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

[WIP] Box complex types in AnyValue enum #1913

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions opentelemetry-appender-tracing/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ opentelemetry_sdk = { path = "../opentelemetry-sdk", features = ["logs", "testin
tracing-log = "0.2"
async-trait = { workspace = true }
criterion = { workspace = true }
smallvec = "1.13"

[target.'cfg(not(target_os = "windows"))'.dev-dependencies]
pprof = { version = "0.13", features = ["flamegraph", "criterion"] }
Expand Down
6 changes: 3 additions & 3 deletions opentelemetry-appender-tracing/benches/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,9 @@ fn benchmark_with_ot_layer(c: &mut Criterion, enabled: bool, bench_name: &str) {
c.bench_function(bench_name, |b| {
b.iter(|| {
error!(
name = "CheckoutFailed",
book_id = "12345",
book_title = "Rust Programming Adventures",
// name = "CheckoutFailed",
//book_id =12345,
// book_title = "Rust Programming Adventures",
"Unable to process checkout."
);
});
Expand Down
20 changes: 10 additions & 10 deletions opentelemetry-appender-tracing/src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,9 @@ impl<'a, LR: LogRecord> EventVisitor<'a, LR> {
fn new(log_record: &'a mut LR) -> Self {
EventVisitor { log_record }
}
fn visit_metadata(&mut self, meta: &Metadata) {
self.log_record
.add_attribute(Key::new("name"), AnyValue::from(meta.name()));

fn visit_metadata(&mut self, _meta: &Metadata) {
#[cfg(feature = "experimental_metadata_attributes")]
self.visit_experimental_metadata(meta);
self.visit_experimental_metadata(_meta);
}

#[cfg(feature = "experimental_metadata_attributes")]
Expand Down Expand Up @@ -168,11 +165,11 @@ where
#[cfg(not(feature = "experimental_metadata_attributes"))]
let meta = event.metadata();

//let mut log_record: LogRecord = LogRecord::default();
let mut log_record = self.logger.create_log_record();
log_record.set_severity_number(severity_of_level(meta.level()));
log_record.set_severity_text(meta.level().to_string().into());
log_record.set_target(meta.target().to_string());
log_record.set_event_name(meta.name());

let mut visitor = EventVisitor::new(&mut log_record);
visitor.visit_metadata(meta);
Expand Down Expand Up @@ -215,9 +212,12 @@ mod tests {
use opentelemetry_sdk::testing::logs::InMemoryLogsExporter;
use opentelemetry_sdk::trace;
use opentelemetry_sdk::trace::{Sampler, TracerProvider};
use smallvec::SmallVec;
use tracing::error;
use tracing_subscriber::layer::SubscriberExt;

const PREALLOCATED_ATTRIBUTE_CAPACITY: usize = 8;

// cargo test --features=testing
#[test]
fn tracing_appender_standalone() {
Expand Down Expand Up @@ -255,7 +255,7 @@ mod tests {
assert!(log.record.trace_context.is_none());

// Validate attributes
let attributes: Vec<(Key, AnyValue)> = log
let attributes: SmallVec<[(Key, AnyValue); PREALLOCATED_ATTRIBUTE_CAPACITY]> = log
.record
.attributes
.clone()
Expand Down Expand Up @@ -352,7 +352,7 @@ mod tests {
);

// validate attributes.
let attributes: Vec<(Key, AnyValue)> = log
let attributes: SmallVec<[(Key, AnyValue); PREALLOCATED_ATTRIBUTE_CAPACITY]> = log
.record
.attributes
.clone()
Expand Down Expand Up @@ -419,7 +419,7 @@ mod tests {
assert!(log.record.trace_context.is_none());

// Validate attributes
let attributes: Vec<(Key, AnyValue)> = log
let attributes: SmallVec<[(Key, AnyValue); PREALLOCATED_ATTRIBUTE_CAPACITY]> = log
.record
.attributes
.clone()
Expand Down Expand Up @@ -516,7 +516,7 @@ mod tests {
);

// validate attributes.
let attributes: Vec<(Key, AnyValue)> = log
let attributes: SmallVec<[(Key, AnyValue); PREALLOCATED_ATTRIBUTE_CAPACITY]> = log
.record
.attributes
.clone()
Expand Down
3 changes: 2 additions & 1 deletion opentelemetry-proto/src/transform/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ pub mod tonic {
})
.collect(),
}),
LogsAnyValue::Bytes(v) => Value::BytesValue(v),

LogsAnyValue::Bytes(v) => Value::BytesValue(*v),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions opentelemetry-sdk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ url = { workspace = true, optional = true }
tokio = { workspace = true, features = ["rt", "time"], optional = true }
tokio-stream = { workspace = true, optional = true }
http = { workspace = true, optional = true }
smallvec = "1.12"

[package.metadata.docs.rs]
all-features = true
Expand Down
34 changes: 21 additions & 13 deletions opentelemetry-sdk/benches/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,74 +100,82 @@ fn criterion_benchmark(c: &mut Criterion) {
logger.emit(log_record);
});

let bytes = AnyValue::Bytes(vec![25u8, 30u8, 40u8]);
let bytes = AnyValue::Bytes(Box::new(vec![25u8, 30u8, 40u8]));
log_benchmark_group(c, "simple-log-with-bytes", |logger| {
let mut log_record = logger.create_log_record();
log_record.set_body("simple log".into());
log_record.add_attribute("testbytes", bytes.clone());
logger.emit(log_record);
});

let bytes = AnyValue::Bytes(vec![
let bytes = AnyValue::Bytes(Box::new(vec![
25u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8,
30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8,
40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8,
30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8,
40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8,
30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8,
]);
]));
log_benchmark_group(c, "simple-log-with-a-lot-of-bytes", |logger| {
let mut log_record = logger.create_log_record();
log_record.set_body("simple log".into());
log_record.add_attribute("testbytes", bytes.clone());
logger.emit(log_record);
});

let vec_any_values = AnyValue::ListAny(vec![AnyValue::Int(25), "test".into(), true.into()]);
let vec_any_values = AnyValue::ListAny(Box::new(vec![
AnyValue::Int(25),
"test".into(),
true.into(),
]));
log_benchmark_group(c, "simple-log-with-vec-any-value", |logger| {
let mut log_record = logger.create_log_record();
log_record.set_body("simple log".into());
log_record.add_attribute("testvec", vec_any_values.clone());
logger.emit(log_record);
});

let vec_any_values = AnyValue::ListAny(vec![AnyValue::Int(25), "test".into(), true.into()]);
let vec_any_values = AnyValue::ListAny(vec![
let vec_any_values = AnyValue::ListAny(Box::new(vec![
AnyValue::Int(25),
"test".into(),
true.into(),
]));
let vec_any_values = AnyValue::ListAny(Box::new(vec![
AnyValue::Int(25),
"test".into(),
true.into(),
vec_any_values,
]);
]));
log_benchmark_group(c, "simple-log-with-inner-vec-any-value", |logger| {
let mut log_record = logger.create_log_record();
log_record.set_body("simple log".into());
log_record.add_attribute("testvec", vec_any_values.clone());
logger.emit(log_record);
});

let map_any_values = AnyValue::Map(HashMap::from([
let map_any_values = AnyValue::Map(Box::new(HashMap::from([
("testint".into(), 2.into()),
("testdouble".into(), 2.2.into()),
("teststring".into(), "test".into()),
]));
])));
log_benchmark_group(c, "simple-log-with-map-any-value", |logger| {
let mut log_record = logger.create_log_record();
log_record.set_body("simple log".into());
log_record.add_attribute("testmap", map_any_values.clone());
logger.emit(log_record);
});

let map_any_values = AnyValue::Map(HashMap::from([
let map_any_values = AnyValue::Map(Box::new(HashMap::from([
("testint".into(), 2.into()),
("testdouble".into(), 2.2.into()),
("teststring".into(), "test".into()),
]));
let map_any_values = AnyValue::Map(HashMap::from([
])));
let map_any_values = AnyValue::Map(Box::new(HashMap::from([
("testint".into(), 2.into()),
("testdouble".into(), 2.2.into()),
("teststring".into(), "test".into()),
("testmap".into(), map_any_values),
]));
])));
log_benchmark_group(c, "simple-log-with-inner-map-any-value", |logger| {
let mut log_record = logger.create_log_record();
log_record.set_body("simple log".into());
Expand Down
9 changes: 1 addition & 8 deletions opentelemetry-sdk/src/logs/log_emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ pub struct LoggerProvider {

/// Default logger name if empty string is provided.
const DEFAULT_COMPONENT_NAME: &str = "rust.opentelemetry.io/sdk/logger";
// According to a Go-specific study mentioned on https://go.dev/blog/slog,
// up to 5 attributes is the most common case. We have chosen 8 as the default
// capacity for attributes to avoid reallocation in common scenarios.
const PREALLOCATED_ATTRIBUTE_CAPACITY: usize = 8;

impl opentelemetry::logs::LoggerProvider for LoggerProvider {
type Logger = Logger;
Expand Down Expand Up @@ -251,10 +247,7 @@ impl opentelemetry::logs::Logger for Logger {

fn create_log_record(&self) -> Self::LogRecord {
// Reserve attributes memory for perf optimization. This may change in future.
LogRecord {
attributes: Some(Vec::with_capacity(PREALLOCATED_ATTRIBUTE_CAPACITY)),
..Default::default()
}
LogRecord::default()
}

/// Emit a `LogRecord`.
Expand Down
3 changes: 2 additions & 1 deletion opentelemetry-sdk/src/logs/log_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,7 @@ mod tests {
use opentelemetry::logs::{Logger, LoggerProvider as _};
use opentelemetry::Key;
use opentelemetry::{logs::LogResult, KeyValue};
use smallvec::smallvec;
use std::borrow::Cow;
use std::sync::{Arc, Mutex};
use std::time::Duration;
Expand Down Expand Up @@ -816,7 +817,7 @@ mod tests {
impl LogProcessor for FirstProcessor {
fn emit(&self, data: &mut LogData) {
// add attribute
data.record.attributes.get_or_insert(vec![]).push((
data.record.attributes.get_or_insert(smallvec![]).push((
Key::from_static_str("processed_by"),
AnyValue::String("FirstProcessor".into()),
));
Expand Down
4 changes: 3 additions & 1 deletion opentelemetry-sdk/src/logs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ pub use record::{LogRecord, TraceContext};
#[cfg(all(test, feature = "testing"))]
mod tests {
use super::*;
use crate::logs::record::PREALLOCATED_ATTRIBUTE_CAPACITY;
use crate::testing::logs::InMemoryLogsExporter;
use crate::Resource;
use opentelemetry::logs::LogRecord;
use opentelemetry::logs::{Logger, LoggerProvider as _, Severity};
use opentelemetry::{logs::AnyValue, Key, KeyValue};
use smallvec::SmallVec;
use std::borrow::Borrow;
use std::collections::HashMap;

Expand Down Expand Up @@ -80,7 +82,7 @@ mod tests {
.expect("Atleast one log is expected to be present.");
assert_eq!(log.instrumentation.name, "test-logger");
assert_eq!(log.record.severity_number, Some(Severity::Error));
let attributes: Vec<(Key, AnyValue)> = log
let attributes: SmallVec<[(Key, AnyValue); PREALLOCATED_ATTRIBUTE_CAPACITY]> = log
.record
.attributes
.clone()
Expand Down
21 changes: 13 additions & 8 deletions opentelemetry-sdk/src/logs/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@ use opentelemetry::{
trace::{SpanContext, SpanId, TraceFlags, TraceId},
Key,
};
use smallvec::{smallvec, SmallVec};
use std::{borrow::Cow, time::SystemTime};

// According to a Go-specific study mentioned on https://go.dev/blog/slog,
// up to 5 attributes is the most common case. We have chosen 8 as the default
// capacity for attributes to avoid reallocation in common scenarios.
pub(crate) const PREALLOCATED_ATTRIBUTE_CAPACITY: usize = 8;

#[derive(Debug, Default, Clone)]
#[non_exhaustive]
/// LogRecord represents all data carried by a log record, and
Expand Down Expand Up @@ -34,7 +40,7 @@ pub struct LogRecord {
pub body: Option<AnyValue>,

/// Additional attributes associated with this record
pub attributes: Option<Vec<(Key, AnyValue)>>,
pub attributes: Option<SmallVec<[(Key, AnyValue); PREALLOCATED_ATTRIBUTE_CAPACITY]>>,
}

impl opentelemetry::logs::LogRecord for LogRecord {
Expand Down Expand Up @@ -91,8 +97,6 @@ impl opentelemetry::logs::LogRecord for LogRecord {
{
if let Some(ref mut attrs) = self.attributes {
attrs.push((key.into(), value.into()));
} else {
self.attributes = Some(vec![(key.into(), value.into())]);
}
}
}
Expand Down Expand Up @@ -186,16 +190,17 @@ mod tests {
let mut log_record = LogRecord::default();
let attributes = vec![(Key::new("key"), AnyValue::String("value".into()))];
log_record.add_attributes(attributes.clone());
assert_eq!(log_record.attributes, Some(attributes));
let expected_attributes: SmallVec<[(Key, AnyValue); PREALLOCATED_ATTRIBUTE_CAPACITY]> =
smallvec![(Key::new("key"), AnyValue::String("value".into()))];
assert_eq!(log_record.attributes, Some(expected_attributes));
}

#[test]
fn test_set_attribute() {
let mut log_record = LogRecord::default();
log_record.add_attribute("key", "value");
assert_eq!(
log_record.attributes,
Some(vec![(Key::new("key"), AnyValue::String("value".into()))])
);
let expected_attributes: SmallVec<[(Key, AnyValue); PREALLOCATED_ATTRIBUTE_CAPACITY]> =
smallvec![(Key::new("key"), AnyValue::String("value".into()))];
assert_eq!(log_record.attributes, Some(expected_attributes));
}
}
2 changes: 1 addition & 1 deletion opentelemetry-stdout/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ impl From<opentelemetry::logs::AnyValue> for Value {
})
.collect(),
),
opentelemetry::logs::AnyValue::Bytes(b) => Value::BytesValue(b),
opentelemetry::logs::AnyValue::Bytes(b) => Value::BytesValue(*b),
}
}
}
Expand Down
14 changes: 13 additions & 1 deletion opentelemetry/benches/anyvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,23 @@ fn criterion_benchmark(c: &mut Criterion) {
fn attributes_creation(c: &mut Criterion) {
c.bench_function("CreateOTelValueString", |b| {
b.iter(|| {
let _v = black_box(Value::String("value1".into()));
let _v = black_box(Value::String(String::from("value1").into()));
});
});

c.bench_function("CreateOTelAnyValueString", |b| {
b.iter(|| {
let _v = black_box(AnyValue::String(String::from("value1").into()));
});
});

c.bench_function("CreateOTelValueStaticStr", |b| {
b.iter(|| {
let _v = black_box(Value::String("value1".into()));
});
});

c.bench_function("CreateOTelAnyValueStaticStr", |b| {
b.iter(|| {
let _v = black_box(AnyValue::String("value1".into()));
});
Expand Down
Loading
Loading