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

Generate full Beats field definitions, including nested fields #379

Merged
merged 11 commits into from
Mar 8, 2019

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Mar 7, 2019

This work has revealed a subtle bug in the generated files schema.csv and both ES templates, which were missing the group fieldset in all places where user is reused. (e.g. host.user.group.*)

Notes

  • ECS has more keys to describe fields than Beats requires (e.g. reusable). This new generator will filter them, to let through only what matters. Question: should we let through the ECS level?

Mathieu Martin added 3 commits March 7, 2019 11:11
This also fixes a bug where the `group` fieldset was not actually being
nested in all places where user is nested (e.g. `source.user.group.*`).
@webmat
Copy link
Contributor Author

webmat commented Mar 8, 2019

@ruflin Could you check this out? A big difference between this file and the previous one we crafted manually (other than the YAML rendering of strings) is that fields no longer are in the original order.

If that's good with you for this, I think this is ready for final review.

@webmat webmat changed the title WIP Generate full Beats field definitions, including nested fields Generate full Beats field definitions, including nested fields Mar 8, 2019
@webmat webmat removed the in progress label Mar 8, 2019
@webmat
Copy link
Contributor Author

webmat commented Mar 8, 2019

Fields listed under each type:group field set are now sorted alphabetically by name.

@andrewkroh
Copy link
Member

Question: should we let through the ECS level?

If it's not being used then I'd drop it.

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

LGTM. I looked over the generated fields.ecs.yml. This is nice. I didn't spend much time on the Python.

@webmat
Copy link
Contributor Author

webmat commented Mar 8, 2019

It's a Rubyist's Python, so probably better like that 😆

@webmat webmat merged commit 81aecf9 into elastic:master Mar 8, 2019
@webmat
Copy link
Contributor Author

webmat commented Mar 8, 2019

@ruflin Will ping you on the backport for review on Monday. Happy to adjust anything needed.

But I'm moving forward here to prepare the 1.0 backport of this, then make elastic/beats#11150 final, so it can hopefully be merged Monday.

webmat added a commit to webmat/ecs that referenced this pull request Mar 8, 2019
…ncluding nested fields (elastic#379)

Backport of PR elastic#379 to 1.0 branch. Original message:

This work has revealed a subtle bug in the generated files schema.csv and both
ES templates, which were missing the group fieldset in all places where user
is reused. (e.g. `host.user.group.*`)

Upon comparing this new file to the fields definition file we had handcrafted for Beats (prior to this), it also revealed we had missed a few things in the Beats field definitions:

- We had forgotten to define the reusable `user` fieldset `client`, `destination`, `server` and `source`. They previously had been missed.
- We had forgotten to define the reusable `geo` fieldset at `host.geo.*` and `observer.geo.*`
webmat added a commit that referenced this pull request Mar 11, 2019
…g nested fields (#379) (#381)

Backport of PR #379 to 1.0 branch. Original message:

This work has revealed a subtle bug in the generated files schema.csv and both
ES templates, which were missing the group fieldset in all places where user
is reused. (e.g. `host.user.group.*`)

Upon comparing this new file to the fields definition file we had handcrafted for Beats (prior to this), it also revealed we had missed a few things in the Beats field definitions:

- We had forgotten to define the reusable `user` fieldset in `client`, `destination`, `server` and `source`. They previously had been missed.
- We had forgotten to define the reusable `geo` fieldset at `host.geo.*` and `observer.geo.*`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants