tracing: Only decode the traceparent and tracestate headers #1021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We all change the UnicodeDecodeError exception to be a debug message. This really shouldn't happen oncee we are restricting the headers to tracestate and traceparent though.
salomon-smekecohen
approved these changes
Nov 12, 2024
chriskuehl
approved these changes
Nov 12, 2024
Comment on lines
86
to
87
| key = k.decode() | ||
| if key == "traceparent" or key == "tracestate": |
There was a problem hiding this comment.
Just checking, are we sure these are always lowercase? I would expect them to be since WSGI normalizes header capitalization but probably worth checking.
There was a problem hiding this comment.
They should only be being set by the otel lib at this point and the spec is lowercase. I think we should be good.
There was a problem hiding this comment.
But maybe we should just do a non case-sensitive test now that we have the old trace headers.
| header_dict[k.decode()] = v.decode() | ||
| # this is only for w3c trace headers | ||
| key = k.decode() | ||
| if key in ["traceparent", "tracestate", "Trace", "Span", "Sampled", "Flags"]: |
There was a problem hiding this comment.
can we move this slightly magic array to a set somewhere?
W3C_HEADERS = {...} ?
There was a problem hiding this comment.
+1, maybe W3C_HEADERS = frozenset((...)) as a general best practice so it's not mutable.
chriskuehl
reviewed
Nov 12, 2024
chriskuehl
force-pushed
the
push-swukuupqynll
branch
from
18c9856 to
99dca60
Compare
salomon-smekecohen
approved these changes
Nov 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We all change the UnicodeDecodeError exception to be a debug message. This really shouldn't happen oncee we are restricting the headers to tracestate and traceparent though.