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

Remove CoW for LogRecord::event_name and LogRecord::severity_text #2014

Merged
merged 5 commits into from
Aug 13, 2024
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
2 changes: 1 addition & 1 deletion opentelemetry-appender-log/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ where
if self.enabled(record.metadata()) {
let mut log_record = self.logger.create_log_record();
log_record.set_severity_number(severity_of_level(record.level()));
log_record.set_severity_text(record.level().as_str().into());
log_record.set_severity_text(record.level().as_str());
log_record.set_body(AnyValue::from(record.args().to_string()));
log_record.add_attributes(log_attributes(record.key_values()));
log_record.set_target(record.metadata().target().to_string());
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-appender-tracing/src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ where
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().as_str().into());
log_record.set_severity_text(meta.level().as_str());
let mut visitor = EventVisitor::new(&mut log_record);
#[cfg(feature = "experimental_metadata_attributes")]
visitor.visit_experimental_metadata(meta);
Expand Down
10 changes: 5 additions & 5 deletions opentelemetry-sdk/benches/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ fn logging_comparable_to_appender(c: &mut Criterion) {
log_record.set_target("my-target".to_string());
log_record.set_event_name("CheckoutFailed");
log_record.set_severity_number(Severity::Warn);
log_record.set_severity_text("WARN".into());
log_record.set_severity_text("WARN");
log_record.add_attribute("book_id", "12345");
log_record.add_attribute("book_title", "Rust Programming Adventures");
log_record.add_attribute("message", "Unable to process checkout.");
Expand Down Expand Up @@ -275,7 +275,7 @@ fn criterion_benchmark(c: &mut Criterion) {
log_record.set_timestamp(now);
log_record.set_observed_timestamp(now);
log_record.set_severity_number(Severity::Warn);
log_record.set_severity_text(Severity::Warn.name().into());
log_record.set_severity_text(Severity::Warn.name());
logger.emit(log_record);
});

