OpenTelemetry: Custom sampler config; rate-limiting sampler

[calohmn]

When migrating from the OpenTracing Jaeger client to using quarkus-opentelemetry with the OpenTracing shim, we want to keep the possibility of letting a rate-limiting sampler be configured (see Jaeger client configuration).

With quarkus-opentelemetry, this currently isn't supported out-of-the-box. Possible values for quarkus.opentelemetry.tracer.sampler are off, on, ratio, meaning either a constant (on/off) or probabilistic sampler can be used.

Of course a rate-limiting sampler can easily be integrated in a Quarkus application by providing a custom Sampler CDI bean.
But then the standard quarkus.opentelemetry.tracer.sampler configuration properties can't be used. I will have to think about some other configuration structure with which I allow my Quarkus application to use either an On/Off Sampler or a rate-limiting sampler for example; the rate-limiting sampler also needing a rate-limit configuration value.

Therefore:

• Would it be feasible to add support for a rate-limiting sampler as one of the "quarkus.opentelemetry.tracer.sampler" options?
  I.e. having off, on, ratio, rate-limiting as possible values there. The sampler implementation would be straightforward, using the Opentelemetry SDK RateLimiter (see here). This feature would make it easier to migrate from the Jaeger client.

• And/or: Could a new quarkus.opentelemetry.tracer.sampler.custom configuration property be added - with value of type Map<String,String>?
  This would allow arbitrary configuration properties under the standard quarkus.opentelemetry.tracer.sampler. prefix to be used by a custom Sampler. This would make it easier to add a custom Sampler that needs configuration.

I would appreciate feedback here on whether to create a feature request for one and/or the other option.

[quarkus-bot[bot]]

/cc @radcortez

[calohmn]

The plan is to support OTel Configuration OOTB.

Ah, good to know.
So, that means the actual otel.traces.sampler and otel.traces.sampler.arg (system/env) properties shall be used then?
Or will this kind of sampler configuration be mapped to the existing quarkus.opentelemetry.tracer.sampler and an additional quarkus.opentelemetry.tracer.sampler.arg property?

BTW the Jaeger RateLimiter Sampler is not listed in the standard configuration options, but it is registered as an SPI, so you would use jaeger_remote to configure it.

Good to know about the SPI mechanism here.
jaeger_remote wouldn't fit though, as I wouldn't want to configure fetching the sampling config from a remote Jaeger server. And the RateLimitingSampler class alongside the JaegerRemoteSampler isn't public.
But I could easily use my own RateLimitingSamplerProvider and RateLimitingSampler, I guess.

[radcortez]

The plan is to support OTel Configuration OOTB.

Ah, good to know. So, that means the actual otel.traces.sampler and otel.traces.sampler.arg (system/env) properties shall be used then? Or will this kind of sampler configuration be mapped to the existing quarkus.opentelemetry.tracer.sampler and an additional quarkus.opentelemetry.tracer.sampler.arg property?

We usually wrap config integrations in our own namespace, so we can benefit from all our configuration mechanisms. I guess we will do something similar here, but most likely we will need to change some names to make the mapping more consistent. For instance otel.traces.sampler should become quarkus.otel.traces.sampler and not quarkus.opentelemetry.tracer.sampler. Anyway, there is no final decision on this yet.

BTW the Jaeger RateLimiter Sampler is not listed in the standard configuration options, but it is registered as an SPI, so you would use jaeger_remote to configure it.

Good to know about the SPI mechanism here. jaeger_remote wouldn't fit though, as I wouldn't want to configure fetching the sampling config from a remote Jaeger server. And the RateLimitingSampler class alongside the JaegerRemoteSampler isn't public. But I could easily use my own RateLimitingSamplerProvider and RateLimitingSampler, I guess.

I've mentioned that one because that is the one that links with the RateLimiter that you mentioned :) Maybe they have more options available (I haven't checked).

[calohmn]

Ok, thanks for the input. Looking forward to these changes then.

[calohmn]

Speaking of the sampler config property names:
When choosing a new property name for the sampler name, I think it should end with ".name" or ".id" then.

I mean, with the current
quarkus.opentelemetry.tracer.sampler and
quarkus.opentelemetry.tracer.sampler.ratio
properties, I don't see how the sampler configuration can be done in a YAML file (along with other quarkus properties), as the sampler field would need both a string and a map value.

[radcortez]

Check here: https://quarkus.io/guides/config-yaml#configuration-key-conflicts