fix(histogram): fix maximum when only values < -1 are provided (#3086)

open-telemetry · Jul 15, 2022 · db32860 · db32860
1 parent 747c404
commit db32860
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 4 deletions.
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -10,6 +10,8 @@ All notable changes to experimental packages in this project will be documented
 
 ### :bug: (Bug Fix)
 
+* fix(histogram): fix maximum when only values < -1 are provided [#3086](https://github.com/open-telemetry/opentelemetry-js/pull/3086) @pichlermarc
+
 ### :books: (Refine Doc)
 
 ### :house: (Internal)

diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Histogram.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Histogram.ts
@@ -39,7 +39,7 @@ function createNewEmptyCheckpoint(boundaries: number[]): Histogram {
     count: 0,
     hasMinMax: false,
     min: Infinity,
-    max: -1
+    max: -Infinity
   };
 }
 
@@ -115,7 +115,7 @@ export class HistogramAggregator implements Aggregator<HistogramAccumulation> {
     }
 
     let min = Infinity;
-    let max = -1;
+    let max = -Infinity;
 
     if (this._recordMinMax) {
       if (previousValue.hasMinMax && deltaValue.hasMinMax) {
@@ -167,7 +167,7 @@ export class HistogramAggregator implements Aggregator<HistogramAccumulation> {
       sum: currentValue.sum - previousValue.sum,
       hasMinMax: false,
       min: Infinity,
-      max: -1
+      max: -Infinity
     });
   }
 

diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Histogram.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/aggregator/Histogram.test.ts
@@ -51,6 +51,29 @@ describe('HistogramAggregator', () => {
 
       assert.deepStrictEqual(aggregator.merge(prev, delta), expected);
     });
+
+    it('with only negatives', () => {
+      const aggregator = new HistogramAggregator([1, 10, 100], true);
+      const prev = aggregator.createAccumulation([0, 0]);
+      prev.record(-10);
+      prev.record(-20);
+
+      const delta = aggregator.createAccumulation([1, 1]);
+      delta.record(-5);
+      delta.record(-30);
+
+      assert.deepStrictEqual(aggregator.merge(prev, delta).toPointValue(), {
+        buckets: {
+          boundaries: [1, 10, 100],
+          counts: [4, 0, 0, 0]
+        },
+        count: 4,
+        hasMinMax: true,
+        max: -5,
+        min: -30,
+        sum: -65
+      });
+    });
   });
 
   describe('diff', () => {
@@ -77,7 +100,7 @@ describe('HistogramAggregator', () => {
         sum: 13,
         hasMinMax: false,
         min: Infinity,
-        max: -1
+        max: -Infinity
       });
 
       assert.deepStrictEqual(aggregator.diff(prev, curr), expected);