Skip to content

Commit 41a8857

Browse files
cijothomascijothomas
committed
Remove orderedfloat dependeny and absorb functionality to opentelemetry
1 parent d3b3f70 commit 41a8857

File tree

6 files changed

+167
-108
lines changed

6 files changed

+167
-108
lines changed

‎opentelemetry-sdk/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
- Add "metrics", "logs" to default features. With this, default feature list is
66
"trace", "metrics" and "logs".
7+
- Removed dependency on `ordered-float`.
78

89
## v0.23.0
910

‎opentelemetry-sdk/Cargo.toml

-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ futures-channel = "0.3"
1818
futures-executor = { workspace = true }
1919
futures-util = { workspace = true, features = ["std", "sink", "async-await-macro"] }
2020
once_cell = { workspace = true }
21-
ordered-float = { workspace = true }
2221
percent-encoding = { version = "2.0", optional = true }
2322
rand = { workspace = true, features = ["std", "std_rng","small_rng"], optional = true }
2423
glob = { version = "0.3.1", optional =true}

‎opentelemetry-sdk/src/metrics/mod.rs

+8-106
Original file line numberDiff line numberDiff line change
@@ -66,71 +66,16 @@ pub use view::*;
6666

6767
use std::collections::hash_map::DefaultHasher;
6868
use std::collections::HashSet;
69-
use std::{
70-
cmp::Ordering,
71-
hash::{Hash, Hasher},
72-
};
69+
use std::hash::{Hash, Hasher};
7370

74-
use opentelemetry::{Array, Key, KeyValue, Value};
75-
use ordered_float::OrderedFloat;
76-
77-
#[derive(Clone, Debug)]
78-
struct HashKeyValue(KeyValue);
79-
80-
impl Hash for HashKeyValue {
81-
fn hash<H: Hasher>(&self, state: &mut H) {
82-
self.0.key.hash(state);
83-
match &self.0.value {
84-
Value::F64(f) => OrderedFloat(*f).hash(state),
85-
Value::Array(a) => match a {
86-
Array::Bool(b) => b.hash(state),
87-
Array::I64(i) => i.hash(state),
88-
Array::F64(f) => f.iter().for_each(|f| OrderedFloat(*f).hash(state)),
89-
Array::String(s) => s.hash(state),
90-
},
91-
Value::Bool(b) => b.hash(state),
92-
Value::I64(i) => i.hash(state),
93-
Value::String(s) => s.hash(state),
94-
};
95-
}
96-
}
97-
98-
impl PartialOrd for HashKeyValue {
99-
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
100-
Some(self.cmp(other))
101-
}
102-
}
103-
104-
impl Ord for HashKeyValue {
105-
fn cmp(&self, other: &Self) -> Ordering {
106-
self.0.key.cmp(&other.0.key)
107-
}
108-
}
109-
110-
impl PartialEq for HashKeyValue {
111-
fn eq(&self, other: &Self) -> bool {
112-
self.0.key == other.0.key
113-
&& match (&self.0.value, &other.0.value) {
114-
(Value::F64(f), Value::F64(of)) => OrderedFloat(*f).eq(&OrderedFloat(*of)),
115-
(Value::Array(Array::F64(f)), Value::Array(Array::F64(of))) => {
116-
f.len() == of.len()
117-
&& f.iter()
118-
.zip(of.iter())
119-
.all(|(f, of)| OrderedFloat(*f).eq(&OrderedFloat(*of)))
120-
}
121-
(non_float, other_non_float) => non_float.eq(other_non_float),
122-
}
123-
}
124-
}
125-
126-
impl Eq for HashKeyValue {}
71+
use opentelemetry::{Key, KeyValue, Value};
12772

12873
/// A unique set of attributes that can be used as instrument identifiers.
12974
///
13075
/// This must implement [Hash], [PartialEq], and [Eq] so it may be used as
13176
/// HashMap keys and other de-duplication methods.
13277
#[derive(Clone, Default, Debug, PartialEq, Eq)]
133-
pub struct AttributeSet(Vec<HashKeyValue>, u64);
78+
pub struct AttributeSet(Vec<KeyValue>, u64);
13479

