Skip to content

Commit

Permalink
Box complex types in AnyValue enum (Improve perf by 10%, and size red…
Browse files Browse the repository at this point in the history 
…uction by ~60%) (#1993)

Co-authored-by: Utkarsh Umesan Pillai <[email protected]>
Co-authored-by: Cijo Thomas <[email protected]>
  • Loading branch information
@lalitb @utpilla @cijothomas
3 people authored Aug 8, 2024
1 parent fe10ab1 commit 2409c18
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 44 deletions.
55 changes: 32 additions & 23 deletions opentelemetry-appender-log/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ mod any_value {
}

fn serialize_bytes(self, v: &[u8]) -> Result<Self::Ok, Self::Error> {
Ok(Some(AnyValue::Bytes(v.to_owned())))
Ok(Some(AnyValue::Bytes(Box::new(v.to_owned()))))
}

fn serialize_none(self) -> Result<Self::Ok, Self::Error> {
Expand Down Expand Up @@ -557,7 +557,7 @@ mod any_value {
}

fn end(self) -> Result<Self::Ok, Self::Error> {
Ok(Some(AnyValue::ListAny(self.value)))
Ok(Some(AnyValue::ListAny(Box::new(self.value))))
}
}

Expand All @@ -578,7 +578,7 @@ mod any_value {
}

fn end(self) -> Result<Self::Ok, Self::Error> {
Ok(Some(AnyValue::ListAny(self.value)))
Ok(Some(AnyValue::ListAny(Box::new(self.value))))
}
}

Expand All @@ -599,7 +599,7 @@ mod any_value {
}

fn end(self) -> Result<Self::Ok, Self::Error> {
Ok(Some(AnyValue::ListAny(self.value)))
Ok(Some(AnyValue::ListAny(Box::new(self.value))))
}
}

Expand All @@ -621,8 +621,11 @@ mod any_value {

fn end(self) -> Result<Self::Ok, Self::Error> {
Ok(Some(AnyValue::Map({
let mut variant = HashMap::new();
variant.insert(Key::from(self.variant), AnyValue::ListAny(self.value));
let mut variant = Box::<HashMap<Key, AnyValue>>::default();
variant.insert(
Key::from(self.variant),
AnyValue::ListAny(Box::new(self.value)),
);
variant
})))
}
Expand Down Expand Up @@ -664,7 +667,7 @@ mod any_value {
}

fn end(self) -> Result<Self::Ok, Self::Error> {
Ok(Some(AnyValue::Map(self.value)))
Ok(Some(AnyValue::Map(Box::new(self.value))))
}
}

Expand All @@ -688,7 +691,7 @@ mod any_value {
}

fn end(self) -> Result<Self::Ok, Self::Error> {
Ok(Some(AnyValue::Map(self.value)))
Ok(Some(AnyValue::Map(Box::new(self.value))))
}
}

Expand All @@ -713,8 +716,8 @@ mod any_value {

fn end(self) -> Result<Self::Ok, Self::Error> {
Ok(Some(AnyValue::Map({
let mut variant = HashMap::new();
variant.insert(Key::from(self.variant), AnyValue::Map(self.value));
let mut variant = Box::<HashMap<Key, AnyValue>>::default();
variant.insert(Key::from(self.variant), AnyValue::Map(Box::new(self.value)));
variant
})))
}
Expand Down Expand Up @@ -1025,13 +1028,17 @@ mod tests {
assert_eq!(AnyValue::Int(42), get("some_value").unwrap());

assert_eq!(
AnyValue::ListAny(vec![AnyValue::Int(1), AnyValue::Int(1), AnyValue::Int(1)]),
AnyValue::ListAny(Box::new(vec![
AnyValue::Int(1),
AnyValue::Int(1),
AnyValue::Int(1)
])),
get("slice_value").unwrap()
);

assert_eq!(
AnyValue::Map({
let mut map = HashMap::new();
let mut map = Box::<HashMap<Key, AnyValue>>::default();

map.insert(Key::from("a"), AnyValue::Int(1));
map.insert(Key::from("b"), AnyValue::Int(1));
Expand All @@ -1044,7 +1051,7 @@ mod tests {

assert_eq!(
AnyValue::Map({
let mut map = HashMap::new();
let mut map = Box::<HashMap<Key, AnyValue>>::default();

map.insert(Key::from("a"), AnyValue::Int(1));
map.insert(Key::from("b"), AnyValue::Int(1));
Expand All @@ -1056,7 +1063,11 @@ mod tests {
);

assert_eq!(
AnyValue::ListAny(vec![AnyValue::Int(1), AnyValue::Int(1), AnyValue::Int(1)]),
AnyValue::ListAny(Box::new(vec![
AnyValue::Int(1),
AnyValue::Int(1),
AnyValue::Int(1)
])),
get("tuple_value").unwrap()
);

Expand All @@ -1071,7 +1082,7 @@ mod tests {

map.insert(Key::from("Newtype"), AnyValue::Int(42));

map
Box::new(map)
}),
get("newtype_variant_value").unwrap()
);
Expand All @@ -1082,18 +1093,16 @@ mod tests {

map.insert(
Key::from("Struct"),
AnyValue::Map({
AnyValue::Map(Box::new({
let mut map = HashMap::new();

map.insert(Key::from("a"), AnyValue::Int(1));
map.insert(Key::from("b"), AnyValue::Int(1));
map.insert(Key::from("c"), AnyValue::Int(1));

map
}),
})),
);

map
Box::new(map)
}),
get("struct_variant_value").unwrap()
);
Expand All @@ -1104,14 +1113,14 @@ mod tests {

map.insert(
Key::from("Tuple"),
AnyValue::ListAny(vec![
AnyValue::ListAny(Box::new(vec![
AnyValue::Int(1),
AnyValue::Int(1),
AnyValue::Int(1),
]),
])),
);

map
Box::new(map)
}),
get("tuple_variant_value").unwrap()
);
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-proto/src/transform/logs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub mod tonic {
})
.collect(),
}),
LogsAnyValue::Bytes(v) => Value::BytesValue(v),
LogsAnyValue::Bytes(v) => Value::BytesValue(*v),
}
}
}
Expand Down
34 changes: 21 additions & 13 deletions opentelemetry-sdk/benches/log.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,74 +151,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
2 changes: 1 addition & 1 deletion opentelemetry-stdout/src/common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,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
46 changes: 46 additions & 0 deletions opentelemetry/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,52 @@

