Don't normalize headers with "hyphen -> underscore" #3104
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
Kludex
commented
Dec 13, 2024
Comment on lines
186
to
193
| def normalise_request_header_name(header: str) -> str: | ||
| key = header.lower().replace("-", "_") | ||
| key = header.lower() | ||
| return f"http.request.header.{key}" | ||
|
|
||
|
|
||
| def normalise_response_header_name(header: str) -> str: | ||
| key = header.lower().replace("-", "_") | ||
| key = header.lower() | ||
| return f"http.response.header.{key}" |
There was a problem hiding this comment.
Those are the relevant changes. The rest is just to comply with this.
|
I don't know what's wrong with the docs... 😢 |
This reverts commit d2b0460.
emdneto
reviewed
Dec 19, 2024
emdneto
reviewed
Dec 20, 2024
.../opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py
Outdated
Show resolved
Hide resolved
…telemetry/instrumentation/tornado/__init__.py
|
Thanks @emdneto |
|
Relevant pr in semconv: open-telemetry/semantic-conventions#369 Would this breaking change fall under semconv migration? @emdneto @aabmass @xrmx wdyt? |
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.
I think this can be considered a breaking change, and I'm not sure how it affects other services, but it fixes the behavior according to the OTel specification.
Changes
-to_on the HTTP header normalizer.Details
The current behavior is incorrect according to the HTTP RFCs regarding HTTP headers (because they are just case-insensitive), and the OpenTelemetry specification itself.
The following headers are not the same:
But those two, should be treated as same: