-
Notifications
You must be signed in to change notification settings - Fork 129
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
add config support for BaggageSpanProcessor #1330
add config support for BaggageSpanProcessor #1330
Conversation
@jack-berg @trask please check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks @zeitlinger!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
public class BaggageSpanProcessorCustomizer implements AutoConfigurationCustomizerProvider { | ||
@Override | ||
public void customize(AutoConfigurationCustomizer autoConfigurationCustomizer) { | ||
autoConfigurationCustomizer.addTracerProviderCustomizer( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a moment I thought this wouldn't work because this customizers are applied after the batch exporter is registered, but then I realized that order of registration doesn't matter for span processors which want to take advantage of modifying the span on start.
We would need to make changes in the SDK if this was a BaggageLogRecordProcessor
: Either providing a hook to customize the logger provider before batch processor is added, or by extending SdkLoggerProviderBuilder
with a method to insert a processor at the front of the queue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either providing a hook to customize the logger provider before batch processor is added
I think this is a great idea - would make sense for all signals
BTW, I've made the test end-to-end to cover that "onStast" is actually called
Co-authored-by: Mike Goldsmith <goldsmith.mike@gmail.com>
Add config support for BaggageSpanProcessor to add it to java agent - simply by adding the library.