From f953257ff07ef398b5e7bc82de1001c5545ba4d5 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 31 Oct 2020 12:45:50 +0100 Subject: [PATCH] Remove support for u64 and bytes values (see #317) --- benches/trace.rs | 15 ---------- opentelemetry-jaeger/src/lib.rs | 3 -- opentelemetry-otlp/src/transform/common.rs | 2 -- src/api/core.rs | 33 +--------------------- src/api/labels/mod.rs | 2 -- src/sdk/propagation/baggage.rs | 2 -- 6 files changed, 1 insertion(+), 56 deletions(-) diff --git a/benches/trace.rs b/benches/trace.rs index 58b44f1c7b..134f86ee9d 100644 --- a/benches/trace.rs +++ b/benches/trace.rs @@ -12,7 +12,6 @@ fn criterion_benchmark(c: &mut Criterion) { let span = tracer.start("foo"); span.set_attribute(Key::new("key1").bool(false)); span.set_attribute(Key::new("key2").string("hello")); - span.set_attribute(Key::new("key3").u64(123)); span.set_attribute(Key::new("key4").f64(123.456)); span.end(); }); @@ -21,11 +20,9 @@ fn criterion_benchmark(c: &mut Criterion) { let span = tracer.start("foo"); span.set_attribute(Key::new("key1").bool(false)); span.set_attribute(Key::new("key2").string("hello")); - span.set_attribute(Key::new("key3").u64(123)); span.set_attribute(Key::new("key4").f64(123.456)); span.set_attribute(Key::new("key11").bool(false)); span.set_attribute(Key::new("key12").string("hello")); - span.set_attribute(Key::new("key13").u64(123)); span.set_attribute(Key::new("key14").f64(123.456)); span.end(); }); @@ -35,11 +32,7 @@ fn criterion_benchmark(c: &mut Criterion) { span.set_attribute(Key::new("key1").bool(false)); span.set_attribute(Key::new("key2").string("hello")); span.set_attribute(Key::new("key3").i64(123)); - span.set_attribute(Key::new("key4").u64(123)); span.set_attribute(Key::new("key5").f64(123.456)); - span.set_attribute( - Key::new("key6").bytes(vec![104, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100]), - ); span.end(); }); @@ -48,19 +41,11 @@ fn criterion_benchmark(c: &mut Criterion) { span.set_attribute(Key::new("key1").bool(false)); span.set_attribute(Key::new("key2").string("hello")); span.set_attribute(Key::new("key3").i64(123)); - span.set_attribute(Key::new("key4").u64(123)); span.set_attribute(Key::new("key5").f64(123.456)); - span.set_attribute( - Key::new("key6").bytes(vec![104, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100]), - ); span.set_attribute(Key::new("key11").bool(false)); span.set_attribute(Key::new("key12").string("hello")); span.set_attribute(Key::new("key13").i64(123)); - span.set_attribute(Key::new("key14").u64(123)); span.set_attribute(Key::new("key15").f64(123.456)); - span.set_attribute( - Key::new("key16").bytes(vec![104, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100]), - ); span.end(); }); } diff --git a/opentelemetry-jaeger/src/lib.rs b/opentelemetry-jaeger/src/lib.rs index 7525bfdb9a..d804991fe1 100644 --- a/opentelemetry-jaeger/src/lib.rs +++ b/opentelemetry-jaeger/src/lib.rs @@ -478,9 +478,6 @@ impl Into for KeyValue { 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), - Value::Bytes(b) => jaeger::Tag::new(key.into(), jaeger::TagType::Binary, None, None, None, None, Some(b)), - // TODO: better u64 handling, jaeger thrift only has i64 support - Value::U64(u) => jaeger::Tag::new(key.into(), jaeger::TagType::String, Some(u.to_string()), None, None, None, 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), } diff --git a/opentelemetry-otlp/src/transform/common.rs b/opentelemetry-otlp/src/transform/common.rs index 0df0f760ac..0baa14a8a8 100644 --- a/opentelemetry-otlp/src/transform/common.rs +++ b/opentelemetry-otlp/src/transform/common.rs @@ -43,10 +43,8 @@ impl From for AnyValue { match value { Value::Bool(val) => any_value.set_bool_value(val), Value::I64(val) => any_value.set_int_value(val), - Value::U64(val) => any_value.set_int_value(val as i64), Value::F64(val) => any_value.set_double_value(val), Value::String(val) => any_value.set_string_value(val), - Value::Bytes(_val) => any_value.set_string_value("INVALID".to_string()), Value::Array(vals) => any_value.set_array_value({ let mut array_value = ArrayValue::new(); array_value.set_values(RepeatedField::from_vec( diff --git a/src/api/core.rs b/src/api/core.rs index a219dbd49b..420b637897 100644 --- a/src/api/core.rs +++ b/src/api/core.rs @@ -35,14 +35,6 @@ impl Key { } } - /// Create a `KeyValue` pair for `u64` values. - pub fn u64(&self, value: u64) -> KeyValue { - KeyValue { - key: self.clone(), - value: Value::U64(value), - } - } - /// Create a `KeyValue` pair for `f64` values. pub fn f64(&self, value: f64) -> KeyValue { KeyValue { @@ -59,14 +51,6 @@ impl Key { } } - /// Create a `KeyValue` pair for byte arrays. - pub fn bytes>>(&self, value: T) -> KeyValue { - KeyValue { - key: self.clone(), - value: Value::Bytes(value.into()), - } - } - /// Create a `KeyValue` pair for arrays. pub fn array>>(&self, value: T) -> KeyValue { KeyValue { @@ -110,14 +94,10 @@ pub enum Value { Bool(bool), /// i64 values I64(i64), - /// u64 values - U64(u64), /// f64 values F64(f64), /// String values String(String), - /// Byte array values - Bytes(Vec), /// Array of homogeneous values Array(Vec), } @@ -141,10 +121,8 @@ macro_rules! from_values { from_values!( (bool, Value::Bool); (i64, Value::I64); - (u64, Value::U64); (f64, Value::F64); (String, Value::String); - (Vec, Value::Bytes); (Vec, Value::Array); ); @@ -162,10 +140,8 @@ impl From for String { match value { Value::Bool(value) => value.to_string(), Value::I64(value) => value.to_string(), - Value::U64(value) => value.to_string(), Value::F64(value) => value.to_string(), Value::String(value) => value, - Value::Bytes(value) => String::from_utf8(value).unwrap_or_else(|_| String::new()), Value::Array(value) => format_value_array_as_string(&value), } } @@ -178,12 +154,8 @@ impl From<&Value> for String { match value { Value::Bool(value) => value.to_string(), Value::I64(value) => value.to_string(), - Value::U64(value) => value.to_string(), Value::F64(value) => value.to_string(), Value::String(value) => value.clone(), - Value::Bytes(value) => { - String::from_utf8(value.clone()).unwrap_or_else(|_| String::new()) - } Value::Array(value) => format_value_array_as_string(value), } } @@ -193,10 +165,7 @@ fn format_value_array_as_string(v: &[Value]) -> String { format!( "[{}]", v.iter() - .map(|elem| match elem { - v @ Value::String(_) | v @ Value::Bytes(_) => format!(r#""{}""#, String::from(v)), - v => String::from(v), - }) + .map(|elem| String::from(elem)) .collect::>() .join(",") ) diff --git a/src/api/labels/mod.rs b/src/api/labels/mod.rs index e37f4e01d3..01d4bc3856 100644 --- a/src/api/labels/mod.rs +++ b/src/api/labels/mod.rs @@ -77,13 +77,11 @@ fn hash_value(state: &mut H, value: &Value) { match value { Value::Bool(b) => b.hash(state), Value::I64(i) => i.hash(state), - Value::U64(u) => u.hash(state), Value::F64(f) => { // FIXME: f64 does not impl hash, this impl may have incorrect outcomes. f.to_bits().hash(state) } Value::String(s) => s.hash(state), - Value::Bytes(b) => state.write(b), Value::Array(arr) => { // recursively hash array values for val in arr { diff --git a/src/sdk/propagation/baggage.rs b/src/sdk/propagation/baggage.rs index be5db58259..59c22ac9a1 100644 --- a/src/sdk/propagation/baggage.rs +++ b/src/sdk/propagation/baggage.rs @@ -233,7 +233,6 @@ mod tests { vec![ KeyValue::new("key1", true), KeyValue::new("key2", Value::I64(123)), - KeyValue::new("key3", Value::U64(123)), KeyValue::new("key4", Value::F64(123.567)), ], vec![ @@ -249,7 +248,6 @@ mod tests { 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("key4", Value::Array(vec![Value::Bytes(vec![118, 97, 108, 49]), Value::Bytes(vec![118, 97, 108, 50])])), ], vec![ "key1=[true%2Cfalse]",