From e7505a37434377f1bd1c2e6255c4cb4ce5bf49f0 Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Fri, 20 May 2022 13:17:43 -0700 Subject: [PATCH 1/6] opentelemetry: add semconv exception fields When an error value is recorded, it should add the `exception.message` and `exception.stacktrace` fields from the opentelemetry semantic conventions to the span/event. Ideally the `exception.type` field would be provided also, but there is currently not a mechanism to determine the type name of a trait object. --- tracing-opentelemetry/src/layer.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tracing-opentelemetry/src/layer.rs b/tracing-opentelemetry/src/layer.rs index 2c4749a1c1..49b3db4156 100644 --- a/tracing-opentelemetry/src/layer.rs +++ b/tracing-opentelemetry/src/layer.rs @@ -276,7 +276,17 @@ impl<'a> field::Visit for SpanAttributeVisitor<'a> { } self.record(Key::new(field.name()).string(value.to_string())); - self.record(Key::new(format!("{}.chain", field.name())).array(chain)); + self.record(Key::new(format!("{}.chain", field.name())).array(chain.clone())); + + self.record(Key::new("exception.message").string(value.to_string())); + + // NOTE: This is actually not the stacktrace of the exception. This is + // the "source chain". It represents the heirarchy of errors from the + // app level to the lowest level such as IO. It does not represent all + // of the callsites in the code that led to the error happening. + // `std::error::Error::backtrace` is a nightly-only API and cannot be + // used here until the feature is stabilized. + self.record(Key::new("exception.stacktrace").array(chain)); } } @@ -1055,6 +1065,18 @@ mod tests { .into() ) ); + + assert_eq!(key_values["exception.message"].as_str(), "user error"); + assert_eq!( + key_values["exception.stacktrace"], + Value::Array( + vec![ + Cow::Borrowed("intermediate error"), + Cow::Borrowed("base error") + ] + .into() + ) + ); } #[test] From e200ee2a2ac613af894491d5ad0d23e1d4a444f9 Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Thu, 9 Jun 2022 12:34:27 -0700 Subject: [PATCH 2/6] fixup! opentelemetry: add semconv exception fields --- tracing-opentelemetry/src/layer.rs | 255 +++++++++++++++++++++++------ 1 file changed, 207 insertions(+), 48 deletions(-) diff --git a/tracing-opentelemetry/src/layer.rs b/tracing-opentelemetry/src/layer.rs index 49b3db4156..02badd78b2 100644 --- a/tracing-opentelemetry/src/layer.rs +++ b/tracing-opentelemetry/src/layer.rs @@ -33,6 +33,7 @@ pub struct OpenTelemetryLayer { location: bool, tracked_inactivity: bool, with_threads: bool, + exception_config: ExceptionFieldConfig, get_context: WithContext, _registry: marker::PhantomData, } @@ -110,9 +111,13 @@ fn str_to_status_code(s: &str) -> Option { } } -struct SpanEventVisitor<'a>(&'a mut otel::Event); +struct SpanEventVisitor<'a, 'b>( + &'a mut otel::Event, + Option<&'b mut otel::SpanBuilder>, + ExceptionFieldConfig, +); -impl<'a> field::Visit for SpanEventVisitor<'a> { +impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { /// Record events on the underlying OpenTelemetry [`Span`] from `bool` values. /// /// [`Span`]: opentelemetry::trace::Span @@ -192,9 +197,72 @@ impl<'a> field::Visit for SpanEventVisitor<'a> { } } } + + /// Set attributes on the underlying OpenTelemetry [`Span`] using a [`std::error::Error`]'s + /// [`std::fmt::Display`] implementation. Also adds the `source` chain as an extra field + /// + /// [`Span`]: opentelemetry::trace::Span + fn record_error( + &mut self, + field: &tracing_core::Field, + value: &(dyn std::error::Error + 'static), + ) { + let mut chain = Vec::new(); + let mut next_err = value.source(); + + while let Some(err) = next_err { + chain.push(Cow::Owned(err.to_string())); + next_err = err.source(); + } + + self.0 + .attributes + .push(Key::new(field.name()).string(value.to_string())); + self.0 + .attributes + .push(Key::new(format!("{}.chain", field.name())).array(chain.clone())); + + if self.2.record { + self.0 + .attributes + .push(Key::new("exception.message").string(value.to_string())); + + // NOTE: This is actually not the stacktrace of the exception. This is + // the "source chain". It represents the heirarchy of errors from the + // app level to the lowest level such as IO. It does not represent all + // of the callsites in the code that led to the error happening. + // `std::error::Error::backtrace` is a nightly-only API and cannot be + // used here until the feature is stabilized. + self.0 + .attributes + .push(Key::new("exception.stacktrace").array(chain.clone())); + } + + if self.2.propagate { + if let Some(span) = &mut self.1 { + if let Some(attrs) = span.attributes.as_mut() { + attrs.push(Key::new("exception.message").string(value.to_string())); + + // NOTE: This is actually not the stacktrace of the exception. This is + // the "source chain". It represents the heirarchy of errors from the + // app level to the lowest level such as IO. It does not represent all + // of the callsites in the code that led to the error happening. + // `std::error::Error::backtrace` is a nightly-only API and cannot be + // used here until the feature is stabilized. + attrs.push(Key::new("exception.stacktrace").array(chain)); + } + } + } + } } -struct SpanAttributeVisitor<'a>(&'a mut otel::SpanBuilder); +#[derive(Clone, Copy)] +struct ExceptionFieldConfig { + record: bool, + propagate: bool, +} + +struct SpanAttributeVisitor<'a>(&'a mut otel::SpanBuilder, ExceptionFieldConfig); impl<'a> SpanAttributeVisitor<'a> { fn record(&mut self, attribute: KeyValue) { @@ -278,15 +346,17 @@ impl<'a> field::Visit for SpanAttributeVisitor<'a> { self.record(Key::new(field.name()).string(value.to_string())); self.record(Key::new(format!("{}.chain", field.name())).array(chain.clone())); - self.record(Key::new("exception.message").string(value.to_string())); + if self.1.record { + self.record(Key::new("exception.message").string(value.to_string())); - // NOTE: This is actually not the stacktrace of the exception. This is - // the "source chain". It represents the heirarchy of errors from the - // app level to the lowest level such as IO. It does not represent all - // of the callsites in the code that led to the error happening. - // `std::error::Error::backtrace` is a nightly-only API and cannot be - // used here until the feature is stabilized. - self.record(Key::new("exception.stacktrace").array(chain)); + // NOTE: This is actually not the stacktrace of the exception. This is + // the "source chain". It represents the heirarchy of errors from the + // app level to the lowest level such as IO. It does not represent all + // of the callsites in the code that led to the error happening. + // `std::error::Error::backtrace` is a nightly-only API and cannot be + // used here until the feature is stabilized. + self.record(Key::new("exception.stacktrace").array(chain)); + } } } @@ -328,6 +398,10 @@ where location: true, tracked_inactivity: true, with_threads: true, + exception_config: ExceptionFieldConfig { + record: true, + propagate: true, + }, get_context: WithContext(Self::get_context), _registry: marker::PhantomData, } @@ -368,11 +442,32 @@ where location: self.location, tracked_inactivity: self.tracked_inactivity, with_threads: self.with_threads, + exception_config: self.exception_config, get_context: WithContext(OpenTelemetryLayer::::get_context), _registry: self._registry, } } + pub fn with_exception_fields(self, exception_fields: bool) -> Self { + Self { + exception_config: ExceptionFieldConfig { + record: exception_fields, + ..self.exception_config + }, + ..self + } + } + + pub fn with_exception_field_propagation(self, exception_field_propagation: bool) -> Self { + Self { + exception_config: ExceptionFieldConfig { + propagate: exception_field_propagation, + ..self.exception_config + }, + ..self + } + } + /// Sets whether or not span and event metadata should include OpenTelemetry /// attributes with location information, such as the file, module and line number. /// @@ -569,7 +664,10 @@ where } } - attrs.record(&mut SpanAttributeVisitor(&mut builder)); + attrs.record(&mut SpanAttributeVisitor( + &mut builder, + self.exception_config, + )); extensions.insert(OtelData { builder, parent_cx }); } @@ -610,7 +708,10 @@ where let span = ctx.span(id).expect("Span not found, this is a bug"); let mut extensions = span.extensions_mut(); if let Some(data) = extensions.get_mut::() { - values.record(&mut SpanAttributeVisitor(&mut data.builder)); + values.record(&mut SpanAttributeVisitor( + &mut data.builder, + self.exception_config, + )); } } @@ -675,15 +776,23 @@ where #[cfg(not(feature = "tracing-log"))] let target = target.string(meta.target()); + let mut extensions = span.extensions_mut(); + let span_builder = extensions + .get_mut::() + .map(|data| &mut data.builder); + let mut otel_event = otel::Event::new( String::new(), SystemTime::now(), vec![Key::new("level").string(meta.level().as_str()), target], 0, ); - event.record(&mut SpanEventVisitor(&mut otel_event)); + event.record(&mut SpanEventVisitor( + &mut otel_event, + span_builder, + self.exception_config, + )); - let mut extensions = span.extensions_mut(); if let Some(OtelData { builder, .. }) = extensions.get_mut::() { if builder.status_code.is_none() && *meta.level() == tracing_core::Level::ERROR { builder.status_code = Some(otel::StatusCode::Error); @@ -809,6 +918,8 @@ mod tests { use std::{ borrow::Cow, collections::HashMap, + error::Error, + fmt::Display, sync::{Arc, Mutex}, thread, time::SystemTime, @@ -886,6 +997,36 @@ mod tests { fn end_with_timestamp(&mut self, _timestamp: SystemTime) {} } + #[derive(Debug)] + struct TestDynError { + msg: &'static str, + source: Option>, + } + impl Display for TestDynError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.msg) + } + } + impl Error for TestDynError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match &self.source { + Some(source) => Some(source), + None => None, + } + } + } + impl TestDynError { + fn new(msg: &'static str) -> Self { + Self { msg, source: None } + } + fn with_parent(self, parent_msg: &'static str) -> Self { + Self { + msg: parent_msg, + source: Some(Box::new(self)), + } + } + } + #[test] fn dynamic_span_names() { let dynamic_name = "GET http://example.com".to_string(); @@ -996,39 +1137,9 @@ mod tests { let tracer = TestTracer(Arc::new(Mutex::new(None))); let subscriber = tracing_subscriber::registry().with(layer().with_tracer(tracer.clone())); - use std::error::Error; - use std::fmt::Display; - - #[derive(Debug)] - struct DynError { - msg: &'static str, - source: Option>, - } - - impl Display for DynError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.msg) - } - } - impl Error for DynError { - fn source(&self) -> Option<&(dyn Error + 'static)> { - match &self.source { - Some(source) => Some(source), - None => None, - } - } - } - - let err = DynError { - msg: "user error", - source: Some(Box::new(DynError { - msg: "intermediate error", - source: Some(Box::new(DynError { - msg: "base error", - source: None, - })), - })), - }; + let err = TestDynError::new("base error") + .with_parent("intermediate error") + .with_parent("user error"); tracing::subscriber::with_default(subscriber, || { tracing::debug_span!( @@ -1162,4 +1273,52 @@ mod tests { assert!(!keys.contains(&"thread.name")); assert!(!keys.contains(&"thread.id")); } + + #[test] + fn propagates_error_fields_from_event_to_span() { + let tracer = TestTracer(Arc::new(Mutex::new(None))); + let subscriber = tracing_subscriber::registry().with(layer().with_tracer(tracer.clone())); + + let err = TestDynError::new("base error") + .with_parent("intermediate error") + .with_parent("user error"); + + tracing::subscriber::with_default(subscriber, || { + let _guard = tracing::debug_span!("request",).entered(); + + tracing::error!( + error = &err as &(dyn std::error::Error + 'static), + "request error!" + ) + }); + + let attributes = tracer + .0 + .lock() + .unwrap() + .as_ref() + .unwrap() + .builder + .attributes + .as_ref() + .unwrap() + .clone(); + + let key_values = attributes + .into_iter() + .map(|attr| (attr.key.as_str().to_owned(), attr.value)) + .collect::>(); + + assert_eq!(key_values["exception.message"].as_str(), "user error"); + assert_eq!( + key_values["exception.stacktrace"], + Value::Array( + vec![ + Cow::Borrowed("intermediate error"), + Cow::Borrowed("base error") + ] + .into() + ) + ); + } } From 2cd337fcbae2f40cb99c5f7edd49008ded68fb5b Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Mon, 13 Jun 2022 15:23:18 -0700 Subject: [PATCH 3/6] fixup! opentelemetry: add semconv exception fields --- tracing-opentelemetry/src/layer.rs | 46 ++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/tracing-opentelemetry/src/layer.rs b/tracing-opentelemetry/src/layer.rs index 02badd78b2..82a94d6c56 100644 --- a/tracing-opentelemetry/src/layer.rs +++ b/tracing-opentelemetry/src/layer.rs @@ -215,13 +215,6 @@ impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { next_err = err.source(); } - self.0 - .attributes - .push(Key::new(field.name()).string(value.to_string())); - self.0 - .attributes - .push(Key::new(format!("{}.chain", field.name())).array(chain.clone())); - if self.2.record { self.0 .attributes @@ -249,10 +242,17 @@ impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { // of the callsites in the code that led to the error happening. // `std::error::Error::backtrace` is a nightly-only API and cannot be // used here until the feature is stabilized. - attrs.push(Key::new("exception.stacktrace").array(chain)); + attrs.push(Key::new("exception.stacktrace").array(chain.clone())); } } } + + self.0 + .attributes + .push(Key::new(field.name()).string(value.to_string())); + self.0 + .attributes + .push(Key::new(format!("{}.chain", field.name())).array(chain)); } } @@ -343,9 +343,6 @@ impl<'a> field::Visit for SpanAttributeVisitor<'a> { next_err = err.source(); } - self.record(Key::new(field.name()).string(value.to_string())); - self.record(Key::new(format!("{}.chain", field.name())).array(chain.clone())); - if self.1.record { self.record(Key::new("exception.message").string(value.to_string())); @@ -355,8 +352,11 @@ impl<'a> field::Visit for SpanAttributeVisitor<'a> { // of the callsites in the code that led to the error happening. // `std::error::Error::backtrace` is a nightly-only API and cannot be // used here until the feature is stabilized. - self.record(Key::new("exception.stacktrace").array(chain)); + self.record(Key::new("exception.stacktrace").array(chain.clone())); } + + self.record(Key::new(field.name()).string(value.to_string())); + self.record(Key::new(format!("{}.chain", field.name())).array(chain)); } } @@ -448,6 +448,17 @@ where } } + /// Sets whether or not span and event metadata should include OpenTelemetry + /// exception fields such as `exception.message` and `exception.backtrace` + /// when an `Error` value is recorded. This is completely independent of + /// `with_exception_field_propagation`. + /// + /// These attributes follow the [OpenTelemetry semantic conventions for + /// exceptions][conv]. + /// + /// By default, these fields are enabled + /// + /// [conv]: https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/exceptions/ pub fn with_exception_fields(self, exception_fields: bool) -> Self { Self { exception_config: ExceptionFieldConfig { @@ -458,6 +469,17 @@ where } } + /// Sets whether or not reporting an `Error` value on an event will + /// propagate the OpenTelemetry exception fields such as `exception.message` + /// and `exception.backtrace` to the corresponding span. This is completely + /// independent of `with_exception_fields`. + /// + /// These attributes follow the [OpenTelemetry semantic conventions for + /// exceptions][conv]. + /// + /// By default, this is enabled + /// + /// [conv]: https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/exceptions/ pub fn with_exception_field_propagation(self, exception_field_propagation: bool) -> Self { Self { exception_config: ExceptionFieldConfig { From 33ed5d4a7f4b5be5ea79aee337b6f49d67118907 Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Tue, 14 Jun 2022 10:25:12 -0700 Subject: [PATCH 4/6] fixup! opentelemetry: add semconv exception fields --- tracing-opentelemetry/src/layer.rs | 153 +++++++++++++++++------------ 1 file changed, 91 insertions(+), 62 deletions(-) diff --git a/tracing-opentelemetry/src/layer.rs b/tracing-opentelemetry/src/layer.rs index 82a94d6c56..98718d46ac 100644 --- a/tracing-opentelemetry/src/layer.rs +++ b/tracing-opentelemetry/src/layer.rs @@ -23,6 +23,9 @@ const SPAN_KIND_FIELD: &str = "otel.kind"; const SPAN_STATUS_CODE_FIELD: &str = "otel.status_code"; const SPAN_STATUS_MESSAGE_FIELD: &str = "otel.status_message"; +const FIELD_EXCEPTION_MESSAGE: &str = "exception.message"; +const FIELD_EXCEPTION_STACKTRACE: &str = "exception.stacktrace"; + /// An [OpenTelemetry] propagation layer for use in a project that uses /// [tracing]. /// @@ -111,11 +114,11 @@ fn str_to_status_code(s: &str) -> Option { } } -struct SpanEventVisitor<'a, 'b>( - &'a mut otel::Event, - Option<&'b mut otel::SpanBuilder>, - ExceptionFieldConfig, -); +struct SpanEventVisitor<'a, 'b> { + event_builder: &'a mut otel::Event, + span_builder: Option<&'b mut otel::SpanBuilder>, + exception_config: ExceptionFieldConfig, +} impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { /// Record events on the underlying OpenTelemetry [`Span`] from `bool` values. @@ -123,12 +126,14 @@ impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { /// [`Span`]: opentelemetry::trace::Span fn record_bool(&mut self, field: &field::Field, value: bool) { match field.name() { - "message" => self.0.name = value.to_string().into(), + "message" => self.event_builder.name = value.to_string().into(), // Skip fields that are actually log metadata that have already been handled #[cfg(feature = "tracing-log")] name if name.starts_with("log.") => (), name => { - self.0.attributes.push(KeyValue::new(name, value)); + self.event_builder + .attributes + .push(KeyValue::new(name, value)); } } } @@ -138,12 +143,14 @@ impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { /// [`Span`]: opentelemetry::trace::Span fn record_f64(&mut self, field: &field::Field, value: f64) { match field.name() { - "message" => self.0.name = value.to_string().into(), + "message" => self.event_builder.name = value.to_string().into(), // Skip fields that are actually log metadata that have already been handled #[cfg(feature = "tracing-log")] name if name.starts_with("log.") => (), name => { - self.0.attributes.push(KeyValue::new(name, value)); + self.event_builder + .attributes + .push(KeyValue::new(name, value)); } } } @@ -153,12 +160,14 @@ impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { /// [`Span`]: opentelemetry::trace::Span fn record_i64(&mut self, field: &field::Field, value: i64) { match field.name() { - "message" => self.0.name = value.to_string().into(), + "message" => self.event_builder.name = value.to_string().into(), // Skip fields that are actually log metadata that have already been handled #[cfg(feature = "tracing-log")] name if name.starts_with("log.") => (), name => { - self.0.attributes.push(KeyValue::new(name, value)); + self.event_builder + .attributes + .push(KeyValue::new(name, value)); } } } @@ -168,12 +177,12 @@ impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { /// [`Span`]: opentelemetry::trace::Span fn record_str(&mut self, field: &field::Field, value: &str) { match field.name() { - "message" => self.0.name = value.to_string().into(), + "message" => self.event_builder.name = value.to_string().into(), // Skip fields that are actually log metadata that have already been handled #[cfg(feature = "tracing-log")] name if name.starts_with("log.") => (), name => { - self.0 + self.event_builder .attributes .push(KeyValue::new(name, value.to_string())); } @@ -186,12 +195,12 @@ impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { /// [`Span`]: opentelemetry::trace::Span fn record_debug(&mut self, field: &field::Field, value: &dyn fmt::Debug) { match field.name() { - "message" => self.0.name = format!("{:?}", value).into(), + "message" => self.event_builder.name = format!("{:?}", value).into(), // Skip fields that are actually log metadata that have already been handled #[cfg(feature = "tracing-log")] name if name.starts_with("log.") => (), name => { - self.0 + self.event_builder .attributes .push(KeyValue::new(name, format!("{:?}", value))); } @@ -215,10 +224,12 @@ impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { next_err = err.source(); } - if self.2.record { - self.0 + let error_msg = value.to_string(); + + if self.exception_config.record { + self.event_builder .attributes - .push(Key::new("exception.message").string(value.to_string())); + .push(Key::new(FIELD_EXCEPTION_MESSAGE).string(error_msg.clone())); // NOTE: This is actually not the stacktrace of the exception. This is // the "source chain". It represents the heirarchy of errors from the @@ -226,15 +237,15 @@ impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { // of the callsites in the code that led to the error happening. // `std::error::Error::backtrace` is a nightly-only API and cannot be // used here until the feature is stabilized. - self.0 + self.event_builder .attributes - .push(Key::new("exception.stacktrace").array(chain.clone())); + .push(Key::new(FIELD_EXCEPTION_STACKTRACE).array(chain.clone())); } - if self.2.propagate { - if let Some(span) = &mut self.1 { + if self.exception_config.propagate { + if let Some(span) = &mut self.span_builder { if let Some(attrs) = span.attributes.as_mut() { - attrs.push(Key::new("exception.message").string(value.to_string())); + attrs.push(Key::new(FIELD_EXCEPTION_MESSAGE).string(error_msg.clone())); // NOTE: This is actually not the stacktrace of the exception. This is // the "source chain". It represents the heirarchy of errors from the @@ -242,32 +253,41 @@ impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { // of the callsites in the code that led to the error happening. // `std::error::Error::backtrace` is a nightly-only API and cannot be // used here until the feature is stabilized. - attrs.push(Key::new("exception.stacktrace").array(chain.clone())); + attrs.push(Key::new(FIELD_EXCEPTION_STACKTRACE).array(chain.clone())); } } } - self.0 + self.event_builder .attributes - .push(Key::new(field.name()).string(value.to_string())); - self.0 + .push(Key::new(field.name()).string(error_msg)); + self.event_builder .attributes .push(Key::new(format!("{}.chain", field.name())).array(chain)); } } #[derive(Clone, Copy)] +/// Control over opentelemetry conventional exception fields struct ExceptionFieldConfig { + /// If an error value is recorded on an event/span, should the otel fields + /// be added record: bool, + + /// If an error value is recorded on an event, should the otel fields be + /// added to the corresponding span propagate: bool, } -struct SpanAttributeVisitor<'a>(&'a mut otel::SpanBuilder, ExceptionFieldConfig); +struct SpanAttributeVisitor<'a> { + span_builder: &'a mut otel::SpanBuilder, + exception_config: ExceptionFieldConfig, +} impl<'a> SpanAttributeVisitor<'a> { fn record(&mut self, attribute: KeyValue) { - debug_assert!(self.0.attributes.is_some()); - if let Some(v) = self.0.attributes.as_mut() { + debug_assert!(self.span_builder.attributes.is_some()); + if let Some(v) = self.span_builder.attributes.as_mut() { v.push(attribute); } } @@ -300,10 +320,12 @@ impl<'a> field::Visit for SpanAttributeVisitor<'a> { /// [`Span`]: opentelemetry::trace::Span fn record_str(&mut self, field: &field::Field, value: &str) { match field.name() { - SPAN_NAME_FIELD => self.0.name = value.to_string().into(), - SPAN_KIND_FIELD => self.0.span_kind = str_to_span_kind(value), - SPAN_STATUS_CODE_FIELD => self.0.status_code = str_to_status_code(value), - SPAN_STATUS_MESSAGE_FIELD => self.0.status_message = Some(value.to_owned().into()), + SPAN_NAME_FIELD => self.span_builder.name = value.to_string().into(), + SPAN_KIND_FIELD => self.span_builder.span_kind = str_to_span_kind(value), + SPAN_STATUS_CODE_FIELD => self.span_builder.status_code = str_to_status_code(value), + SPAN_STATUS_MESSAGE_FIELD => { + self.span_builder.status_message = Some(value.to_owned().into()) + } _ => self.record(KeyValue::new(field.name(), value.to_string())), } } @@ -314,13 +336,15 @@ impl<'a> field::Visit for SpanAttributeVisitor<'a> { /// [`Span`]: opentelemetry::trace::Span fn record_debug(&mut self, field: &field::Field, value: &dyn fmt::Debug) { match field.name() { - SPAN_NAME_FIELD => self.0.name = format!("{:?}", value).into(), - SPAN_KIND_FIELD => self.0.span_kind = str_to_span_kind(&format!("{:?}", value)), + SPAN_NAME_FIELD => self.span_builder.name = format!("{:?}", value).into(), + SPAN_KIND_FIELD => { + self.span_builder.span_kind = str_to_span_kind(&format!("{:?}", value)) + } SPAN_STATUS_CODE_FIELD => { - self.0.status_code = str_to_status_code(&format!("{:?}", value)) + self.span_builder.status_code = str_to_status_code(&format!("{:?}", value)) } SPAN_STATUS_MESSAGE_FIELD => { - self.0.status_message = Some(format!("{:?}", value).into()) + self.span_builder.status_message = Some(format!("{:?}", value).into()) } _ => self.record(Key::new(field.name()).string(format!("{:?}", value))), } @@ -343,8 +367,10 @@ impl<'a> field::Visit for SpanAttributeVisitor<'a> { next_err = err.source(); } - if self.1.record { - self.record(Key::new("exception.message").string(value.to_string())); + let error_msg = value.to_string(); + + if self.exception_config.record { + self.record(Key::new(FIELD_EXCEPTION_MESSAGE).string(error_msg.clone())); // NOTE: This is actually not the stacktrace of the exception. This is // the "source chain". It represents the heirarchy of errors from the @@ -352,10 +378,10 @@ impl<'a> field::Visit for SpanAttributeVisitor<'a> { // of the callsites in the code that led to the error happening. // `std::error::Error::backtrace` is a nightly-only API and cannot be // used here until the feature is stabilized. - self.record(Key::new("exception.stacktrace").array(chain.clone())); + self.record(Key::new(FIELD_EXCEPTION_STACKTRACE).array(chain.clone())); } - self.record(Key::new(field.name()).string(value.to_string())); + self.record(Key::new(field.name()).string(error_msg)); self.record(Key::new(format!("{}.chain", field.name())).array(chain)); } } @@ -450,8 +476,9 @@ where /// Sets whether or not span and event metadata should include OpenTelemetry /// exception fields such as `exception.message` and `exception.backtrace` - /// when an `Error` value is recorded. This is completely independent of - /// `with_exception_field_propagation`. + /// when an `Error` value is recorded. If multiple error values are recorded + /// on the same span/event, only the most recently recorded error value will + /// show up under these fields. /// /// These attributes follow the [OpenTelemetry semantic conventions for /// exceptions][conv]. @@ -471,8 +498,10 @@ where /// Sets whether or not reporting an `Error` value on an event will /// propagate the OpenTelemetry exception fields such as `exception.message` - /// and `exception.backtrace` to the corresponding span. This is completely - /// independent of `with_exception_fields`. + /// and `exception.backtrace` to the corresponding span. You do not need to + /// enable `with_exception_fields` in order to enable this. If multiple + /// error values are recorded on the same span/event, only the most recently + /// recorded error value will show up under these fields. /// /// These attributes follow the [OpenTelemetry semantic conventions for /// exceptions][conv]. @@ -686,10 +715,10 @@ where } } - attrs.record(&mut SpanAttributeVisitor( - &mut builder, - self.exception_config, - )); + attrs.record(&mut SpanAttributeVisitor { + span_builder: &mut builder, + exception_config: self.exception_config, + }); extensions.insert(OtelData { builder, parent_cx }); } @@ -730,10 +759,10 @@ where let span = ctx.span(id).expect("Span not found, this is a bug"); let mut extensions = span.extensions_mut(); if let Some(data) = extensions.get_mut::() { - values.record(&mut SpanAttributeVisitor( - &mut data.builder, - self.exception_config, - )); + values.record(&mut SpanAttributeVisitor { + span_builder: &mut data.builder, + exception_config: self.exception_config, + }); } } @@ -809,11 +838,11 @@ where vec![Key::new("level").string(meta.level().as_str()), target], 0, ); - event.record(&mut SpanEventVisitor( - &mut otel_event, + event.record(&mut SpanEventVisitor { + event_builder: &mut otel_event, span_builder, - self.exception_config, - )); + exception_config: self.exception_config, + }); if let Some(OtelData { builder, .. }) = extensions.get_mut::() { if builder.status_code.is_none() && *meta.level() == tracing_core::Level::ERROR { @@ -1199,9 +1228,9 @@ mod tests { ) ); - assert_eq!(key_values["exception.message"].as_str(), "user error"); + assert_eq!(key_values[FIELD_EXCEPTION_MESSAGE].as_str(), "user error"); assert_eq!( - key_values["exception.stacktrace"], + key_values[FIELD_EXCEPTION_STACKTRACE], Value::Array( vec![ Cow::Borrowed("intermediate error"), @@ -1331,9 +1360,9 @@ mod tests { .map(|attr| (attr.key.as_str().to_owned(), attr.value)) .collect::>(); - assert_eq!(key_values["exception.message"].as_str(), "user error"); + assert_eq!(key_values[FIELD_EXCEPTION_MESSAGE].as_str(), "user error"); assert_eq!( - key_values["exception.stacktrace"], + key_values[FIELD_EXCEPTION_STACKTRACE], Value::Array( vec![ Cow::Borrowed("intermediate error"), From 6ab55113e2b94d3c6ec74b7d5e0edf2c6bd60216 Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Wed, 22 Jun 2022 12:54:43 -0700 Subject: [PATCH 5/6] fixup! opentelemetry: add semconv exception fields --- tracing-opentelemetry/src/layer.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tracing-opentelemetry/src/layer.rs b/tracing-opentelemetry/src/layer.rs index 98718d46ac..de891eb131 100644 --- a/tracing-opentelemetry/src/layer.rs +++ b/tracing-opentelemetry/src/layer.rs @@ -267,8 +267,8 @@ impl<'a, 'b> field::Visit for SpanEventVisitor<'a, 'b> { } } -#[derive(Clone, Copy)] /// Control over opentelemetry conventional exception fields +#[derive(Clone, Copy)] struct ExceptionFieldConfig { /// If an error value is recorded on an event/span, should the otel fields /// be added @@ -425,8 +425,8 @@ where tracked_inactivity: true, with_threads: true, exception_config: ExceptionFieldConfig { - record: true, - propagate: true, + record: false, + propagate: false, }, get_context: WithContext(Self::get_context), _registry: marker::PhantomData, From 4feda0c5c767101d7b3bb3762a98add47505682b Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Wed, 22 Jun 2022 17:20:46 -0700 Subject: [PATCH 6/6] fixup! opentelemetry: add semconv exception fields --- tracing-opentelemetry/src/layer.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tracing-opentelemetry/src/layer.rs b/tracing-opentelemetry/src/layer.rs index de891eb131..a722dfd185 100644 --- a/tracing-opentelemetry/src/layer.rs +++ b/tracing-opentelemetry/src/layer.rs @@ -1186,7 +1186,11 @@ mod tests { #[test] fn records_error_fields() { let tracer = TestTracer(Arc::new(Mutex::new(None))); - let subscriber = tracing_subscriber::registry().with(layer().with_tracer(tracer.clone())); + let subscriber = tracing_subscriber::registry().with( + layer() + .with_tracer(tracer.clone()) + .with_exception_fields(true), + ); let err = TestDynError::new("base error") .with_parent("intermediate error") @@ -1328,7 +1332,11 @@ mod tests { #[test] fn propagates_error_fields_from_event_to_span() { let tracer = TestTracer(Arc::new(Mutex::new(None))); - let subscriber = tracing_subscriber::registry().with(layer().with_tracer(tracer.clone())); + let subscriber = tracing_subscriber::registry().with( + layer() + .with_tracer(tracer.clone()) + .with_exception_field_propagation(true), + ); let err = TestDynError::new("base error") .with_parent("intermediate error")