-
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
Import fields.ecs.yml as generated by the ECS repo #11150
Conversation
Here's a tip if you want to diff the previous file with this one. It doesn't help for the array sort order differences, but it helps for the YAML formatting differences and key order in YAML dictionaries. Using Python 2.7 (so the dictionaries are sorted by key):
|
- with agent.hostname fix - and warning header that includes the version
Hey @webmat, this is the only PR (open or closed) in the beats repo that is using the |
@ycombinator Nah, the space instead of the underscore is really important |
I used "needs backport" in ECS, and I open my PRs with hub instead of the web interface. Will adjust to needs_backport in my things :-) Thanks for pointing out |
No harm was done to the fields between 1.0 and master
This is now based on the ECS v1.0 backport PR elastic/ecs#381. This is ready for final review. |
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.
LGTM. A follow up PR should be done to update the ecs version in each event.
One thing that keeps bugging me is that each Beat gets all ECS fields even if they are not used which leads to a much larger template then needed. This "should" not be a technical issue but I think it could be confusing for users to see fields in the template that are not used. The benefit of it is that users can't introduce fields which conflict with ECS in Beats with the rename processor for example. As you see, I'm on the fence. But this is for a later discussion, it just popped up because more fields are added here to each Beat.
The vendored copy of the generated Go code should get updated too.
|
@ruflin I'll open the follow-up PR shortly, to set the version on the events. Didn't want the massive amounts of test JSON files conflicting with all the other PRs to slow down the merge of this straightforward field update :-) @andrewkroh The code generation hasn't been backported to 1.0 yet, but I should be able to get that done and open a PR here today. |
Upon further inspection, it appears that Is this now solely based on the vendored Go ECS library's |
Moved the definition of `agent.hostname` to fields.common.yml
Moved the definition of `agent.hostname` to fields.common.yml
@webmat Yes, looks like it depends on the go lib. |
They were accidentally removed by elastic#11150.
They were accidentally removed by #11150.
They were accidentally removed by elastic#11150.
Moved the definition of `agent.hostname` to fields.common.yml
They were accidentally removed by elastic#11150.
This PR is based on elastic/ecs#379. Once this ECS issue is merged and backported to the 1.0 branch in ECS, a final export of the fields should be imported in this PR.
A few things of note:
agent.hostname
, which is not an ECS field. So this field definition has been moved tofields.common.yml
.user.*
reusable fields underclient
,destination
,server
andsource
. They previously had been missed.geo
fieldset athost.geo.*
andobserver.geo.*
. They previously had been missed.Update
This is now based on the ECS v1.0 backport PR elastic/ecs#381.
This is ready for final review.