Skip to content

chore(deps): update opentelemetry #4199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 1, 2025
Merged

Conversation

rami3l
Copy link
Member

@rami3l rami3l commented Mar 1, 2025

Supersedes #4182.

Following #4182 (comment), I realized that since otel is not an essential part of our project (it's used in benchmarks for the timeline view only), the following change has brought us no benefits:

opentelemetry::global::shutdown_tracer_provider() is removed.
Now, you should explicitly call shutdown() on the created tracer provider.
[..]
This now makes shutdown consistent across signals.
https://github.com/open-telemetry/opentelemetry-rust/blob/main/docs/migration_0.28.md#tracing-shutdown-changes

... so it'd be nice to just stick to the original behavior of shutdown_tracer_provider(), which is simply replacing the global tracer provider with NoopTracerProvider::new() and dropping the replaced provider immediately:

pub fn set_tracer_provider<P, T, S>(new_provider: P) -> GlobalTracerProvider
where
    S: trace::Span + Send + Sync + 'static,
    T: trace::Tracer<Span = S> + Send + Sync + 'static,
    P: trace::TracerProvider<Tracer = T> + Send + Sync + 'static,
{
    let mut tracer_provider = GLOBAL_TRACER_PROVIDER
        .write()
        .expect("GLOBAL_TRACER_PROVIDER RwLock poisoned");
    mem::replace(
        &mut *tracer_provider,
        GlobalTracerProvider::new(new_provider),
    )
}

pub fn shutdown_tracer_provider() {
    let mut tracer_provider = GLOBAL_TRACER_PROVIDER
        .write()
        .expect("GLOBAL_TRACER_PROVIDER RwLock poisoned");
    let _ = mem::replace(
        &mut *tracer_provider,
        GlobalTracerProvider::new(NoopTracerProvider::new()),
    );
}

https://github.com/open-telemetry/opentelemetry-rust/blob/99d24b7b8ca25652b955678a1109af0f9d65e242/opentelemetry/src/global/trace.rs#L398-L430

Regarding the setup/shutdown ergonomics, I've made a simple guard to bring back (before|after)_test_async() with minimal efforts, so this has also addressed the test-related part of #4195, although it'd probably still be nice to be able to fetch e.g. logger filter levels directly from within Process, so the remainder of that issue stays valid.

@rami3l rami3l requested a review from djc March 1, 2025 03:30
@rami3l rami3l force-pushed the deps/bump-otel branch 2 times, most recently from a5e622c to d5e4d57 Compare March 1, 2025 03:39
Copy link
Contributor

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff!

@rami3l rami3l added this to the 1.28.0 milestone Mar 1, 2025
@rami3l rami3l added this pull request to the merge queue Mar 1, 2025
Merged via the queue into rust-lang:master with commit 580c94f Mar 1, 2025
29 checks passed
@rami3l rami3l deleted the deps/bump-otel branch March 1, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants