Skip to content
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

Fix base fields in ECS file #9619

Merged
merged 2 commits into from
Dec 21, 2018
Merged

Conversation

ruflin
Copy link
Member

@ruflin ruflin commented Dec 18, 2018

So far the base fields of ECS were not read as they were nested under base. This brings them to the top level and resolves all the conflicts with field definitions in libbeat and other Beats.

Moving forward we must find a better way for writing the ECS file to Beats as this step has to be done manually each time.

@@ -3,7 +3,50 @@
description: >
ECS fields.
fields:


- name: "@timestamp"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should/could we change how the fields.yml file in elastic/ecs is generated to make it match this format?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current fields.yml file is a simple concatenation of all of the different field definition files. ECS has more keys in there, which are ECS specific.

So we'll likely need to put together a script that strips out these keys and produces what Beats expects...

Would it make sense to have this script live in Beats, since it's an adaptation of the schema to Beats?

Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

It looks like the Jenkins failure is due to capacity issues over there.

@@ -3,7 +3,50 @@
description: >
ECS fields.
fields:


- name: "@timestamp"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current fields.yml file is a simple concatenation of all of the different field definition files. ECS has more keys in there, which are ECS specific.

So we'll likely need to put together a script that strips out these keys and produces what Beats expects...

Would it make sense to have this script live in Beats, since it's an adaptation of the schema to Beats?

@ruflin
Copy link
Member Author

ruflin commented Dec 18, 2018

I'm actually thinking to adjust the ECS fields.yml but need to figure out why. I think the two should be in sync.

So far the base fields of ECS were not read as they were nested under base. This brings them to the top level and resolves all the conflicts with field definitions in libbeat and other Beats.
@ruflin
Copy link
Member Author

ruflin commented Dec 21, 2018

I created a follow up issue for the discussion around the structure unification. elastic/ecs#292

@ruflin ruflin merged commit 690e40e into elastic:master Dec 21, 2018
@ruflin ruflin deleted the add-base-fields-correctly branch December 21, 2018 10:15
DStape pushed a commit to DStape/beats that referenced this pull request Aug 20, 2019
So far the base fields of ECS were not read as they were nested under base. This brings them to the top level and resolves all the conflicts with field definitions in libbeat and other Beats.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants