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

Make user.group a nesting of the group field set #308

Merged
merged 4 commits into from
Jan 23, 2019

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Jan 23, 2019

An error slipped in, and instead of nesting the group field set at user.group, we originally made this a keyword field.

This PR introduces a breaking change, but we are aiming to introduce this fix in ECS 1.0.0 GA.

The initial implementation went against our principle of not reusing top level object names elsewhere in the hierarchy, using different semantics.

This PR closes #304.

@webmat
Copy link
Contributor Author

webmat commented Jan 23, 2019

Philosophical question: the changelog entry goes under "breaking", or "bugfixes"? :-)

@webmat webmat requested a review from ruflin January 23, 2019 01:58
@webmat webmat self-assigned this Jan 23, 2019
@webmat webmat added bug Something isn't working needs_backport 1.0.0-ga labels Jan 23, 2019
@webmat
Copy link
Contributor Author

webmat commented Jan 23, 2019

@cwurm for awareness

@ruflin
Copy link
Contributor

ruflin commented Jan 23, 2019

For the changelog, it should be mentioned in both places. It's a bug fix but a breaking bug fix.

@webmat
Copy link
Contributor Author

webmat commented Jan 23, 2019

I just duplicated the whole changelog entry as is. Considered wording it differently (why it's a bugfix), but I figure if people care, they'll check out the PR and the issue.

@webmat webmat requested a review from MikePaquette January 23, 2019 14:13
@ruflin
Copy link
Contributor

ruflin commented Jan 23, 2019

@webmat I stumbled over this but was not sure if I should comment. If you can word it differently would appreciate it.

@webmat
Copy link
Contributor Author

webmat commented Jan 23, 2019

Ok, will do ;-)

Copy link
Contributor

@MikePaquette MikePaquette left a comment

Choose a reason for hiding this comment

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

LGTM

@webmat webmat merged commit 337ddd4 into elastic:master Jan 23, 2019
@webmat webmat deleted the reuse-group branch January 23, 2019 18:47
webmat pushed a commit to webmat/beats that referenced this pull request Jan 24, 2019
webmat added a commit to elastic/beats that referenced this pull request Jan 24, 2019
This change enables us to nest the `group` field set at `user.group`, rather than being limited to only group name.

Imports the changes from ECS elastic/ecs#308, which solves elastic/ecs#304.
webmat added a commit to webmat/ecs that referenced this pull request Mar 5, 2019
Breaking change.

Field set name "group" was being used as a leaf field at `user.group`. It had different semantics as the field set: it was a keyword field, instead of being a nesting of the field set. This goes against a driving principle of ECS, and has been corrected.

We removed the `user.group` `keyword` field (introduced in elastic#204), and made the `group` field set nestable at `user.group`.
webmat added a commit that referenced this pull request Mar 5, 2019
…#355)

Cherry-pick of PR #308 to 1.0 branch. Original message:

Breaking change.

Field set name "group" was being used as a leaf field at `user.group`. It had different semantics as the field set: it was a keyword field, instead of being a nesting of the field set. This goes against a driving principle of ECS, and has been corrected.

We removed the `user.group` `keyword` field (introduced in #204), and made the `group` field set nestable at `user.group`.
DStape pushed a commit to DStape/beats that referenced this pull request Aug 20, 2019
This change enables us to nest the `group` field set at `user.group`, rather than being limited to only group name.

Imports the changes from ECS elastic/ecs#308, which solves elastic/ecs#304.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0.0-ga bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

user.group should not be a keyword field, but a place where we can nest the group field set
3 participants