From 73049213d368089b32484e031bdac8d6fd8b5222 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 7 Feb 2022 16:43:12 +0100 Subject: [PATCH 1/6] opentelemetry: forward event metadata --- tracing-opentelemetry/src/subscriber.rs | 33 +++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tracing-opentelemetry/src/subscriber.rs b/tracing-opentelemetry/src/subscriber.rs index 67257f7f5d..4c27f24866 100644 --- a/tracing-opentelemetry/src/subscriber.rs +++ b/tracing-opentelemetry/src/subscriber.rs @@ -3,6 +3,8 @@ use opentelemetry::{ trace::{self as otel, noop, TraceContextExt}, Context as OtelContext, Key, KeyValue, }; +#[cfg(not(feature = "tracing-log"))] +use std::borrow::Cow; use std::fmt; use std::marker; use std::time::{Instant, SystemTime}; @@ -27,6 +29,7 @@ const SPAN_STATUS_MESSAGE_FIELD: &str = "otel.status_message"; /// [tracing]: https://github.com/tokio-rs/tracing pub struct OpenTelemetrySubscriber { tracer: T, + event_location: bool, tracked_inactivity: bool, get_context: WithContext, _registry: marker::PhantomData, @@ -289,6 +292,7 @@ where pub fn new(tracer: T) -> Self { OpenTelemetrySubscriber { tracer, + event_location: true, tracked_inactivity: true, get_context: WithContext(Self::get_context), _registry: marker::PhantomData, @@ -327,12 +331,22 @@ where { OpenTelemetrySubscriber { tracer, + event_location: self.event_location, tracked_inactivity: self.tracked_inactivity, get_context: WithContext(OpenTelemetrySubscriber::::get_context), _registry: self._registry, } } + /// Sets whether or not event span's metadata should include detailed location + /// information, such as the file, module and line number. + pub fn with_event_location(self, event_location: bool) -> Self { + Self { + event_location, + ..self + } + } + /// Sets whether or not spans metadata should include the _busy time_ /// (total time for which it was entered), and _idle time_ (total time /// the span existed but was not entered). @@ -555,6 +569,25 @@ where builder.status_code = Some(otel::StatusCode::Error); } + if self.event_location { + let builder_attrs = builder.attributes.get_or_insert(Vec::new()); + if let Some(file) = meta.file() { + #[cfg(feature = "tracing-log")] + builder_attrs.push(KeyValue::new("code.filepath", file.to_owned())); + #[cfg(not(feature = "tracing-log"))] + builder_attrs.push(KeyValue::new("code.filepath", Cow::Borrowed(file))); + } + if let Some(module) = meta.module_path() { + #[cfg(feature = "tracing-log")] + builder_attrs.push(KeyValue::new("code.namespace", module.to_owned())); + #[cfg(not(feature = "tracing-log"))] + builder_attrs.push(KeyValue::new("code.namespace", Cow::Borrowed(module))); + } + if let Some(line) = meta.line() { + builder_attrs.push(KeyValue::new("code.lineno", line as i64)); + } + } + if let Some(ref mut events) = builder.events { events.push(otel_event); } else { From c160ef5d1588b0674a9d77b2f3fe434f942ca33c Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 9 Feb 2022 11:19:31 +0100 Subject: [PATCH 2/6] Avoid unnecessary allocations --- tracing-opentelemetry/src/subscriber.rs | 34 +++++++++++++++++-------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/tracing-opentelemetry/src/subscriber.rs b/tracing-opentelemetry/src/subscriber.rs index 4c27f24866..b90e102a9d 100644 --- a/tracing-opentelemetry/src/subscriber.rs +++ b/tracing-opentelemetry/src/subscriber.rs @@ -1,7 +1,7 @@ use crate::{OtelData, PreSampledTracer}; use opentelemetry::{ trace::{self as otel, noop, TraceContextExt}, - Context as OtelContext, Key, KeyValue, + Context as OtelContext, Key, KeyValue, Value, }; #[cfg(not(feature = "tracing-log"))] use std::borrow::Cow; @@ -571,17 +571,29 @@ where if self.event_location { let builder_attrs = builder.attributes.get_or_insert(Vec::new()); - if let Some(file) = meta.file() { - #[cfg(feature = "tracing-log")] - builder_attrs.push(KeyValue::new("code.filepath", file.to_owned())); - #[cfg(not(feature = "tracing-log"))] - builder_attrs.push(KeyValue::new("code.filepath", Cow::Borrowed(file))); + + #[cfg(feature = "tracing-log")] + let (file, module) = match &normalized_meta { + Some(meta) => ( + meta.file().map(|s| Value::from(s.to_owned())), + meta.module_path().map(|s| Value::from(s.to_owned())), + ), + None => ( + event.metadata().file().map(|s| Value::from(s)), + event.metadata().module_path().map(|s| Value::from(s)), + ), + }; + #[cfg(not(feature = "tracing-log"))] + let (file, module) = ( + event.metadata().file().map(|s| Value::from(s)), + event.metadata().module_path().map(|s| Value::from(s)), + ); + + if let Some(file) = file { + builder_attrs.push(KeyValue::new("code.filepath", file)); } - if let Some(module) = meta.module_path() { - #[cfg(feature = "tracing-log")] - builder_attrs.push(KeyValue::new("code.namespace", module.to_owned())); - #[cfg(not(feature = "tracing-log"))] - builder_attrs.push(KeyValue::new("code.namespace", Cow::Borrowed(module))); + if let Some(module) = module { + builder_attrs.push(KeyValue::new("code.namespace", module)); } if let Some(line) = meta.line() { builder_attrs.push(KeyValue::new("code.lineno", line as i64)); From 7b49d1cd1c821217acaee05bb1e5e97332030b52 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 9 Feb 2022 11:25:56 +0100 Subject: [PATCH 3/6] Simplify some more --- tracing-opentelemetry/src/subscriber.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tracing-opentelemetry/src/subscriber.rs b/tracing-opentelemetry/src/subscriber.rs index b90e102a9d..6735af8d0c 100644 --- a/tracing-opentelemetry/src/subscriber.rs +++ b/tracing-opentelemetry/src/subscriber.rs @@ -548,10 +548,10 @@ where // See https://github.com/tokio-rs/tracing/issues/763 #[cfg(feature = "tracing-log")] let normalized_meta = event.normalized_metadata(); - #[cfg(feature = "tracing-log")] - let meta = normalized_meta.as_ref().unwrap_or_else(|| event.metadata()); #[cfg(not(feature = "tracing-log"))] - let meta = event.metadata(); + let normalized_meta = None; + + let meta = normalized_meta.as_ref().unwrap_or_else(|| event.metadata()); let mut otel_event = otel::Event::new( String::new(), SystemTime::now(), @@ -572,7 +572,6 @@ where if self.event_location { let builder_attrs = builder.attributes.get_or_insert(Vec::new()); - #[cfg(feature = "tracing-log")] let (file, module) = match &normalized_meta { Some(meta) => ( meta.file().map(|s| Value::from(s.to_owned())), @@ -583,11 +582,6 @@ where event.metadata().module_path().map(|s| Value::from(s)), ), }; - #[cfg(not(feature = "tracing-log"))] - let (file, module) = ( - event.metadata().file().map(|s| Value::from(s)), - event.metadata().module_path().map(|s| Value::from(s)), - ); if let Some(file) = file { builder_attrs.push(KeyValue::new("code.filepath", file)); From 75215d7f89e7157244634671bf81e41ea1520da8 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 9 Feb 2022 11:27:19 +0100 Subject: [PATCH 4/6] Avoid clippy warnings --- tracing-opentelemetry/src/subscriber.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tracing-opentelemetry/src/subscriber.rs b/tracing-opentelemetry/src/subscriber.rs index 6735af8d0c..c4ee37c8d1 100644 --- a/tracing-opentelemetry/src/subscriber.rs +++ b/tracing-opentelemetry/src/subscriber.rs @@ -578,8 +578,8 @@ where meta.module_path().map(|s| Value::from(s.to_owned())), ), None => ( - event.metadata().file().map(|s| Value::from(s)), - event.metadata().module_path().map(|s| Value::from(s)), + event.metadata().file().map(Value::from), + event.metadata().module_path().map(Value::from), ), }; From 9f2c6c24449d22b01fce5d183d642488bbc6e837 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 9 Feb 2022 11:30:48 +0100 Subject: [PATCH 5/6] Don't pessimize setup code --- tracing-opentelemetry/src/subscriber.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tracing-opentelemetry/src/subscriber.rs b/tracing-opentelemetry/src/subscriber.rs index c4ee37c8d1..eb8e356521 100644 --- a/tracing-opentelemetry/src/subscriber.rs +++ b/tracing-opentelemetry/src/subscriber.rs @@ -548,10 +548,10 @@ where // See https://github.com/tokio-rs/tracing/issues/763 #[cfg(feature = "tracing-log")] let normalized_meta = event.normalized_metadata(); - #[cfg(not(feature = "tracing-log"))] - let normalized_meta = None; - + #[cfg(feature = "tracing-log")] let meta = normalized_meta.as_ref().unwrap_or_else(|| event.metadata()); + #[cfg(not(feature = "tracing-log"))] + let meta = event.metadata(); let mut otel_event = otel::Event::new( String::new(), SystemTime::now(), @@ -572,6 +572,8 @@ where if self.event_location { let builder_attrs = builder.attributes.get_or_insert(Vec::new()); + #[cfg(not(feature = "tracing-log"))] + let normalized_meta = None; let (file, module) = match &normalized_meta { Some(meta) => ( meta.file().map(|s| Value::from(s.to_owned())), From 0213f3e0ec1ff5a5fc971e23f7e0653e1c602768 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 9 Feb 2022 09:06:17 -0800 Subject: [PATCH 6/6] Apply suggestions from code review --- tracing-opentelemetry/src/subscriber.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tracing-opentelemetry/src/subscriber.rs b/tracing-opentelemetry/src/subscriber.rs index eb8e356521..c637da394c 100644 --- a/tracing-opentelemetry/src/subscriber.rs +++ b/tracing-opentelemetry/src/subscriber.rs @@ -340,6 +340,8 @@ where /// Sets whether or not event span's metadata should include detailed location /// information, such as the file, module and line number. + /// + /// By default, event locations are enabled. pub fn with_event_location(self, event_location: bool) -> Self { Self { event_location,