Merge branch 'main' into refactor-baggage-context

Author: TommyCpp

Cargo.toml

@@ -85,5 +85,12 @@ opentelemetry = { path = "opentelemetry" }
 opentelemetry_sdk = { path = "opentelemetry-sdk" }
 opentelemetry-stdout = { path = "opentelemetry-stdout" }
 
+[workspace.lints.rust]
+rust_2024_compatibility = { level = "warn", priority = -1 }
+# No need to enable those, because it either not needed or results in ugly syntax
+edition_2024_expr_fragment_specifier = "allow"
+if_let_rescope = "allow"
+tail_expr_drop_order = "allow"
+
 [workspace.lints.clippy]
 all = { level = "warn", priority = 1 }

docs/design/logs.md

@@ -296,6 +296,19 @@ convey that decision back to logger, allowing appender to avoid even the cost of
 creating a `LogRecord` in the first place if there is no listener. This check is
 done for each log emission, and can react dynamically to changes in interest, by
 enabling/disabling ETW/user-event listener.
+5. `tracing` has a notion of "target", which is expected to be mapped to OTel's
+concept of Instrumentation Scope for Logs, when `OpenTelemetry-Tracing-Appender`
+bridges `tracing` to OpenTelemetry. Since scopes are tied to Loggers, a naive
+approach would require creating a separate logger for each unique target. This
+would necessitate an RWLock-protected HashMap lookup, introducing contention and
+reducing throughput. To avoid this, `OpenTelemetry-Tracing-Appender` instead
+stores the target directly in the LogRecord as a top-level field, ensuring fast
+access in the hot path. Components processing the LogRecord can retrieve the
+target via LogRecord.target(), treating it as the scope. The OTLP Exporter
+already handles this automatically, so end-users will see “target” reflected in
+the Instrumentation Scope. An alternative design would be to use thread-local
+HashMaps - but it can cause increased memory usage, as there can be 100s of
+unique targets. (because `tracing` defaults to using module path as target).
 
 ### Perf test - benchmarks
 

opentelemetry-appender-tracing/CHANGELOG.md

