Skip to content

Commit

Permalink
Remove support for u64 and bytes values (see #317) (#323)
Browse files Browse the repository at this point in the history
  • Loading branch information
djc authored Oct 31, 2020
1 parent 396dd88 commit be76c0a
Show file tree
Hide file tree
Showing 7 changed files with 6 additions and 60 deletions.
15 changes: 0 additions & 15 deletions benches/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand All @@ -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();
});
Expand All @@ -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();
});

Expand All @@ -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();
});
}
Expand Down
3 changes: 0 additions & 3 deletions opentelemetry-jaeger/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,9 +478,6 @@ impl Into<jaeger::Tag> 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),
}
Expand Down
2 changes: 0 additions & 2 deletions opentelemetry-otlp/src/transform/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,8 @@ impl From<Value> 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(
Expand Down
30 changes: 1 addition & 29 deletions src/api/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -59,14 +51,6 @@ impl Key {
}
}

/// Create a `KeyValue` pair for byte arrays.
pub fn bytes<T: Into<Vec<u8>>>(&self, value: T) -> KeyValue {
KeyValue {
key: self.clone(),
value: Value::Bytes(value.into()),
}
}

/// Create a `KeyValue` pair for arrays.
pub fn array<T: Into<Vec<Value>>>(&self, value: T) -> KeyValue {
KeyValue {
Expand Down Expand Up @@ -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<u8>),
/// Array of homogeneous values
Array(Vec<Value>),
}
Expand All @@ -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<u8>, Value::Bytes);
(Vec<Value>, Value::Array);
);

Expand All @@ -162,10 +140,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,
Value::Bytes(value) => String::from_utf8(value).unwrap_or_else(|_| String::new()),
Value::Array(value) => format_value_array_as_string(&value),
}
}
Expand All @@ -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),
}
}
Expand All @@ -194,7 +166,7 @@ fn format_value_array_as_string(v: &[Value]) -> String {
"[{}]",
v.iter()
.map(|elem| match elem {
v @ Value::String(_) | v @ Value::Bytes(_) => format!(r#""{}""#, String::from(v)),
Value::String(s) => format!(r#""{}""#, s),
v => String::from(v),
})
.collect::<Vec<_>>()
Expand Down
2 changes: 0 additions & 2 deletions src/api/labels/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,11 @@ fn hash_value<H: Hasher>(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 {
Expand Down
2 changes: 1 addition & 1 deletion src/api/trace/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,6 @@ pub enum StatusCode {
Ok = 0,
/// The default status.
Unset = 1,
/// The operation contains an error.
/// The operation contains an error.
Error = 2,
}
12 changes: 4 additions & 8 deletions src/sdk/propagation/baggage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
//! }
//!
//! // Add new baggage
//! let cx_with_additions = cx.with_baggage(vec![Key::new("server_id").u64(42)]);
//! let cx_with_additions = cx.with_baggage(vec![Key::new("server_id").i64(42)]);
//!
//! // Inject aggage into http request
//! propagator.inject_context(&cx_with_additions, &mut headers);
Expand Down Expand Up @@ -88,7 +88,7 @@ lazy_static::lazy_static! {
/// }
///
/// // Add new baggage
/// let cx_with_additions = cx.with_baggage(vec![Key::new("server_id").u64(42)]);
/// let cx_with_additions = cx.with_baggage(vec![Key::new("server_id").i64(42)]);
///
/// // Inject aggage into http request
/// propagator.inject_context(&cx_with_additions, &mut headers);
Expand Down Expand Up @@ -233,14 +233,12 @@ 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)),
KeyValue::new("key3", Value::F64(123.567)),
],
vec![
"key1=true",
"key2=123",
"key3=123",
"key4=123.567",
"key3=123.567",
],
),
// "values of array types"
Expand All @@ -249,13 +247,11 @@ 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]",
"key2=[123%2C456]",
"key3=[%22val1%22%2C%22val2%22]",
"key4=[%22val1%22%2C%22val2%22]",
],
),
]
Expand Down

0 comments on commit be76c0a

Please sign in to comment.