fix(proto): preserve InstrumentationScope version and attributes (#3332)

Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com>

Author: djvcom

opentelemetry-proto/CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Update proto definitions to v1.10.0.
 - Updated `schemars` dependency to version 1.0.0.
+- **Bug fix**: `InstrumentationScope` version and attributes are now preserved when logs have a target set. Previously, setting a log target would discard the scope's version and attributes. ([#3276](https://github.com/open-telemetry/opentelemetry-rust/issues/3276))
 
 ## 0.31.0
 

opentelemetry-proto/src/transform/common.rs

@@ -55,20 +55,11 @@ pub mod tonic {
             ),
         ) -> Self {
             let (library, target) = data;
-            if let Some(t) = target {
-                InstrumentationScope {
-                    name: t.to_string(),
-                    version: String::new(),
-                    attributes: vec![],
-                    ..Default::default()
-                }
-            } else {
-                InstrumentationScope {
-                    name: library.name().to_owned(),
-                    version: library.version().map(ToOwned::to_owned).unwrap_or_default(),
-                    attributes: Attributes::from(library.attributes().cloned()).0,
-                    ..Default::default()
-                }
+            InstrumentationScope {
+                name: target.map_or_else(|| library.name().to_owned(), Cow::into_owned),
+                version: library.version().unwrap_or_default().to_owned(),
+                attributes: Attributes::from(library.attributes().cloned()).0,
+                ..Default::default()
             }
         }
     }
@@ -86,20 +77,11 @@ pub mod tonic {
             ),
         ) -> Self {
             let (library, target) = data;
-            if let Some(t) = target {
-                InstrumentationScope {
-                    name: t.to_string(),
-                    version: String::new(),
-                    attributes: vec![],
-                    ..Default::default()
-                }
-            } else {
-                InstrumentationScope {
-                    name: library.name().to_owned(),
-                    version: library.version().map(ToOwned::to_owned).unwrap_or_default(),
-                    attributes: Attributes::from(library.attributes().cloned()).0,
-                    ..Default::default()
-                }
+            InstrumentationScope {
+                name: target.map_or_else(|| library.name().to_owned(), Cow::into_owned),
+                version: library.version().unwrap_or_default().to_owned(),
+                attributes: Attributes::from(library.attributes().cloned()).0,
+                ..Default::default()
             }
         }
     }
@@ -178,4 +160,53 @@ pub mod tonic {
             .collect::<Vec<_>>()
             .into()
     }
+
+    #[cfg(test)]
+    mod tests {
+        use super::*;
+        use opentelemetry::KeyValue;
+        use std::borrow::Cow;
+
+        fn assert_scope_fields(
+            proto_scope: &InstrumentationScope,
+            expected_name: &str,
+            expected_version: &str,
+            expected_attr_key: &str,
+        ) {
+            assert_eq!(proto_scope.name, expected_name);
+            assert_eq!(proto_scope.version, expected_version);
+            assert_eq!(proto_scope.attributes.len(), 1);
+            assert_eq!(proto_scope.attributes[0].key, expected_attr_key);
+        }
+
+        #[test]
+        fn instrumentation_scope_with_target_overrides_name_but_preserves_version_and_attributes() {
+            let scope = opentelemetry::InstrumentationScope::builder("my-lib")
+                .with_version("1.0.0")
+                .with_attributes([KeyValue::new("feature", "metrics")])
+                .build();
+            let target: Option<Cow<'static, str>> = Some(Cow::Borrowed("my_app::handlers"));
+
+            let from_owned = InstrumentationScope::from((scope.clone(), target.clone()));
+            let from_ref = InstrumentationScope::from((&scope, target));
+
+            assert_scope_fields(&from_owned, "my_app::handlers", "1.0.0", "feature");
+            assert_scope_fields(&from_ref, "my_app::handlers", "1.0.0", "feature");
+        }
+
+        #[test]
+        fn instrumentation_scope_without_target_preserves_all_fields() {
+            let scope = opentelemetry::InstrumentationScope::builder("my-lib")
+                .with_version("1.0.0")
+                .with_attributes([KeyValue::new("feature", "metrics")])
+                .build();
+            let target: Option<Cow<'static, str>> = None;
+
+            let from_owned = InstrumentationScope::from((scope.clone(), target.clone()));
+            let from_ref = InstrumentationScope::from((&scope, target));
+
+            assert_scope_fields(&from_owned, "my-lib", "1.0.0", "feature");
+            assert_scope_fields(&from_ref, "my-lib", "1.0.0", "feature");
+        }
+    }
 }

opentelemetry-proto/src/transform/logs.rs

@@ -228,11 +228,12 @@ mod tests {
     use opentelemetry::logs::Logger;
     use opentelemetry::logs::LoggerProvider;
     use opentelemetry::time::now;
-    use opentelemetry::InstrumentationScope;
+    use opentelemetry::{InstrumentationScope, KeyValue};
     use opentelemetry_sdk::error::OTelSdkResult;
     use opentelemetry_sdk::logs::LogProcessor;
     use opentelemetry_sdk::logs::SdkLoggerProvider;
     use opentelemetry_sdk::{logs::LogBatch, logs::SdkLogRecord, Resource};
+    use std::borrow::Cow;
 
     #[derive(Debug)]
     struct MockProcessor;
@@ -317,4 +318,41 @@ mod tests {
         assert_eq!(scope_logs_1.log_records.len(), 1);
         assert_eq!(scope_logs_2.log_records.len(), 1);
     }
+
+    #[test]
+    fn test_group_logs_preserves_scope_version_and_attributes_when_target_set() {
+        let resource = Resource::builder().build();
+        let processor = MockProcessor {};
+        let logger = SdkLoggerProvider::builder()
+            .with_log_processor(processor)
+            .build()
+            .logger("test");
+
+        let mut logrecord = logger.create_log_record();
+        logrecord.set_timestamp(now());
+        logrecord.set_observed_timestamp(now());
+        logrecord.set_target(Cow::Borrowed("my_app::handlers"));
+
+        let instrumentation = InstrumentationScope::builder("my-lib")
+            .with_version("1.0.0")
+            .with_attributes([KeyValue::new("feature", "metrics")])
+            .build();
+
+        let logs = [(&logrecord, &instrumentation)];
+        let log_batch = LogBatch::new(&logs);
+        let resource: ResourceAttributesWithSchema = (&resource).into();
+
+        let grouped_logs =
+            crate::transform::logs::tonic::group_logs_by_resource_and_scope(&log_batch, &resource);
+
+        assert_eq!(grouped_logs.len(), 1);
+        let resource_logs = &grouped_logs[0];
+        assert_eq!(resource_logs.scope_logs.len(), 1);
+
+        let scope = resource_logs.scope_logs[0].scope.as_ref().unwrap();
+        assert_eq!(scope.name, "my_app::handlers");
+        assert_eq!(scope.version, "1.0.0");
+        assert_eq!(scope.attributes.len(), 1);
+        assert_eq!(scope.attributes[0].key, "feature");
+    }
 }