-
Notifications
You must be signed in to change notification settings - Fork 169
add baggage before it can be exported #2152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
We need to add the baggage processor before the batch span processor (that is usually already configured) - because it can start to export as soon as it's offered the span - see https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessor.java#L177-L182 @trask fyi since we discussed this |
...sor/src/main/java/io/opentelemetry/contrib/baggage/processor/BaggageProcessorCustomizer.java
Show resolved
Hide resolved
...sor/src/main/java/io/opentelemetry/contrib/baggage/processor/BaggageProcessorCustomizer.java
Show resolved
Hide resolved
hm, I feel like this would have been a bigger problem if batch span processor wasn't always added last in this typical situation (and not sporadic then) |
sorry, not sure why this would be the fix still
Possible - but I didn't spot anything else. @laurit do you have an idea? |
maybe it happens in the test due to the short delay: https://github.com/zeitlinger/opentelemetry-java-contrib/blob/00c7b2841f7c16371a6c4a7c3eae5358fd0e3516/baggage-processor/src/test/java/io/opentelemetry/contrib/baggage/processor/BaggageProcessorCustomizerTest.java#L109 I've included this PR in #2031 where the error is happening now |
This seems to have helped |
|
@zeitlinger thanks for sorting this out, I have high hopes of this resolving the flaky test issue |
…ge/processor/BaggageProcessorCustomizer.java Co-authored-by: Trask Stalnaker <[email protected]>
…ge/processor/BaggageProcessorCustomizer.java Co-authored-by: Trask Stalnaker <[email protected]>
Fixes #2150