@@ -7,23 +7,22 @@ use crate::metrics::data::HistogramDataPoint;
7
7
use crate :: metrics:: data:: { self , Aggregation , Temporality } ;
8
8
use opentelemetry:: KeyValue ;
9
9
10
- use super :: Number ;
11
- use super :: { AtomicTracker , AtomicallyUpdate , Operation , ValueMap } ;
12
-
13
- struct HistogramUpdate ;
14
-
15
- impl Operation for HistogramUpdate {
16
- fn update_tracker < T : Default , AT : AtomicTracker < T > > ( tracker : & AT , value : T , index : usize ) {
17
- tracker. update_histogram ( index, value) ;
18
- }
19
- }
10
+ use super :: ValueMap ;
11
+ use super :: { Aggregator , Number } ;
20
12
21
13
struct HistogramTracker < T > {
22
14
buckets : Mutex < Buckets < T > > ,
23
15
}
24
16
25
- impl < T : Number > AtomicTracker < T > for HistogramTracker < T > {
26
- fn update_histogram ( & self , index : usize , value : T ) {
17
+ impl < T > Aggregator < T > for HistogramTracker < T >
18
+ where
19
+ T : Number ,
20
+ {
21
+ type InitConfig = usize ;
22
+ /// Value and bucket index
23
+ type PreComputedValue = ( T , usize ) ;
24
+
25
+ fn update ( & self , ( value, index) : ( T , usize ) ) {
27
26
let mut buckets = match self . buckets . lock ( ) {
28
27
Ok ( guard) => guard,
29
28
Err ( _) => return ,
@@ -32,15 +31,10 @@ impl<T: Number> AtomicTracker<T> for HistogramTracker<T> {
32
31
buckets. bin ( index, value) ;
33
32
buckets. sum ( value) ;
34
33
}
35
- }
36
34
37
- impl < T : Number > AtomicallyUpdate < T > for HistogramTracker < T > {
38
- type AtomicTracker = HistogramTracker < T > ;
39
-
40
- fn new_atomic_tracker ( buckets_count : Option < usize > ) -> Self :: AtomicTracker {
41
- let count = buckets_count. unwrap ( ) ;
35
+ fn create ( count : & usize ) -> Self {
42
36
HistogramTracker {
43
- buckets : Mutex :: new ( Buckets :: < T > :: new ( count) ) ,
37
+ buckets : Mutex :: new ( Buckets :: < T > :: new ( * count) ) ,
44
38
}
45
39
}
46
40
}
@@ -94,7 +88,7 @@ impl<T: Number> Buckets<T> {
94
88
/// Summarizes a set of measurements as a histogram with explicitly defined
95
89
/// buckets.
96
90
pub ( crate ) struct Histogram < T : Number > {
97
- value_map : ValueMap < HistogramTracker < T > , T , HistogramUpdate > ,
91
+ value_map : ValueMap < T , HistogramTracker < T > > ,
98
92
bounds : Vec < f64 > ,
99
93
record_min_max : bool ,
100
94
record_sum : bool ,
@@ -103,9 +97,11 @@ pub(crate) struct Histogram<T: Number> {
103
97
104
98
impl < T : Number > Histogram < T > {
105
99
pub ( crate ) fn new ( boundaries : Vec < f64 > , record_min_max : bool , record_sum : bool ) -> Self {
100
+ // TODO fix the bug, by first removing NaN and only then getting buckets_count
101
+ // once we know the reason for performance degradation
106
102
let buckets_count = boundaries. len ( ) + 1 ;
107
103
let mut histogram = Histogram {
108
- value_map : ValueMap :: new_with_buckets_count ( buckets_count) ,
104
+ value_map : ValueMap :: new ( buckets_count) ,
109
105
bounds : boundaries,
110
106
record_min_max,
111
107
record_sum,
@@ -122,14 +118,20 @@ impl<T: Number> Histogram<T> {
122
118
123
119
pub ( crate ) fn measure ( & self , measurement : T , attrs : & [ KeyValue ] ) {
124
120
let f = measurement. into_float ( ) ;
125
-
121
+ // Ignore NaN and infinity.
122
+ // Only makes sense if T is f64, maybe this could be no-op for other cases?
123
+ // TODO: uncomment once we know the reason for performance degradation
124
+ // if f.is_infinite() || f.is_nan() {
125
+ // return;
126
+ // }
126
127
// This search will return an index in the range `[0, bounds.len()]`, where
127
128
// it will return `bounds.len()` if value is greater than the last element
128
129
// of `bounds`. This aligns with the buckets in that the length of buckets
129
130
// is `bounds.len()+1`, with the last bucket representing:
130
131
// `(bounds[bounds.len()-1], +∞)`.
131
132
let index = self . bounds . partition_point ( |& x| x < f) ;
132
- self . value_map . measure ( measurement, attrs, index) ;
133
+
134
+ self . value_map . measure ( ( measurement, index) , attrs) ;
133
135
}
134
136
135
137
pub ( crate ) fn delta (
@@ -350,3 +352,68 @@ impl<T: Number> Histogram<T> {
350
352
( h. data_points . len ( ) , new_agg. map ( |a| Box :: new ( a) as Box < _ > ) )
351
353
}
352
354
}
355
+
356
+ #[ cfg( test) ]
357
+ mod tests {
358
+
359
+ use super :: * ;
360
+
361
+ #[ test]
362
+ fn when_f64_is_nan_or_infinity_then_ignore ( ) {
363
+ struct Expected {
364
+ min : f64 ,
365
+ max : f64 ,
366
+ sum : f64 ,
367
+ count : u64 ,
368
+ }
369
+ impl Expected {
370
+ fn new ( min : f64 , max : f64 , sum : f64 , count : u64 ) -> Self {
371
+ Expected {
372
+ min,
373
+ max,
374
+ sum,
375
+ count,
376
+ }
377
+ }
378
+ }
379
+ struct TestCase {
380
+ values : Vec < f64 > ,
381
+ expected : Expected ,
382
+ }
383
+
384
+ let test_cases = vec ! [
385
+ TestCase {
386
+ values: vec![ 2.0 , 4.0 , 1.0 ] ,
387
+ expected: Expected :: new( 1.0 , 4.0 , 7.0 , 3 ) ,
388
+ } ,
389
+ TestCase {
390
+ values: vec![ 2.0 , 4.0 , 1.0 , f64 :: INFINITY ] ,
391
+ expected: Expected :: new( 1.0 , 4.0 , 7.0 , 3 ) ,
392
+ } ,
393
+ TestCase {
394
+ values: vec![ 2.0 , 4.0 , 1.0 , -f64 :: INFINITY ] ,
395
+ expected: Expected :: new( 1.0 , 4.0 , 7.0 , 3 ) ,
396
+ } ,
397
+ TestCase {
398
+ values: vec![ 2.0 , f64 :: NAN , 4.0 , 1.0 ] ,
399
+ expected: Expected :: new( 1.0 , 4.0 , 7.0 , 3 ) ,
400
+ } ,
401
+ TestCase {
402
+ values: vec![ 4.0 , 4.0 , 4.0 , 2.0 , 16.0 , 1.0 ] ,
403
+ expected: Expected :: new( 1.0 , 16.0 , 31.0 , 6 ) ,
404
+ } ,
405
+ ] ;
406
+
407
+ for test in test_cases {
408
+ let h = Histogram :: new ( vec ! [ ] , true , true ) ;
409
+ for v in test. values {
410
+ h. measure ( v, & [ ] ) ;
411
+ }
412
+ let res = h. value_map . no_attribute_tracker . buckets . lock ( ) . unwrap ( ) ;
413
+ assert_eq ! ( test. expected. max, res. max) ;
414
+ assert_eq ! ( test. expected. min, res. min) ;
415
+ assert_eq ! ( test. expected. sum, res. total) ;
416
+ assert_eq ! ( test. expected. count, res. count) ;
417
+ }
418
+ }
419
+ }
0 commit comments