fix: Report error using OTel convention (#2808)

Co-authored-by: Anton Grübel <[email protected]>
Co-authored-by: Utkarsh Umesan Pillai <[email protected]>

Author: 3 people

opentelemetry-appender-tracing/CHANGELOG.md

@@ -42,12 +42,16 @@ 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`
+- `u64`, `i128`, `u128` 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.
 - Byte arrays are stored as `opentelemetry::logs::AnyValue::Bytes` instead
 of string.
+- `Error` fields are reported using attribute named "exception.message". For
+  example, the below will now report an attribute named "exception.message",
+  instead of previously reporting the user provided attribute "error".
+  `error!(....error = &OTelSdkError::AlreadyShutdown as &dyn std::error::Error...)`
 - perf - small perf improvement by avoiding string allocation of `target`
 
 ## 0.28.1

opentelemetry-appender-tracing/src/layer.rs

@@ -80,6 +80,18 @@ impl<LR: LogRecord> tracing::field::Visit for EventVisitor<'_, LR> {
         }
     }
 
+    fn record_error(
+        &mut self,
+        _field: &tracing_core::Field,
+        value: &(dyn std::error::Error + 'static),
+    ) {
+        self.log_record.add_attribute(
+            Key::new("exception.message"),
+            AnyValue::from(value.to_string()),
+        );
+        // No ability to get exception.stacktrace or exception.type from the error today.
+    }
+
     fn record_bytes(&mut self, field: &tracing_core::Field, value: &[u8]) {
         self.log_record
             .add_attribute(Key::new(field.name()), AnyValue::from(value));
@@ -135,6 +147,34 @@ impl<LR: LogRecord> tracing::field::Visit for EventVisitor<'_, LR> {
         }
     }
 
+    fn record_i128(&mut self, field: &tracing::field::Field, value: i128) {
+        #[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:?}")));
+        }
+    }
+
+    fn record_u128(&mut self, field: &tracing::field::Field, value: u128) {
+        #[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
 }
 
@@ -247,7 +287,7 @@ mod tests {
     use opentelemetry::trace::{TraceContextExt, TraceFlags, Tracer};
     use opentelemetry::InstrumentationScope;
     use opentelemetry::{logs::AnyValue, Key};
-    use opentelemetry_sdk::error::OTelSdkResult;
+    use opentelemetry_sdk::error::{OTelSdkError, OTelSdkResult};
     use opentelemetry_sdk::logs::{InMemoryLogExporter, LogProcessor};
     use opentelemetry_sdk::logs::{LogBatch, LogExporter};
     use opentelemetry_sdk::logs::{SdkLogRecord, SdkLoggerProvider};
@@ -355,7 +395,11 @@ mod tests {
         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, bytes = &b"abc"[..], small_u64value, big_u64value, small_usizevalue, big_usizevalue, user_name = "otel", user_email = "[email protected]");
+        let small_u128value: u128 = 42;
+        let big_u128value: u128 = u128::MAX;
+        let small_i128value: i128 = 42;
+        let big_i128value: i128 = i128::MAX;
+        error!(name: "my-event-name", target: "my-system", event_id = 20, bytes = &b"abc"[..], error = &OTelSdkError::AlreadyShutdown as &dyn std::error::Error, small_u64value, big_u64value, small_usizevalue, big_usizevalue, small_u128value, big_u128value, small_i128value, big_i128value, user_name = "otel", user_email = "[email protected]");
         assert!(logger_provider.force_flush().is_ok());
 
         // Assert TODO: move to helper methods
@@ -386,9 +430,9 @@ mod tests {
 
         // Validate attributes
         #[cfg(not(feature = "experimental_metadata_attributes"))]
-        assert_eq!(log.record.attributes_iter().count(), 8);
+        assert_eq!(log.record.attributes_iter().count(), 13);
         #[cfg(feature = "experimental_metadata_attributes")]
-        assert_eq!(log.record.attributes_iter().count(), 12);
+        assert_eq!(log.record.attributes_iter().count(), 17);
         assert!(attributes_contains(
             &log.record,
             &Key::new("event_id"),
@@ -404,6 +448,11 @@ mod tests {
             &Key::new("user_email"),
             &AnyValue::String("[email protected]".into())
         ));
+        assert!(attributes_contains(
+            &log.record,
+            &Key::new("exception.message"),
+            &AnyValue::String(OTelSdkError::AlreadyShutdown.to_string().into())
+        ));
         assert!(attributes_contains(
             &log.record,
             &Key::new("small_u64value"),
@@ -424,6 +473,26 @@ mod tests {
             &Key::new("big_usizevalue"),
             &AnyValue::String(format!("{}", u64::MAX).into())
         ));
+        assert!(attributes_contains(
+            &log.record,
+            &Key::new("small_u128value"),
+            &AnyValue::Int(42.into())
+        ));
+        assert!(attributes_contains(
+            &log.record,
+            &Key::new("big_u128value"),
+            &AnyValue::String(format!("{}", u128::MAX).into())
+        ));
+        assert!(attributes_contains(
+            &log.record,
+            &Key::new("small_i128value"),
+            &AnyValue::Int(42.into())
+        ));
+        assert!(attributes_contains(
+            &log.record,
+            &Key::new("big_i128value"),
+            &AnyValue::String(format!("{}", i128::MAX).into())
+        ));
         assert!(attributes_contains(
             &log.record,
             &Key::new("bytes"),

opentelemetry-appender-tracing/src/lib.rs

@@ -54,14 +54,16 @@
 //!
 //! ### 3. Create the OpenTelemetry-Tracing Bridge
 //!
+//! Create `OpenTelemetryTracingBridge` layer using the `SdkLoggerProvider` created in the previous step.
+//!
 //! ```rust
 //! # use opentelemetry_sdk::logs::SdkLoggerProvider;
 //! # use opentelemetry_stdout::LogExporter;
 //! # use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge;
 //! # let exporter = LogExporter::default();
 //! # let provider = SdkLoggerProvider::builder()
-//!     .with_simple_exporter(exporter)
-//!     .build();
+//! #    .with_simple_exporter(exporter)
+//! #    .build();
 //! let otel_layer = OpenTelemetryTracingBridge::new(&provider);
 //! ```
 //!
@@ -80,7 +82,7 @@
 //!
 //! tracing_subscriber::registry()
 //!     .with(otel_layer)
-//!     .with(tracing_subscriber::fmt::layer())
+//!     .with(tracing_subscriber::fmt::layer()) // In this example, `fmt` layer is also added.
 //!     .init();
 //! ```
 //!
@@ -112,7 +114,7 @@
 //! |----------------|-------------------------------|
 //! | `i64`          | `Int`                         |
 //! | `f32`, `f64`   | `Double`                      |
-//! | `u64`          | `Int` (if convertible without loss) else `String` |
+//! | `u64`,`u128` ,`i128`         | `Int` (if convertible to `i64` without loss) else `String` |
 //! | `&str`         | `String`                      |
 //! | `bool`         | `Bool`                        |
 //! | `&[u8]`        | `Bytes`                       |