Merge branch 'main' into feat/otel_0.26_promo

Author: cijothomas

opentelemetry-appender-tracing/Cargo.toml

@@ -26,6 +26,7 @@ tracing-subscriber = { workspace = true, features = ["registry", "std", "env-fil
 tracing-log = "0.2"
 async-trait = { workspace = true }
 criterion = { workspace = true }
+tokio = { workspace = true, features = ["full"]}
 
 [target.'cfg(not(target_os = "windows"))'.dev-dependencies]
 pprof = { version = "0.13", features = ["flamegraph", "criterion"] }

opentelemetry-appender-tracing/src/layer.rs

@@ -208,17 +208,20 @@ const fn severity_of_level(level: &Level) -> Severity {
 #[cfg(test)]
 mod tests {
     use crate::layer;
-    use opentelemetry::logs::Severity;
+    use async_trait::async_trait;
+    use opentelemetry::logs::{LogResult, Severity};
     use opentelemetry::trace::TracerProvider as _;
     use opentelemetry::trace::{TraceContextExt, TraceFlags, Tracer};
     use opentelemetry::{logs::AnyValue, Key};
+    use opentelemetry_sdk::export::logs::{LogBatch, LogExporter};
     use opentelemetry_sdk::logs::{LogRecord, LoggerProvider};
     use opentelemetry_sdk::testing::logs::InMemoryLogsExporter;
     use opentelemetry_sdk::trace;
     use opentelemetry_sdk::trace::{Sampler, TracerProvider};
-    use tracing::error;
+    use tracing::{error, warn};
     use tracing_subscriber::prelude::__tracing_subscriber_SubscriberExt;
-    use tracing_subscriber::Layer;
+    use tracing_subscriber::util::SubscriberInitExt;
+    use tracing_subscriber::{EnvFilter, Layer};
 
     pub fn attributes_contains(log_record: &LogRecord, key: &Key, value: &AnyValue) -> bool {
         log_record
@@ -238,6 +241,70 @@ mod tests {
     }
 
     // cargo test --features=testing
+
+    #[derive(Clone, Debug, Default)]
+    struct ReentrantLogExporter;
+
+    #[async_trait]
+    impl LogExporter for ReentrantLogExporter {
+        async fn export(&mut self, _batch: LogBatch<'_>) -> LogResult<()> {
+            // This will cause a deadlock as the export itself creates a log
+            // while still within the lock of the SimpleLogProcessor.
+            warn!(name: "my-event-name", target: "reentrant", event_id = 20, user_name = "otel", user_email = "[email protected]");
+            Ok(())
+        }
+    }
+
+    #[test]
+    #[ignore = "See issue: https://github.com/open-telemetry/opentelemetry-rust/issues/1745"]
+    fn simple_processor_deadlock() {
+        let exporter: ReentrantLogExporter = ReentrantLogExporter;
+        let logger_provider = LoggerProvider::builder()
+            .with_simple_exporter(exporter.clone())
+            .build();
+
+        let layer = layer::OpenTelemetryTracingBridge::new(&logger_provider);
+
+        // Setting subscriber as global as that is the only way to test this scenario.
+        tracing_subscriber::registry().with(layer).init();
+        warn!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "[email protected]");
+    }
+
+    #[test]
+    #[ignore = "While this test runs fine, this uses global subscriber and does not play well with other tests."]
+    fn simple_processor_no_deadlock() {
+        let exporter: ReentrantLogExporter = ReentrantLogExporter;
+        let logger_provider = LoggerProvider::builder()
+            .with_simple_exporter(exporter.clone())
+            .build();
+
+        let layer = layer::OpenTelemetryTracingBridge::new(&logger_provider);
+
+        // This filter will prevent the deadlock as the reentrant log will be
+        // ignored.
+        let filter = EnvFilter::new("debug").add_directive("reentrant=error".parse().unwrap());
+        // Setting subscriber as global as that is the only way to test this scenario.
+        tracing_subscriber::registry()
+            .with(filter)
+            .with(layer)
+            .init();
+        warn!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "[email protected]");
+    }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+    #[ignore = "While this test runs fine, this uses global subscriber and does not play well with other tests."]
+    async fn batch_processor_no_deadlock() {
+        let exporter: ReentrantLogExporter = ReentrantLogExporter;
+        let logger_provider = LoggerProvider::builder()
+            .with_batch_exporter(exporter.clone(), opentelemetry_sdk::runtime::Tokio)
+            .build();
+
+        let layer = layer::OpenTelemetryTracingBridge::new(&logger_provider);
+
+        tracing_subscriber::registry().with(layer).init();
+        warn!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "[email protected]");
+    }
+
     #[test]
     fn tracing_appender_standalone() {
         // Arrange

opentelemetry-sdk/src/metrics/meter.rs

@@ -4,7 +4,6 @@ use std::{borrow::Cow, sync::Arc};
 use opentelemetry::{
     global,
     metrics::{
-        noop::{NoopAsyncInstrument, NoopSyncInstrument},
         AsyncInstrumentBuilder, Counter, Gauge, Histogram, HistogramBuilder, InstrumentBuilder,
         InstrumentProvider, MetricsError, ObservableCounter, ObservableGauge,
         ObservableUpDownCounter, Result, UpDownCounter,
@@ -18,6 +17,8 @@ use crate::metrics::{
     pipeline::{Pipelines, Resolver},
 };
 
+use super::noop::{NoopAsyncInstrument, NoopSyncInstrument};
+
 // maximum length of instrument name
 const INSTRUMENT_NAME_MAX_LENGTH: usize = 255;
 // maximum length of instrument unit name

opentelemetry-sdk/src/metrics/meter_provider.rs

@@ -9,13 +9,15 @@ use std::{
 
 use opentelemetry::{
     global,
-    metrics::{noop::NoopMeter, Meter, MeterProvider, MetricsError, Result},
+    metrics::{Meter, MeterProvider, MetricsError, Result},
     KeyValue,
 };
 
 use crate::{instrumentation::Scope, Resource};
 
-use super::{meter::SdkMeter, pipeline::Pipelines, reader::MetricReader, view::View};
+use super::{
+    meter::SdkMeter, noop::NoopMeter, pipeline::Pipelines, reader::MetricReader, view::View,
+};
 
 /// Handles the creation and coordination of [Meter]s.
 ///

opentelemetry-sdk/src/metrics/mod.rs

@@ -47,6 +47,7 @@ pub(crate) mod internal;
 pub(crate) mod manual_reader;
 pub(crate) mod meter;
 mod meter_provider;
+pub(crate) mod noop;
 pub(crate) mod periodic_reader;
 pub(crate) mod pipeline;
 pub mod reader;

opentelemetry-sdk/src/metrics/noop.rs

@@ -0,0 +1,78 @@
+use opentelemetry::{
+    metrics::{
+        AsyncInstrument, InstrumentProvider, SyncCounter, SyncGauge, SyncHistogram,
+        SyncUpDownCounter,
+    },
+    KeyValue,
+};
+
+/// A no-op instance of a `Meter`
+#[derive(Debug, Default)]
+pub(crate) struct NoopMeter {
+    _private: (),
+}
+
+impl NoopMeter {
+    /// Create a new no-op meter core.
+    pub(crate) fn new() -> Self {
+        NoopMeter { _private: () }
+    }
+}
+
+impl InstrumentProvider for NoopMeter {}
+
+/// A no-op sync instrument
+#[derive(Debug, Default)]
+pub(crate) struct NoopSyncInstrument {
+    _private: (),
+}
+
+impl NoopSyncInstrument {
+    /// Create a new no-op sync instrument
+    pub(crate) fn new() -> Self {
+        NoopSyncInstrument { _private: () }
+    }
+}
+
+impl<T> SyncCounter<T> for NoopSyncInstrument {
+    fn add(&self, _value: T, _attributes: &[KeyValue]) {
+        // Ignored
+    }
+}
+
+impl<T> SyncUpDownCounter<T> for NoopSyncInstrument {
+    fn add(&self, _value: T, _attributes: &[KeyValue]) {
+        // Ignored
+    }
+}
+
+impl<T> SyncHistogram<T> for NoopSyncInstrument {
+    fn record(&self, _value: T, _attributes: &[KeyValue]) {
+        // Ignored
+    }
+}
+
+impl<T> SyncGauge<T> for NoopSyncInstrument {
+    fn record(&self, _value: T, _attributes: &[KeyValue]) {
+        // Ignored
+    }
+}
+
+/// A no-op async instrument.
+#[derive(Debug, Default)]
+pub(crate) struct NoopAsyncInstrument {
+    _private: (),
+}
+
+impl NoopAsyncInstrument {
+    /// Create a new no-op async instrument
+    pub(crate) fn new() -> Self {
+        NoopAsyncInstrument { _private: () }
+    }
+}
+
+impl<T> AsyncInstrument<T> for NoopAsyncInstrument {
+    fn observe(&self, _value: T, _attributes: &[KeyValue]) {
+        // Ignored
+    }
+}

opentelemetry/src/common.rs

@@ -227,7 +227,7 @@ pub enum Value {
 }
 
 /// Wrapper for string-like values
-#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
+#[derive(Clone, PartialEq, Eq, Hash)]
 pub struct StringValue(OtelString);
 
 impl fmt::Debug for StringValue {

opentelemetry/src/global/internal_logging.rs

@@ -23,12 +23,20 @@ macro_rules! otel_info {
         {
             tracing::info!( name: $name, target: env!("CARGO_PKG_NAME"), "");
         }
+        #[cfg(not(feature = "internal-logs"))]
+        {
+            let _ = $name; // Compiler will optimize this out as it's unused.
+        }
     };
     (name: $name:expr, $($key:ident = $value:expr),+ $(,)?) => {
         #[cfg(feature = "internal-logs")]
         {
             tracing::info!(name: $name, target: env!("CARGO_PKG_NAME"), $($key = $value),+, "");
         }
+        #[cfg(not(feature = "internal-logs"))]
+        {
+            let _ = ($name, $($value),+); // Compiler will optimize this out as it's unused.
+        }
     };
 }
 
@@ -50,12 +58,20 @@ macro_rules! otel_warn {
         {
             tracing::warn!(name: $name, target: env!("CARGO_PKG_NAME"), "");
         }
+        #[cfg(not(feature = "internal-logs"))]
+        {
+            let _ = $name; // Compiler will optimize this out as it's unused.
+        }
     };
     (name: $name:expr, $($key:ident = $value:expr),+ $(,)?) => {
         #[cfg(feature = "internal-logs")]
         {
             tracing::warn!(name: $name, target: env!("CARGO_PKG_NAME"), $($key = $value),+, "");
         }
+        #[cfg(not(feature = "internal-logs"))]
+        {
+            let _ = ($name, $($value),+); // Compiler will optimize this out as it's unused.
+        }
     };
 }
 
@@ -77,12 +93,20 @@ macro_rules! otel_debug {
         {
             tracing::debug!(name: $name, target: env!("CARGO_PKG_NAME"),"");
         }
+        #[cfg(not(feature = "internal-logs"))]
+        {
+            let _ = $name; // Compiler will optimize this out as it's unused.
+        }
     };
     (name: $name:expr, $($key:ident = $value:expr),+ $(,)?) => {
         #[cfg(feature = "internal-logs")]
         {
             tracing::debug!(name: $name, target: env!("CARGO_PKG_NAME"), $($key = $value),+, "");
         }
+        #[cfg(not(feature = "internal-logs"))]
+        {
+            let _ = ($name, $($value),+); // Compiler will optimize this out as it's unused.
+        }
     };
 }
 
@@ -104,11 +128,19 @@ macro_rules! otel_error {
         {
             tracing::error!(name: $name, target: env!("CARGO_PKG_NAME"), "");
         }
+        #[cfg(not(feature = "internal-logs"))]
+        {
+            let _ = $name; // Compiler will optimize this out as it's unused.
+        }
     };
     (name: $name:expr, $($key:ident = $value:expr),+ $(,)?) => {
         #[cfg(feature = "internal-logs")]
         {
             tracing::error!(name: $name, target: env!("CARGO_PKG_NAME"), $($key = $value),+, "");
         }
+        #[cfg(not(feature = "internal-logs"))]
+        {
+            let _ = ($name, $($value),+); // Compiler will optimize this out as it's unused.
+        }
     };
 }

opentelemetry/src/global/metrics.rs

@@ -7,7 +7,7 @@ type GlobalMeterProvider = Arc<dyn MeterProvider + Send + Sync>;
 
 /// The global `MeterProvider` singleton.
 static GLOBAL_METER_PROVIDER: Lazy<RwLock<GlobalMeterProvider>> =
-    Lazy::new(|| RwLock::new(Arc::new(metrics::noop::NoopMeterProvider::new())));
+    Lazy::new(|| RwLock::new(Arc::new(crate::metrics::noop::NoopMeterProvider::new())));
 
 /// Sets the given [`MeterProvider`] instance as the current global meter
 /// provider.

opentelemetry/src/metrics/instruments/counter.rs

@@ -10,6 +10,7 @@ pub trait SyncCounter<T> {
 
 /// An instrument that records increasing values.
 #[derive(Clone)]
+#[non_exhaustive]
 pub struct Counter<T>(Arc<dyn SyncCounter<T> + Send + Sync>);
 
 impl<T> fmt::Debug for Counter<T>
@@ -35,6 +36,7 @@ impl<T> Counter<T> {
 
 /// An async instrument that records increasing values.
 #[derive(Clone)]
+#[non_exhaustive]
 pub struct ObservableCounter<T>(Arc<dyn AsyncInstrument<T>>);
 
 impl<T> ObservableCounter<T> {