Skip to content

Commit fab6940

Browse files
djcdjc
authored
Make sure value arrays are homogeneous (see open-telemetry#317) (open-telemetry#333)
1 parent 734524b commit fab6940

File tree

5 files changed

+121
-35
lines changed

5 files changed

+121
-35
lines changed

‎opentelemetry-otlp/src/transform/common.rs

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::proto::common::{AnyValue, ArrayValue, KeyValue};
22
use opentelemetry::sdk::trace::EvictedHashMap;
3-
use opentelemetry::Value;
3+
use opentelemetry::{Array, Value};
44
use protobuf::RepeatedField;
55
use std::time::{Duration, SystemTime, UNIX_EPOCH};
66

@@ -45,19 +45,36 @@ impl From<Value> for AnyValue {
4545
Value::I64(val) => any_value.set_int_value(val),
4646
Value::F64(val) => any_value.set_double_value(val),
4747
Value::String(val) => any_value.set_string_value(val.into_owned()),
48-
Value::Array(vals) => any_value.set_array_value({
49-
let mut array_value = ArrayValue::new();
50-
array_value.set_values(RepeatedField::from_vec(
51-
vals.into_iter().map(AnyValue::from).collect(),
52-
));
53-
array_value
48+
Value::Array(array) => any_value.set_array_value(match array {
49+
Array::Bool(vals) => array_into_proto(vals),
50+
Array::I64(vals) => array_into_proto(vals),
51+
Array::F64(vals) => array_into_proto(vals),
52+
Array::String(vals) => array_into_proto(vals),
5453
}),
5554
};
5655

5756
any_value
5857
}
5958
}
6059

60+
fn array_into_proto<T>(vals: Vec<Option<T>>) -> ArrayValue
61+
where
62+
Value: From<T>,
63+
{
64+
let values = RepeatedField::from_vec(
65+
vals.into_iter()
66+
.map(|val| match val {
67+
Some(v) => AnyValue::from(Value::from(v)),
68+
None => AnyValue::new(),
69+
})
70+
.collect(),
71+
);
72+
73+
let mut array_value = ArrayValue::new();
74+
array_value.set_values(values);
75+
array_value
76+
}
77+
6178
pub(crate) fn to_nanos(time: SystemTime) -> u64 {
6279
time.duration_since(UNIX_EPOCH)
6380
.unwrap_or_else(|_| Duration::from_secs(0))

‎opentelemetry/src/api/core.rs

Lines changed: 83 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#[cfg(feature = "serialize")]
33
use serde::{Deserialize, Serialize};
44
use std::borrow::Cow;
5+
use std::fmt;
56

67
/// Key used for metric `LabelSet`s and trace `Span` attributes.
78
#[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))]
@@ -52,7 +53,7 @@ impl Key {
5253
}
5354

5455
/// Create a `KeyValue` pair for arrays.
55-
pub fn array<T: Into<Vec<Value>>>(&self, value: T) -> KeyValue {
56+
pub fn array<T: Into<Array>>(&self, value: T) -> KeyValue {
5657
KeyValue {
5758
key: self.clone(),
5859
value: Value::Array(value.into()),
@@ -86,6 +87,83 @@ impl From<Key> for String {
8687
}
8788
}
8889

90+
/// Array of homogeneous values
91+
#[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))]
92+
#[derive(Clone, Debug, PartialEq)]
93+
pub enum Array {
94+
/// Array of bools
95+
Bool(Vec<Option<bool>>),
96+
/// Array of integers
97+
I64(Vec<Option<i64>>),
98+
/// Array of floats
99+
F64(Vec<Option<f64>>),
100+
/// Array of strings
101+
String(Vec<Option<Cow<'static, str>>>),
102+
}
103+
104+
impl fmt::Display for Array {
105+
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
106+
match self {
107+
Array::Bool(values) => display_array_str(&values, fmt),
108+
Array::I64(values) => display_array_str(&values, fmt),
109+
Array::F64(values) => display_array_str(&values, fmt),
110+
Array::String(values) => {
111+
write!(fmt, "[")?;
112+
for (i, t) in values.iter().enumerate() {
113+
if i > 0 {
114+
write!(fmt, ",")?;
115+
}
116+
match t {
117+
Some(t) => write!(fmt, "{:?}", t)?,
118+
None => write!(fmt, "null")?,
119+
}
120+
}
121+
write!(fmt, "]")
122+
}
123+
}
124+
}
125+
}
126+
127+
fn display_array_str<T: fmt::Display>(
128+
slice: &[Option<T>],
129+
fmt: &mut fmt::Formatter<'_>,
130+
) -> fmt::Result {
131+
write!(fmt, "[")?;
132+
for (i, t) in slice.iter().enumerate() {
133+
if i > 0 {
134+
write!(fmt, ",")?;
135+
}
136+
match t {
137+
Some(t) => write!(fmt, "{}", t)?,
138+
None => write!(fmt, "null")?,
139+
}
140+
}
141+
write!(fmt, "]")
142+
}
143+
144+
macro_rules! into_array {
145+
($(($t:ty, $val:expr, $src:ident, $convert:expr),)+) => {
146+
$(
147+
impl From<$t> for Array {
148+
fn from($src: $t) -> Self {
149+
$val($convert)
150+
}
151+
}
152+
)+
153+
}
154+
}
155+
156+
into_array!(
157+
(Vec<Option<bool>>, Array::Bool, t, t),
158+
(Vec<Option<i64>>, Array::I64, t, t),
159+
(Vec<Option<f64>>, Array::F64, t, t),
160+
(Vec<Option<Cow<'static, str>>>, Array::String, t, t),
161+
(Vec<bool>, Array::Bool, t, t.into_iter().map(Some).collect()),
162+
(Vec<i64>, Array::I64, t, t.into_iter().map(Some).collect()),
163+
(Vec<f64>, Array::F64, t, t.into_iter().map(Some).collect()),
164+
(Vec<Cow<'static, str>>, Array::String, t, t.into_iter().map(Some).collect()),
165+
);
166+
89167
/// Value types for use in `KeyValue` pairs.
90168
#[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))]
91169
#[derive(Clone, Debug, PartialEq)]
@@ -99,7 +177,7 @@ pub enum Value {
99177
/// String values
100178
String(Cow<'static, str>),
101179
/// Array of homogeneous values
102-
Array(Vec<Value>),
180+
Array(Array),
103181
}
104182

105183
macro_rules! from_values {
@@ -122,7 +200,7 @@ from_values!(
122200
(bool, Value::Bool);
123201
(i64, Value::I64);
124202
(f64, Value::F64);
125-
(Vec<Value>, Value::Array);
203+
(Cow<'static, str>, Value::String);
126204
);
127205

128206
impl From<&'static str> for Value {
@@ -148,7 +226,7 @@ impl From<Value> for String {
148226
Value::I64(value) => value.to_string(),
149227
Value::F64(value) => value.to_string(),
150228
Value::String(value) => value.into_owned(),
151-
Value::Array(value) => format_value_array_as_string(&value),
229+
Value::Array(value) => value.to_string(),
152230
}
153231
}
154232
}
@@ -162,24 +240,11 @@ impl From<&Value> for String {
162240
Value::I64(value) => value.to_string(),
163241
Value::F64(value) => value.to_string(),
164242
Value::String(value) => value.to_string(),
165-
Value::Array(value) => format_value_array_as_string(value),
243+
Value::Array(value) => value.to_string(),
166244
}
167245
}
168246
}
169247

170-
fn format_value_array_as_string(v: &[Value]) -> String {
171-
format!(
172-
"[{}]",
173-
v.iter()
174-
.map(|elem| match elem {
175-
Value::String(s) => format!(r#""{}""#, s),
176-
v => String::from(v),
177-
})
178-
.collect::<Vec<_>>()
179-
.join(",")
180-
)
181-
}
182-
183248
/// `KeyValue` pairs are used by `LabelSet`s and `Span` attributes.
184249
#[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))]
185250
#[derive(Clone, Debug, PartialEq)]

‎opentelemetry/src/api/labels/mod.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//! OpenTelemetry Labels
2-
use crate::{Key, KeyValue, Value};
2+
use crate::{Array, Key, KeyValue, Value};
33
use std::cmp::Ordering;
44
use std::collections::{btree_map, BTreeMap};
55
use std::hash::{Hash, Hasher};
@@ -82,12 +82,15 @@ fn hash_value<H: Hasher>(state: &mut H, value: &Value) {
8282
f.to_bits().hash(state)
8383
}
8484
Value::String(s) => s.hash(state),
85-
Value::Array(arr) => {
85+
Value::Array(arr) => match arr {
8686
// recursively hash array values
87-
for val in arr {
88-
hash_value(state, val);
89-
}
90-
}
87+
Array::Bool(values) => values.iter().for_each(|v| v.hash(state)),
88+
Array::I64(values) => values.iter().for_each(|v| v.hash(state)),
89+
Array::F64(values) => values
90+
.iter()
91+
.for_each(|v| v.map(|f| f.to_bits()).hash(state)),
92+
Array::String(values) => values.iter().for_each(|v| v.hash(state)),
93+
},
9194
}
9295
}
9396

‎opentelemetry/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,6 @@ pub use api::trace;
142142
pub use api::{
143143
baggage,
144144
context::{Context, ContextGuard},
145-
core::{Key, KeyValue, Unit, Value},
145+
core::{Array, Key, KeyValue, Unit, Value},
146146
propagation,
147147
};

‎opentelemetry/src/sdk/propagation/baggage.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ impl TextMapPropagator for BaggagePropagator {
186186
mod tests {
187187
use super::*;
188188
use crate::{baggage::BaggageMetadata, propagation::TextMapPropagator, Key, KeyValue, Value};
189+
use std::borrow::Cow;
189190
use std::collections::HashMap;
190191

191192
#[rustfmt::skip]
@@ -244,9 +245,9 @@ mod tests {
244245
// "values of array types"
245246
(
246247
vec![
247-
KeyValue::new("key1", Value::Array(vec![Value::Bool(true), Value::Bool(false)])),
248-
KeyValue::new("key2", Value::Array(vec![Value::I64(123), Value::I64(456)])),
249-
KeyValue::new("key3", Value::Array(vec!["val1".into(), "val2".into()])),
248+
KeyValue::new("key1", Value::Array(vec![true, false].into())),
249+
KeyValue::new("key2", Value::Array(vec![123, 456].into())),
250+
KeyValue::new("key3", Value::Array(vec![Cow::from("val1"), Cow::from("val2")].into())),
250251
],
251252
vec![
252253
"key1=[true%2Cfalse]",

0 commit comments

Comments
 (0)