-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add event.dataset to filebeat #9457
Conversation
This allows 6.6 / 6.7 data to be compatible with 7.x.
Pinging @elastic/infrastructure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any need to enforce that event.dataset is always fileset.module + fileset.name (say with a test)?
@@ -54,7 +55,7 @@ | |||
"tags": [ | |||
"suricata" | |||
], | |||
"url.hostname": "example.net", | |||
"url.hostname": "example.net", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bunch of these extra spaces, not sure if you care
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the files are auto generated so more wonder why there were missing before :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I may have saved it in a commit, before I was familiar with the Beats integration tests :-)
- name: event.dataset | ||
description: > | ||
The Filebeat dataset that generated this event. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, so we have per-Beat override 👍
@graphaelli Good question about the test. I don't think we should enforce it (event though at the moment it is because it's in the code). More important for me is that it's correct in the generate files. Ok if for now we skip a test? |
sounds reasonable to me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Straightforward, LGTM.
This allows 6.6 / 6.7 data to be compatible with 7.x.