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 78.77% 78.77%
- Complexity 8579 8580 +1
=========================================
Files 1009 1009
Lines 28993 28994 +1
Branches 3599 3599
=========================================
+ Hits 22839 22841 +2
Misses 5311 5311
+ Partials 843 842 -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.
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.