Remove global set_tracer_provider from trace pipelines #156
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #156 +/- ##
=======================================
- Coverage 54.1% 54.0% -0.1%
=======================================
Files 42 42
Lines 6282 6261 -21
=======================================
- Hits 3400 3386 -14
+ Misses 2882 2875 -7 ☔ View full report in Codecov by Sentry. |
| @@ -49,7 +49,7 @@ impl<R: JaegerJsonRuntime> JaegerJsonExporter<R> { | |||
| } | |||
|
|
|||
| /// Install the exporter using the internal provided runtime | |||
| pub fn install_batch(self) -> Tracer { | |||
| pub fn install_batch(self) -> (Tracer, TracerProvider) { | |||
There was a problem hiding this comment.
this need to return the provider only. The caller then can create Tracer using the returned provider.
There was a problem hiding this comment.
Yeah, I thought about that, but my problem is the InstrumentationScope which is defined here and used to setup the tracer. Any advice on how to handle it? Or should I also remove it?
There was a problem hiding this comment.
You can remove it. The instrumentation scope is expected to capture the component emitting the traces, so whoever is emitting traces, they should call provider.tracer("component-name"), and use that tracer, rather than creating a tracer with hardcoded name here. (OpenTelemetry-OTLP was also doing this mistake - it was returning a tracer with hardcode named "otel-otlp", which defeats the purpose of InstrumentationScope, as every telemetry would be associated incorrectly with "otel-otlp", instead of reflecting the component)
There was a problem hiding this comment.
hey @cijothomas I did as asked, please have an other look again.
| @@ -295,7 +295,7 @@ impl DatadogPipelineBuilder { | |||
| .with_attributes(None) | |||
| .build(); | |||
| let tracer = provider.tracer_with_scope(scope); | |||
| let _ = global::set_tracer_provider(provider.clone()); | |||
|
|
|||
There was a problem hiding this comment.
return provider only, not tracer.
Changes
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes