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

Update to ECS 1.4.0 #14844

Merged
merged 14 commits into from
Jan 10, 2020
Merged

Update to ECS 1.4.0 #14844

merged 14 commits into from
Jan 10, 2020

Conversation

adriansr
Copy link
Contributor

@adriansr adriansr commented Nov 28, 2019

Updated vendored elastic/ecs to v1.3.0 v1.3.1 v1.4.0 and fields.ecs.yml from same version.

@adriansr adriansr added the ecs label Nov 28, 2019
@adriansr adriansr requested a review from a team as a code owner November 28, 2019 14:56
@adriansr
Copy link
Contributor Author

jenkins, test this

@@ -30,7 +30,7 @@ import (
// MaxDefaultFieldLength is the limit on the number of default_field values
// allowed by the test. This is less that the 1024 limit of Elasticsearch to
// give a little room for custom fields and the expansion of `fields.*`.
const MaxDefaultFieldLength = 1000
const MaxDefaultFieldLength = 1010
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about this change. It was complaining that there were 1006 fields after the ECS update.

Will exceed 1000 have an impact or as long as it's below 1024 we're safe?

Copy link

Choose a reason for hiding this comment

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

We configure the total_fields.limit per index to 10000. The default_fields limit is impacted by the indices.query.bool.max_clause_count setting (default 1024, but not modifyable via templates).

Copy link
Member

Choose a reason for hiding this comment

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

@andrewkroh I think touched this in the past?

Copy link
Member

Choose a reason for hiding this comment

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

One option to deal with this would be to add default_field: false to the new fields being added in ECS. This problem will keep occurring each time new fields are added and this could be the solution until we move to a better indexing strategy. Bumping the limit up won't work for much longer.

@adriansr adriansr requested review from a team, andrewkroh and ruflin and removed request for a team December 2, 2019 11:47
@adriansr adriansr added the review label Dec 2, 2019
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Overall I'm good with this change. We should keep in mind that this keeps increasing the size of all templates even thought he fields are not used in winlogbeat for example.

I didn't check CI but I think we should ;-)

@adriansr adriansr requested a review from ruflin December 4, 2019 10:59
@adriansr adriansr changed the title Update to ECS 1.3.0 Update to ECS 1.3.1 Dec 4, 2019
@adriansr
Copy link
Contributor Author

adriansr commented Dec 4, 2019

Updated to ECS 1.3.1

@ruflin
Copy link
Member

ruflin commented Dec 9, 2019

@adriansr What are your thoughts around the field limit (see comment from @andrewkroh ).

@adriansr
Copy link
Contributor Author

adriansr commented Dec 10, 2019

I misunderstood the default_fields suggestion.

I can add them to the new fields this PR adds, but this implies a change in ECS (to add default_fields in there) and a new release (say 1.3.2/1.4.0). Alternatively we can merge as is and apply the change in the next release.

I don't mind doing either.

Edit: Created a PR to elastic/ecs with the change: elastic/ecs#687

@adriansr
Copy link
Contributor Author

This is still a work in progress as it currently depends on adding the default_field: false option to ECS (elastic/ecs#687).

A new ECS release (1.4.0?) will be needed after that.

@webmat
Copy link
Contributor

webmat commented Dec 24, 2019

elastic/ecs#687 has been merged and backported to branch 1.3.

@adriansr adriansr changed the title Update to ECS 1.3.1 Update to ECS 1.4.0 Jan 7, 2020
@@ -1,5 +1,5 @@
# WARNING! Do not edit this file directly, it was generated by the ECS project,
# based on ECS version 1.4.0.
# based on ECS version 1.5.0-dev.
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to grab the version from elastic/ecs#718 ;-)

@adriansr
Copy link
Contributor Author

adriansr commented Jan 8, 2020

Jenkins, test this

@adriansr
Copy link
Contributor Author

Jenkins, test this

@adriansr adriansr merged commit cab56a3 into elastic:master Jan 10, 2020
adriansr added a commit to adriansr/beats that referenced this pull request Jan 13, 2020
- Update vendored elastic/ecs and fields.ecs.yml from ECS 1.4 branch

Includes elastic/ecs#717 and  elastic/ecs#687 not in released v1.4.0

- Remove field `process.exit_code` from Metricbeat, now in ECS.

(cherry picked from commit cab56a3)
adriansr added a commit that referenced this pull request Jan 14, 2020
- Update vendored elastic/ecs and fields.ecs.yml from ECS 1.4 branch

Includes elastic/ecs#717 and  elastic/ecs#687 not in released v1.4.0

- Remove field `process.exit_code` from Metricbeat, now in ECS.

(cherry picked from commit cab56a3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ecs in progress Pull request is currently in progress. v7.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants