Implement Display trait for Key and Value types (#353)

djc · web-flow · commit bd696420f332 · 2020-11-09T09:06:44.000-08:00
diff --git a/opentelemetry-contrib/src/trace/exporter/datadog/model/v03.rs b/opentelemetry-contrib/src/trace/exporter/datadog/model/v03.rs
@@ -63,9 +63,8 @@ pub(crate) fn encode(service_name: &str, spans: Vec<trace::SpanData>) -> Result<
         rmp::encode::write_str(&mut encoded, "meta")?;
         rmp::encode::write_map_len(&mut encoded, span.attributes.len() as u32)?;
         for (key, value) in span.attributes.iter() {
-            let value_string: String = value.into();
             rmp::encode::write_str(&mut encoded, key.as_str())?;
-            rmp::encode::write_str(&mut encoded, value_string.as_str())?;
+            rmp::encode::write_str(&mut encoded, value.as_str().as_ref())?;
         }
     }
 
diff --git a/opentelemetry-contrib/src/trace/exporter/datadog/model/v05.rs b/opentelemetry-contrib/src/trace/exporter/datadog/model/v05.rs
@@ -111,9 +111,8 @@ fn encode_spans(
         rmp::encode::write_i32(&mut encoded, span.status_code as i32)?;
         rmp::encode::write_map_len(&mut encoded, span.attributes.len() as u32)?;
         for (key, value) in span.attributes.iter() {
-            let value_string: String = value.into();
             rmp::encode::write_u32(&mut encoded, interner.intern(key.as_str()))?;
-            rmp::encode::write_u32(&mut encoded, interner.intern(value_string.as_str()))?;
+            rmp::encode::write_u32(&mut encoded, interner.intern(value.as_str().as_ref()))?;
         }
         rmp::encode::write_map_len(&mut encoded, 0)?;
         rmp::encode::write_u32(&mut encoded, span_type)?;
diff --git a/opentelemetry-jaeger/src/lib.rs b/opentelemetry-jaeger/src/lib.rs
@@ -486,7 +486,7 @@ impl Into<jaeger::Tag> for KeyValue {
             Value::Bool(b) => jaeger::Tag::new(key.into(), jaeger::TagType::Bool, None, None, Some(b), None, None),
             Value::I64(i) => jaeger::Tag::new(key.into(), jaeger::TagType::Long, None, None, None, Some(i), None),
             // TODO: better Array handling, jaeger thrift doesn't support arrays
-            v @ Value::Array(_) => jaeger::Tag::new(key.into(), jaeger::TagType::String, Some(v.into()), None, None, None, None),
+            v @ Value::Array(_) => jaeger::Tag::new(key.into(), jaeger::TagType::String, Some(v.to_string()), None, None, None, None),
         }
     }
 }
diff --git a/opentelemetry-prometheus/src/lib.rs b/opentelemetry-prometheus/src/lib.rs
@@ -394,7 +394,7 @@ fn build_histogram(
 fn build_label_pair(label: KeyValue) -> prometheus::proto::LabelPair {
     let mut lp = prometheus::proto::LabelPair::new();
     lp.set_name(label.key.into());
-    lp.set_value(label.value.into());
+    lp.set_value(label.value.to_string());
 
     lp
 }
diff --git a/opentelemetry-zipkin/src/model/mod.rs b/opentelemetry-zipkin/src/model/mod.rs
@@ -138,7 +138,7 @@ where
 {
     let mut map: HashMap<String, String> = HashMap::new();
     for kv in kvs {
-        map.insert(kv.key.into(), kv.value.into());
+        map.insert(kv.key.into(), kv.value.to_string());
     }
     map
 }
diff --git a/opentelemetry/src/api/baggage.rs b/opentelemetry/src/api/baggage.rs
@@ -211,8 +211,8 @@ impl Baggage {
         if !key.as_str().is_ascii() {
             return false;
         }
-        let value = String::from(value);
-        if key_value_metadata_bytes_size(key.as_str(), value.as_str(), metadata.as_str())
+        let value = value.as_str();
+        if key_value_metadata_bytes_size(key.as_str(), value.as_ref(), metadata.as_str())
             < MAX_BYTES_FOR_ONE_PAIR
         {
             match self.inner.get(key) {
@@ -234,7 +234,7 @@ impl Baggage {
                         metadata.as_str().len() + value.len() + key.as_str().len()
                 }
                 Some((old_value, old_metadata)) => {
-                    let old_value = String::from(old_value);
+                    let old_value = old_value.as_str();
                     if self.kv_content_len - old_metadata.as_str().len() - old_value.len()
                         + metadata.as_str().len()
                         + value.len()
diff --git a/opentelemetry/src/api/core.rs b/opentelemetry/src/api/core.rs
@@ -87,6 +87,12 @@ impl From<Key> for String {
     }
 }
 
+impl fmt::Display for Key {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.0.fmt(fmt)
+    }
+}
+
 /// Array of homogeneous values
 #[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))]
 #[derive(Clone, Debug, PartialEq)]
@@ -180,6 +186,21 @@ pub enum Value {
     Array(Array),
 }
 
+impl Value {
+    /// String representation of the `Value`
+    ///
+    /// This will allocate iff the underlying value is not a `String`.
+    pub fn as_str(&self) -> Cow<'_, str> {
+        match self {
+            Value::Bool(v) => format!("{}", v).into(),
+            Value::I64(v) => format!("{}", v).into(),
+            Value::F64(v) => format!("{}", v).into(),
+            Value::String(v) => Cow::Borrowed(v.as_ref()),
+            Value::Array(v) => format!("{}", v).into(),
+        }
+    }
+}
+
 macro_rules! from_values {
    (
         $(
@@ -217,30 +238,14 @@ impl From<String> for Value {
     }
 }
 
-impl From<Value> for String {
-    /// Convert `Value` types to `String` for use by exporters that only use
-    /// `String` values.
-    fn from(value: Value) -> Self {
-        match value {
-            Value::Bool(value) => value.to_string(),
-            Value::I64(value) => value.to_string(),
-            Value::F64(value) => value.to_string(),
-            Value::String(value) => value.into_owned(),
-            Value::Array(value) => value.to_string(),
-        }
-    }
-}
-
-impl From<&Value> for String {
-    /// Convert `&Value` types to `String` for use by exporters that only use
-    /// `String` values.
-    fn from(value: &Value) -> Self {
-        match value {
-            Value::Bool(value) => value.to_string(),
-            Value::I64(value) => value.to_string(),
-            Value::F64(value) => value.to_string(),
-            Value::String(value) => value.to_string(),
-            Value::Array(value) => value.to_string(),
+impl fmt::Display for Value {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Value::Bool(v) => fmt.write_fmt(format_args!("{}", v)),
+            Value::I64(v) => fmt.write_fmt(format_args!("{}", v)),
+            Value::F64(v) => fmt.write_fmt(format_args!("{}", v)),
+            Value::String(v) => fmt.write_fmt(format_args!("{}", v)),
+            Value::Array(v) => fmt.write_fmt(format_args!("{}", v)),
         }
     }
 }
diff --git a/opentelemetry/src/api/labels/encoder.rs b/opentelemetry/src/api/labels/encoder.rs
@@ -1,5 +1,5 @@
 use crate::{Key, Value};
-use std::fmt;
+use std::fmt::{self, Write};
 use std::sync::atomic::{AtomicUsize, Ordering};
 
 static ENCODER_ID_COUNTER: AtomicUsize = AtomicUsize::new(0);
@@ -39,12 +39,22 @@ impl Encoder for DefaultLabelEncoder {
         labels
             .enumerate()
             .fold(String::new(), |mut acc, (idx, (key, value))| {
+                let offset = acc.len();
                 if idx > 0 {
                     acc.push(',')
                 }
-                acc.push_str(key.as_str());
+
+                if write!(acc, "{}", key).is_err() {
+                    acc.truncate(offset);
+                    return acc;
+                }
+
                 acc.push('=');
-                acc.push_str(String::from(value).as_str());
+                if write!(acc, "{}", value).is_err() {
+                    acc.truncate(offset);
+                    return acc;
+                }
+
                 acc
             })
     }
diff --git a/opentelemetry/src/sdk/propagation/baggage.rs b/opentelemetry/src/sdk/propagation/baggage.rs
@@ -123,7 +123,7 @@ impl TextMapPropagator for BaggagePropagator {
                     let metadata_prefix = if metadata_str.is_empty() { "" } else { ";" };
                     utf8_percent_encode(name.as_str().trim(), FRAGMENT)
                         .chain(iter::once("="))
-                        .chain(utf8_percent_encode(String::from(value).trim(), FRAGMENT))
+                        .chain(utf8_percent_encode(value.as_str().trim(), FRAGMENT))
                         .chain(iter::once(metadata_prefix))
                         .chain(iter::once(metadata_str))
                         .collect()

Original file line number	Diff line number	Diff line change
`@@ -63,9 +63,8 @@ pub(crate) fn encode(service_name: &str, spans: Vec<trace::SpanData>) -> Result<`
`63`	`63`	`rmp::encode::write_str(&mut encoded, "meta")?;`
`64`	`64`	`rmp::encode::write_map_len(&mut encoded, span.attributes.len() as u32)?;`
`65`	`65`	`for (key, value) in span.attributes.iter() {`
`66`		`- let value_string: String = value.into();`
`67`	`66`	`rmp::encode::write_str(&mut encoded, key.as_str())?;`
`68`		`- rmp::encode::write_str(&mut encoded, value_string.as_str())?;`
	`67`	`+ rmp::encode::write_str(&mut encoded, value.as_str().as_ref())?;`
`69`	`68`	`}`
`70`	`69`	`}`
`71`	`70`
Original file line number	Diff line number	Diff line change
`@@ -486,7 +486,7 @@ impl Into<jaeger::Tag> for KeyValue {`
`486`	`486`	`Value::Bool(b) => jaeger::Tag::new(key.into(), jaeger::TagType::Bool, None, None, Some(b), None, None),`
`487`	`487`	`Value::I64(i) => jaeger::Tag::new(key.into(), jaeger::TagType::Long, None, None, None, Some(i), None),`
`488`	`488`	`// TODO: better Array handling, jaeger thrift doesn't support arrays`
`489`		`- v @ Value::Array(_) => jaeger::Tag::new(key.into(), jaeger::TagType::String, Some(v.into()), None, None, None, None),`
	`489`	`+ v @ Value::Array(_) => jaeger::Tag::new(key.into(), jaeger::TagType::String, Some(v.to_string()), None, None, None, None),`
`490`	`490`	`}`
`491`	`491`	`}`
`492`	`492`	`}`
Original file line number	Diff line number	Diff line change
`@@ -394,7 +394,7 @@ fn build_histogram(`
`394`	`394`	`fn build_label_pair(label: KeyValue) -> prometheus::proto::LabelPair {`
`395`	`395`	`let mut lp = prometheus::proto::LabelPair::new();`
`396`	`396`	`lp.set_name(label.key.into());`
`397`		`- lp.set_value(label.value.into());`
	`397`	`+ lp.set_value(label.value.to_string());`
`398`	`398`
`399`	`399`	`lp`
`400`	`400`	`}`
Original file line number	Diff line number	Diff line change
`@@ -138,7 +138,7 @@ where`
`138`	`138`	`{`
`139`	`139`	`let mut map: HashMap<String, String> = HashMap::new();`
`140`	`140`	`for kv in kvs {`
`141`		`- map.insert(kv.key.into(), kv.value.into());`
	`141`	`+ map.insert(kv.key.into(), kv.value.to_string());`
`142`	`142`	`}`
`143`	`143`	`map`
`144`	`144`	`}`
Original file line number	Diff line number	Diff line change
`@@ -211,8 +211,8 @@ impl Baggage {`
`211`	`211`	`if !key.as_str().is_ascii() {`
`212`	`212`	`return false;`
`213`	`213`	`}`
`214`		`- let value = String::from(value);`
`215`		`- if key_value_metadata_bytes_size(key.as_str(), value.as_str(), metadata.as_str())`
	`214`	`+ let value = value.as_str();`
	`215`	`+ if key_value_metadata_bytes_size(key.as_str(), value.as_ref(), metadata.as_str())`
`216`	`216`	`< MAX_BYTES_FOR_ONE_PAIR`
`217`	`217`	`{`
`218`	`218`	`match self.inner.get(key) {`
`@@ -234,7 +234,7 @@ impl Baggage {`
`234`	`234`	`metadata.as_str().len() + value.len() + key.as_str().len()`
`235`	`235`	`}`
`236`	`236`	`Some((old_value, old_metadata)) => {`
`237`		`- let old_value = String::from(old_value);`
	`237`	`+ let old_value = old_value.as_str();`
`238`	`238`	`if self.kv_content_len - old_metadata.as_str().len() - old_value.len()`
`239`	`239`	`+ metadata.as_str().len()`
`240`	`240`	`+ value.len()`