Expand All @@ -285,7 +285,7 @@ fn criterion_benchmark(c: &mut Criterion) {
log_record.set_timestamp(now);
log_record.set_observed_timestamp(now);
log_record.set_severity_number(Severity::Warn);
log_record.set_severity_text(Severity::Warn.name().into());
log_record.set_severity_text(Severity::Warn.name());
log_record.add_attribute("name", "my-event-name");
log_record.add_attribute("event.id", 20);
log_record.add_attribute("user.name", "otel");
Expand All @@ -299,7 +299,7 @@ fn criterion_benchmark(c: &mut Criterion) {
log_record.set_timestamp(now);
log_record.set_observed_timestamp(now);
log_record.set_severity_number(Severity::Warn);
log_record.set_severity_text(Severity::Warn.name().into());
log_record.set_severity_text(Severity::Warn.name());
log_record.add_attribute("name", "my-event-name");
log_record.add_attribute("event.id", 20);
log_record.add_attribute("user.name", "otel");
Expand Down Expand Up @@ -338,7 +338,7 @@ fn criterion_benchmark(c: &mut Criterion) {
log_record.set_timestamp(now);
log_record.set_observed_timestamp(now);
log_record.set_severity_number(Severity::Warn);
log_record.set_severity_text(Severity::Warn.name().into());
log_record.set_severity_text(Severity::Warn.name());
log_record.add_attributes(attributes.clone());
logger.emit(log_record);
});
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/logs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ mod tests {
let logger = logger_provider.logger("test-logger");
let mut log_record = logger.create_log_record();
log_record.set_severity_number(Severity::Error);
log_record.set_severity_text("Error".into());
log_record.set_severity_text("Error");

// Adding attributes using a vector with explicitly constructed Key and AnyValue objects.
log_record.add_attributes(vec![
Expand Down
31 changes: 14 additions & 17 deletions opentelemetry-sdk/src/logs/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::{borrow::Cow, time::SystemTime};
/// 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<Cow<'static, str>>,
pub event_name: Option<&'static str>,

/// Target of the log record
pub target: Option<Cow<'static, str>>,
Expand All @@ -26,7 +26,8 @@ pub struct LogRecord {
pub trace_context: Option<TraceContext>,

/// The original severity string from the source
pub severity_text: Option<Cow<'static, str>>,
pub severity_text: Option<&'static str>,

/// The corresponding severity value, normalized
pub severity_number: Option<Severity>,

Expand All @@ -38,11 +39,8 @@ pub struct LogRecord {
}

impl opentelemetry::logs::LogRecord for LogRecord {
fn set_event_name<T>(&mut self, name: T)
where
T: Into<Cow<'static, str>>,
{
self.event_name = Some(name.into());
fn set_event_name(&mut self, name: &'static str) {
self.event_name = Some(name);
}

// Sets the `target` of a record
Expand All @@ -61,7 +59,7 @@ impl opentelemetry::logs::LogRecord for LogRecord {
self.observed_timestamp = Some(timestamp);
}

fn set_severity_text(&mut self, severity_text: Cow<'static, str>) {
fn set_severity_text(&mut self, severity_text: &'static str) {
self.severity_text = Some(severity_text);
}

Expand Down Expand Up @@ -154,7 +152,7 @@ mod tests {
fn test_set_eventname() {
let mut log_record = LogRecord::default();
log_record.set_event_name("test_event");
assert_eq!(log_record.event_name, Some(Cow::Borrowed("test_event")));
assert_eq!(log_record.event_name, Some("test_event"));
}

#[test]
Expand Down Expand Up @@ -183,9 +181,8 @@ mod tests {
#[test]
fn test_set_severity_text() {
let mut log_record = LogRecord::default();
let severity_text: Cow<'static, str> = "ERROR".into(); // Explicitly typed
log_record.set_severity_text(severity_text);
assert_eq!(log_record.severity_text, Some(Cow::Borrowed("ERROR")));
log_record.set_severity_text("ERROR");
assert_eq!(log_record.severity_text, Some("ERROR"));
}

#[test]
Expand Down Expand Up @@ -247,11 +244,11 @@ mod tests {
#[test]
fn compare_log_record() {
let log_record = LogRecord {
event_name: Some(Cow::Borrowed("test_event")),
event_name: Some("test_event"),
target: Some(Cow::Borrowed("foo::bar")),
timestamp: Some(SystemTime::now()),
observed_timestamp: Some(SystemTime::now()),
severity_text: Some(Cow::Borrowed("ERROR")),
severity_text: Some("ERROR"),
severity_number: Some(Severity::Error),
body: Some(AnyValue::String("Test body".into())),
attributes: Some(vec![(Key::new("key"), AnyValue::String("value".into()))]),
Expand All @@ -267,20 +264,20 @@ mod tests {
assert_eq!(log_record, log_record_cloned);

let mut log_record_different = log_record.clone();
log_record_different.event_name = Some(Cow::Borrowed("different_event"));
log_record_different.event_name = Some("different_event");

assert_ne!(log_record, log_record_different);
}

#[test]
fn compare_log_record_target_borrowed_eq_owned() {
let log_record_borrowed = LogRecord {
event_name: Some(Cow::Borrowed("test_event")),
event_name: Some("test_event"),
..Default::default()
};

let log_record_owned = LogRecord {
event_name: Some(Cow::Owned("test_event".to_string())),
event_name: Some("test_event"),
..Default::default()
};

Expand Down
6 changes: 3 additions & 3 deletions opentelemetry-stdout/src/logs/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
observed_time: SystemTime,
severity_number: u32,
#[serde(skip_serializing_if = "Option::is_none")]
severity_text: Option<Cow<'static, str>>,
severity_text: Option<&'static str>,
#[serde(skip_serializing_if = "Option::is_none")]
body: Option<Value>,
attributes: Vec<KeyValue>,
Expand All @@ -115,11 +115,11 @@
.collect::<Vec<KeyValue>>(); // Collect into a Vec<KeyValue>s

#[cfg(feature = "populate-logs-event-name")]
if let Some(event_name) = &value.record.event_name {
if let Some(event_name) = value.record.event_name {

Check warning on line 118 in opentelemetry-stdout/src/logs/transform.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-stdout/src/logs/transform.rs#L118

Added line #L118 was not covered by tests
let mut attributes_with_name = attributes;
attributes_with_name.push(KeyValue::from((
"name".into(),
opentelemetry::Value::String(event_name.clone().into()),
opentelemetry::Value::String(event_name.into()),

Check warning on line 122 in opentelemetry-stdout/src/logs/transform.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-stdout/src/logs/transform.rs#L122

Added line #L122 was not covered by tests
)));
attributes_with_name
} else {
Expand Down
8 changes: 2 additions & 6 deletions opentelemetry/src/logs/noop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,13 @@
impl LogRecord for NoopLogRecord {
// Implement the LogRecord trait methods with empty bodies.
#[inline]
fn set_event_name<T>(&mut self, _name: T)
where
T: Into<Cow<'static, str>>,
{
}
fn set_event_name(&mut self, _name: &'static str) {}

Check warning on line 44 in opentelemetry/src/logs/noop.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry/src/logs/noop.rs#L44

Added line #L44 was not covered by tests
#[inline]
fn set_timestamp(&mut self, _timestamp: SystemTime) {}
#[inline]
fn set_observed_timestamp(&mut self, _timestamp: SystemTime) {}
#[inline]
fn set_severity_text(&mut self, _text: Cow<'static, str>) {}
fn set_severity_text(&mut self, _text: &'static str) {}

Check warning on line 50 in opentelemetry/src/logs/noop.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry/src/logs/noop.rs#L50

Added line #L50 was not covered by tests
#[inline]
fn set_severity_number(&mut self, _number: Severity) {}
#[inline]
Expand Down
8 changes: 2 additions & 6 deletions opentelemetry/src/logs/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ use std::{borrow::Cow, collections::HashMap, time::SystemTime};
/// SDK implemented trait for managing log records
pub trait LogRecord {
/// Sets the `event_name` of a record
fn set_event_name<T>(&mut self, _name: T)
where
T: Into<Cow<'static, str>>,
{
}
fn set_event_name(&mut self, name: &'static str);

/// Sets the `target` of a record.
/// Currently, both `opentelemetry-appender-tracing` and `opentelemetry-appender-log` create a single logger
Expand All @@ -25,7 +21,7 @@ pub trait LogRecord {
fn set_observed_timestamp(&mut self, timestamp: SystemTime);

/// Sets severity as text.
fn set_severity_text(&mut self, text: Cow<'static, str>);
fn set_severity_text(&mut self, text: &'static str);

/// Sets severity as a numeric value.
fn set_severity_number(&mut self, number: Severity);
Expand Down
Loading