From 8a4e5ca41933b7b48a5d6c6ca9e8d9304dc05f25 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 24 Jan 2025 07:08:49 -0800 Subject: [PATCH] Restrict LogRecord creation access outside opentelemetry_sdk crate. (#2549) --- opentelemetry-proto/src/transform/logs.rs | 26 ++++++++++++- opentelemetry-sdk/CHANGELOG.md | 4 ++ opentelemetry-sdk/src/logs/log_emitter.rs | 2 +- opentelemetry-sdk/src/logs/log_processor.rs | 22 +++++------ .../logs/log_processor_with_async_runtime.rs | 4 +- opentelemetry-sdk/src/logs/record.rs | 39 +++++++++++++------ 6 files changed, 70 insertions(+), 27 deletions(-) diff --git a/opentelemetry-proto/src/transform/logs.rs b/opentelemetry-proto/src/transform/logs.rs index 37be35709b..8bbefe0c65 100644 --- a/opentelemetry-proto/src/transform/logs.rs +++ b/opentelemetry-proto/src/transform/logs.rs @@ -221,15 +221,39 @@ pub mod tonic { mod tests { use crate::transform::common::tonic::ResourceAttributesWithSchema; use opentelemetry::logs::LogRecord as _; + use opentelemetry::logs::Logger as _; + use opentelemetry::logs::LoggerProvider as _; use opentelemetry::InstrumentationScope; + use opentelemetry_sdk::logs::LogProcessor; + use opentelemetry_sdk::logs::{LogResult, LoggerProvider}; use opentelemetry_sdk::{logs::LogBatch, logs::LogRecord, Resource}; use std::time::SystemTime; + #[derive(Debug)] + struct MockProcessor; + + impl LogProcessor for MockProcessor { + fn emit(&self, _record: &mut LogRecord, _instrumentation: &InstrumentationScope) {} + + fn force_flush(&self) -> LogResult<()> { + Ok(()) + } + + fn shutdown(&self) -> LogResult<()> { + Ok(()) + } + } + fn create_test_log_data( instrumentation_name: &str, _message: &str, ) -> (LogRecord, InstrumentationScope) { - let mut logrecord = LogRecord::default(); + let processor = MockProcessor {}; + let logger = LoggerProvider::builder() + .with_log_processor(processor) + .build() + .logger("test"); + let mut logrecord = logger.create_log_record(); logrecord.set_timestamp(SystemTime::now()); logrecord.set_observed_timestamp(SystemTime::now()); let instrumentation = diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index ceb2df532b..6d355db452 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -282,6 +282,10 @@ limit. now: `opentelemetry_sdk::logs::{ExportResult, LogBatch, LogExporter};` +- *Breaking* `opentelemetry_sdk::LogRecord::default()` method is removed. + The only way to create log record outside opentelemetry_sdk crate is using + `Logger::create_log_record()` method. + - *Breaking*: Rename namespaces for InMemoryExporters. (The module is still under "testing" feature flag) before: `opentelemetry_sdk::testing::logs::{InMemoryLogExporter, InMemoryLogExporterBuilder};` diff --git a/opentelemetry-sdk/src/logs/log_emitter.rs b/opentelemetry-sdk/src/logs/log_emitter.rs index 94546f7c61..7c7e3a9509 100644 --- a/opentelemetry-sdk/src/logs/log_emitter.rs +++ b/opentelemetry-sdk/src/logs/log_emitter.rs @@ -293,7 +293,7 @@ impl opentelemetry::logs::Logger for Logger { type LogRecord = LogRecord; fn create_log_record(&self) -> Self::LogRecord { - LogRecord::default() + LogRecord::new() } /// Emit a `LogRecord`. diff --git a/opentelemetry-sdk/src/logs/log_processor.rs b/opentelemetry-sdk/src/logs/log_processor.rs index 68ba64bb7a..222c9e186b 100644 --- a/opentelemetry-sdk/src/logs/log_processor.rs +++ b/opentelemetry-sdk/src/logs/log_processor.rs @@ -1079,7 +1079,7 @@ mod tests { .build(); let processor = BatchLogProcessor::new(exporter.clone(), BatchConfig::default()); - let mut record = LogRecord::default(); + let mut record = LogRecord::new(); let instrumentation = InstrumentationScope::default(); processor.emit(&mut record, &instrumentation); @@ -1097,7 +1097,7 @@ mod tests { .build(); let processor = SimpleLogProcessor::new(exporter.clone()); - let mut record: LogRecord = Default::default(); + let mut record: LogRecord = LogRecord::new(); let instrumentation: InstrumentationScope = Default::default(); processor.emit(&mut record, &instrumentation); @@ -1255,7 +1255,7 @@ mod tests { let exporter = InMemoryLogExporterBuilder::default().build(); let processor = SimpleLogProcessor::new(exporter.clone()); - let mut record: LogRecord = Default::default(); + let mut record: LogRecord = LogRecord::new(); let instrumentation: InstrumentationScope = Default::default(); processor.emit(&mut record, &instrumentation); @@ -1268,7 +1268,7 @@ mod tests { let exporter = InMemoryLogExporterBuilder::default().build(); let processor = SimpleLogProcessor::new(exporter.clone()); - let mut record: LogRecord = Default::default(); + let mut record: LogRecord = LogRecord::new(); let instrumentation: InstrumentationScope = Default::default(); processor.emit(&mut record, &instrumentation); @@ -1285,7 +1285,7 @@ mod tests { for _ in 0..10 { let processor_clone = Arc::clone(&processor); let handle = tokio::spawn(async move { - let mut record: LogRecord = Default::default(); + let mut record: LogRecord = LogRecord::new(); let instrumentation: InstrumentationScope = Default::default(); processor_clone.emit(&mut record, &instrumentation); }); @@ -1304,7 +1304,7 @@ mod tests { let exporter = InMemoryLogExporterBuilder::default().build(); let processor = SimpleLogProcessor::new(exporter.clone()); - let mut record: LogRecord = Default::default(); + let mut record: LogRecord = LogRecord::new(); let instrumentation: InstrumentationScope = Default::default(); processor.emit(&mut record, &instrumentation); @@ -1356,7 +1356,7 @@ mod tests { let exporter = LogExporterThatRequiresTokio::new(); let processor = SimpleLogProcessor::new(exporter.clone()); - let mut record: LogRecord = Default::default(); + let mut record: LogRecord = LogRecord::new(); let instrumentation: InstrumentationScope = Default::default(); // This will panic because an tokio async operation within exporter without a runtime. @@ -1412,7 +1412,7 @@ mod tests { for _ in 0..concurrent_emit { let processor_clone = Arc::clone(&processor); let handle = tokio::spawn(async move { - let mut record: LogRecord = Default::default(); + let mut record: LogRecord = LogRecord::new(); let instrumentation: InstrumentationScope = Default::default(); processor_clone.emit(&mut record, &instrumentation); }); @@ -1436,7 +1436,7 @@ mod tests { let exporter = LogExporterThatRequiresTokio::new(); let processor = SimpleLogProcessor::new(exporter.clone()); - let mut record: LogRecord = Default::default(); + let mut record: LogRecord = LogRecord::new(); let instrumentation: InstrumentationScope = Default::default(); processor.emit(&mut record, &instrumentation); @@ -1455,7 +1455,7 @@ mod tests { let processor = SimpleLogProcessor::new(exporter.clone()); - let mut record: LogRecord = Default::default(); + let mut record: LogRecord = LogRecord::new(); let instrumentation: InstrumentationScope = Default::default(); processor.emit(&mut record, &instrumentation); @@ -1475,7 +1475,7 @@ mod tests { let processor = SimpleLogProcessor::new(exporter.clone()); - let mut record: LogRecord = Default::default(); + let mut record: LogRecord = LogRecord::new(); let instrumentation: InstrumentationScope = Default::default(); processor.emit(&mut record, &instrumentation); diff --git a/opentelemetry-sdk/src/logs/log_processor_with_async_runtime.rs b/opentelemetry-sdk/src/logs/log_processor_with_async_runtime.rs index 781c7bb054..765d1ebd98 100644 --- a/opentelemetry-sdk/src/logs/log_processor_with_async_runtime.rs +++ b/opentelemetry-sdk/src/logs/log_processor_with_async_runtime.rs @@ -555,7 +555,7 @@ mod tests { let processor = BatchLogProcessor::new(exporter.clone(), BatchConfig::default(), runtime::Tokio); - let mut record = LogRecord::default(); + let mut record = LogRecord::new(); let instrumentation = InstrumentationScope::default(); processor.emit(&mut record, &instrumentation); @@ -817,7 +817,7 @@ mod tests { let processor = BatchLogProcessor::new(exporter.clone(), BatchConfig::default(), runtime::Tokio); - let mut record = LogRecord::default(); + let mut record = LogRecord::new(); let instrumentation = InstrumentationScope::default(); processor.emit(&mut record, &instrumentation); diff --git a/opentelemetry-sdk/src/logs/record.rs b/opentelemetry-sdk/src/logs/record.rs index 3e4f5c8c18..740ee14c97 100644 --- a/opentelemetry-sdk/src/logs/record.rs +++ b/opentelemetry-sdk/src/logs/record.rs @@ -18,7 +18,7 @@ const PREALLOCATED_ATTRIBUTE_CAPACITY: usize = 5; pub(crate) type LogRecordAttributes = GrowableArray, PREALLOCATED_ATTRIBUTE_CAPACITY>; -#[derive(Debug, Default, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq)] #[non_exhaustive] /// LogRecord represents all data carried by a log record, and /// is provided to `LogExporter`s as input. @@ -118,6 +118,21 @@ impl opentelemetry::logs::LogRecord for LogRecord { } impl LogRecord { + /// Crate only default constructor + pub(crate) fn new() -> Self { + LogRecord { + event_name: None, + target: None, + timestamp: None, + observed_timestamp: None, + trace_context: None, + severity_text: None, + severity_number: None, + body: None, + attributes: LogRecordAttributes::default(), + } + } + /// Returns the event name #[inline] pub fn event_name(&self) -> Option<&'static str> { @@ -220,21 +235,21 @@ mod tests { #[test] fn test_set_eventname() { - let mut log_record = LogRecord::default(); + let mut log_record = LogRecord::new(); log_record.set_event_name("test_event"); assert_eq!(log_record.event_name, Some("test_event")); } #[test] fn test_set_target() { - let mut log_record = LogRecord::default(); + let mut log_record = LogRecord::new(); log_record.set_target("foo::bar"); assert_eq!(log_record.target, Some(Cow::Borrowed("foo::bar"))); } #[test] fn test_set_timestamp() { - let mut log_record = LogRecord::default(); + let mut log_record = LogRecord::new(); let now = SystemTime::now(); log_record.set_timestamp(now); assert_eq!(log_record.timestamp, Some(now)); @@ -242,7 +257,7 @@ mod tests { #[test] fn test_set_observed_timestamp() { - let mut log_record = LogRecord::default(); + let mut log_record = LogRecord::new(); let now = SystemTime::now(); log_record.set_observed_timestamp(now); assert_eq!(log_record.observed_timestamp, Some(now)); @@ -250,14 +265,14 @@ mod tests { #[test] fn test_set_severity_text() { - let mut log_record = LogRecord::default(); + let mut log_record = LogRecord::new(); log_record.set_severity_text("ERROR"); assert_eq!(log_record.severity_text, Some("ERROR")); } #[test] fn test_set_severity_number() { - let mut log_record = LogRecord::default(); + let mut log_record = LogRecord::new(); let severity_number = Severity::Error; log_record.set_severity_number(severity_number); assert_eq!(log_record.severity_number, Some(Severity::Error)); @@ -265,7 +280,7 @@ mod tests { #[test] fn test_set_body() { - let mut log_record = LogRecord::default(); + let mut log_record = LogRecord::new(); let body = AnyValue::String("Test body".into()); log_record.set_body(body.clone()); assert_eq!(log_record.body, Some(body)); @@ -273,7 +288,7 @@ mod tests { #[test] fn test_set_attributes() { - let mut log_record = LogRecord::default(); + let mut log_record = LogRecord::new(); let attributes = vec![(Key::new("key"), AnyValue::String("value".into()))]; log_record.add_attributes(attributes.clone()); for (key, value) in attributes { @@ -283,7 +298,7 @@ mod tests { #[test] fn test_set_attribute() { - let mut log_record = LogRecord::default(); + let mut log_record = LogRecord::new(); log_record.add_attribute("key", "value"); let key = Key::new("key"); let value = AnyValue::String("value".into()); @@ -344,12 +359,12 @@ mod tests { fn compare_log_record_target_borrowed_eq_owned() { let log_record_borrowed = LogRecord { event_name: Some("test_event"), - ..Default::default() + ..LogRecord::new() }; let log_record_owned = LogRecord { event_name: Some("test_event"), - ..Default::default() + ..LogRecord::new() }; assert_eq!(log_record_borrowed, log_record_owned);