@@ -31,6 +31,13 @@ simple string, but require format arguments as in the below example.
 error!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "[email protected]", "This is an example message with format arguments {} and {}", "foo", "bar");
 ```
 
+Fixes [2658](https://github.com/open-telemetry/opentelemetry-rust/issues/2658)
+InstrumentationScope(Logger) used by the appender now uses an empty ("") named
+Logger. Previously, a Logger with name and version of the crate was used.
+Receivers (processors, exporters) are expected to use `LogRecord.target()` as
+scope name. This is already done in OTLP Exporters, so this change should be
+transparent to most users.
+
 ## 0.28.1
 
 Released 2025-Feb-12

opentelemetry-appender-tracing/src/layer.rs

@@ -1,17 +1,14 @@
 use opentelemetry::{
     logs::{AnyValue, LogRecord, Logger, LoggerProvider, Severity},
-    InstrumentationScope, Key,
+    Key,
 };
-use std::borrow::Cow;
 use tracing_core::Level;
 #[cfg(feature = "experimental_metadata_attributes")]
 use tracing_core::Metadata;
 #[cfg(feature = "experimental_metadata_attributes")]
 use tracing_log::NormalizeEvent;
 use tracing_subscriber::{registry::LookupSpan, Layer};
 
-const INSTRUMENTATION_LIBRARY_NAME: &str = "opentelemetry-appender-tracing";
-
 /// Visitor to record the fields from the event record.
 struct EventVisitor<'a, LR: LogRecord> {
     log_record: &'a mut LR,
@@ -135,12 +132,13 @@ where
     L: Logger + Send + Sync,
 {
     pub fn new(provider: &P) -> Self {
-        let scope = InstrumentationScope::builder(INSTRUMENTATION_LIBRARY_NAME)
-            .with_version(Cow::Borrowed(env!("CARGO_PKG_VERSION")))
-            .build();
-
         OpenTelemetryTracingBridge {
-            logger: provider.logger_with_scope(scope),
+            // Using empty scope name.
+            // The name/version of this library itself can be added
+            // as a Scope attribute, once a semantic convention is
+            // defined for the same.
+            // See https://github.com/open-telemetry/semantic-conventions/issues/1550
+            logger: provider.logger(""),
             _phantom: Default::default(),
         }
     }
@@ -245,6 +243,7 @@ mod tests {
             .any(|(k, v)| k == key && v == value)
     }
 
+    #[allow(impl_trait_overcaptures)] // can only be fixed with Rust 1.82+
     fn create_tracing_subscriber(
         _exporter: InMemoryLogExporter,
         logger_provider: &SdkLoggerProvider,
@@ -348,8 +347,18 @@ mod tests {
             .expect("Atleast one log is expected to be present.");
 
         // Validate common fields
-        assert_eq!(log.instrumentation.name(), "opentelemetry-appender-tracing");
+        assert_eq!(log.instrumentation.name(), "");
         assert_eq!(log.record.severity_number(), Some(Severity::Error));
+        // Validate target
+        assert_eq!(
+            log.record.target().expect("target is expected").to_string(),
+            "my-system"
+        );
+        // Validate event name
+        assert_eq!(
+            log.record.event_name().expect("event_name is expected"),
+            "my-event-name"
+        );
 
         // Validate trace context is none.
         assert!(log.record.trace_context().is_none());
@@ -398,6 +407,39 @@ mod tests {
             assert!(attributes_key.contains(&Key::new("code.lineno")));
             assert!(!attributes_key.contains(&Key::new("log.target")));
         }
+
+        // Test when target, eventname are not explicitly provided
+        exporter.reset();
+        error!(
+            event_id = 20,
+            user_name = "otel",
+            user_email = "[email protected]"
+        );
+        assert!(logger_provider.force_flush().is_ok());
+
+        // Assert TODO: move to helper methods
+        let exported_logs = exporter
+            .get_emitted_logs()
+            .expect("Logs are expected to be exported.");
+        assert_eq!(exported_logs.len(), 1);
+        let log = exported_logs
+            .first()
+            .expect("Atleast one log is expected to be present.");
+
+        // Validate target - tracing defaults to module path
+        assert_eq!(
+            log.record.target().expect("target is expected").to_string(),
+            "opentelemetry_appender_tracing::layer::tests"
+        );
+        // Validate event name - tracing defaults to event followed source & line number
+        // Assert is doing "contains" check to avoid tests failing when line number changes.
+        // and also account for the fact that the module path is different on different platforms.
+        // Ex.: The path will be different on a Windows and Linux machine.
+        assert!(log
+            .record
+            .event_name()
+            .expect("event_name is expected")
+            .contains("event opentelemetry-appender-tracing"),);
     }
 
     #[test]
@@ -442,8 +484,18 @@ mod tests {
             .expect("Atleast one log is expected to be present.");
 
         // validate common fields.
-        assert_eq!(log.instrumentation.name(), "opentelemetry-appender-tracing");
+        assert_eq!(log.instrumentation.name(), "");
         assert_eq!(log.record.severity_number(), Some(Severity::Error));
+        // Validate target
+        assert_eq!(
+            log.record.target().expect("target is expected").to_string(),
+            "my-system"
+        );
+        // Validate event name
+        assert_eq!(
+            log.record.event_name().expect("event_name is expected"),
+            "my-event-name"
+        );
 
         // validate trace context.
         assert!(log.record.trace_context().is_some());
@@ -583,7 +635,7 @@ mod tests {
         drop(tracing_log::LogTracer::init());
 
         // Act
-        log::error!(target: "my-system", "log from log crate");
+        log::error!("log from log crate");
         assert!(logger_provider.force_flush().is_ok());
 
         // Assert TODO: move to helper methods
@@ -596,8 +648,19 @@ mod tests {
             .expect("Atleast one log is expected to be present.");
 
         // Validate common fields
-        assert_eq!(log.instrumentation.name(), "opentelemetry-appender-tracing");
+        assert_eq!(log.instrumentation.name(), "");
         assert_eq!(log.record.severity_number(), Some(Severity::Error));
+        // Target and EventName from Log crate are "log" and "log event" respectively.
+        // Validate target
+        assert_eq!(
+            log.record.target().expect("target is expected").to_string(),
+            "log"
+        );
+        // Validate event name
+        assert_eq!(
+            log.record.event_name().expect("event_name is expected"),
+            "log event"
+        );
 
         // Validate trace context is none.
         assert!(log.record.trace_context().is_none());
@@ -675,7 +738,7 @@ mod tests {
             .expect("Atleast one log is expected to be present.");
 
         // validate common fields.
-        assert_eq!(log.instrumentation.name(), "opentelemetry-appender-tracing");
+        assert_eq!(log.instrumentation.name(), "");
         assert_eq!(log.record.severity_number(), Some(Severity::Error));
 
         // validate trace context.

opentelemetry-sdk/src/logs/mod.rs

@@ -35,10 +35,11 @@ pub mod log_processor_with_async_runtime;
 mod tests {
     use super::*;
     use crate::Resource;
+    use opentelemetry::baggage::BaggageExt;
     use opentelemetry::logs::LogRecord;
     use opentelemetry::logs::{Logger, LoggerProvider, Severity};
-    use opentelemetry::InstrumentationScope;
     use opentelemetry::{logs::AnyValue, Key, KeyValue};
+    use opentelemetry::{Context, InstrumentationScope};
     use std::borrow::Borrow;
     use std::collections::HashMap;
 
@@ -150,4 +151,63 @@ mod tests {
             .attributes()
             .eq(&[KeyValue::new("test_k", "test_v")]));
     }
+
+    #[derive(Debug)]
+    struct EnrichWithBaggageProcessor;
+    impl LogProcessor for EnrichWithBaggageProcessor {
+        fn emit(&self, data: &mut SdkLogRecord, _instrumentation: &InstrumentationScope) {
+            Context::map_current(|cx| {
+                for (kk, vv) in cx.baggage().iter() {
+                    data.add_attribute(kk.clone(), vv.0.clone());
+                }
+            });
+        }
+
+        fn force_flush(&self) -> crate::error::OTelSdkResult {
+            Ok(())
+        }
+
+        fn shutdown(&self) -> crate::error::OTelSdkResult {
+            Ok(())
+        }
+    }
+    #[test]
+    fn log_and_baggage() {
+        // Arrange
+        let exporter: InMemoryLogExporter = InMemoryLogExporter::default();
+        let logger_provider = SdkLoggerProvider::builder()
+            .with_log_processor(EnrichWithBaggageProcessor)
+            .with_log_processor(SimpleLogProcessor::new(exporter.clone()))
+            .build();
+
+        // Act
+        let logger = logger_provider.logger("test-logger");
+        let context_with_baggage =
+            Context::current_with_baggage(vec![KeyValue::new("key-from-bag", "value-from-bag")]);
+        let _cx_guard = context_with_baggage.attach();
+        let mut log_record = logger.create_log_record();
+        log_record.add_attribute("key", "value");
+        logger.emit(log_record);
+
+        // Assert
+        let exported_logs = exporter
+            .get_emitted_logs()
+            .expect("Logs are expected to be exported.");
+        assert_eq!(exported_logs.len(), 1);
+        let log = exported_logs
+            .first()
+            .expect("Atleast one log is expected to be present.");
+        assert_eq!(log.instrumentation.name(), "test-logger");
+        assert_eq!(log.record.attributes_len(), 2);
+
+        // Assert that the log record contains the baggage attribute
+        // and the attribute added to the log record.
+        assert!(log
+            .record
+            .attributes_contains(&Key::new("key"), &AnyValue::String("value".into())));
+        assert!(log.record.attributes_contains(
+            &Key::new("key-from-bag"),
+            &AnyValue::String("value-from-bag".into())
+        ));
+    }
 }

opentelemetry-sdk/src/trace/id_generator/mod.rs

@@ -22,11 +22,11 @@ pub struct RandomIdGenerator {
 
 impl IdGenerator for RandomIdGenerator {
     fn new_trace_id(&self) -> TraceId {
-        CURRENT_RNG.with(|rng| TraceId::from(rng.borrow_mut().gen::<u128>()))
+        CURRENT_RNG.with(|rng| TraceId::from(rng.borrow_mut().random::<u128>()))
     }
 
     fn new_span_id(&self) -> SpanId {
-        CURRENT_RNG.with(|rng| SpanId::from(rng.borrow_mut().gen::<u64>()))
+        CURRENT_RNG.with(|rng| SpanId::from(rng.borrow_mut().random::<u64>()))
     }
 }
 

opentelemetry/src/baggage.rs

@@ -13,6 +13,10 @@
 //! Baggage can be sent between systems using a baggage propagator in
 //! accordance with the [W3C Baggage] specification.
 //!
+//! Note: Baggage is not automatically added to any telemetry. Users have to
+//! explicitly add baggage entries to telemetry items.
+//!
+//!
 //! [W3C Baggage]: https://w3c.github.io/baggage
 use crate::{Context, Key, KeyValue, StringValue};
 use std::collections::hash_map::Entry;
@@ -75,10 +79,10 @@ impl Baggage {
     /// ```
     /// use opentelemetry::{baggage::Baggage, StringValue};
     ///
