Merge branch 'main' into cijothomas/otlp-error-return

Author: cijothomas

opentelemetry-appender-tracing/CHANGELOG.md

@@ -42,6 +42,13 @@ transparent to most users.
   implementations (SDK, processor, exporters) to leverage this additional
   information to determine if an event is enabled.
 
+- `u64` and `usize` values are stored as `opentelemetry::logs::AnyValue::Int`
+when conversion is feasible. Otherwise stored as
+`opentelemetry::logs::AnyValue::String`. This avoids unnecessary string
+allocation when values can be represented in their original types.
+
+- perf - small perf improvement by avoiding string allocation of `target`
+
 ## 0.28.1
 
 Released 2025-Feb-12

opentelemetry-appender-tracing/benches/logs.rs

@@ -10,7 +10,7 @@
     | noop_layer_disabled         | 12 ns       |
     | noop_layer_enabled          | 25 ns       |
     | ot_layer_disabled           | 19 ns       |
-    | ot_layer_enabled            | 167 ns      |
+    | ot_layer_enabled            | 155 ns      |
 
     Hardware: Apple M4 Pro
     Total Number of Cores:	14 (10 performance and 4 efficiency)
@@ -20,7 +20,7 @@
     | noop_layer_disabled         | 8 ns       |
     | noop_layer_enabled          | 14 ns       |
     | ot_layer_disabled           | 12 ns       |
-    | ot_layer_enabled            | 186 ns      |
+    | ot_layer_enabled            | 130 ns      |
 */
 
 use criterion::{criterion_group, criterion_main, Criterion};

opentelemetry-appender-tracing/src/layer.rs

@@ -114,6 +114,22 @@ impl<LR: LogRecord> tracing::field::Visit for EventVisitor<'_, LR> {
             .add_attribute(Key::new(field.name()), AnyValue::from(value));
     }
 
+    // TODO: We might need to do similar for record_i128,record_u128 too
+    // to avoid stringification, unless needed.
+    fn record_u64(&mut self, field: &tracing::field::Field, value: u64) {
+        #[cfg(feature = "experimental_metadata_attributes")]
+        if is_duplicated_metadata(field.name()) {
+            return;
+        }
+        if let Ok(signed) = i64::try_from(value) {
+            self.log_record
+                .add_attribute(Key::new(field.name()), AnyValue::from(signed));
+        } else {
+            self.log_record
+                .add_attribute(Key::new(field.name()), AnyValue::from(format!("{value:?}")));
+        }
+    }
+
     // TODO: Remaining field types from AnyValue : Bytes, ListAny, Boolean
 }
 
@@ -173,8 +189,7 @@ where
 
         let mut log_record = self.logger.create_log_record();
 
-        // TODO: Fix heap allocation
-        log_record.set_target(target.to_string());
+        log_record.set_target(target);
         log_record.set_event_name(name);
         log_record.set_severity_number(severity);
         log_record.set_severity_text(metadata.level().as_str());
@@ -331,7 +346,11 @@ mod tests {
         let _guard = tracing::subscriber::set_default(subscriber);
 
         // Act
-        error!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "[email protected]");
+        let small_u64value: u64 = 42;
+        let big_u64value: u64 = u64::MAX;
+        let small_usizevalue: usize = 42;
+        let big_usizevalue: usize = usize::MAX;
+        error!(name: "my-event-name", target: "my-system", event_id = 20, small_u64value, big_u64value, small_usizevalue, big_usizevalue, user_name = "otel", user_email = "[email protected]");
         assert!(logger_provider.force_flush().is_ok());
 
         // Assert TODO: move to helper methods
@@ -362,9 +381,9 @@ mod tests {
 
         // Validate attributes
         #[cfg(not(feature = "experimental_metadata_attributes"))]
-        assert_eq!(log.record.attributes_iter().count(), 3);
-        #[cfg(feature = "experimental_metadata_attributes")]
         assert_eq!(log.record.attributes_iter().count(), 7);
+        #[cfg(feature = "experimental_metadata_attributes")]
+        assert_eq!(log.record.attributes_iter().count(), 11);
         assert!(attributes_contains(
             &log.record,
             &Key::new("event_id"),
@@ -380,6 +399,26 @@ mod tests {
             &Key::new("user_email"),
             &AnyValue::String("[email protected]".into())
         ));
+        assert!(attributes_contains(
+            &log.record,
+            &Key::new("small_u64value"),
+            &AnyValue::Int(42.into())
+        ));
+        assert!(attributes_contains(
+            &log.record,
+            &Key::new("big_u64value"),
+            &AnyValue::String(format!("{}", u64::MAX).into())
+        ));
+        assert!(attributes_contains(
+            &log.record,
+            &Key::new("small_usizevalue"),
+            &AnyValue::Int(42.into())
+        ));
+        assert!(attributes_contains(
+            &log.record,
+            &Key::new("big_usizevalue"),
+            &AnyValue::String(format!("{}", u64::MAX).into())
+        ));
         #[cfg(feature = "experimental_metadata_attributes")]
         {
             assert!(attributes_contains(
@@ -753,6 +792,10 @@ mod tests {
             TraceFlags::SAMPLED
         );
 
+        for attribute in log.record.attributes_iter() {
+            println!("key: {:?}, value: {:?}", attribute.0, attribute.1);
+        }
+
         // Attributes can be polluted when we don't use this feature.
         #[cfg(feature = "experimental_metadata_attributes")]
         assert_eq!(log.record.attributes_iter().count(), 4);

opentelemetry-sdk/src/logs/logger.rs

@@ -34,14 +34,11 @@ impl opentelemetry::logs::Logger for SdkLogger {
 
         //let mut log_record = record;
         if record.trace_context.is_none() {
-            let trace_context = Context::map_current(|cx| {
-                cx.has_active_span()
-                    .then(|| TraceContext::from(cx.span().span_context()))
+            Context::map_current(|cx| {
+                cx.has_active_span().then(|| {
+                    record.trace_context = Some(TraceContext::from(cx.span().span_context()))
+                })
             });
-
-            if let Some(ref trace_context) = trace_context {
-                record.trace_context = Some(trace_context.clone());
-            }
         }
         if record.observed_timestamp.is_none() {
             record.observed_timestamp = Some(now());