Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict LogRecord creation access outside opentelemetry_sdk crate. #2549

Merged
merged 3 commits into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion opentelemetry-proto/src/transform/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,15 +221,39 @@
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) {}

Check warning on line 236 in opentelemetry-proto/src/transform/logs.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-proto/src/transform/logs.rs#L236

Added line #L236 was not covered by tests

fn force_flush(&self) -> LogResult<()> {
Ok(())
}

Check warning on line 240 in opentelemetry-proto/src/transform/logs.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-proto/src/transform/logs.rs#L238-L240

Added lines #L238 - L240 were not covered by tests

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 =
Expand Down
4 changes: 4 additions & 0 deletions opentelemetry-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,10 @@ limit.
now:
`opentelemetry_sdk::logs::{ExportResult, LogBatch, LogExporter};`

- *Breaking* `opentelemetry_sdk::LogRecord::default()` method is removed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Breaking opentelemetry_sdk::LogRecord::default() method is removed.
    Use Logger::create_log_record() method instead.

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};`
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/logs/log_emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
22 changes: 11 additions & 11 deletions opentelemetry-sdk/src/logs/log_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1079,7 +1079,7 @@
.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);
Expand All @@ -1097,7 +1097,7 @@
.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);
Expand Down Expand Up @@ -1255,7 +1255,7 @@
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);
Expand All @@ -1268,7 +1268,7 @@
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);
Expand All @@ -1285,7 +1285,7 @@
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);
});
Expand All @@ -1304,7 +1304,7 @@
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);
Expand Down Expand Up @@ -1356,7 +1356,7 @@
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.
Expand Down Expand Up @@ -1412,7 +1412,7 @@
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();

Check warning on line 1415 in opentelemetry-sdk/src/logs/log_processor.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/logs/log_processor.rs#L1415

Added line #L1415 was not covered by tests
let instrumentation: InstrumentationScope = Default::default();
processor_clone.emit(&mut record, &instrumentation);
});
Expand All @@ -1436,7 +1436,7 @@
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);
Expand All @@ -1455,7 +1455,7 @@

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);
Expand All @@ -1475,7 +1475,7 @@

let processor = SimpleLogProcessor::new(exporter.clone());

let mut record: LogRecord = Default::default();
let mut record: LogRecord = LogRecord::new();

Check warning on line 1478 in opentelemetry-sdk/src/logs/log_processor.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/logs/log_processor.rs#L1478

Added line #L1478 was not covered by tests
let instrumentation: InstrumentationScope = Default::default();

processor.emit(&mut record, &instrumentation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@
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);
Expand Down Expand Up @@ -817,7 +817,7 @@
let processor =
BatchLogProcessor::new(exporter.clone(), BatchConfig::default(), runtime::Tokio);

let mut record = LogRecord::default();
let mut record = LogRecord::new();

Check warning on line 820 in opentelemetry-sdk/src/logs/log_processor_with_async_runtime.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/logs/log_processor_with_async_runtime.rs#L820

Added line #L820 was not covered by tests
let instrumentation = InstrumentationScope::default();

processor.emit(&mut record, &instrumentation);
Expand Down
39 changes: 27 additions & 12 deletions opentelemetry-sdk/src/logs/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const PREALLOCATED_ATTRIBUTE_CAPACITY: usize = 5;
pub(crate) type LogRecordAttributes =
GrowableArray<Option<(Key, AnyValue)>, 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.
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -220,60 +235,60 @@ 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));
}

#[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));
}

#[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));
}

#[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));
}

#[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 {
Expand All @@ -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());
Expand Down Expand Up @@ -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);
Expand Down
Loading