Don't normalize headers with "hyphen -> underscore" #3104
Conversation
| 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.
.../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? |
@lzchen Taking a second look: yes. We do still have an open PR to migrate util-http: #2648 . In that case, I think the problem is we are breaking already migrated instrumentation, like django, flask. edit: maybe I wasn't so clear, but I'm saying that my concern is we should do that change only for new semconv at least for instrumentation that were migrated |
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: