From cc389bc1e90c2f2961ffb46a709b58dac41ebcc5 Mon Sep 17 00:00:00 2001 From: Julian Tescher Date: Wed, 22 Jun 2022 11:52:56 -0700 Subject: [PATCH 1/2] Allow ref counted keys and values This updates attribute `Key` and `Value` inner types from `Cow<'static, str>` to a new private `OtelString` enum to support static, owned, or ref counted values. --- .../external-otlp-grpcio-async-std/Cargo.toml | 2 +- .../external-otlp-tonic-tokio/src/main.rs | 2 +- opentelemetry-api/src/common.rs | 234 ++++++++++++++++-- opentelemetry-contrib/Cargo.toml | 2 +- .../src/trace/exporter/jaeger_json.rs | 2 +- .../src/exporter/model/v03.rs | 2 +- .../src/exporter/model/v05.rs | 2 +- opentelemetry-dynatrace/Cargo.toml | 2 +- .../src/transform/metrics.rs | 12 +- opentelemetry-jaeger/Cargo.toml | 2 +- opentelemetry-proto/src/transform/common.rs | 4 +- opentelemetry-sdk/Cargo.toml | 2 +- .../src/export/metrics/stdout.rs | 2 +- opentelemetry-sdk/src/propagation/baggage.rs | 3 +- opentelemetry-sdk/src/resource/process.rs | 12 +- opentelemetry-stackdriver/src/lib.rs | 2 +- 16 files changed, 233 insertions(+), 54 deletions(-) diff --git a/examples/external-otlp-grpcio-async-std/Cargo.toml b/examples/external-otlp-grpcio-async-std/Cargo.toml index 43dcbf475a..3b70e775e6 100644 --- a/examples/external-otlp-grpcio-async-std/Cargo.toml +++ b/examples/external-otlp-grpcio-async-std/Cargo.toml @@ -5,7 +5,7 @@ edition = "2018" publish = false [dependencies] -async-std = { version = "= 1.8.0", features = ["attributes"] } +async-std = { version = "= 1.10.0", features = ["attributes"] } env_logger = "0.8.2" opentelemetry = { path = "../../opentelemetry", features = ["rt-async-std"] } opentelemetry-otlp = { path = "../../opentelemetry-otlp", features = [ diff --git a/examples/external-otlp-tonic-tokio/src/main.rs b/examples/external-otlp-tonic-tokio/src/main.rs index ed71df3108..28e819c6a8 100644 --- a/examples/external-otlp-tonic-tokio/src/main.rs +++ b/examples/external-otlp-tonic-tokio/src/main.rs @@ -62,7 +62,7 @@ fn init_tracer() -> Result { opentelemetry_otlp::new_exporter() .tonic() .with_endpoint(endpoint.as_str()) - .with_metadata(dbg!(metadata)) + .with_metadata(metadata) .with_tls_config( ClientTlsConfig::new().domain_name( endpoint diff --git a/opentelemetry-api/src/common.rs b/opentelemetry-api/src/common.rs index d1b77c7894..6b078f1748 100644 --- a/opentelemetry-api/src/common.rs +++ b/opentelemetry-api/src/common.rs @@ -1,23 +1,35 @@ use std::borrow::Cow; -use std::fmt; +use std::sync::Arc; +use std::{fmt, hash}; /// The key part of attribute [KeyValue] pairs. /// /// See the [attribute naming] spec for guidelines. /// /// [attribute naming]: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.9.0/specification/common/attribute-naming.md -#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] -pub struct Key(Cow<'static, str>); +#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct Key(OtelString); impl Key { /// Create a new `Key`. - pub fn new>>(value: S) -> Self { - Key(value.into()) + /// + /// # Examples + /// + /// ``` + /// use opentelemetry_api::Key; + /// use std::sync::Arc; + /// + /// let key1 = Key::new("my_static_str"); + /// let key2 = Key::new(String::from("my_owned_string")); + /// let key3 = Key::new(Arc::from("my_ref_counted_str")); + /// ``` + pub fn new(value: impl Into) -> Self { + value.into() } /// Create a new const `Key`. pub const fn from_static_str(value: &'static str) -> Self { - Key(Cow::Borrowed(value)) + Key(OtelString::Static(value)) } /// Create a `KeyValue` pair for `bool` values. @@ -44,8 +56,8 @@ impl Key { } } - /// Create a `KeyValue` pair for `String` values. - pub fn string>>(self, value: T) -> KeyValue { + /// Create a `KeyValue` pair for string-like values. + pub fn string(self, value: impl Into) -> KeyValue { KeyValue { key: self, value: Value::String(value.into()), @@ -62,34 +74,95 @@ impl Key { /// Returns a reference to the underlying key name pub fn as_str(&self) -> &str { - self.0.as_ref() + self.0.as_str() } } impl From<&'static str> for Key { /// Convert a `&str` to a `Key`. fn from(key_str: &'static str) -> Self { - Key(Cow::from(key_str)) + Key(OtelString::Static(key_str)) } } impl From for Key { /// Convert a `String` to a `Key`. fn from(string: String) -> Self { - Key(Cow::from(string)) + Key(OtelString::Owned(string)) + } +} + +impl From> for Key { + /// Convert a `String` to a `Key`. + fn from(string: Arc) -> Self { + Key(OtelString::RefCounted(string)) + } +} + +impl fmt::Debug for Key { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(fmt) } } impl From for String { - /// Converts `Key` instances into `String`. fn from(key: Key) -> Self { - key.0.into_owned() + match key.0 { + OtelString::Owned(s) => s, + OtelString::Static(s) => s.to_string(), + OtelString::RefCounted(s) => s.to_string(), + } } } impl fmt::Display for Key { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - self.0.fmt(fmt) + match &self.0 { + OtelString::Owned(s) => s.fmt(fmt), + OtelString::Static(s) => s.fmt(fmt), + OtelString::RefCounted(s) => s.fmt(fmt), + } + } +} + +#[derive(Clone, Debug, Eq)] +enum OtelString { + Static(&'static str), + Owned(String), + RefCounted(Arc), +} + +impl OtelString { + fn as_str(&self) -> &str { + match self { + OtelString::Owned(s) => s.as_ref(), + OtelString::Static(s) => s, + OtelString::RefCounted(s) => s.as_ref(), + } + } +} + +impl PartialOrd for OtelString { + fn partial_cmp(&self, other: &Self) -> Option { + self.as_str().partial_cmp(other.as_str()) + } +} + +impl Ord for OtelString { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.as_str().cmp(other.as_str()) + } +} + +impl PartialEq for OtelString { + fn eq(&self, other: &Self) -> bool { + self.as_str().eq(other.as_str()) + } +} + +impl hash::Hash for OtelString { + fn hash(&self, state: &mut H) { + self.as_str().hash(state) } } @@ -103,7 +176,7 @@ pub enum Array { /// Array of floats F64(Vec), /// Array of strings - String(Vec>), + String(Vec), } impl fmt::Display for Array { @@ -118,7 +191,7 @@ impl fmt::Display for Array { if i > 0 { write!(fmt, ",")?; } - write!(fmt, "{:?}", t)?; + write!(fmt, "\"{}\"", t)?; } write!(fmt, "]") } @@ -153,9 +226,42 @@ into_array!( (Vec, Array::Bool), (Vec, Array::I64), (Vec, Array::F64), - (Vec>, Array::String), + (Vec, Array::String), ); +impl From> for Array { + /// Convenience method for creating a `Value` from a `&'static str`. + fn from(ss: Vec<&'static str>) -> Self { + Array::String( + ss.into_iter() + .map(|s| StringValue(OtelString::Static(s))) + .collect(), + ) + } +} + +impl From> for Array { + /// Convenience method for creating a `Value` from a `String`. + fn from(ss: Vec) -> Self { + Array::String( + ss.into_iter() + .map(|s| StringValue(OtelString::Owned(s))) + .collect(), + ) + } +} + +impl From>> for Array { + /// Convenience method for creating a `Value` from a `String`. + fn from(ss: Vec>) -> Self { + Array::String( + ss.into_iter() + .map(|s| StringValue(OtelString::RefCounted(s))) + .collect(), + ) + } +} + /// The value part of attribute [KeyValue] pairs. #[derive(Clone, Debug, PartialEq)] pub enum Value { @@ -166,11 +272,75 @@ pub enum Value { /// f64 values F64(f64), /// String values - String(Cow<'static, str>), + String(StringValue), /// Array of homogeneous values Array(Array), } +/// Wrapper for string-like values +#[derive(Clone, PartialEq, Hash)] +pub struct StringValue(OtelString); + +impl fmt::Debug for StringValue { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl fmt::Display for StringValue { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match &self.0 { + OtelString::Owned(s) => s.fmt(f), + OtelString::Static(s) => s.fmt(f), + OtelString::RefCounted(s) => s.fmt(f), + } + } +} + +impl StringValue { + /// Returns a string slice to this value + pub fn as_str(&self) -> &str { + self.0.as_str() + } +} + +impl From for String { + fn from(s: StringValue) -> Self { + match s.0 { + OtelString::Owned(s) => s, + OtelString::Static(s) => s.to_string(), + OtelString::RefCounted(s) => s.to_string(), + } + } +} + +impl From<&'static str> for StringValue { + fn from(s: &'static str) -> Self { + StringValue(OtelString::Static(s)) + } +} + +impl From for StringValue { + fn from(s: String) -> Self { + StringValue(OtelString::Owned(s)) + } +} + +impl From> for StringValue { + fn from(s: Arc) -> Self { + StringValue(OtelString::RefCounted(s)) + } +} + +impl From> for StringValue { + fn from(s: Cow<'static, str>) -> Self { + match s { + Cow::Owned(s) => StringValue(OtelString::Owned(s)), + Cow::Borrowed(s) => StringValue(OtelString::Static(s)), + } + } +} + impl Value { /// String representation of the `Value` /// @@ -180,7 +350,7 @@ impl Value { 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::String(v) => Cow::Borrowed(v.as_str()), Value::Array(v) => format!("{}", v).into(), } } @@ -206,7 +376,7 @@ from_values!( (bool, Value::Bool); (i64, Value::I64); (f64, Value::F64); - (Cow<'static, str>, Value::String); + (StringValue, Value::String); ); impl From<&'static str> for Value { @@ -223,14 +393,28 @@ impl From for Value { } } +impl From> for Value { + /// Convenience method for creating a `Value` from a `String`. + fn from(s: Arc) -> Self { + Value::String(s.into()) + } +} + +impl From> for Value { + /// Convenience method for creating a `Value` from a `String`. + fn from(s: Cow<'static, str>) -> Self { + Value::String(s.into()) + } +} + 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)), + Value::Bool(v) => v.fmt(fmt), + Value::I64(v) => v.fmt(fmt), + Value::F64(v) => v.fmt(fmt), + Value::String(v) => fmt.write_str(v.as_str()), + Value::Array(v) => v.fmt(fmt), } } } diff --git a/opentelemetry-contrib/Cargo.toml b/opentelemetry-contrib/Cargo.toml index 93a2ef35a5..1f3a1919a6 100644 --- a/opentelemetry-contrib/Cargo.toml +++ b/opentelemetry-contrib/Cargo.toml @@ -34,7 +34,7 @@ serde_json = { version = "1", optional = true} futures = {version = "0.3", optional = true} async-trait = {version = "0.1", optional = true} tokio = { version = "1.0", features = ["fs", "io-util"], optional = true } -async-std = { version = "1.6", optional = true } +async-std = { version = "1.10", optional = true } [dev-dependencies] diff --git a/opentelemetry-contrib/src/trace/exporter/jaeger_json.rs b/opentelemetry-contrib/src/trace/exporter/jaeger_json.rs index 6b5a8bfafe..35f0e9631d 100644 --- a/opentelemetry-contrib/src/trace/exporter/jaeger_json.rs +++ b/opentelemetry-contrib/src/trace/exporter/jaeger_json.rs @@ -193,7 +193,7 @@ fn opentelemetry_value_to_json(value: &opentelemetry::Value) -> (&str, serde_jso opentelemetry::Value::Bool(b) => ("bool", serde_json::json!(b)), opentelemetry::Value::I64(i) => ("int64", serde_json::json!(i)), opentelemetry::Value::F64(f) => ("float64", serde_json::json!(f)), - opentelemetry::Value::String(s) => ("string", serde_json::json!(s)), + opentelemetry::Value::String(s) => ("string", serde_json::json!(s.as_str())), v @ opentelemetry::Value::Array(_) => ("string", serde_json::json!(v.to_string())), } } diff --git a/opentelemetry-datadog/src/exporter/model/v03.rs b/opentelemetry-datadog/src/exporter/model/v03.rs index 1315176387..b9310d75fc 100644 --- a/opentelemetry-datadog/src/exporter/model/v03.rs +++ b/opentelemetry-datadog/src/exporter/model/v03.rs @@ -41,7 +41,7 @@ where if let Some(Value::String(s)) = span.attributes.get(&Key::new("span.type")) { rmp::encode::write_map_len(&mut encoded, 11)?; rmp::encode::write_str(&mut encoded, "type")?; - rmp::encode::write_str(&mut encoded, s.as_ref())?; + rmp::encode::write_str(&mut encoded, s.as_str())?; } else { rmp::encode::write_map_len(&mut encoded, 10)?; } diff --git a/opentelemetry-datadog/src/exporter/model/v05.rs b/opentelemetry-datadog/src/exporter/model/v05.rs index 08708b700c..94ab1df9b1 100644 --- a/opentelemetry-datadog/src/exporter/model/v05.rs +++ b/opentelemetry-datadog/src/exporter/model/v05.rs @@ -121,7 +121,7 @@ where .unwrap_or(0); let span_type = match span.attributes.get(&Key::new("span.type")) { - Some(Value::String(s)) => interner.intern(s.as_ref()), + Some(Value::String(s)) => interner.intern(s.as_str()), _ => interner.intern(""), }; diff --git a/opentelemetry-dynatrace/Cargo.toml b/opentelemetry-dynatrace/Cargo.toml index 99dd343639..a5b122b408 100644 --- a/opentelemetry-dynatrace/Cargo.toml +++ b/opentelemetry-dynatrace/Cargo.toml @@ -49,7 +49,7 @@ wasm = [ ] [dependencies] -async-std = { version = "= 1.8.0", features = ["unstable"], optional = true } +async-std = { version = "= 1.10.0", features = ["unstable"], optional = true } base64 = { version = "0.13", optional = true } futures = "0.3" futures-util = { version = "0.3", optional = true } diff --git a/opentelemetry-dynatrace/src/transform/metrics.rs b/opentelemetry-dynatrace/src/transform/metrics.rs index f0aca6ffdf..3b409b5f10 100644 --- a/opentelemetry-dynatrace/src/transform/metrics.rs +++ b/opentelemetry-dynatrace/src/transform/metrics.rs @@ -801,7 +801,7 @@ mod tests { let dimensions = DimensionSet::from(vec![ KeyValue::new("KEY", "VALUE"), KeyValue::new("test.abc_123-", "value.123_foo-bar"), - KeyValue::new(METRICS_SOURCE, "opentelemetry".to_string()), + KeyValue::new(METRICS_SOURCE, "opentelemetry"), ]); let expect = vec![ @@ -892,7 +892,7 @@ mod tests { let dimensions = DimensionSet::from(vec![ KeyValue::new("KEY", "VALUE"), KeyValue::new("test.abc_123-", "value.123_foo-bar"), - KeyValue::new(METRICS_SOURCE, "opentelemetry".to_string()), + KeyValue::new(METRICS_SOURCE, "opentelemetry"), ]); let expect = vec![MetricLine { @@ -936,7 +936,7 @@ mod tests { let dimensions = DimensionSet::from(vec![ KeyValue::new("KEY", "VALUE"), KeyValue::new("test.abc_123-", "value.123_foo-bar"), - KeyValue::new(METRICS_SOURCE, "opentelemetry".to_string()), + KeyValue::new(METRICS_SOURCE, "opentelemetry"), ]); let expect = vec![MetricLine { @@ -1005,7 +1005,7 @@ mod tests { let dimensions = DimensionSet::from(vec![ KeyValue::new("KEY", "VALUE"), KeyValue::new("test.abc_123-", "value.123_foo-bar"), - KeyValue::new(METRICS_SOURCE, "opentelemetry".to_string()), + KeyValue::new(METRICS_SOURCE, "opentelemetry"), ]); let expect = vec![MetricLine { @@ -1072,7 +1072,7 @@ mod tests { let dimensions = DimensionSet::from(vec![ KeyValue::new("KEY", "VALUE"), KeyValue::new("test.abc_123-", "value.123_foo-bar"), - KeyValue::new(METRICS_SOURCE, "opentelemetry".to_string()), + KeyValue::new(METRICS_SOURCE, "opentelemetry"), ]); let expect = vec![MetricLine { @@ -1142,7 +1142,7 @@ mod tests { let dimensions = DimensionSet::from(vec![ KeyValue::new("KEY", "VALUE"), KeyValue::new("test.abc_123-", "value.123_foo-bar"), - KeyValue::new(METRICS_SOURCE, "opentelemetry".to_string()), + KeyValue::new(METRICS_SOURCE, "opentelemetry"), ]); let expect = vec![MetricLine { diff --git a/opentelemetry-jaeger/Cargo.toml b/opentelemetry-jaeger/Cargo.toml index 686dba584c..085cc7ca32 100644 --- a/opentelemetry-jaeger/Cargo.toml +++ b/opentelemetry-jaeger/Cargo.toml @@ -19,7 +19,7 @@ all-features = true rustdoc-args = ["--cfg", "docsrs"] [dependencies] -async-std = { version = "= 1.8.0", optional = true } +async-std = { version = "= 1.10.0", optional = true } async-trait = "0.1" base64 = { version = "0.13", optional = true } futures = "0.3" diff --git a/opentelemetry-proto/src/transform/common.rs b/opentelemetry-proto/src/transform/common.rs index 590ee0e31b..adc55381aa 100644 --- a/opentelemetry-proto/src/transform/common.rs +++ b/opentelemetry-proto/src/transform/common.rs @@ -62,7 +62,7 @@ pub mod tonic { Value::Bool(val) => Some(any_value::Value::BoolValue(val)), Value::I64(val) => Some(any_value::Value::IntValue(val)), Value::F64(val) => Some(any_value::Value::DoubleValue(val)), - Value::String(val) => Some(any_value::Value::StringValue(val.into_owned())), + Value::String(val) => Some(any_value::Value::StringValue(val.to_string())), Value::Array(array) => Some(any_value::Value::ArrayValue(match array { Array::Bool(vals) => array_into_proto(vals), Array::I64(vals) => array_into_proto(vals), @@ -144,7 +144,7 @@ pub mod grpcio { Value::Bool(val) => any_value.set_bool_value(val), 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::String(val) => any_value.set_string_value(val.to_string()), Value::Array(array) => any_value.set_array_value(match array { Array::Bool(vals) => array_into_proto(vals), Array::I64(vals) => array_into_proto(vals), diff --git a/opentelemetry-sdk/Cargo.toml b/opentelemetry-sdk/Cargo.toml index d9077436bd..0d47eb3199 100644 --- a/opentelemetry-sdk/Cargo.toml +++ b/opentelemetry-sdk/Cargo.toml @@ -4,7 +4,7 @@ version = "0.1.0" edition = "2018" [dependencies] -async-std = { version = "= 1.8.0", features = ["unstable"], optional = true } +async-std = { version = "= 1.10.0", features = ["unstable"], optional = true } async-trait = { version = "0.1", optional = true } crossbeam-channel = { version = "0.5", optional = true } dashmap = { version = "4.0.1", optional = true } diff --git a/opentelemetry-sdk/src/export/metrics/stdout.rs b/opentelemetry-sdk/src/export/metrics/stdout.rs index aaf1ae722d..4668c57dbb 100644 --- a/opentelemetry-sdk/src/export/metrics/stdout.rs +++ b/opentelemetry-sdk/src/export/metrics/stdout.rs @@ -94,7 +94,7 @@ where let encoded_inst_attributes = if !desc.instrumentation_name().is_empty() { let inst_attributes = AttributeSet::from_attributes(iter::once(KeyValue::new( "instrumentation.name", - desc.instrumentation_name().to_owned(), + desc.instrumentation_name(), ))); inst_attributes.encoded(Some(self.attribute_encoder.as_ref())) } else { diff --git a/opentelemetry-sdk/src/propagation/baggage.rs b/opentelemetry-sdk/src/propagation/baggage.rs index 8b16c8eff1..bc169afcb4 100644 --- a/opentelemetry-sdk/src/propagation/baggage.rs +++ b/opentelemetry-sdk/src/propagation/baggage.rs @@ -186,7 +186,6 @@ mod tests { use opentelemetry_api::{ baggage::BaggageMetadata, propagation::TextMapPropagator, Key, KeyValue, Value, }; - use std::borrow::Cow; use std::collections::HashMap; #[rustfmt::skip] @@ -247,7 +246,7 @@ mod tests { vec![ 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())), + KeyValue::new("key3", Value::Array(vec!["val1", "val2"].into())), ], vec![ "key1=[true%2Cfalse]", diff --git a/opentelemetry-sdk/src/resource/process.rs b/opentelemetry-sdk/src/resource/process.rs index 5d74a73a81..8e51135d7f 100644 --- a/opentelemetry-sdk/src/resource/process.rs +++ b/opentelemetry-sdk/src/resource/process.rs @@ -4,8 +4,7 @@ use crate::resource::ResourceDetector; use crate::Resource; -use opentelemetry_api::{Array, KeyValue, Value}; -use std::borrow::Cow; +use opentelemetry_api::{KeyValue, Value}; use std::env::args_os; use std::process::id; use std::time::Duration; @@ -25,13 +24,10 @@ impl ResourceDetector for ProcessResourceDetector { let arguments = args_os(); let cmd_arg_val = arguments .into_iter() - .map(|arg| Cow::from(arg.to_string_lossy().into_owned())) - .collect::>>(); + .map(|arg| arg.to_string_lossy().into_owned()) + .collect::>(); Resource::new(vec![ - KeyValue::new( - "process.command_args", - Value::Array(Array::String(cmd_arg_val)), - ), + KeyValue::new("process.command_args", Value::Array(cmd_arg_val.into())), KeyValue::new("process.pid", id() as i64), ]) } diff --git a/opentelemetry-stackdriver/src/lib.rs b/opentelemetry-stackdriver/src/lib.rs index 0f1cfab4e5..a917aff8fc 100644 --- a/opentelemetry-stackdriver/src/lib.rs +++ b/opentelemetry-stackdriver/src/lib.rs @@ -497,7 +497,7 @@ impl From for AttributeValue { Value::Bool(v) => attribute_value::Value::BoolValue(v), Value::F64(v) => attribute_value::Value::StringValue(to_truncate(v.to_string())), Value::I64(v) => attribute_value::Value::IntValue(v), - Value::String(v) => attribute_value::Value::StringValue(to_truncate(v.into_owned())), + Value::String(v) => attribute_value::Value::StringValue(to_truncate(v.to_string())), Value::Array(_) => attribute_value::Value::StringValue(to_truncate(v.to_string())), }; AttributeValue { From 39b86a7f42ec517c32f6d8cb966ca34ae47cdaf9 Mon Sep 17 00:00:00 2001 From: Julian Tescher Date: Fri, 24 Jun 2022 17:53:29 -0700 Subject: [PATCH 2/2] Remove easily misused array conversions --- opentelemetry-api/src/common.rs | 43 +++----------------- opentelemetry-api/src/lib.rs | 2 +- opentelemetry-sdk/src/propagation/baggage.rs | 4 +- opentelemetry-sdk/src/resource/process.rs | 6 +-- 4 files changed, 12 insertions(+), 43 deletions(-) diff --git a/opentelemetry-api/src/common.rs b/opentelemetry-api/src/common.rs index 6b078f1748..44eae5cf1a 100644 --- a/opentelemetry-api/src/common.rs +++ b/opentelemetry-api/src/common.rs @@ -229,39 +229,6 @@ into_array!( (Vec, Array::String), ); -impl From> for Array { - /// Convenience method for creating a `Value` from a `&'static str`. - fn from(ss: Vec<&'static str>) -> Self { - Array::String( - ss.into_iter() - .map(|s| StringValue(OtelString::Static(s))) - .collect(), - ) - } -} - -impl From> for Array { - /// Convenience method for creating a `Value` from a `String`. - fn from(ss: Vec) -> Self { - Array::String( - ss.into_iter() - .map(|s| StringValue(OtelString::Owned(s))) - .collect(), - ) - } -} - -impl From>> for Array { - /// Convenience method for creating a `Value` from a `String`. - fn from(ss: Vec>) -> Self { - Array::String( - ss.into_iter() - .map(|s| StringValue(OtelString::RefCounted(s))) - .collect(), - ) - } -} - /// The value part of attribute [KeyValue] pairs. #[derive(Clone, Debug, PartialEq)] pub enum Value { @@ -297,6 +264,12 @@ impl fmt::Display for StringValue { } } +impl AsRef for StringValue { + fn as_ref(&self) -> &str { + self.0.as_str() + } +} + impl StringValue { /// Returns a string slice to this value pub fn as_str(&self) -> &str { @@ -380,28 +353,24 @@ from_values!( ); impl From<&'static str> for Value { - /// Convenience method for creating a `Value` from a `&'static str`. fn from(s: &'static str) -> Self { Value::String(s.into()) } } impl From for Value { - /// Convenience method for creating a `Value` from a `String`. fn from(s: String) -> Self { Value::String(s.into()) } } impl From> for Value { - /// Convenience method for creating a `Value` from a `String`. fn from(s: Arc) -> Self { Value::String(s.into()) } } impl From> for Value { - /// Convenience method for creating a `Value` from a `String`. fn from(s: Cow<'static, str>) -> Self { Value::String(s.into()) } diff --git a/opentelemetry-api/src/lib.rs b/opentelemetry-api/src/lib.rs index 54f5131279..7fcdc079d8 100644 --- a/opentelemetry-api/src/lib.rs +++ b/opentelemetry-api/src/lib.rs @@ -55,7 +55,7 @@ mod common; #[doc(hidden)] pub mod testing; -pub use common::{Array, ExportError, InstrumentationLibrary, Key, KeyValue, Value}; +pub use common::{Array, ExportError, InstrumentationLibrary, Key, KeyValue, StringValue, Value}; #[cfg(feature = "metrics")] #[cfg_attr(docsrs, doc(cfg(feature = "metrics")))] diff --git a/opentelemetry-sdk/src/propagation/baggage.rs b/opentelemetry-sdk/src/propagation/baggage.rs index bc169afcb4..025dd62ae0 100644 --- a/opentelemetry-sdk/src/propagation/baggage.rs +++ b/opentelemetry-sdk/src/propagation/baggage.rs @@ -184,7 +184,7 @@ impl TextMapPropagator for BaggagePropagator { mod tests { use super::*; use opentelemetry_api::{ - baggage::BaggageMetadata, propagation::TextMapPropagator, Key, KeyValue, Value, + baggage::BaggageMetadata, propagation::TextMapPropagator, Key, KeyValue, StringValue, Value, }; use std::collections::HashMap; @@ -246,7 +246,7 @@ mod tests { vec![ KeyValue::new("key1", Value::Array(vec![true, false].into())), KeyValue::new("key2", Value::Array(vec![123, 456].into())), - KeyValue::new("key3", Value::Array(vec!["val1", "val2"].into())), + KeyValue::new("key3", Value::Array(vec![StringValue::from("val1"), StringValue::from("val2")].into())), ], vec![ "key1=[true%2Cfalse]", diff --git a/opentelemetry-sdk/src/resource/process.rs b/opentelemetry-sdk/src/resource/process.rs index 8e51135d7f..1c1a8d7154 100644 --- a/opentelemetry-sdk/src/resource/process.rs +++ b/opentelemetry-sdk/src/resource/process.rs @@ -4,7 +4,7 @@ use crate::resource::ResourceDetector; use crate::Resource; -use opentelemetry_api::{KeyValue, Value}; +use opentelemetry_api::{KeyValue, StringValue, Value}; use std::env::args_os; use std::process::id; use std::time::Duration; @@ -24,8 +24,8 @@ impl ResourceDetector for ProcessResourceDetector { let arguments = args_os(); let cmd_arg_val = arguments .into_iter() - .map(|arg| arg.to_string_lossy().into_owned()) - .collect::>(); + .map(|arg| arg.to_string_lossy().into_owned().into()) + .collect::>(); Resource::new(vec![ KeyValue::new("process.command_args", Value::Array(cmd_arg_val.into())), KeyValue::new("process.pid", id() as i64),