search

open-telemetry / opentelemetry-rust

The Rust OpenTelemetry implementation
https://opentelemetry.io
Apache License 2.0
1.82k stars 423 forks source link

OTLP Exporter builder issues #1592

Open cijothomas opened 7 months ago

cijothomas commented 7 months ago

Methods like install_batch created unintended side-effects https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-otlp/src/span.rs#L143-L150

  1. It sets global providers, which may not be what user has intended.
  2. It returns a tracer with instrumentation scope set to "opentelemetry-otlp", which is incorrect as well.

The builder methods used in OTLP to setup the pipeline must be revisited to ensure that there are no accidental side effects like above. Also, returning tracer with hardcoded name is defeating the purpose of instrumentation scope, as all tracers will have same scope (and that too incorrect one). This is particularly hard when used with tracer-opentelemetry, as it'll result in traces which all have same scope!

cijothomas commented 7 months ago

I used traces as an example - same/similar issue for all signals exist and need a unified fix.

mattbodd commented 7 months ago

I'm interested in taking this on

NickLarsenNZ commented 5 months ago

Adding a permalink (for the link in the description): https://github.com/open-telemetry/opentelemetry-rust/blob/d5392dc1df13b51553577a51d736eac9b29e7125/opentelemetry-otlp/src/span.rs#L143-L150

cijothomas commented 4 months ago

Both logs, and metrics have completed this cleanup. Neither returns a Logger/Meter, nor set a Global Provider. Tracing is still needing to be fixed to make it consistent with the rest.

TommyCpp commented 4 months ago

I will do it for the tracing pipelines if no one else pick it up within next week

NickLarsenNZ commented 4 months ago

Both logs, and metrics have completed this cleanup. Neither returns a Logger/Meter, nor set a Global Provider. Tracing is still needing to be fixed to make it consistent with the rest.

Is the logs one linked anywhere in this issue? I didn't see it, but am on mobile and maybe missing the obvious.

lalitb commented 4 months ago

Is the logs one linked anywhere in this issue?

Not linked, but it was removed as part of global cleanups in #1691 .

cijothomas commented 4 months ago

Is the logs one linked anywhere in this issue?

Not linked, but it was removed as part of global cleanups in #1691 .

@NickLarsenNZ The logging should not affect end users.. It is only going to affect authors of bridges/appenders.. Curious if you are writing one?

utpilla commented 4 months ago

I can work on this if this still needs help.

cijothomas commented 2 months ago

@ThomsonTan Will see if this is something he can take.