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

Agent Auto Configuration can overwrite an extension's customer Sampler #315

Closed
GrahamLea opened this issue Jun 23, 2022 · 4 comments · Fixed by #316
Closed

Agent Auto Configuration can overwrite an extension's customer Sampler #315

GrahamLea opened this issue Jun 23, 2022 · 4 comments · Fixed by #316
Assignees
Labels
status: oncall Flagged for awareness from Honeycomb Telemetry Oncall type: bug Something isn't working

Comments

@GrahamLea
Copy link
Contributor

Headline

The way HoneycombAutoConfigurationCustomizerProvider is written, (calling SdkTracerProviderBuilder.setSampler()), it can overwrite a Sampler installed by an OTel Java Agent extension that is intended to decorate whatever other sampling is in place.

Versions

  • Honeycomb OpenTelemetry Distribution: 1.2.0

Steps to reproduce

  1. Create an OTel extension that auto-configures a custom OTel Sampler, decorating the already installed one. (This is not a trivial task. You can see some of the steps I've taken to do this here. But the bug is pretty obvious from the code.)
  2. Run an app with the Honeycomb agent and the custom extension.

Expected Behaviour

HoneycombAutoConfigurationCustomizerProvider should customize (i.e. decorate) any Sampler already in the configuration, so that the extension's Sampler decorating behaviour is effective, irrespective of what order AutoConfigurationCustomizerProviders are loaded.

Actual Behaviour

Depending on the order in which Java's ServiceLoader loads the two AutoConfigurationCustomizerProviders (Honeycomb's and the extension's), it's possible (and happens consistently, for me) that Honeycomb's code overwrites the extension's custom Sampler.

Additional context

The HoneycombAutoConfigurationCustomizerProvider overwrites whatever sampler is in the configuration already:

return tracerProvider
.setSampler(new DeterministicTraceSampler(sampleRate))
.addSpanProcessor(new BaggageSpanProcessor());

I can't see any way to control the order in which AutoConfigurationCustomizerProviders are executed, either in the ServiceLoader feature or in the design/usage of AutoConfigurationCustomizerProvider (in OTel's AutoConfiguredOpenTelemetrySdkBuilder.build()).

Honeycomb's auto config should probably, instead of setting the Sampler, decorate whatever Sampler is already installed, using the addSamplerCustomizer() extension point, as shown here:
open-telemetry/opentelemetry-java-instrumentation#5008 (comment)

@GrahamLea GrahamLea added the type: bug Something isn't working label Jun 23, 2022
@trask
Copy link

trask commented Jun 23, 2022

it may be worth raising an issue in the opentelemetry-java repo to ask if there's a way for setSampler and addSamplerCustomizer to work together, it seems like ideally addSamplerCustomizer would apply to the sampler that was set via setSampler

@GrahamLea
Copy link
Contributor Author

Here's a commit that I think would make DeterministicTraceSampler and HoneycombAutoConfigurationCustomizerProvider work how I think they should be working.
Let me know if you want me to turn it into a PR.
main...Archium:autoconfig-decorate-sampler

@GrahamLea
Copy link
Contributor Author

FYI, I've built and used the fork above, and it fixes my issue as expected. 👍🏼

@MikeGoldsmith
Copy link
Contributor

Thanks for raising the issue @GrahamLea and sorry for the delay getting back to you. Most of the team were unavailable last week.

We'll look into the issue and thanks again for linking to your work around. We're always open to community contributions, so we'd welcome a PR to update our implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: oncall Flagged for awareness from Honeycomb Telemetry Oncall type: bug Something isn't working
Projects
None yet
3 participants