-
Notifications
You must be signed in to change notification settings - Fork 93
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
feat(spans): Copy transaction tags to segment #3386
Conversation
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.
The code LGTM, please have a look at the comments below.
This PR introduces the Map -> Array, which implicitly also changes the LenientString
. Existing tests, including the kafka schema checks, pass; so I think it should be good. However, I'm a bit worried this could break something so I'd suggest deploying in S4S first.
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! Please update the scope of the integration test before merging
For the trace explorer, filtering on user tags is important so we'll start copying them to the tags.