-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[prometheusremotewriteexporter] Support sending trace id and span id for exemplars #8380
[prometheusremotewriteexporter] Support sending trace id and span id for exemplars #8380
Conversation
adefca3
to
fcd65d7
Compare
fcd65d7
to
271a8e8
Compare
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.
Just one nit to make this more consistent with the rest of the codebase
db6a5a3
to
175196f
Compare
cc @anneelisabethlelievre, who initially implemented exemplars for PRW |
@dashpole, could you please address the conflict? Given the approvals, I'll merge this PR afterward. |
175196f
to
fc6a5fe
Compare
It is non-trivial to rebase (since we now need to handle max exemplar size including these new values), so i'd like another review from someone before we merge. |
The additional change is that we have to keep track of the number of runes for the new trace id and span id to make sure the total doesn't exceed limits. If it does exceed the limit, we need to keep trace and span id, and ignore filtered attributes. The relevant line from the spec is: |
Good to go now! |
Description:
Fixes #8351. This is aligned with the specification for OTLP -> prometheus.
The specification does not say how to handle collisions between attributes named
trace_id
andspan_id
and the trace id or span id of the exemplar itself. This PR has attributes take precedence over trace id and span id from the exemplar itself.Testing:
Unit test checks that trace id and span id are added as labels on prometheus exemplars.