Fix context propagation into webflux7 client callbacks#15336
Fix context propagation into webflux7 client callbacks#15336trask merged 1 commit intoopen-telemetry:mainfrom
Conversation
zeitlinger
left a comment
There was a problem hiding this comment.
Were those tests failing before or how did you test this?
| } | ||
|
|
||
| private static void registerReactorHook() { | ||
| ContextPropagationOperator.builder().build().registerOnEachOperator(); |
There was a problem hiding this comment.
is there any issue with calling this twice?
is the instrumentation broken in 7.0 if you don't call this?
There was a problem hiding this comment.
is there any issue with calling this twice?
There shouldn't be an issue, webflux server instrumentation is doing it exactly the same way. ContextPropagationOperator has a boolean that avoids doing anything after it has already been registered once.
is the instrumentation broken in 7.0 if you don't call this?
This is only needed for the library instrumentation. Instrumentation is mostly functional just that context isn't propagated into reactive callbacks. I believe that this is not a regression and the same issue was already present for older webflux versions, our tests just didn't catch it.
The tests didn't fail because manual context propagation was used. That manual propagation was removed in this PR. |
I believe that this not a regression, but rather caused by using
exchangeToMonoinstead of theexchangethat was removed in webflux 7.