opentelemetry-rust icon indicating copy to clipboard operation
opentelemetry-rust copied to clipboard

[Docs]: Compatibility matrix

Open xd009642 opened this issue 1 year ago • 9 comments

Related Problems?

No response

Describe the solution you'd like:

This is very much just a this-crate issue. But the usage of traits from different otel crates is widespread enough that whenever I attempt to upgrade I end up in some compatibility nightmare trying to get all the versions aligned to the correct one. As far as I see if it, the solution would be either to move versions up lockstep and disregarding semver (or at least broadening it's definition to cover all the various crates), or alternatively document for each opentelemetry version what the latest compatible version for the other crates are.

I'd rather not spend another hour of my life trying to solve issues like the trait JaegerTraceRuntimeis not implemented forTokio`` whenever I upgrade this crate. Unfortunately, the opentelemetry upgrades are the most painful upgrades I've found in the Rust ecosystem :cry:

Considered Alternatives

No response

Additional Context

No response

xd009642 avatar Jul 25 '24 13:07 xd009642

solution would be either to move versions up lockstep and disregarding semver (or at least broadening it's definition to cover all the various crates), or alternatively document for each opentelemetry version what the latest compatible version for the other crates are.

@xd009642 In the current publishing process, we coordinate the versions of all OpenTelemetry-related crates to be updated simultaneously, ensuring compatibility across the board. The tracing-opentelemetry crate is an exception as it is part of tokio ecosystem, and so is upgraded separately afterwards. This could bring the is not implemented for errors in interim. While discussions around #1571 are still ongoing, I agree that there needs to be better synchronization to address these issues.

lalitb avatar Jul 25 '24 20:07 lalitb

vote up for this.

I'm still facing trait issue when every individual package is up to date

# opentelemetry
opentelemetry = "0.24"
opentelemetry_sdk = { version = "0.24.1", features = ["rt-tokio"] }
tracing-opentelemetry = "0.25"
opentelemetry-jaeger-propagator = "0.3"
opentelemetry-otlp = { version = "0.17", features = ["grpc-tonic"] }
error[E0277]: the trait bound `opentelemetry_sdk::trace::TracerProvider: opentelemetry::trace::Tracer` is not satisfied
   --> clash_lib/src/app/logging.rs:119:57
    |
119 |         Some(tracing_opentelemetry::layer().with_tracer(tracer))
    |                                             ----------- ^^^^^^ the trait `opentelemetry::trace::Tracer` is not implemented for `opentelemetry_sdk::trace::TracerProvider`
    |                                             |
    |                                             required by a bound introduced by this call
    |
    = help: the following other types implement trait `opentelemetry::trace::Tracer`:
              BoxedTracer
              NoopTracer
              opentelemetry_sdk::trace::Tracer
note: required by a bound in `OpenTelemetryLayer::<S, T>::with_tracer`
   --> /home/wtf/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tracing-opentelemetry-0.25.0/src/layer.rs:591:17
    |
589 |     pub fn with_tracer<Tracer>(self, tracer: Tracer) -> OpenTelemetryLayer<S, Tracer>
    |            ----------- required by a bound in this associated function
590 |     where
591 |         Tracer: otel::Tracer + PreSampledTracer + 'static,
    |                 ^^^^^^^^^^^^ required by this bound in `OpenTelemetryLayer::<S, T>::with_tracer`

just giving a concrete example

ibigbug avatar Jul 29 '24 04:07 ibigbug

@ibigbug Are you facing issues without tracing-opentelemetry as well? If the issue is when tracing-opentelemetry is involved, then not much we can do in this repo. The final answer depends on resolving https://github.com/open-telemetry/opentelemetry-rust/issues/1571

cijothomas avatar Jul 29 '24 16:07 cijothomas

@cijothomas the thing is there is something you can do, if for every version you release you have a table of the versions of each ecosystem crate that is compatible. We can go back to the crates.io or docs.rs page for the opentelemetry version tracing-opentelemetry is using and see what versions we have to use.

xd009642 avatar Jul 29 '24 16:07 xd009642

 Some(tracing_opentelemetry::layer().with_tracer(tracer))
    |                                             ----------- ^^^^^^ the trait `opentelemetry::trace::Tracer` is not implemented for `opentelemetry_sdk::trace::TracerProvider`
    |                                             |
    |                                             required by a bound introduced by this call

@ibigbug You need to modify your code as the opentelemetry_otlp::new_pipeline() now returns TracerProvider instead of Tracer. Please follow the example here.

lalitb avatar Jul 29 '24 16:07 lalitb

@cijothomas the thing is there is something you can do, if for every version you release you have a table of the versions of each ecosystem crate that is compatible. We can go back to the crates.io or docs.rs page for the opentelemetry version tracing-opentelemetry is using and see what versions we have to use.

I don't support the idea of opentelemetry repo (this repo) documenting the compatibility matrix of other ecosystem crates, in particular tracing-opentelemetry, without formally deciding and closing https://github.com/open-telemetry/opentelemetry-rust/issues/1571.

From my observation, I am not even sure if tracing-opentelemetry maintainers are interested in actively reviewing PRs in this repo or they plan to ignore opentelemetry and write an otlp exporter directly https://github.com/open-telemetry/opentelemetry-rust/pull/1937

The current state is highly unacceptable. Once #1571 is resolved, do you still see more compat issues? @TommyCpp and other maintainers (including me), were discussing making an equivalent of tracing-opentelemetry right in this repo's sdk itself

cijothomas avatar Jul 29 '24 16:07 cijothomas

Oh no I just want you to document your own compatibility. opentelemetry_sdk, opentelemetry-semantic-conventions, opentelemetry-otlp, opentelemetry-contrib, opentelemetry, opentelemetry-http etc etc etc. Most of my trait mismatches have came with the interplay between those crates. Matching tracing_opentelemetry to opentelemetry is fairly simple but then going back in time and getting all of those matched takes a lot of effort.

xd009642 avatar Jul 29 '24 16:07 xd009642

I see.. Thanks for clarifying. Is the below reply from @lalitb good enough? i.e we already release all crates together. (sorry about missing zipkin crate by mistake). Is that not sufficient?

@xd009642 In the current publishing process, we coordinate the versions of all OpenTelemetry-related crates to be updated simultaneously, ensuring compatibility across the board.

Stepping back - is the issue really because we are breaking things in every release? (Something we have an end in sight!)

cijothomas avatar Jul 29 '24 17:07 cijothomas

I find the issue personally, is the trait impedance mismatch in rust. So as some of the core traits may be reimplemented in different places and then consumed by other traits the error messages don't necessarily point me to the crate causing the actual issue. The APIs may be otherwise compatible but monomorphisation is more fussy than a programmer

xd009642 avatar Jul 29 '24 17:07 xd009642

close with #2084

TommyCpp avatar Sep 09 '24 23:09 TommyCpp

@xd009642 Please let us know if you have more suggestions on this. We just unified the version across all crates in this repo

cijothomas avatar Sep 10 '24 02:09 cijothomas