opentelemetry: Add PreSampledTracer interface #962
Conversation
## Motivation Currently the `OpenTelemetryLayer` is coupled to the opentelemetry SDK in a way that makes using alternate API implementations difficult (they would have to use the SDK's sampler and id generator) as well as causing duplication when specifying sampler configuration (it must currently be set on both the tracer provider configuration as well as the layer configuration, which is difficult to remember to keep in sync). ## Solution This change creates an interface for working with pre-sampled tracers, and implements it for the current opentelemetry SDK. This simplifies the layer as it no longer needs to track an id generator or sampler, and allows other otel API implementations to implement the trait rather than rely on the current SDK.
| pub fn new<Sampler>(tracer: T, sampler: Sampler) -> Self | ||
| where | ||
| Sampler: sdk::ShouldSample + 'static, | ||
| { |
There was a problem hiding this comment.
This is a breaking API change --- does it make sense to leave the new constructor as-is, and add a new constructor that takes just a tracer, or does that not make sense?
There was a problem hiding this comment.
Could keep as is for a deprecation cycle, the sampler is no longer needed but the method could still accept one.
There was a problem hiding this comment.
Actually deprecation isn't really what we want here unless we want another name, but with_tracer already exists. Maybe best to just go with the breaking change as it is trivial to not include the parameter and it would only be called once during initialization.
There was a problem hiding this comment.
Okay, I'm fine with the breaking change if it's the best option.
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Motivation
Currently the
OpenTelemetryLayeris coupled to the opentelemetry SDK in a way that makes using alternate API implementations difficult (they would have to use the SDK's sampler and id generator) as well as causing duplication when specifying sampler configuration (it must currently be set on both the tracer provider configuration as well as the layer configuration, which is difficult to remember to keep in sync).Solution
This change creates an interface for working with pre-sampled tracers, and implements it for the current opentelemetry SDK. This simplifies the layer as it no longer needs to track an id generator or sampler, and allows other otel API implementations to implement the trait rather than rely on the current SDK.