Fix spring-webflux header parsing for 7+#15502
Conversation
github-actions
Bot
added
the
test native
trask
left a comment
trask
left a comment
There was a problem hiding this comment.
any idea how we could get coverage of keys() via the common http server tests, so it would cover all of the instrumentations?
We could introduce a custom verifying propagator that calls the |
| // The keys() method internally calls HttpHeaders.keySet() | ||
| // This will throw NoSuchMethodError with Spring Web 7 if not properly handled |
There was a problem hiding this comment.
somewhat contradictory, NoSuchMethodError is avoided by not calling HttpHeaders.keySet() for spring web 7
|
|
||
| @Test | ||
| void testKeysWithBaggageHeader() { | ||
| // Test that baggage headers are properly returned by keys() |
There was a problem hiding this comment.
I wouldn't have added that many tests since most of the functionality is already tested elsewhere. Guess having more tests can't hurt.
| @Test | ||
| void testKeysWithMultipleBaggageHeaders() { | ||
| // Test that multiple baggage headers are properly returned by keys() | ||
| // The W3C Baggage propagator needs to iterate through all headers to find baggage entries |
There was a problem hiding this comment.
W3C Baggage propagator doesn't really use the keys() method
3 participants
Spring Web 7 introduced a breaking change where
HttpHeadersno longer extendsMapand removed thekeySet()method. This causedNoSuchMethodErrorinWebfluxTextMapGetter#keys()when extracting baggage headers, but existing tests didn't catch it because they never sent baggage headers that would trigger thekeys()code path.I am still trying to get the javaagent instrumentation working with latestDeps, but this at least fixes this one piece of the puzzle.
Related to #14906