Fix HTTP header normalization

[samuelcolvin]

Description

HTTP request and response headers (attributes http.request.header.* and http.response.header.*) should be normalized by just being converted to lowercase, not replacing underscore with dash.

This is made clear in the docs here:

You can also see the correct behavior in the go library here.

The current incorrect behavior of replace('_', '-') is extremely problematic since you can't know what the original header was - e.g. if you see a key http.request.header.foo_bar what was the original value? It could have been foo-bar or foo_bar.

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• replaced all the tests expecting the incorrect behavior

Does This PR Require a Core Repo Change?

• Yes. - Link to PR:
• No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added
• Documentation has been updated

[aabmass]

Thanks for the PR @samuelcolvin. I did some digging and it looks like the OTel conventions were changed in open-telemetry/semantic-conventions#369 but used to require this normalization. We should definitely fix it, but I think it might need to be gated behind the OTEL_SEMCONV_STABILITY_OPT_IN environment variable described here.

@lmolkova @lzchen could we merge this PR today or does it need to be controlled by OTEL_SEMCONV_STABILITY_OPT_IN? I'm not sure how we could emit http/dup if there is only one attribute key in question.

[lzchen]

@aabmass

This might be a bigger discussion on how we should be handling the new http semantic convention migrations. For now, please hold off on this PR. Will bring this up in the weekly SIG.

[samuelcolvin]

@lzchen ok understood, can we at least run tests, so I know it's passing.

Also, if this won't get merged yet, could we merge #2266 instead first please? There's no "http semantic convention migrations" issue there, just a bug.

[aabmass]

can we at least run tests, so I know it's passing.

Running now

Also, if this won't get merged yet, could we merge #2266 instead first please? There's no "http semantic convention migrations" issue there, just a bug.

That SGTM as it's just a bug 👍

[lzchen]

From discussion of 07/11/2024 SIG, just an update on what is blocking this PR. Some of the instrumentations that this pr touches still has not been migrated to new semantic conventions as of yet, most notably django, falcon, pyramid, starlette and tornado. Since django is the only one remaining for the stable http semantic migration plan, once that instrumentation is migrated, we can go ahead with this change as we have agreed that it is fine to make breaking/semantic convention changes outright to the other instrumentations since they are all in beta.