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

Allow overriding service information from context. #1901

Merged
merged 4 commits into from
Feb 8, 2019

Conversation

simitt
Copy link
Contributor

@simitt simitt commented Feb 7, 2019

fixes #1175

Copy link
Member

@graphaelli graphaelli left a comment

Choose a reason for hiding this comment

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

does this mean events can remove service and user information from an event when those are set in the metadata? are there tests for that?

docs/spec/user.json Show resolved Hide resolved
utility/map_str_enhancer.go Show resolved Hide resolved
@simitt
Copy link
Contributor Author

simitt commented Feb 8, 2019

I added some tests showing that data from the metadata.service cannot be deleted but only overriden if set in context.service. This means if there is null set in the event information this does not override the information from the metadata.
One can set an empty string though, in event.context.service as well as in metadata.service. Since we are treating zero values as values, I would not call this deleting. To be consistent and less error prone I think we should either allow zero values on both places or on none.

For users it behaves differently. Since we are not merging the user information but either take the whole information from metadata or from context one could kind of delete information on an event.

I tried to explain this here, but I'll probably have to rephrase as it doesn't seem to be clear enough:

user:

Describes the correlated user for this event. If user data are provided here, all user related information from metadata is ignored, otherwise the metadata's user information will be stored with the event.

service:

Service related information can be sent per event. Provided information will override the more generic information from metadata, non provided fields will be set according to the metadata information.

docs/spec/metadata.json Show resolved Hide resolved
utility/map_str_enhancer.go Outdated Show resolved Hide resolved
@simitt simitt merged commit 62fbb81 into elastic:master Feb 8, 2019
simitt added a commit to simitt/apm-server that referenced this pull request Feb 11, 2019
* Allow overriding `service` information from `context`.

fixes elastic#1175
simitt added a commit to simitt/apm-server that referenced this pull request Feb 11, 2019
* Allow overriding `service` information from `context`.

fixes elastic#1175
simitt added a commit that referenced this pull request Feb 11, 2019
* Allow overriding `service` information from `context`.

fixes #1175
simitt added a commit to simitt/apm-server that referenced this pull request Feb 11, 2019
* Allow overriding `service` information from `context`.

fixes elastic#1175
@simitt simitt deleted the 1175-service-per-event branch February 11, 2019 17:09
graphaelli added a commit to graphaelli/apm-integration-testing that referenced this pull request Feb 12, 2019
simitt added a commit that referenced this pull request Feb 14, 2019
* Allow overriding `service` information from `context`.

fixes #1175
@eyalkoren
Copy link
Contributor

This is not backported to 6.x, right?

@simitt
Copy link
Contributor Author

simitt commented Mar 13, 2019

@eyalkoren confirm, it is not backported.

@eyalkoren
Copy link
Contributor

Users with agents that support that upgrading the APM server to 7.x may get their views and aggregations completely changed. Is this documented somewhere?

@simitt
Copy link
Contributor Author

simitt commented Mar 13, 2019

No, you're right, this is missing a changelog entry. I think this should go into the APM wide release notes and also into the server release notes.

There was kind of a it implicitly works situation though with 6.x I first need to check what really changes for the users when upgrading.

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.

Allow overriding of meta data in intake V2
3 participants