From 6207882b4b7e303915919ecd2824153788990cef Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 12 Jul 2024 17:16:06 -0700 Subject: [PATCH] Insert tracing event name into LogRecord::event_name instead of attributes (#1928) Co-authored-by: Cijo Thomas --- opentelemetry-appender-tracing/CHANGELOG.md | 3 ++ opentelemetry-appender-tracing/src/layer.rs | 39 +++++++++------------ opentelemetry-proto/src/transform/logs.rs | 21 ++++++++--- opentelemetry-stdout/src/logs/transform.rs | 30 ++++++++++------ 4 files changed, 55 insertions(+), 38 deletions(-) diff --git a/opentelemetry-appender-tracing/CHANGELOG.md b/opentelemetry-appender-tracing/CHANGELOG.md index 67d1f55dc7..59edf5fe80 100644 --- a/opentelemetry-appender-tracing/CHANGELOG.md +++ b/opentelemetry-appender-tracing/CHANGELOG.md @@ -5,6 +5,9 @@ - [1869](https://github.com/open-telemetry/opentelemetry-rust/pull/1869) Utilize the `LogRecord::set_target()` method to pass the tracing target to the SDK. Exporters might use the target to override the instrumentation scope, which previously contained "opentelemetry-appender-tracing". +- **Breaking** [1928](https://github.com/open-telemetry/opentelemetry-rust/pull/1928) Insert tracing event name into LogRecord::event_name instead of attributes. + - If using a custom exporter, then they must serialize this field directly from LogRecord::event_name instead of iterating over the attributes. OTLP Exporter is modified to handle this. + ## v0.4.0 - Removed unwanted dependency on opentelemetry-sdk. diff --git a/opentelemetry-appender-tracing/src/layer.rs b/opentelemetry-appender-tracing/src/layer.rs index 45eb85dc92..91124c8464 100644 --- a/opentelemetry-appender-tracing/src/layer.rs +++ b/opentelemetry-appender-tracing/src/layer.rs @@ -3,7 +3,9 @@ use opentelemetry::{ Key, }; use std::borrow::Cow; -use tracing_core::{Level, Metadata}; +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::Layer; @@ -39,13 +41,6 @@ impl<'a, LR: LogRecord> EventVisitor<'a, LR> { fn new(log_record: &'a mut LR) -> Self { EventVisitor { log_record } } - fn visit_metadata(&mut self, meta: &Metadata) { - self.log_record - .add_attribute(Key::new("name"), AnyValue::from(meta.name())); - - #[cfg(feature = "experimental_metadata_attributes")] - self.visit_experimental_metadata(meta); - } #[cfg(feature = "experimental_metadata_attributes")] fn visit_experimental_metadata(&mut self, meta: &Metadata) { @@ -170,15 +165,17 @@ where //let mut log_record: LogRecord = LogRecord::default(); let mut log_record = self.logger.create_log_record(); + log_record.set_target(meta.target().to_string()); + log_record.set_event_name(meta.name()); log_record.set_severity_number(severity_of_level(meta.level())); log_record.set_severity_text(meta.level().to_string().into()); - log_record.set_target(meta.target().to_string()); - let mut visitor = EventVisitor::new(&mut log_record); - visitor.visit_metadata(meta); + #[cfg(feature = "experimental_metadata_attributes")] + visitor.visit_experimental_metadata(meta); // Visit fields. event.record(&mut visitor); + //emit record self.logger.emit(log_record); } @@ -261,10 +258,9 @@ mod tests { .clone() .expect("Attributes are expected"); #[cfg(not(feature = "experimental_metadata_attributes"))] - assert_eq!(attributes.len(), 4); + assert_eq!(attributes.len(), 3); #[cfg(feature = "experimental_metadata_attributes")] - assert_eq!(attributes.len(), 9); - assert!(attributes.contains(&(Key::new("name"), "my-event-name".into()))); + assert_eq!(attributes.len(), 8); assert!(attributes.contains(&(Key::new("event_id"), 20.into()))); assert!(attributes.contains(&(Key::new("user_name"), "otel".into()))); assert!(attributes.contains(&(Key::new("user_email"), "otel@opentelemetry.io".into()))); @@ -358,10 +354,9 @@ mod tests { .clone() .expect("Attributes are expected"); #[cfg(not(feature = "experimental_metadata_attributes"))] - assert_eq!(attributes.len(), 4); + assert_eq!(attributes.len(), 3); #[cfg(feature = "experimental_metadata_attributes")] - assert_eq!(attributes.len(), 9); - assert!(attributes.contains(&(Key::new("name"), "my-event-name".into()))); + assert_eq!(attributes.len(), 8); assert!(attributes.contains(&(Key::new("event_id"), 20.into()))); assert!(attributes.contains(&(Key::new("user_name"), "otel".into()))); assert!(attributes.contains(&(Key::new("user_email"), "otel@opentelemetry.io".into()))); @@ -419,6 +414,7 @@ mod tests { assert!(log.record.trace_context.is_none()); // Validate attributes + #[cfg(feature = "experimental_metadata_attributes")] let attributes: Vec<(Key, AnyValue)> = log .record .attributes @@ -427,9 +423,7 @@ mod tests { // Attributes can be polluted when we don't use this feature. #[cfg(feature = "experimental_metadata_attributes")] - assert_eq!(attributes.len(), 6); - - assert!(attributes.contains(&(Key::new("name"), "log event".into()))); + assert_eq!(attributes.len(), 5); #[cfg(feature = "experimental_metadata_attributes")] { @@ -516,6 +510,7 @@ mod tests { ); // validate attributes. + #[cfg(feature = "experimental_metadata_attributes")] let attributes: Vec<(Key, AnyValue)> = log .record .attributes @@ -524,9 +519,7 @@ mod tests { // Attributes can be polluted when we don't use this feature. #[cfg(feature = "experimental_metadata_attributes")] - assert_eq!(attributes.len(), 6); - - assert!(attributes.contains(&(Key::new("name"), "log event".into()))); + assert_eq!(attributes.len(), 5); #[cfg(feature = "experimental_metadata_attributes")] { diff --git a/opentelemetry-proto/src/transform/logs.rs b/opentelemetry-proto/src/transform/logs.rs index 0a672d3d51..6e5235f1d9 100644 --- a/opentelemetry-proto/src/transform/logs.rs +++ b/opentelemetry-proto/src/transform/logs.rs @@ -92,11 +92,22 @@ pub mod tonic { severity_number: severity_number.into(), severity_text: log_record.severity_text.map(Into::into).unwrap_or_default(), body: log_record.body.map(Into::into), - attributes: log_record - .attributes - .map(Attributes::from_iter) - .unwrap_or_default() - .0, + attributes: { + let mut attributes = log_record + .attributes + .map(Attributes::from_iter) + .unwrap_or_default() + .0; + if let Some(event_name) = log_record.event_name.as_ref() { + attributes.push(KeyValue { + key: "name".into(), + value: Some(AnyValue { + value: Some(Value::StringValue(event_name.to_string())), + }), + }) + } + attributes + }, dropped_attributes_count: 0, flags: trace_context .map(|ctx| { diff --git a/opentelemetry-stdout/src/logs/transform.rs b/opentelemetry-stdout/src/logs/transform.rs index b2f5a43a57..80c4a877ef 100644 --- a/opentelemetry-stdout/src/logs/transform.rs +++ b/opentelemetry-stdout/src/logs/transform.rs @@ -131,16 +131,26 @@ impl From for LogRecord { .severity_number .map(|u| u as u32) .unwrap_or_default(), - attributes: value - .record - .attributes - .map(|attrs| { - attrs - .into_iter() - .map(|(key, value)| (key, value).into()) - .collect() - }) - .unwrap_or_default(), + attributes: { + let mut attributes = value + .record + .attributes + .unwrap_or_default() + .into_iter() + .map(|(key, value)| (key, value).into()) + .collect::>(); + + if let Some(event_name) = &value.record.event_name { + attributes.push( + ( + opentelemetry::Key::from("name"), + opentelemetry::Value::String(event_name.clone().into()), + ) + .into(), + ) + } + attributes + }, dropped_attributes_count: 0, severity_text: value.record.severity_text, body: value.record.body.map(|a| a.into()),