Support exclusion of non-application URIs if custom Sampler is used

[mun711]

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

[quarkus-bot]

/cc @radcortez

[radcortez]

Thank you for the PR!

I'm not totally sure if we want to support adding samplers as CDI Beans. I know that we already have a few other things that we configure with CDI Beans, but I believe that was a mistake.

OTel now provides a well-defined configuration to set a lot of stuff, we just need to support it first. I think our goal should be to move to https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk-extensions/autoconfigure/README.md and use otel.traces.sampler to set up the Sampler.

[mun711]

Thank you for the PR!

I'm not totally sure if we want to support adding samplers as CDI Beans. I know that we already have a few other things that we configure with CDI Beans, but I believe that was a mistake.

OTel now provides a well-defined configuration to set a lot of stuff, we just need to support it first. I think our goal should be to move to https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk-extensions/autoconfigure/README.md and use otel.traces.sampler to set up the Sampler.

@radcortez - Thanks for the reply
I checked out the link for opentelemetry autoconfiguration. It seems that such a migration will require quite a few changes. I don't have a full understanding of the quarkus recorder pattern, but I could see that the current opentelemetrysdk is created at that point, and my guess is that we want to somehow replace with AutoConfiguredOpenTelemetrySdk, as well as add some customizations to support custom sampler enhancements and possibly url dropping as a configuration property both for custom and existing samplers.

If we go ahead with the autoconfigure ticket I guess this PR and the relevant issue may be closed as no longer wanted ?

[radcortez]

If we go ahead with the autoconfigure ticket I guess this PR and the relevant issue may be closed as no longer wanted ?

Not sure yet.

I do know that we want to go with the OTel way to configure things. Since they already have a way to configure all of their APIs we should go with that instead of providing specific ways to configure stuff (CDI Beans).

We don't even have support configuration as CDI Beans for everything, so we would either need to add it to make it consistent or drop it altogether. This means that we need to somehow migrate / deprecate the current CDI support.

Maybe we just need to offer our custom Sampler that users can extend that excludes the non-application uris out of the box.

[luneo7]

The way that Quarkus configures samplers today gives the possibility of using CDI beans already, so the PR is not adding this functionality, it is just extending the exclusion of non-application URIs also if the CDI bean is used. I know that when that sampler as a bean was added, was because AWS X-Rray Sampler needed custom configuration and needed to use a custom bean as the OTEL auto configuration thing wouldn't configure it, and the auto configuration stuff was in alpha.
I think that maybe we can add support as this PR proposes, and when we migrate to use OTEL auto configuration stuff, we should make it support everything that we already do with Quarkus current approach.

The thing about exposing a custom Sampler, is that people will need to extend that, and also extend another Sampler or create a custom delegate on top of Quarkus custom sampler to use the desired sampler (AWS X-Ray for instance), I think that is it adds more complexity.

[radcortez]

I know, but I want to avoid additional behavior relying on CDI. Ideally, we should replace the CDI Sampler with the OTel config, but I agree that it may take a while.

[gsmet]

Thanks!