diff --git a/opentelemetry-sdk/Cargo.toml b/opentelemetry-sdk/Cargo.toml index b57bd2f043..b535dd5438 100644 --- a/opentelemetry-sdk/Cargo.toml +++ b/opentelemetry-sdk/Cargo.toml @@ -18,7 +18,6 @@ futures-channel = "0.3" futures-executor = { workspace = true } futures-util = { workspace = true, features = ["std", "sink", "async-await-macro"] } once_cell = { workspace = true } -ordered-float = { workspace = true } percent-encoding = { version = "2.0", optional = true } rand = { workspace = true, features = ["std", "std_rng","small_rng"], optional = true } glob = { version = "0.3.1", optional =true} diff --git a/opentelemetry-sdk/src/attributes/mod.rs b/opentelemetry-sdk/src/attributes/mod.rs index 1182e996fb..60949b23b2 100644 --- a/opentelemetry-sdk/src/attributes/mod.rs +++ b/opentelemetry-sdk/src/attributes/mod.rs @@ -1,3 +1,4 @@ mod set; +#[cfg(feature = "metrics")] pub use set::AttributeSet; diff --git a/opentelemetry-sdk/src/attributes/set.rs b/opentelemetry-sdk/src/attributes/set.rs index 7775d20692..0ddb36f2e0 100644 --- a/opentelemetry-sdk/src/attributes/set.rs +++ b/opentelemetry-sdk/src/attributes/set.rs @@ -1,70 +1,15 @@ use std::collections::hash_map::DefaultHasher; use std::collections::HashSet; -use std::{ - cmp::Ordering, - hash::{Hash, Hasher}, -}; +use std::hash::{Hash, Hasher}; -use opentelemetry::{Array, Key, KeyValue, Value}; -use ordered_float::OrderedFloat; - -#[derive(Clone, Debug)] -struct HashKeyValue(KeyValue); - -impl Hash for HashKeyValue { - fn hash(&self, state: &mut H) { - self.0.key.hash(state); - match &self.0.value { - Value::F64(f) => OrderedFloat(*f).hash(state), - Value::Array(a) => match a { - Array::Bool(b) => b.hash(state), - Array::I64(i) => i.hash(state), - Array::F64(f) => f.iter().for_each(|f| OrderedFloat(*f).hash(state)), - Array::String(s) => s.hash(state), - }, - Value::Bool(b) => b.hash(state), - Value::I64(i) => i.hash(state), - Value::String(s) => s.hash(state), - }; - } -} - -impl PartialOrd for HashKeyValue { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for HashKeyValue { - fn cmp(&self, other: &Self) -> Ordering { - self.0.key.cmp(&other.0.key) - } -} - -impl PartialEq for HashKeyValue { - fn eq(&self, other: &Self) -> bool { - self.0.key == other.0.key - && match (&self.0.value, &other.0.value) { - (Value::F64(f), Value::F64(of)) => OrderedFloat(*f).eq(&OrderedFloat(*of)), - (Value::Array(Array::F64(f)), Value::Array(Array::F64(of))) => { - f.len() == of.len() - && f.iter() - .zip(of.iter()) - .all(|(f, of)| OrderedFloat(*f).eq(&OrderedFloat(*of))) - } - (non_float, other_non_float) => non_float.eq(other_non_float), - } - } -} - -impl Eq for HashKeyValue {} +use opentelemetry::{Key, KeyValue, Value}; /// A unique set of attributes that can be used as instrument identifiers. /// /// This must implement [Hash], [PartialEq], and [Eq] so it may be used as /// HashMap keys and other de-duplication methods. #[derive(Clone, Default, Debug, PartialEq, Eq)] -pub struct AttributeSet(Vec, u64); +pub struct AttributeSet(Vec, u64); impl From<&[KeyValue]> for AttributeSet { fn from(values: &[KeyValue]) -> Self { @@ -74,7 +19,7 @@ impl From<&[KeyValue]> for AttributeSet { .rev() .filter_map(|kv| { if seen_keys.insert(kv.key.clone()) { - Some(HashKeyValue(kv.clone())) + Some(kv.clone()) } else { None } @@ -85,7 +30,7 @@ impl From<&[KeyValue]> for AttributeSet { } } -fn calculate_hash(values: &[HashKeyValue]) -> u64 { +fn calculate_hash(values: &[KeyValue]) -> u64 { let mut hasher = DefaultHasher::new(); values.iter().fold(&mut hasher, |mut hasher, item| { item.hash(&mut hasher); @@ -95,7 +40,7 @@ fn calculate_hash(values: &[HashKeyValue]) -> u64 { } impl AttributeSet { - fn new(mut values: Vec) -> Self { + fn new(mut values: Vec) -> Self { values.sort_unstable(); let hash = calculate_hash(&values); AttributeSet(values, hash) @@ -116,7 +61,7 @@ impl AttributeSet { where F: Fn(&KeyValue) -> bool, { - self.0.retain(|kv| f(&kv.0)); + self.0.retain(|kv| f(kv)); // Recalculate the hash as elements are changed. self.1 = calculate_hash(&self.0); @@ -124,7 +69,7 @@ impl AttributeSet { /// Iterate over key value pairs in the set pub fn iter(&self) -> impl Iterator { - self.0.iter().map(|kv| (&kv.0.key, &kv.0.value)) + self.0.iter().map(|kv| (&kv.key, &kv.value)) } } @@ -133,52 +78,3 @@ impl Hash for AttributeSet { state.write_u64(self.1) } } - -#[cfg(test)] -mod tests { - use std::hash::DefaultHasher; - use std::hash::{Hash, Hasher}; - - use crate::attributes::set::HashKeyValue; - use opentelemetry::KeyValue; - - #[test] - fn equality_kv_float() { - let kv1 = HashKeyValue(KeyValue::new("key", 1.0)); - let kv2 = HashKeyValue(KeyValue::new("key", 1.0)); - assert_eq!(kv1, kv2); - - let kv1 = HashKeyValue(KeyValue::new("key", 1.0)); - let kv2 = HashKeyValue(KeyValue::new("key", 1.01)); - assert_ne!(kv1, kv2); - - let kv1 = HashKeyValue(KeyValue::new("key", std::f64::NAN)); - let kv2 = HashKeyValue(KeyValue::new("key", std::f64::NAN)); - assert_eq!(kv1, kv2); - - let kv1 = HashKeyValue(KeyValue::new("key", std::f64::INFINITY)); - let kv2 = HashKeyValue(KeyValue::new("key", std::f64::INFINITY)); - assert_eq!(kv1, kv2); - } - - #[test] - fn hash_kv_float() { - let kv1 = HashKeyValue(KeyValue::new("key", 1.0)); - let kv2 = HashKeyValue(KeyValue::new("key", 1.0)); - assert_eq!(hash_helper(&kv1), hash_helper(&kv2)); - - let kv1 = HashKeyValue(KeyValue::new("key", std::f64::NAN)); - let kv2 = HashKeyValue(KeyValue::new("key", std::f64::NAN)); - assert_eq!(hash_helper(&kv1), hash_helper(&kv2)); - - let kv1 = HashKeyValue(KeyValue::new("key", std::f64::INFINITY)); - let kv2 = HashKeyValue(KeyValue::new("key", std::f64::INFINITY)); - assert_eq!(hash_helper(&kv1), hash_helper(&kv2)); - } - - fn hash_helper(item: &T) -> u64 { - let mut hasher = DefaultHasher::new(); - item.hash(&mut hasher); - hasher.finish() - } -} diff --git a/opentelemetry/Cargo.toml b/opentelemetry/Cargo.toml index 24ff2fe215..f9e7d9c42e 100644 --- a/opentelemetry/Cargo.toml +++ b/opentelemetry/Cargo.toml @@ -42,6 +42,7 @@ otel_unstable = [] [dev-dependencies] opentelemetry_sdk = { path = "../opentelemetry-sdk", features = ["logs_level_enabled"]} # for documentation tests criterion = { version = "0.3" } +rand = { workspace = true } [[bench]] name = "metrics" diff --git a/opentelemetry/src/common.rs b/opentelemetry/src/common.rs index 511ca6e257..ce911552a9 100644 --- a/opentelemetry/src/common.rs +++ b/opentelemetry/src/common.rs @@ -1,4 +1,5 @@ use std::borrow::Cow; +use std::hash::Hash; use std::sync::Arc; use std::{fmt, hash}; diff --git a/opentelemetry/src/metrics/mod.rs b/opentelemetry/src/metrics/mod.rs index 34ed16fa39..51f090608b 100644 --- a/opentelemetry/src/metrics/mod.rs +++ b/opentelemetry/src/metrics/mod.rs @@ -1,6 +1,8 @@ //! # OpenTelemetry Metrics API use std::any::Any; +use std::cmp::Ordering; +use std::hash::{Hash, Hasher}; use std::result; use std::sync::PoisonError; use std::{borrow::Cow, sync::Arc}; @@ -10,7 +12,7 @@ mod instruments; mod meter; pub mod noop; -use crate::ExportError; +use crate::{Array, ExportError, KeyValue, Value}; pub use instruments::{ counter::{Counter, ObservableCounter, SyncCounter}, gauge::{Gauge, ObservableGauge, SyncGauge}, @@ -81,6 +83,55 @@ impl AsRef for Unit { } } +#[derive(Debug, Clone, Copy)] +struct F64Hashable(f64); + +impl PartialEq for F64Hashable { + fn eq(&self, other: &Self) -> bool { + self.0.to_bits() == other.0.to_bits() + } +} + +impl Eq for F64Hashable {} + +impl Hash for F64Hashable { + fn hash(&self, state: &mut H) { + self.0.to_bits().hash(state); + } +} + +impl Hash for KeyValue { + fn hash(&self, state: &mut H) { + self.key.hash(state); + match &self.value { + Value::F64(f) => F64Hashable(*f).hash(state), + Value::Array(a) => match a { + Array::Bool(b) => b.hash(state), + Array::I64(i) => i.hash(state), + Array::F64(f) => f.iter().for_each(|f| F64Hashable(*f).hash(state)), + Array::String(s) => s.hash(state), + }, + Value::Bool(b) => b.hash(state), + Value::I64(i) => i.hash(state), + Value::String(s) => s.hash(state), + }; + } +} + +impl PartialOrd for KeyValue { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for KeyValue { + fn cmp(&self, other: &Self) -> Ordering { + self.key.cmp(&other.key) + } +} + +impl Eq for KeyValue {} + /// SDK implemented trait for creating instruments pub trait InstrumentProvider { /// creates an instrument for recording increasing values. @@ -279,3 +330,102 @@ pub trait InstrumentProvider { } type MultiInstrumentCallback = dyn Fn(&dyn Observer) + Send + Sync; + +#[cfg(test)] +mod tests { + use rand::Rng; + + use crate::KeyValue; + use std::hash::DefaultHasher; + use std::hash::{Hash, Hasher}; + + #[test] + fn equality_kv_float() { + let kv1 = KeyValue::new("key", 1.0); + let kv2 = KeyValue::new("key", 1.0); + assert_eq!(kv1, kv2); + + let kv1 = KeyValue::new("key", 1.0); + let kv2 = KeyValue::new("key", 1.01); + assert_ne!(kv1, kv2); + + let kv1 = KeyValue::new("key", std::f64::NAN); + let kv2 = KeyValue::new("key", std::f64::NAN); + assert_ne!(kv1, kv2, "NAN is not equal to itself"); + + let kv1 = KeyValue::new("key", std::f64::INFINITY); + let kv2 = KeyValue::new("key", std::f64::INFINITY); + assert_eq!(kv1, kv2); + + let kv1 = KeyValue::new("key", std::f64::NEG_INFINITY); + let kv2 = KeyValue::new("key", std::f64::NEG_INFINITY); + assert_eq!(kv1, kv2); + + let mut rng = rand::thread_rng(); + + for _ in 0..100 { + let random_value = rng.gen::(); + let kv1 = KeyValue::new("key", random_value); + let kv2 = KeyValue::new("key", random_value); + assert_eq!(kv1, kv2); + } + } + + #[test] + fn hash_kv_float() { + let kv1 = KeyValue::new("key", 1.0); + let kv2 = KeyValue::new("key", 1.0); + assert_eq!(hash_helper(&kv1), hash_helper(&kv2)); + + let kv1 = KeyValue::new("key", 1.001); + let kv2 = KeyValue::new("key", 1.001); + assert_eq!(hash_helper(&kv1), hash_helper(&kv2)); + + let kv1 = KeyValue::new("key", 1.001); + let kv2 = KeyValue::new("key", 1.002); + assert_ne!(hash_helper(&kv1), hash_helper(&kv2)); + + let kv1 = KeyValue::new("key", std::f64::NAN); + let kv2 = KeyValue::new("key", std::f64::NAN); + assert_eq!(hash_helper(&kv1), hash_helper(&kv2)); + + let kv1 = KeyValue::new("key", std::f64::INFINITY); + let kv2 = KeyValue::new("key", std::f64::INFINITY); + assert_eq!(hash_helper(&kv1), hash_helper(&kv2)); + + let mut rng = rand::thread_rng(); + + for _ in 0..100 { + let random_value = rng.gen::(); + let kv1 = KeyValue::new("key", random_value); + let kv2 = KeyValue::new("key", random_value); + assert_eq!(hash_helper(&kv1), hash_helper(&kv2)); + } + } + + #[test] + fn hash_kv_order() { + let float_vals = [ + 0.0, + 1.0, + -1.0, + std::f64::INFINITY, + std::f64::NEG_INFINITY, + std::f64::NAN, + std::f64::MIN, + std::f64::MAX, + ]; + + for v in float_vals { + let kv1 = KeyValue::new("a", v); + let kv2 = KeyValue::new("b", v); + assert!(kv1 < kv2, "Order is solely based on key!"); + } + } + + fn hash_helper(item: &T) -> u64 { + let mut hasher = DefaultHasher::new(); + item.hash(&mut hasher); + hasher.finish() + } +}