Make LogRecord fields private and add getters for encapsulation (#2314)

lalitb · web-flow · commit fbc3c70c2716 · 2024-12-15T20:28:55.000+05:30
diff --git a/opentelemetry-appender-log/src/lib.rs b/opentelemetry-appender-log/src/lib.rs
@@ -846,11 +846,11 @@ mod tests {
 
         assert_eq!(logs.len(), 5);
         for log in logs {
-            let body: String = match log.record.body.as_ref().unwrap() {
+            let body: String = match log.record.body().unwrap() {
                 super::AnyValue::String(s) => s.to_string(),
                 _ => panic!("AnyValue::String expected"),
             };
-            assert_eq!(body, log.record.severity_text.unwrap());
+            assert_eq!(body, log.record.severity_text().unwrap());
         }
     }
 
diff --git a/opentelemetry-appender-tracing/src/layer.rs b/opentelemetry-appender-tracing/src/layer.rs
@@ -334,10 +334,10 @@ mod tests {
 
         // Validate common fields
         assert_eq!(log.instrumentation.name(), "opentelemetry-appender-tracing");
-        assert_eq!(log.record.severity_number, Some(Severity::Error));
+        assert_eq!(log.record.severity_number(), Some(Severity::Error));
 
         // Validate trace context is none.
-        assert!(log.record.trace_context.is_none());
+        assert!(log.record.trace_context().is_none());
 
         // Validate attributes
         #[cfg(not(feature = "experimental_metadata_attributes"))]
@@ -428,25 +428,20 @@ mod tests {
 
         // validate common fields.
         assert_eq!(log.instrumentation.name(), "opentelemetry-appender-tracing");
-        assert_eq!(log.record.severity_number, Some(Severity::Error));
+        assert_eq!(log.record.severity_number(), Some(Severity::Error));
 
         // validate trace context.
-        assert!(log.record.trace_context.is_some());
+        assert!(log.record.trace_context().is_some());
         assert_eq!(
-            log.record.trace_context.as_ref().unwrap().trace_id,
+            log.record.trace_context().unwrap().trace_id,
             trace_id_expected
         );
         assert_eq!(
-            log.record.trace_context.as_ref().unwrap().span_id,
+            log.record.trace_context().unwrap().span_id,
             span_id_expected
         );
         assert_eq!(
-            log.record
-                .trace_context
-                .as_ref()
-                .unwrap()
-                .trace_flags
-                .unwrap(),
+            log.record.trace_context().unwrap().trace_flags.unwrap(),
             TraceFlags::SAMPLED
         );
 
@@ -526,10 +521,10 @@ mod tests {
 
         // Validate common fields
         assert_eq!(log.instrumentation.name(), "opentelemetry-appender-tracing");
-        assert_eq!(log.record.severity_number, Some(Severity::Error));
+        assert_eq!(log.record.severity_number(), Some(Severity::Error));
 
         // Validate trace context is none.
-        assert!(log.record.trace_context.is_none());
+        assert!(log.record.trace_context().is_none());
 
         // Attributes can be polluted when we don't use this feature.
         #[cfg(feature = "experimental_metadata_attributes")]
@@ -605,25 +600,20 @@ mod tests {
 
         // validate common fields.
         assert_eq!(log.instrumentation.name(), "opentelemetry-appender-tracing");
-        assert_eq!(log.record.severity_number, Some(Severity::Error));
+        assert_eq!(log.record.severity_number(), Some(Severity::Error));
 
         // validate trace context.
-        assert!(log.record.trace_context.is_some());
+        assert!(log.record.trace_context().is_some());
         assert_eq!(
-            log.record.trace_context.as_ref().unwrap().trace_id,
+            log.record.trace_context().unwrap().trace_id,
             trace_id_expected
         );
         assert_eq!(
-            log.record.trace_context.as_ref().unwrap().span_id,
+            log.record.trace_context().unwrap().span_id,
             span_id_expected
         );
         assert_eq!(
-            log.record
-                .trace_context
-                .as_ref()
-                .unwrap()
-                .trace_flags
-                .unwrap(),
+            log.record.trace_context().unwrap().trace_flags.unwrap(),
             TraceFlags::SAMPLED
         );
 
diff --git a/opentelemetry-proto/src/transform/logs.rs b/opentelemetry-proto/src/transform/logs.rs
@@ -58,8 +58,8 @@ pub mod tonic {
 
     impl From<&opentelemetry_sdk::logs::LogRecord> for LogRecord {
         fn from(log_record: &opentelemetry_sdk::logs::LogRecord) -> Self {
-            let trace_context = log_record.trace_context.as_ref();
-            let severity_number = match log_record.severity_number {
+            let trace_context = log_record.trace_context();
+            let severity_number = match log_record.severity_number() {
                 Some(Severity::Trace) => SeverityNumber::Trace,
                 Some(Severity::Trace2) => SeverityNumber::Trace2,
                 Some(Severity::Trace3) => SeverityNumber::Trace3,
@@ -88,8 +88,8 @@ pub mod tonic {
             };
 
             LogRecord {
-                time_unix_nano: log_record.timestamp.map(to_nanos).unwrap_or_default(),
-                observed_time_unix_nano: to_nanos(log_record.observed_timestamp.unwrap()),
+                time_unix_nano: log_record.timestamp().map(to_nanos).unwrap_or_default(),
+                observed_time_unix_nano: to_nanos(log_record.observed_timestamp().unwrap()),
                 attributes: {
                     let attributes: Vec<KeyValue> = log_record
                         .attributes_iter()
@@ -102,7 +102,7 @@ pub mod tonic {
                         .collect();
                     #[cfg(feature = "populate-logs-event-name")]
                     {
-                        if let Some(event_name) = &log_record.event_name {
+                        if let Some(event_name) = &log_record.event_name() {
                             let mut attributes_with_name = attributes;
                             attributes_with_name.push(KeyValue {
                                 key: "event.name".into(),
@@ -119,8 +119,11 @@ pub mod tonic {
                     attributes
                 },
                 severity_number: severity_number.into(),
-                severity_text: log_record.severity_text.map(Into::into).unwrap_or_default(),
-                body: log_record.body.clone().map(Into::into),
+                severity_text: log_record
+                    .severity_text()
+                    .map(Into::into)
+                    .unwrap_or_default(),
+                body: log_record.body().cloned().map(Into::into),
                 dropped_attributes_count: 0,
                 flags: trace_context
                     .map(|ctx| {
@@ -170,7 +173,7 @@ pub mod tonic {
                         .schema_url()
                         .map(ToOwned::to_owned)
                         .unwrap_or_default(),
-                    scope: Some((instrumentation, log_record.target.clone()).into()),
+                    scope: Some((instrumentation, log_record.target().cloned()).into()),
                     log_records: vec![log_record.into()],
                 }],
             }
@@ -193,8 +196,8 @@ pub mod tonic {
             >,
              (log_record, instrumentation)| {
                 let key = log_record
-                    .target
-                    .clone()
+                    .target()
+                    .cloned()
                     .unwrap_or_else(|| Cow::Owned(instrumentation.name().to_owned()));
                 scope_map
                     .entry(key)
diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md
@@ -96,6 +96,12 @@
   - Bump msrv to 1.75.0.
 
 
+- *Breaking* : [#2314](https://github.com/open-telemetry/opentelemetry-rust/pull/2314)
+  - The LogRecord struct has been updated:
+    - All fields are now pub(crate) instead of pub.
+    - Getter methods have been introduced to access field values.
+    This change impacts custom exporter and processor developers by requiring updates to code that directly accessed LogRecord fields. They must now use the provided getter methods (e.g., `log_record.event_name()` instead of `log_record.event_name`).
+
 ## 0.27.1
 
 Released 2024-Nov-27
diff --git a/opentelemetry-sdk/src/logs/record.rs b/opentelemetry-sdk/src/logs/record.rs
@@ -24,28 +24,28 @@ pub(crate) type LogRecordAttributes =
 /// is provided to `LogExporter`s as input.
 pub struct LogRecord {
     /// Event name. Optional as not all the logging API support it.
-    pub event_name: Option<&'static str>,
+    pub(crate) event_name: Option<&'static str>,
 
     /// Target of the log record
-    pub target: Option<Cow<'static, str>>,
+    pub(crate) target: Option<Cow<'static, str>>,
 
     /// Record timestamp
-    pub timestamp: Option<SystemTime>,
+    pub(crate) timestamp: Option<SystemTime>,
 
     /// Timestamp for when the record was observed by OpenTelemetry
-    pub observed_timestamp: Option<SystemTime>,
+    pub(crate) observed_timestamp: Option<SystemTime>,
 
     /// Trace context for logs associated with spans
-    pub trace_context: Option<TraceContext>,
+    pub(crate) trace_context: Option<TraceContext>,
 
     /// The original severity string from the source
-    pub severity_text: Option<&'static str>,
+    pub(crate) severity_text: Option<&'static str>,
 
     /// The corresponding severity value, normalized
-    pub severity_number: Option<Severity>,
+    pub(crate) severity_number: Option<Severity>,
 
     /// Record body
-    pub body: Option<AnyValue>,
+    pub(crate) body: Option<AnyValue>,
 
     /// Additional attributes associated with this record
     pub(crate) attributes: LogRecordAttributes,
@@ -118,7 +118,56 @@ impl opentelemetry::logs::LogRecord for LogRecord {
 }
 
 impl LogRecord {
+    /// Returns the event name
+    #[inline]
+    pub fn event_name(&self) -> Option<&'static str> {
+        self.event_name
+    }
+
+    /// Returns the target
+    #[inline]
+    pub fn target(&self) -> Option<&Cow<'static, str>> {
+        self.target.as_ref()
+    }
+
+    /// Returns the timestamp
+    #[inline]
+    pub fn timestamp(&self) -> Option<SystemTime> {
+        self.timestamp
+    }
+
+    /// Returns the observed timestamp
+    #[inline]
+    pub fn observed_timestamp(&self) -> Option<SystemTime> {
+        self.observed_timestamp
+    }
+
+    /// Returns the trace context
+    #[inline]
+    pub fn trace_context(&self) -> Option<&TraceContext> {
+        self.trace_context.as_ref()
+    }
+
+    /// Returns the severity text
+    #[inline]
+    pub fn severity_text(&self) -> Option<&'static str> {
+        self.severity_text
+    }
+
+    /// Returns the severity number
+    #[inline]
+    pub fn severity_number(&self) -> Option<Severity> {
+        self.severity_number
+    }
+
+    /// Returns the body
+    #[inline]
+    pub fn body(&self) -> Option<&AnyValue> {
+        self.body.as_ref()
+    }
+
     /// Provides an iterator over the attributes.
+    #[inline]
     pub fn attributes_iter(&self) -> impl Iterator<Item = &(Key, AnyValue)> {
         self.attributes.iter().filter_map(|opt| opt.as_ref())
     }
diff --git a/opentelemetry-stdout/src/logs/exporter.rs b/opentelemetry-stdout/src/logs/exporter.rs
@@ -73,34 +73,34 @@ fn print_logs(batch: LogBatch<'_>) {
     for (i, log) in batch.iter().enumerate() {
         println!("Log #{}", i);
         let (record, _library) = log;
-        if let Some(event_name) = record.event_name {
+        if let Some(event_name) = record.event_name() {
             println!("\t EventName: {:?}", event_name);
         }
-        if let Some(target) = &record.target {
+        if let Some(target) = record.target() {
             println!("\t Target (Scope): {:?}", target);
         }
-        if let Some(trace_context) = &record.trace_context {
+        if let Some(trace_context) = record.trace_context() {
             println!("\t TraceId: {:?}", trace_context.trace_id);
             println!("\t SpanId: {:?}", trace_context.span_id);
         }
-        if let Some(timestamp) = record.timestamp {
+        if let Some(timestamp) = record.timestamp() {
             let datetime: DateTime<Utc> = timestamp.into();
             println!("\t Timestamp: {}", datetime.format("%Y-%m-%d %H:%M:%S%.6f"));
         }
-        if let Some(timestamp) = record.observed_timestamp {
+        if let Some(timestamp) = record.observed_timestamp() {
             let datetime: DateTime<Utc> = timestamp.into();
             println!(
                 "\t Observed Timestamp: {}",
                 datetime.format("%Y-%m-%d %H:%M:%S%.6f")
             );
         }
-        if let Some(severity) = record.severity_text {
+        if let Some(severity) = record.severity_text() {
             println!("\t SeverityText: {:?}", severity);
         }
-        if let Some(severity) = record.severity_number {
+        if let Some(severity) = record.severity_number() {
             println!("\t SeverityNumber: {:?}", severity);
         }
-        if let Some(body) = &record.body {
+        if let Some(body) = record.body() {
             println!("\t Body: {:?}", body);
         }
 

Original file line number	Diff line number	Diff line change
`@@ -846,11 +846,11 @@ mod tests {`
`846`	`846`
`847`	`847`	`assert_eq!(logs.len(), 5);`
`848`	`848`	`for log in logs {`
`849`		`- let body: String = match log.record.body.as_ref().unwrap() {`
	`849`	`+ let body: String = match log.record.body().unwrap() {`
`850`	`850`	`super::AnyValue::String(s) => s.to_string(),`
`851`	`851`	`_ => panic!("AnyValue::String expected"),`
`852`	`852`	`};`
`853`		`- assert_eq!(body, log.record.severity_text.unwrap());`
	`853`	`+ assert_eq!(body, log.record.severity_text().unwrap());`
`854`	`854`	`}`
`855`	`855`	`}`
`856`	`856`
Original file line number	Diff line number	Diff line change
`@@ -73,34 +73,34 @@ fn print_logs(batch: LogBatch<'_>) {`
`73`	`73`	`for (i, log) in batch.iter().enumerate() {`
`74`	`74`	`println!("Log #{}", i);`
`75`	`75`	`let (record, _library) = log;`
`76`		`- if let Some(event_name) = record.event_name {`
	`76`	`+ if let Some(event_name) = record.event_name() {`
`77`	`77`	`println!("\t EventName: {:?}", event_name);`
`78`	`78`	`}`
`79`		`- if let Some(target) = &record.target {`
	`79`	`+ if let Some(target) = record.target() {`
`80`	`80`	`println!("\t Target (Scope): {:?}", target);`
`81`	`81`	`}`
`82`		`- if let Some(trace_context) = &record.trace_context {`
	`82`	`+ if let Some(trace_context) = record.trace_context() {`
`83`	`83`	`println!("\t TraceId: {:?}", trace_context.trace_id);`
`84`	`84`	`println!("\t SpanId: {:?}", trace_context.span_id);`
`85`	`85`	`}`
`86`		`- if let Some(timestamp) = record.timestamp {`
	`86`	`+ if let Some(timestamp) = record.timestamp() {`
`87`	`87`	`let datetime: DateTime<Utc> = timestamp.into();`
`88`	`88`	`println!("\t Timestamp: {}", datetime.format("%Y-%m-%d %H:%M:%S%.6f"));`
`89`	`89`	`}`
`90`		`- if let Some(timestamp) = record.observed_timestamp {`
	`90`	`+ if let Some(timestamp) = record.observed_timestamp() {`
`91`	`91`	`let datetime: DateTime<Utc> = timestamp.into();`
`92`	`92`	`println!(`
`93`	`93`	`"\t Observed Timestamp: {}",`
`94`	`94`	`datetime.format("%Y-%m-%d %H:%M:%S%.6f")`
`95`	`95`	`);`
`96`	`96`	`}`
`97`		`- if let Some(severity) = record.severity_text {`
	`97`	`+ if let Some(severity) = record.severity_text() {`
`98`	`98`	`println!("\t SeverityText: {:?}", severity);`
`99`	`99`	`}`
`100`		`- if let Some(severity) = record.severity_number {`
	`100`	`+ if let Some(severity) = record.severity_number() {`
`101`	`101`	`println!("\t SeverityNumber: {:?}", severity);`
`102`	`102`	`}`
`103`		`- if let Some(body) = &record.body {`
	`103`	`+ if let Some(body) = record.body() {`
`104`	`104`	`println!("\t Body: {:?}", body);`
`105`	`105`	`}`
`106`	`106`