This was part of the original plan after #3803, but we probably have put it aside for a while:
In #4182 (comment), I've noticed that the current Process abstraction should probably be extended to allow the existence of a tracing_subscriber global state, so that we can handle graceful shutdown more easily.
Having noticed the clear pattern here:
|
#[cfg(feature = "otel")] |
|
opentelemetry::global::set_text_map_propagator( |
|
opentelemetry_sdk::propagation::TraceContextPropagator::new(), |
|
); |
|
let (subscriber, console_filter) = rustup::cli::log::tracing_subscriber(&process); |
|
tracing::subscriber::set_global_default(subscriber)?; |
|
let result = run_rustup(&process, console_filter).await; |
|
// We're tracing, so block until all spans are exported. |
|
#[cfg(feature = "otel")] |
|
opentelemetry::global::shutdown_tracer_provider(); |
... it should probably be modeled as an RAII guard.
I have also noticed that this should resolve another problem, i.e. the corresponding construct in tests is simply no longer used, as these two functions no longer have a caller:
|
pub async fn before_test_async() { |
|
#[cfg(feature = "otel")] |
|
{ |
|
opentelemetry::global::set_text_map_propagator( |
|
opentelemetry_sdk::propagation::TraceContextPropagator::new(), |
|
); |
|
} |
|
} |
|
|
|
pub async fn after_test_async() { |
|
#[cfg(feature = "otel")] |
|
{ |
|
// We're tracing, so block until all spans are exported. |
|
opentelemetry::global::shutdown_tracer_provider(); |
|
} |
|
} |
This is most probably caused by the removal of rustup_macros (d60f005): indeed, the tests are passing on the CI, but sometimes one might want to run tests with otel on as well.
@djc What do you think?
This was part of the original plan after #3803, but we probably have put it aside for a while:
In #4182 (comment), I've noticed that the current
Processabstraction should probably be extended to allow the existence of atracing_subscriberglobal state, so that we can handle graceful shutdown more easily.Having noticed the clear pattern here:
rustup/src/bin/rustup-init.rs
Lines 44 to 53 in 52e6070
... it should probably be modeled as an RAII guard.
I have also noticed that this should resolve another problem, i.e. the corresponding construct in tests is simply no longer used, as these two functions no longer have a caller:
rustup/src/test.rs
Lines 235 to 250 in 52e6070
This is most probably caused by the removal of
rustup_macros(d60f005): indeed, the tests are passing on the CI, but sometimes one might want to run tests withotelon as well.@djc What do you think?