Make sure value arrays are homogeneous (see open-telemetry#317)

opentelemetry-otlp/src/transform/common.rs

@@ -1,6 +1,6 @@
 use crate::proto::common::{AnyValue, ArrayValue, KeyValue};
 use opentelemetry::sdk::trace::EvictedHashMap;
-use opentelemetry::Value;
+use opentelemetry::{Array, Value};
 use protobuf::RepeatedField;
 use std::time::{Duration, SystemTime, UNIX_EPOCH};
 
@@ -45,19 +45,36 @@ impl From<Value> for AnyValue {
             Value::I64(val) => any_value.set_int_value(val),
             Value::F64(val) => any_value.set_double_value(val),
             Value::String(val) => any_value.set_string_value(val.into_owned()),
-            Value::Array(vals) => any_value.set_array_value({
-                let mut array_value = ArrayValue::new();
-                array_value.set_values(RepeatedField::from_vec(
-                    vals.into_iter().map(AnyValue::from).collect(),
-                ));
-                array_value
+            Value::Array(array) => any_value.set_array_value(match array {
+                Array::Bool(vals) => array_into_proto(vals),
+                Array::I64(vals) => array_into_proto(vals),
+                Array::F64(vals) => array_into_proto(vals),
+                Array::String(vals) => array_into_proto(vals),
             }),
         };
 
         any_value
     }
 }
 
+fn array_into_proto<T>(vals: Vec<Option<T>>) -> ArrayValue
+where
+    Value: From<T>,
+{
+    let values = RepeatedField::from_vec(
+        vals.into_iter()
+            .map(|val| match val {
+                Some(v) => AnyValue::from(Value::from(v)),
+                None => AnyValue::new(),
+            })
+            .collect(),
+    );
+
+    let mut array_value = ArrayValue::new();
+    array_value.set_values(values);
+    array_value
+}
+
 pub(crate) fn to_nanos(time: SystemTime) -> u64 {
     time.duration_since(UNIX_EPOCH)
         .unwrap_or_else(|_| Duration::from_secs(0))

opentelemetry/src/api/core.rs

@@ -2,6 +2,7 @@
 #[cfg(feature = "serialize")]
 use serde::{Deserialize, Serialize};
 use std::borrow::Cow;
+use std::fmt;
 
 /// Key used for metric `LabelSet`s and trace `Span` attributes.
 #[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))]
@@ -52,7 +53,7 @@ impl Key {
     }
 
     /// Create a `KeyValue` pair for arrays.
-    pub fn array<T: Into<Vec<Value>>>(&self, value: T) -> KeyValue {
+    pub fn array<T: Into<Array>>(&self, value: T) -> KeyValue {
         KeyValue {
             key: self.clone(),
             value: Value::Array(value.into()),
@@ -86,6 +87,83 @@ impl From<Key> for String {
     }
 }
 
+/// Array of homogeneous values
+#[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))]
+#[derive(Clone, Debug, PartialEq)]
+pub enum Array {
+    /// Array of bools
+    Bool(Vec<Option<bool>>),
+    /// Array of integers
+    I64(Vec<Option<i64>>),
+    /// Array of floats
+    F64(Vec<Option<f64>>),
+    /// Array of strings
+    String(Vec<Option<Cow<'static, str>>>),
+}
+
+impl fmt::Display for Array {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Array::Bool(values) => display_array_str(&values, fmt),
+            Array::I64(values) => display_array_str(&values, fmt),
+            Array::F64(values) => display_array_str(&values, fmt),
+            Array::String(values) => {
+                write!(fmt, "[")?;
+                for (i, t) in values.iter().enumerate() {
+                    if i > 0 {
+                        write!(fmt, ",")?;
+                    }
+                    match t {
+                        Some(t) => write!(fmt, "{:?}", t)?,
+                        None => write!(fmt, "null")?,
+                    }
+                }
+                write!(fmt, "]")
+            }
+        }
+    }
+}
+
+fn display_array_str<T: fmt::Display>(
+    slice: &[Option<T>],
+    fmt: &mut fmt::Formatter<'_>,
+) -> fmt::Result {
+    write!(fmt, "[")?;
+    for (i, t) in slice.iter().enumerate() {
+        if i > 0 {
+            write!(fmt, ",")?;
+        }
+        match t {
+            Some(t) => write!(fmt, "{}", t)?,
+            None => write!(fmt, "null")?,
+        }
+    }
+    write!(fmt, "]")
+}
+
+macro_rules! into_array {
+    ($(($t:ty, $val:expr, $src:ident, $convert:expr),)+) => {
+        $(
+            impl From<$t> for Array {
+                fn from($src: $t) -> Self {
+                    $val($convert)
+                }
+            }
+        )+
+    }
+}
+
+into_array!(
+    (Vec<Option<bool>>, Array::Bool, t, t),
+    (Vec<Option<i64>>, Array::I64, t, t),
+    (Vec<Option<f64>>, Array::F64, t, t),
+    (Vec<Option<Cow<'static, str>>>, Array::String, t, t),
+    (Vec<bool>, Array::Bool, t, t.into_iter().map(|v| Some(v)).collect()),
+    (Vec<i64>, Array::I64, t, t.into_iter().map(|v| Some(v)).collect()),
+    (Vec<f64>, Array::F64, t, t.into_iter().map(|v| Some(v)).collect()),
+    (Vec<Cow<'static, str>>, Array::String, t, t.into_iter().map(|v| Some(v)).collect()),
+);
+
 /// Value types for use in `KeyValue` pairs.
 #[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))]
 #[derive(Clone, Debug, PartialEq)]
@@ -99,7 +177,7 @@ pub enum Value {
     /// String values
     String(Cow<'static, str>),
     /// Array of homogeneous values
-    Array(Vec<Value>),
+    Array(Array),
 }
 
 macro_rules! from_values {
@@ -122,7 +200,7 @@ from_values!(
     (bool, Value::Bool);
     (i64, Value::I64);
     (f64, Value::F64);
-    (Vec<Value>, Value::Array);
+    (Cow<'static, str>, Value::String);
 );
 
 impl From<&'static str> for Value {
@@ -148,7 +226,7 @@ impl From<Value> for String {
             Value::I64(value) => value.to_string(),
             Value::F64(value) => value.to_string(),
             Value::String(value) => value.into_owned(),
-            Value::Array(value) => format_value_array_as_string(&value),
+            Value::Array(value) => value.to_string(),
         }
     }
 }
@@ -162,24 +240,11 @@ impl From<&Value> for String {
             Value::I64(value) => value.to_string(),
             Value::F64(value) => value.to_string(),
             Value::String(value) => value.to_string(),
-            Value::Array(value) => format_value_array_as_string(value),
+            Value::Array(value) => value.to_string(),
         }
     }
 }
 
-fn format_value_array_as_string(v: &[Value]) -> String {
-    format!(
-        "[{}]",
-        v.iter()
-            .map(|elem| match elem {
-                Value::String(s) => format!(r#""{}""#, s),
-                v => String::from(v),
-            })
-            .collect::<Vec<_>>()
-            .join(",")
-    )
-}
-
 /// `KeyValue` pairs are used by `LabelSet`s and `Span` attributes.
 #[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))]
 #[derive(Clone, Debug, PartialEq)]

opentelemetry/src/api/labels/mod.rs

@@ -1,5 +1,5 @@
 //! OpenTelemetry Labels
-use crate::{Key, KeyValue, Value};
+use crate::{Array, Key, KeyValue, Value};
 use std::cmp::Ordering;
 use std::collections::{btree_map, BTreeMap};
 use std::hash::{Hash, Hasher};
@@ -82,12 +82,15 @@ fn hash_value<H: Hasher>(state: &mut H, value: &Value) {
             f.to_bits().hash(state)
         }
         Value::String(s) => s.hash(state),
-        Value::Array(arr) => {
+        Value::Array(arr) => match arr {
             // recursively hash array values
-            for val in arr {
-                hash_value(state, val);
-            }
-        }
+            Array::Bool(values) => values.iter().for_each(|v| v.hash(state)),
+            Array::I64(values) => values.iter().for_each(|v| v.hash(state)),
+            Array::F64(values) => values
+                .iter()
+                .for_each(|v| v.map(|f| f.to_bits()).hash(state)),
+            Array::String(values) => values.iter().for_each(|v| v.hash(state)),
+        },
     }
 }
 

opentelemetry/src/lib.rs

@@ -142,6 +142,6 @@ pub use api::trace;
 pub use api::{
     baggage,
     context::{Context, ContextGuard},
-    core::{Key, KeyValue, Unit, Value},
+    core::{Key, KeyValue, Unit, Value, Array},
     propagation,
 };

opentelemetry/src/sdk/propagation/baggage.rs

@@ -186,6 +186,7 @@ impl TextMapPropagator for BaggagePropagator {
 mod tests {
     use super::*;
     use crate::{baggage::BaggageMetadata, propagation::TextMapPropagator, Key, KeyValue, Value};
+    use std::borrow::Cow;
     use std::collections::HashMap;
 
     #[rustfmt::skip]
@@ -244,9 +245,9 @@ mod tests {
             // "values of array types"
             (
                 vec![
-                    KeyValue::new("key1", Value::Array(vec![Value::Bool(true), Value::Bool(false)])),
-                    KeyValue::new("key2", Value::Array(vec![Value::I64(123), Value::I64(456)])),
-                    KeyValue::new("key3", Value::Array(vec!["val1".into(), "val2".into()])),
+                    KeyValue::new("key1", Value::Array(vec![true, false].into())),
+                    KeyValue::new("key2", Value::Array(vec![123, 456].into())),
+                    KeyValue::new("key3", Value::Array(vec![Cow::from("val1"), Cow::from("val2")].into())),
                 ],
                 vec![
                     "key1=[true%2Cfalse]",