diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 79efb77f3a..3e7a1bc319 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -12,6 +12,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 index 60a1d484d4..8566f4aa7f 100644 --- 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 { } 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 { 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 index b1f6e05c0a..3d0b3a1264 100644 --- 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);