-    /// let mut cc = Baggage::new();
-    /// let _ = cc.insert("my-name", "my-value");
+    /// let mut baggage = Baggage::new();
+    /// let _ = baggage.insert("my-name", "my-value");
     ///
-    /// assert_eq!(cc.get("my-name"), Some(&StringValue::from("my-value")))
+    /// assert_eq!(baggage.get("my-name"), Some(&StringValue::from("my-value")))
     /// ```
     pub fn get<K: AsRef<str>>(&self, key: K) -> Option<&StringValue> {
         self.inner.get(key.as_ref()).map(|(value, _metadata)| value)
@@ -90,11 +94,11 @@ impl Baggage {
     /// ```
     /// use opentelemetry::{baggage::{Baggage, BaggageMetadata}, StringValue};
     ///
-    /// let mut cc = Baggage::new();
-    /// let _ = cc.insert("my-name", "my-value");
+    /// let mut baggage = Baggage::new();
+    /// let _ = baggage.insert("my-name", "my-value");
     ///
     /// // By default, the metadata is empty
-    /// assert_eq!(cc.get_with_metadata("my-name"), Some(&(StringValue::from("my-value"), BaggageMetadata::from(""))))
+    /// assert_eq!(baggage.get_with_metadata("my-name"), Some(&(StringValue::from("my-value"), BaggageMetadata::from(""))))
     /// ```
     pub fn get_with_metadata<K: AsRef<str>>(
         &self,
@@ -113,10 +117,10 @@ impl Baggage {
     /// ```
     /// use opentelemetry::{baggage::Baggage, StringValue};
     ///
-    /// let mut cc = Baggage::new();
-    /// let _ = cc.insert("my-name", "my-value");
+    /// let mut baggage = Baggage::new();
+    /// let _ = baggage.insert("my-name", "my-value");
     ///
-    /// assert_eq!(cc.get("my-name"), Some(&StringValue::from("my-value")))
+    /// assert_eq!(baggage.get("my-name"), Some(&StringValue::from("my-value")))
     /// ```
     pub fn insert<K, V>(&mut self, key: K, value: V) -> Option<StringValue>
     where
@@ -127,7 +131,7 @@ impl Baggage {
             .map(|pair| pair.0)
     }
 
-    /// Inserts a name/value pair into the baggage.
+    /// Inserts a name/value(+metadata) pair into the baggage.
     ///
     /// Same with `insert`, if the name was not present, [`None`] will be returned.
     /// If the name is present, the old value and metadata will be returned.
@@ -139,10 +143,10 @@ impl Baggage {
     /// ```
     /// use opentelemetry::{baggage::{Baggage, BaggageMetadata}, StringValue};
     ///
-    /// let mut cc = Baggage::new();
-    /// let _ = cc.insert_with_metadata("my-name", "my-value", "test");
+    /// let mut baggage = Baggage::new();
+    /// let _ = baggage.insert_with_metadata("my-name", "my-value", "test");
     ///
-    /// assert_eq!(cc.get_with_metadata("my-name"), Some(&(StringValue::from("my-value"), BaggageMetadata::from("test"))))
+    /// assert_eq!(baggage.get_with_metadata("my-name"), Some(&(StringValue::from("my-value"), BaggageMetadata::from("test"))))
     /// ```
     pub fn insert_with_metadata<K, V, S>(
         &mut self,

stress/src/throughput.rs

@@ -155,7 +155,7 @@ impl<'a> UnsafeSlice<'a> {
     #[inline(always)]
     unsafe fn increment(&self, i: usize) {
         let value = self.slice[i].get();
-        (*value).count += 1;
+        unsafe { (*value).count += 1 };
     }
 
     #[inline(always)]