13580
impl From<&[KeyValue]> for AttributeSet {
13681
fn from(values: &[KeyValue]) -> Self {
@@ -140,7 +85,7 @@ impl From<&[KeyValue]> for AttributeSet {
14085
.rev()
14186
.filter_map(|kv| {
14287
if seen_keys.insert(kv.key.clone()) {
143-
Some(HashKeyValue(kv.clone()))
88+
Some(kv.clone())
14489
} else {
14590
None
14691
}
@@ -151,7 +96,7 @@ impl From<&[KeyValue]> for AttributeSet {
15196
}
15297
}
15398

154-
fn calculate_hash(values: &[HashKeyValue]) -> u64 {
99+
fn calculate_hash(values: &[KeyValue]) -> u64 {
155100
let mut hasher = DefaultHasher::new();
156101
values.iter().fold(&mut hasher, |mut hasher, item| {
157102
item.hash(&mut hasher);
@@ -161,7 +106,7 @@ fn calculate_hash(values: &[HashKeyValue]) -> u64 {
161106
}
162107

163108
impl AttributeSet {
164-
fn new(mut values: Vec<HashKeyValue>) -> Self {
109+
fn new(mut values: Vec<KeyValue>) -> Self {
165110
values.sort_unstable();
166111
let hash = calculate_hash(&values);
167112
AttributeSet(values, hash)
@@ -177,15 +122,15 @@ impl AttributeSet {
177122
where
178123
F: Fn(&KeyValue) -> bool,
179124
{
180-
self.0.retain(|kv| f(&kv.0));
125+
self.0.retain(|kv| f(kv));
181126

182127
// Recalculate the hash as elements are changed.
183128
self.1 = calculate_hash(&self.0);
184129
}
185130

186131
/// Iterate over key value pairs in the set
187132
pub fn iter(&self) -> impl Iterator<Item = (&Key, &Value)> {
188-
self.0.iter().map(|kv| (&kv.0.key, &kv.0.value))
133+
self.0.iter().map(|kv| (&kv.key, &kv.value))
189134
}
190135
}
191136

@@ -209,52 +154,9 @@ mod tests {
209154
KeyValue,
210155
};
211156
use std::borrow::Cow;
212-
use std::hash::DefaultHasher;
213-
use std::hash::{Hash, Hasher};
214157

215158
// Run all tests in this mod
216159
// cargo test metrics::tests --features=metrics,testing
217-
218-
#[test]
219-
fn equality_kv_float() {
220-
let kv1 = HashKeyValue(KeyValue::new("key", 1.0));
221-
let kv2 = HashKeyValue(KeyValue::new("key", 1.0));
222-
assert_eq!(kv1, kv2);
223-
224-
let kv1 = HashKeyValue(KeyValue::new("key", 1.0));
225-
let kv2 = HashKeyValue(KeyValue::new("key", 1.01));
226-
assert_ne!(kv1, kv2);
227-
228-
let kv1 = HashKeyValue(KeyValue::new("key", std::f64::NAN));
229-
let kv2 = HashKeyValue(KeyValue::new("key", std::f64::NAN));
230-
assert_eq!(kv1, kv2);
231-
232-
let kv1 = HashKeyValue(KeyValue::new("key", std::f64::INFINITY));
233-
let kv2 = HashKeyValue(KeyValue::new("key", std::f64::INFINITY));
234-
assert_eq!(kv1, kv2);
235-
}
236-
237-
#[test]
238-
fn hash_kv_float() {
239-
let kv1 = HashKeyValue(KeyValue::new("key", 1.0));
240-
let kv2 = HashKeyValue(KeyValue::new("key", 1.0));
241-
assert_eq!(hash_helper(&kv1), hash_helper(&kv2));
242-
243-
let kv1 = HashKeyValue(KeyValue::new("key", std::f64::NAN));
244-
let kv2 = HashKeyValue(KeyValue::new("key", std::f64::NAN));
245-
assert_eq!(hash_helper(&kv1), hash_helper(&kv2));
246-
247-
let kv1 = HashKeyValue(KeyValue::new("key", std::f64::INFINITY));
248-
let kv2 = HashKeyValue(KeyValue::new("key", std::f64::INFINITY));
249-
assert_eq!(hash_helper(&kv1), hash_helper(&kv2));
250-
}
251-
252-
fn hash_helper<T: Hash>(item: &T) -> u64 {
253-
let mut hasher = DefaultHasher::new();
254-
item.hash(&mut hasher);
255-
hasher.finish()
256-
}
257-
258160
// Note for all tests from this point onwards in this mod:
259161
// "multi_thread" tokio flavor must be used else flush won't
260162
// be able to make progress!

‎opentelemetry/CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
- Add "metrics", "logs" to default features. With this, default feature list is
66
"trace", "metrics" and "logs".
7+
- When "metrics" feature is enabled, `KeyValue` implements `PartialEq`, `Eq`,
8+
`PartialOrder`, `Order`, `Hash`. This is meant to be used for metrics
9+
aggregation purposes only.
710

811
## v0.23.0
912

‎opentelemetry/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ otel_unstable = []
4242
[dev-dependencies]
4343
opentelemetry_sdk = { path = "../opentelemetry-sdk", features = ["logs_level_enabled"]} # for documentation tests
4444
criterion = { version = "0.3" }
45+
rand = { workspace = true }
4546

4647
[[bench]]
4748
name = "metrics"

‎opentelemetry/src/metrics/mod.rs

+154-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
//! # OpenTelemetry Metrics API
22
33
use std::any::Any;
4+
use std::cmp::Ordering;
5+
use std::hash::{Hash, Hasher};
46
use std::result;
57
use std::sync::PoisonError;
68
use std::{borrow::Cow, sync::Arc};
@@ -10,7 +12,7 @@ mod instruments;
1012
mod meter;
1113
pub mod noop;
1214

13-
use crate::ExportError;
15+
use crate::{Array, ExportError, KeyValue, Value};
1416
pub use instruments::{
1517
counter::{Counter, ObservableCounter, SyncCounter},
1618
gauge::{Gauge, ObservableGauge, SyncGauge},
@@ -81,6 +83,55 @@ impl AsRef<str> for Unit {
8183
}
8284
}
8385

86+
struct F64Hashable(f64);
87+
88+
impl PartialEq for F64Hashable {
89+
fn eq(&self, other: &Self) -> bool {
90+
self.0.to_bits() == other.0.to_bits()
91+
}
92+
}
93+
94+
impl Eq for F64Hashable {}
95+
96+
impl Hash for F64Hashable {
97+
fn hash<H: Hasher>(&self, state: &mut H) {
98+
self.0.to_bits().hash(state);
99+
}
100+
}
101+
102+
impl Hash for KeyValue {
103+
fn hash<H: Hasher>(&self, state: &mut H) {
104+
self.key.hash(state);
105+
match &self.value {
106+
Value::F64(f) => F64Hashable(*f).hash(state),
107+
Value::Array(a) => match a {
108+
Array::Bool(b) => b.hash(state),
109+
Array::I64(i) => i.hash(state),
110+
Array::F64(f) => f.iter().for_each(|f| F64Hashable(*f).hash(state)),
111+
Array::String(s) => s.hash(state),
112+
},
113+
Value::Bool(b) => b.hash(state),
114+
Value::I64(i) => i.hash(state),
115+
Value::String(s) => s.hash(state),
116+
};
117+
}
118+
}
119+
120+
impl PartialOrd for KeyValue {
121+
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
122+
Some(self.cmp(other))
123+
}
124+
}
125+
126+
/// Ordering is based on the key only.
127+
impl Ord for KeyValue {
128+
fn cmp(&self, other: &Self) -> Ordering {
129+
self.key.cmp(&other.key)
130+
}
131+
}
132+
133+
impl Eq for KeyValue {}
134+
84135
/// SDK implemented trait for creating instruments
85136
pub trait InstrumentProvider {
86137
/// creates an instrument for recording increasing values.
@@ -279,3 +330,105 @@ pub trait InstrumentProvider {
279330
}
280331

281332
type MultiInstrumentCallback = dyn Fn(&dyn Observer) + Send + Sync;
333+
334+
#[cfg(test)]
335+
mod tests {
336+
use rand::Rng;
337+
338+
use crate::KeyValue;
339+
use std::f64;
340+
use std::hash::DefaultHasher;
341+
use std::hash::{Hash, Hasher};
342+
343+
#[test]
344+
fn kv_float_equality() {
345+
let kv1 = KeyValue::new("key", 1.0);
346+
let kv2 = KeyValue::new("key", 1.0);
347+
assert_eq!(kv1, kv2);
348+
349+
let kv1 = KeyValue::new("key", 1.0);
350+
let kv2 = KeyValue::new("key", 1.01);
351+
assert_ne!(kv1, kv2);
352+
353+
let kv1 = KeyValue::new("key", f64::NAN);
354+
let kv2 = KeyValue::new("key", f64::NAN);
355+
assert_ne!(kv1, kv2, "NAN is not equal to itself");
356+
357+
for float_val in [
358+
f64::INFINITY,
359+
f64::NEG_INFINITY,
360+
f64::MAX,
361+
f64::MIN,
362+
f64::MIN_POSITIVE,
363+
]
364+
.iter()
365+
{
366+
let kv1 = KeyValue::new("key", *float_val);
367+
let kv2 = KeyValue::new("key", *float_val);
368+
assert_eq!(kv1, kv2);
369+
}
370+
371+
let mut rng = rand::thread_rng();
372+
373+
for _ in 0..100 {
374+
let random_value = rng.gen::<f64>();
375+
let kv1 = KeyValue::new("key", random_value);
376+
let kv2 = KeyValue::new("key", random_value);
377+
assert_eq!(kv1, kv2);
378+
}
379+
}
380+
381+
#[test]
382+
fn kv_float_hash() {
383+
for float_val in [
384+
f64::NAN,
385+
f64::INFINITY,
386+
f64::NEG_INFINITY,
387+
f64::MAX,
388+
f64::MIN,
389+
f64::MIN_POSITIVE,
390+
]
391+
.iter()
392+
{
393+
let kv1 = KeyValue::new("key", *float_val);
394+
let kv2 = KeyValue::new("key", *float_val);
395+
assert_eq!(hash_helper(&kv1), hash_helper(&kv2));
396+
}
397+
398+
let mut rng = rand::thread_rng();
399+
400+
for _ in 0..100 {
401+
let random_value = rng.gen::<f64>();
402+
let kv1 = KeyValue::new("key", random_value);
403+
let kv2 = KeyValue::new("key", random_value);
404+
assert_eq!(hash_helper(&kv1), hash_helper(&kv2));
405+
}
406+
}
407+
408+
#[test]
409+
fn kv_float_order() {
410+
// TODO: Extend this test to all value types, not just F64
411+
let float_vals = [
412+
0.0,
413+
1.0,
414+
-1.0,
415+
f64::INFINITY,
416+
f64::NEG_INFINITY,
417+
f64::NAN,
418+
f64::MIN,
419+
f64::MAX,
420+
];
421+
422+
for v in float_vals {
423+
let kv1 = KeyValue::new("a", v);
424+
let kv2 = KeyValue::new("b", v);
425+
assert!(kv1 < kv2, "Order is solely based on key!");
426+
}
427+
}
428+
429+
fn hash_helper<T: Hash>(item: &T) -> u64 {
430+
let mut hasher = DefaultHasher::new();
431+
item.hash(&mut hasher);
432+
hasher.finish()
433+
}
434+
}

0 commit comments

Comments
 (0)