Fix Jaeger propagator baggage header case sensitivity - #8496
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8496 +/- ##
=========================================
Coverage 91.59% 91.60%
Complexity 10312 10312
=========================================
Files 1013 1013
Lines 27273 27274 +1
Branches 3202 3202
=========================================
+ Hits 24982 24984 +2
Misses 1563 1563
+ Partials 728 727 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jack-berg
added
the
needs author feedback
|
Ready for review — CI is green. |
github-actions
Bot
removed
the
needs author feedback
| Map<String, String> carrier = new LinkedHashMap<>(); | ||
| // Some TextMapGetters return header keys in their original case (e.g. undertow 1.7), so baggage | ||
| // matching must be case insensitive. | ||
| carrier.put("Uberctx-some-key", "value"); |
There was a problem hiding this comment.
maybe use the constant for the prefix but upper-case it, or init-cap it? I think it would be good use the constants, like in the other tests.
There was a problem hiding this comment.
Good call — switched to BAGGAGE_PREFIX/BAGGAGE_HEADER upper-cased so it still exercises the case-insensitive path, matching the other tests.
|
This PR has review comments. Review suggestions, whether from maintainers or automated reviewers, aren't always correct or required. Please evaluate each comment on its merits, then make sure each thread has a clear outcome. For example, link to the commit if you applied a suggestion, explain why it wasn't applied, or ask a follow-up question. Automation flags a PR for human review once every review thread has a reply or is marked as resolved. Status across open PRs is visible on the pull request dashboard. |
Replace the hardcoded "Uberctx-some-key"/"Jaeger-Baggage" literals with BAGGAGE_PREFIX/BAGGAGE_HEADER upper-cased via Locale.ROOT, matching the other tests in this file. Still exercises the case-insensitive matching path; no assertion changes.
|
needs a rebase for the changelog |
|
Thank you for your contribution @thswlsqls! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey. |
3 participants
Description
JaegerPropagator.getBaggageFromHeader()matched baggage keys case sensitively (uberctx-,jaeger-baggage), so carriers that return header keys in their original case (e.g. undertow 1.7 sendsUberctx-Key,Jaeger-Baggage) had their baggage silently dropped.key.toLowerCase(Locale.ROOT)before matching the prefix and header name; the value lookup still uses the original key.OtTracePropagator.extract(), which already lowercases the key (#5840). Jaeger was missed.Testing done
JaegerPropagatorTest#extract_baggageOnly_capitalizedHeaderscovering both a capitalizedUberctx-*prefix and a capitalizedJaeger-Baggageheader../gradlew :extensions:trace-propagators:check— 39 tests passed inJaegerPropagatorTest, full module check green.jApiCmpproduced no apidiff change.## Unreleasedentry inCHANGELOG.md.