Skip to content

Commit

Permalink
Use Cow<'static, str> for api::Value (open-telemetry#332)
Browse files Browse the repository at this point in the history
  • Loading branch information
djc authored Nov 2, 2020
1 parent 866ae41 commit 734524b
Show file tree
Hide file tree
Showing 12 changed files with 49 additions and 51 deletions.
3 changes: 2 additions & 1 deletion examples/actix-http/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ async fn main() -> std::io::Result<()> {
.wrap(Logger::default())
.wrap_fn(move |req, srv| {
tracer.in_span("middleware", move |cx| {
cx.span().set_attribute(Key::new("path").string(req.path()));
cx.span()
.set_attribute(Key::new("path").string(req.path().to_string()));
srv.call(req).with_context(cx)
})
})
Expand Down
3 changes: 2 additions & 1 deletion examples/actix-udp/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ async fn main() -> std::io::Result<()> {
.wrap_fn(|req, srv| {
let tracer = global::tracer("request");
tracer.in_span("middleware", move |cx| {
cx.span().set_attribute(Key::new("path").string(req.path()));
cx.span()
.set_attribute(Key::new("path").string(req.path().to_string()));
srv.call(req).with_context(cx)
})
})
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-contrib/src/datadog/model/v03.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub(crate) fn encode(service_name: &str, spans: Vec<trace::SpanData>) -> Result<
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_str())?;
rmp::encode::write_str(&mut encoded, s.as_ref())?;
} else {
rmp::encode::write_map_len(&mut encoded, 10)?;
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-contrib/src/datadog/model/v05.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ fn encode_spans(
.unwrap_or(0);

let span_type = match span.attributes.get(&Key::new("span.type")) {
Some(Value::String(s)) => interner.intern(s.as_str()),
Some(Value::String(s)) => interner.intern(s.as_ref()),
_ => interner.intern(""),
};

Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-jaeger/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ impl Into<jaeger::Tag> for KeyValue {
fn into(self) -> jaeger::Tag {
let KeyValue { key, value } = self;
match value {
Value::String(s) => jaeger::Tag::new(key.into(), jaeger::TagType::String, Some(s), None, None, None, None),
Value::String(s) => jaeger::Tag::new(key.into(), jaeger::TagType::String, Some(s.into()), None, None, None, None),
Value::F64(f) => jaeger::Tag::new(key.into(), jaeger::TagType::Double, None, Some(f.into()), None, None, None),
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),
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-otlp/src/transform/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl From<Value> for AnyValue {
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),
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(
Expand Down
12 changes: 6 additions & 6 deletions opentelemetry/src/api/baggage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl Baggage {
/// let mut cc = Baggage::new();
/// let _ = cc.insert("my-name", "my-value");
///
/// assert_eq!(cc.get("my-name"), Some(&Value::String("my-value".to_string())))
/// assert_eq!(cc.get("my-name"), Some(&Value::from("my-value")))
/// ```
pub fn get<T: Into<Key>>(&self, key: T) -> Option<&Value> {
self.inner.get(&key.into()).map(|(value, _metadata)| value)
Expand All @@ -55,7 +55,7 @@ impl Baggage {
/// let _ = cc.insert("my-name", "my-value");
///
/// // By default, the metadata is empty
/// assert_eq!(cc.get_with_metadata("my-name"), Some(&(Value::String("my-value".to_string()), BaggageMetadata::from(""))))
/// assert_eq!(cc.get_with_metadata("my-name"), Some(&(Value::from("my-value"), BaggageMetadata::from(""))))
/// ```
pub fn get_with_metadata<T: Into<Key>>(&self, key: T) -> Option<&(Value, BaggageMetadata)> {
self.inner.get(&key.into())
Expand All @@ -74,7 +74,7 @@ impl Baggage {
/// let mut cc = Baggage::new();
/// let _ = cc.insert("my-name", "my-value");
///
/// assert_eq!(cc.get("my-name"), Some(&Value::String("my-value".to_string())))
/// assert_eq!(cc.get("my-name"), Some(&Value::from("my-value")))
/// ```
pub fn insert<K, V>(&mut self, key: K, value: V) -> Option<Value>
where
Expand All @@ -98,7 +98,7 @@ impl Baggage {
/// let mut cc = Baggage::new();
/// let _ = cc.insert_with_metadata("my-name", "my-value", "test");
///
/// assert_eq!(cc.get_with_metadata("my-name"), Some(&(Value::String("my-value".to_string()), BaggageMetadata::from("test"))))
/// assert_eq!(cc.get_with_metadata("my-name"), Some(&(Value::from("my-value"), BaggageMetadata::from("test"))))
/// ```
pub fn insert_with_metadata<K, V, S>(
&mut self,
Expand Down Expand Up @@ -257,7 +257,7 @@ pub trait BaggageExt {
///
/// assert_eq!(
/// cx.baggage().get("my-name"),
/// Some(&Value::String("my-value".to_string())),
/// Some(&Value::from("my-value")),
/// )
/// ```
fn with_baggage<T: IntoIterator<Item = I>, I: Into<KeyValueMetadata>>(
Expand All @@ -276,7 +276,7 @@ pub trait BaggageExt {
///
/// assert_eq!(
/// cx.baggage().get("my-name"),
/// Some(&Value::String("my-value".to_string())),
/// Some(&Value::from("my-value")),
/// )
/// ```
fn current_with_baggage<T: IntoIterator<Item = I>, I: Into<KeyValueMetadata>>(
Expand Down
24 changes: 15 additions & 9 deletions opentelemetry/src/api/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl Key {
}

/// Create a `KeyValue` pair for `String` values.
pub fn string<T: Into<String>>(&self, value: T) -> KeyValue {
pub fn string<T: Into<Cow<'static, str>>>(&self, value: T) -> KeyValue {
KeyValue {
key: self.clone(),
value: Value::String(value.into()),
Expand Down Expand Up @@ -97,7 +97,7 @@ pub enum Value {
/// f64 values
F64(f64),
/// String values
String(String),
String(Cow<'static, str>),
/// Array of homogeneous values
Array(Vec<Value>),
}
Expand All @@ -122,14 +122,20 @@ from_values!(
(bool, Value::Bool);
(i64, Value::I64);
(f64, Value::F64);
(String, Value::String);
(Vec<Value>, Value::Array);
);

impl From<&str> for Value {
/// Convenience method for creating a `Value` from a `&str`.
fn from(value_str: &str) -> Self {
Value::String(value_str.to_string())
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<String> for Value {
/// Convenience method for creating a `Value` from a `String`.
fn from(s: String) -> Self {
Value::String(s.into())
}
}

Expand All @@ -141,7 +147,7 @@ impl From<Value> for String {
Value::Bool(value) => value.to_string(),
Value::I64(value) => value.to_string(),
Value::F64(value) => value.to_string(),
Value::String(value) => value,
Value::String(value) => value.into_owned(),
Value::Array(value) => format_value_array_as_string(&value),
}
}
Expand All @@ -155,7 +161,7 @@ impl From<&Value> for String {
Value::Bool(value) => value.to_string(),
Value::I64(value) => value.to_string(),
Value::F64(value) => value.to_string(),
Value::String(value) => value.clone(),
Value::String(value) => value.to_string(),
Value::Array(value) => format_value_array_as_string(value),
}
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry/src/exporter/metrics/stdout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ where
let encoded_inst_labels = if !desc.instrumentation_name().is_empty() {
let inst_labels = LabelSet::from_labels(iter::once(KeyValue::new(
"instrumentation.name",
desc.instrumentation_name(),
desc.instrumentation_name().to_owned(),
)));
inst_labels.encoded(Some(self.label_encoder.as_ref()))
} else {
Expand Down
35 changes: 14 additions & 21 deletions opentelemetry/src/sdk/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! Implementation of `ResourceDetector` to extract a `Resource` from environment
//! variables.
use crate::sdk::{resource::ResourceDetector, Resource};
use crate::{Key, KeyValue, Value};
use crate::KeyValue;
use std::env;
use std::time::Duration;

Expand Down Expand Up @@ -45,28 +45,24 @@ impl Default for EnvResourceDetector {
/// Extract key value pairs and construct a resource from resources string like
/// key1=value1,key2=value2,...
fn construct_otel_resources(s: String) -> Resource {
let mut key_values = vec![];
for entries in s.split_terminator(',') {
let key_value_strs = entries.split('=').map(str::trim).collect::<Vec<&str>>();
if key_value_strs.len() != 2 {
continue;
Resource::new(s.split_terminator(',').filter_map(|entry| {
let mut parts = entry.splitn(2, '=');
let key = parts.next()?.trim();
let value = parts.next()?.trim();
if value.find('=').is_some() {
return None;
}

let key = Key::from(key_value_strs.get(0).unwrap().to_string());
let value = Value::String(key_value_strs.get(1).unwrap().to_string());

key_values.push(KeyValue::new(key, value));
}

Resource::new(key_values)
Some(KeyValue::new(key.to_owned(), value.to_owned()))
}))
}

#[cfg(test)]
mod tests {
use crate::sdk::env::OTEL_RESOURCE_ATTRIBUTES;
use crate::sdk::resource::{Resource, ResourceDetector};
use crate::sdk::EnvResourceDetector;
use crate::{Key, KeyValue, Value};
use crate::{Key, KeyValue};
use std::{env, time};

#[test]
Expand All @@ -79,13 +75,10 @@ mod tests {
assert_eq!(
resource,
Resource::new(vec![
KeyValue::new(
Key::new("key".to_string()),
Value::String("value".to_string())
),
KeyValue::new(Key::new("k".to_string()), Value::String("v".to_string())),
KeyValue::new(Key::new("a".to_string()), Value::String("x".to_string())),
KeyValue::new(Key::new("a".to_string()), Value::String("z".to_string()))
KeyValue::new(Key::new("key".to_string()), "value"),
KeyValue::new(Key::new("k".to_string()), "v"),
KeyValue::new(Key::new("a".to_string()), "x"),
KeyValue::new(Key::new("a".to_string()), "z"),
])
);

Expand Down
2 changes: 1 addition & 1 deletion opentelemetry/src/sdk/propagation/baggage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ mod tests {
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![Value::String("val1".to_string()), Value::String("val2".to_string())])),
KeyValue::new("key3", Value::Array(vec!["val1".into(), "val2".into()])),
],
vec![
"key1=[true%2Cfalse]",
Expand Down
11 changes: 4 additions & 7 deletions opentelemetry/src/sdk/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,10 @@ mod tests {
assert_eq!(
resource,
Resource::new(vec![
KeyValue::new(
Key::new("key".to_string()),
Value::String("value".to_string())
),
KeyValue::new(Key::new("k".to_string()), Value::String("v".to_string())),
KeyValue::new(Key::new("a".to_string()), Value::String("x".to_string())),
KeyValue::new(Key::new("a".to_string()), Value::String("z".to_string()))
KeyValue::new(Key::new("key".to_string()), "value"),
KeyValue::new(Key::new("k".to_string()), "v"),
KeyValue::new(Key::new("a".to_string()), "x"),
KeyValue::new(Key::new("a".to_string()), "z")
])
)
}
Expand Down

0 comments on commit 734524b

Please sign in to comment.