## vNext

- **BREAKING** [#1993](https://github.com/open-telemetry/opentelemetry-rust/pull/1993) Box complex types in AnyValue enum
Before:
```rust
#[derive(Debug, Clone, PartialEq)]
pub enum AnyValue {
/// An integer value
Int(i64),
/// A double value
Double(f64),
/// A string value
String(StringValue),
/// A boolean value
Boolean(bool),
/// A byte array
Bytes(Vec<u8>),
/// An array of `Any` values
ListAny(Vec<AnyValue>),
/// A map of string keys to `Any` values, arbitrarily nested.
Map(HashMap<Key, AnyValue>),
}
```

After:
```rust
#[derive(Debug, Clone, PartialEq)]
pub enum AnyValue {
/// An integer value
Int(i64),
/// A double value
Double(f64),
/// A string value
String(StringValue),
/// A boolean value
Boolean(bool),
/// A byte array
Bytes(Box<Vec<u8>>),
/// An array of `Any` values
ListAny(Box<Vec<AnyValue>>),
/// A map of string keys to `Any` values, arbitrarily nested.
Map(Box<HashMap<Key, AnyValue>>),
}
```
So the custom log appenders should box these types while adding them in message body, or
attribute values. Similarly, the custom exporters should dereference these complex type values
before serializing.

## v0.24.0

- Add "metrics", "logs" to default features. With this, default feature list is
Expand Down
12 changes: 6 additions & 6 deletions opentelemetry/src/logs/record.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ pub enum AnyValue {
/// A boolean value
Boolean(bool),
/// A byte array
Bytes(Vec<u8>),
Bytes(Box<Vec<u8>>),
/// An array of `Any` values
ListAny(Vec<AnyValue>),
ListAny(Box<Vec<AnyValue>>),
/// A map of string keys to `Any` values, arbitrarily nested.
Map(HashMap<Key, AnyValue>),
Map(Box<HashMap<Key, AnyValue>>),
}

macro_rules! impl_trivial_from {
Expand Down Expand Up @@ -98,17 +98,17 @@ impl_trivial_from!(bool, AnyValue::Boolean);
impl<T: Into<AnyValue>> FromIterator<T> for AnyValue {
/// Creates an [`AnyValue::ListAny`] value from a sequence of `Into<AnyValue>` values.
fn from_iter<I: IntoIterator<Item = T>>(iter: I) -> Self {
AnyValue::ListAny(iter.into_iter().map(Into::into).collect())
AnyValue::ListAny(Box::new(iter.into_iter().map(Into::into).collect()))
}
}

impl<K: Into<Key>, V: Into<AnyValue>> FromIterator<(K, V)> for AnyValue {
/// Creates an [`AnyValue::Map`] value from a sequence of key-value pairs
/// that can be converted into a `Key` and `AnyValue` respectively.
fn from_iter<I: IntoIterator<Item = (K, V)>>(iter: I) -> Self {
AnyValue::Map(HashMap::from_iter(
AnyValue::Map(Box::new(HashMap::from_iter(
iter.into_iter().map(|(k, v)| (k.into(), v.into())),
))
)))
}
}

Expand Down

0 comments on commit 2409c18

Please sign in to comment.