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

OpenTelemetry: Support exclusion of non-application URIs if custom Sampler is used #25525

Closed
calohmn opened this issue May 12, 2022 · 5 comments · Fixed by #26367
Closed

OpenTelemetry: Support exclusion of non-application URIs if custom Sampler is used #25525

calohmn opened this issue May 12, 2022 · 5 comments · Fixed by #26367
Labels
area/tracing good first issue Good for newcomers kind/enhancement New feature or request
Milestone

Comments

@calohmn
Copy link

calohmn commented May 12, 2022

Description

This refers to the quarkus-opentelemetry extension.

We have opted for using a custom io.opentelemetry.sdk.trace.samplers.Sampler CDI Bean in order to extend the existing sampling logic.
As stated in the documentation, this means that the quarkus.opentelemetry.tracer.suppress-non-application-uris option has no effect, so that the logic for suppressing non-application uris has to be added manually to the custom Sampler.

For applications not using a common /q prefix for the non-application uris, assembling the URIs and applying the path resolution shows to be quite cumbersome.

It would be helpful, if the

List<String> dropNonApplicationUris,
List<String> dropStaticResources

information, given to the TracerRecorder.setupSampler() method is somehow made available for the creation of a custom Sampler.

Implementation ideas

Instead of checking for a Sampler bean, the TracerRecorder.setupSampler() method could check for a bean of a new SamplerFactory class, containing a factory method to which the above mentioned dropNonApplicationUris and dropStaticResources can be supplied by the TracerRecorder.setupSampler() method.

@calohmn calohmn added the kind/enhancement New feature or request label May 12, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented May 12, 2022

/cc @radcortez

@mun711
Copy link
Contributor

mun711 commented Jun 8, 2022

I'm new to quarkus and didn't check out the code related to this issue yet, but the description seems to be quite detailed.
@calohmn @radcortez
Could I take this issue to investigate?

@radcortez
Copy link
Member

Please, go ahead @mun711. Thank you!

@mun711
Copy link
Contributor

mun711 commented Jun 25, 2022

@radcortez
added the changes in #26367
Updated the TracerUtil:mapSampler method to search for a sampler bean and use it instead of a baseSampler if found.
Please let me know if that looks good to you or we still want to move this logic to a SamplerFactory.

@calohmn
Copy link
Author

calohmn commented Jun 27, 2022

FMPOV, this kind of approach of applying the DropNamesSampler and parentBased Sampler on top of the custom sampler also looks ok. The configuration properties documentation would then have to be adapted, dropping the "Providing a custom [...]" part for the quarkus.opentelemetry.tracer.suppress-non-application-uris and quarkus.opentelemetry.tracer.include-static-resources descriptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tracing good first issue Good for newcomers kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants