Skip to content
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

Merged
merged 7 commits into from
Jun 17, 2024

Conversation

zeitlinger
Copy link
Member

Add config support for BaggageSpanProcessor to add it to java agent - simply by adding the library.

@zeitlinger zeitlinger requested a review from a team May 31, 2024 13:44
@zeitlinger
Copy link
Member Author

@jack-berg @trask please check

Copy link
Member

@MikeGoldsmith MikeGoldsmith left a 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!

Copy link
Contributor

@breedx-splk breedx-splk left a 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(
Copy link
Member

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.

Copy link
Member Author

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

@trask trask added this to the v1.37.0 milestone Jun 11, 2024
@trask trask merged commit d23bc11 into open-telemetry:main Jun 17, 2024
14 checks passed
@zeitlinger zeitlinger deleted the baggage-processor-config branch June 18, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants