fixup! fixup! fixup! fixup! refactor(log): reimplement log using tracing

Author: rami3l

src/bin/rustup-init.rs

@@ -56,7 +56,15 @@ async fn maybe_trace_rustup() -> Result<utils::ExitCode> {
     use tracing_subscriber::{layer::SubscriberExt, Registry};
 
     #[cfg(feature = "otel")]
-    let telemetry = rustup::cli::log::telemetry()?;
+    let telemetry = {
+        use rustup::cli::log::{telemetry, telemetry_default_tracer};
+
+        use opentelemetry::global;
+        use opentelemetry_sdk::propagation::TraceContextPropagator;
+
+        global::set_text_map_propagator(TraceContextPropagator::new());
+        telemetry(telemetry_default_tracer()?)
+    };
     let console_logger = rustup::cli::log::console_logger(process());
     let subscriber = {
         #[cfg(feature = "otel")]

src/cli/log.rs

@@ -69,23 +69,29 @@ where
 /// A [`tracing::Subscriber`] [`Layer`][`tracing_subscriber::Layer`] that corresponds to Rustup's
 /// optional `opentelemetry` (a.k.a. `otel`) feature.
 #[cfg(feature = "otel")]
-pub fn telemetry<S>() -> anyhow::Result<impl Layer<S>>
+pub fn telemetry<S>(tracer: opentelemetry_sdk::trace::Tracer) -> impl Layer<S>
 where
     S: Subscriber + for<'span> LookupSpan<'span>,
 {
+    // NOTE: This reads from the real environment variables instead of `process().var_os()`.
+    let env_filter = EnvFilter::try_from_default_env().unwrap_or(EnvFilter::new("INFO"));
+    tracing_opentelemetry::layer()
+        .with_tracer(tracer)
+        .with_filter(env_filter)
+}
+
+#[cfg(feature = "otel")]
+pub fn telemetry_default_tracer() -> anyhow::Result<opentelemetry_sdk::trace::Tracer> {
     use std::time::Duration;
 
-    use opentelemetry::{global, KeyValue};
+    use opentelemetry::KeyValue;
     use opentelemetry_otlp::WithExportConfig;
     use opentelemetry_sdk::{
-        propagation::TraceContextPropagator,
         trace::{self, Sampler},
         Resource,
     };
 
-    global::set_text_map_propagator(TraceContextPropagator::new());
-
-    let tracer = opentelemetry_otlp::new_pipeline()
+    Ok(opentelemetry_otlp::new_pipeline()
         .tracing()
         .with_exporter(
             opentelemetry_otlp::new_exporter()
@@ -97,12 +103,7 @@ where
                 .with_sampler(Sampler::AlwaysOn)
                 .with_resource(Resource::new(vec![KeyValue::new("service.name", "rustup")])),
         )
-        .install_batch(opentelemetry_sdk::runtime::Tokio)?;
-    // NOTE: This reads from the real environment variables instead of `process().var_os()`.
-    let env_filter = EnvFilter::try_from_default_env().unwrap_or(EnvFilter::new("INFO"));
-    Ok(tracing_opentelemetry::layer()
-        .with_tracer(tracer)
-        .with_filter(env_filter))
+        .install_batch(opentelemetry_sdk::runtime::Tokio)?)
 }
 
 // Adapted from

src/currentprocess.rs

@@ -165,10 +165,17 @@ fn ensure_hook() {
 fn tracing_subscriber() -> impl tracing::Subscriber {
     use tracing_subscriber::{layer::SubscriberExt, Registry};
 
+    #[cfg(feature = "otel")]
+    let telemetry = crate::cli::log::telemetry(crate::test::TRACER.clone());
     let console_logger = crate::cli::log::console_logger(process());
-
-    // TODO: What about the original otel logger?
-    Registry::default().with(console_logger)
+    #[cfg(feature = "otel")]
+    {
+        Registry::default().with(console_logger).with(telemetry)
+    }
+    #[cfg(not(feature = "otel"))]
+    {
+        Registry::default().with(console_logger)
+    }
 }
 
 /// Run a function in the context of a process definition and a tokio runtime.

src/test.rs

@@ -238,70 +238,37 @@ type Tracer = opentelemetry_sdk::trace::Tracer;
 type Tracer = ();
 
 /// A tracer for the tests.
-static TRACER: Lazy<Tracer> = Lazy::new(|| {
-    use tokio::runtime::Handle;
+pub(crate) static TRACER: Lazy<Tracer> = Lazy::new(|| {
+    #[cfg(feature = "otel")]
+    {
+        use tokio::runtime::Handle;
 
-    // Use the current runtime, or the sync test runtime otherwise.
-    let handle = match Handle::try_current() {
-        Ok(handle) => handle,
-        Err(_) => TRACE_RUNTIME.handle().clone(),
-    };
+        // Use the current runtime, or the sync test runtime otherwise.
+        let handle = match Handle::try_current() {
+            Ok(handle) => handle,
+            Err(_) => TRACE_RUNTIME.handle().clone(),
+        };
 
-    let _guard = handle.enter();
+        let _guard = handle.enter();
+
+        use crate::cli::log::{telemetry, telemetry_default_tracer};
 
-    #[cfg(feature = "otel")]
-    let tracer = {
-        use std::time::Duration;
-
-        use opentelemetry::KeyValue;
-        use opentelemetry_otlp::WithExportConfig;
-        use opentelemetry_sdk::{
-            trace::{self, Sampler},
-            Resource,
-        };
-        opentelemetry_otlp::new_pipeline()
-            .tracing()
-            .with_exporter(
-                opentelemetry_otlp::new_exporter()
-                    .tonic()
-                    .with_timeout(Duration::from_secs(3)),
-            )
-            .with_trace_config(
-                trace::config()
-                    .with_sampler(Sampler::AlwaysOn)
-                    .with_resource(Resource::new(vec![KeyValue::new("service.name", "rustup")])),
-            )
-            .install_batch(opentelemetry_sdk::runtime::Tokio)
-            .unwrap()
-    };
-    #[cfg(feature = "otel")]
-    let telemetry = {
         use opentelemetry::global;
         use opentelemetry_sdk::propagation::TraceContextPropagator;
-        use tracing_subscriber::{EnvFilter, Layer};
+        use tracing_subscriber::{layer::SubscriberExt as _, Registry};
 
         global::set_text_map_propagator(TraceContextPropagator::new());
-
-        let env_filter = EnvFilter::try_from_default_env().unwrap_or(EnvFilter::new("INFO"));
-        tracing_opentelemetry::layer()
-            .with_tracer(tracer.clone())
-            .with_filter(env_filter)
-    };
-    let subscriber = {
-        use tracing_subscriber::Registry;
-        #[cfg(feature = "otel")]
-        {
-            use tracing_subscriber::layer::SubscriberExt;
-            Registry::default().with(telemetry)
-        }
-        #[cfg(not(feature = "otel"))]
-        {
-            Registry::default()
-        }
-    };
-    tracing::subscriber::set_global_default(subscriber).unwrap();
-    #[cfg(feature = "otel")]
-    tracer
+        let tracer = telemetry_default_tracer().unwrap();
+        let telemetry = telemetry(tracer.clone());
+        let subscriber = Registry::default().with(telemetry);
+        tracing::subscriber::set_global_default(subscriber).unwrap();
+        tracer
+    }
+    #[cfg(not(feature = "otel"))]
+    {
+        let subscriber = Registry::default();
+        tracing::subscriber::set_global_default(subscriber).unwrap();
+    }
 });
 
 pub fn before_test() {