From f9d8f6b973f91e35b5debf4ce44773dc89ed4e94 Mon Sep 17 00:00:00 2001 From: andrewgrachov Date: Fri, 28 Aug 2020 13:51:50 +0300 Subject: [PATCH 1/2] fix: proper histogram boundaries sort --- .../opentelemetry-metrics/src/export/aggregators/Histogram.ts | 2 +- .../test/export/aggregators/Histogram.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-metrics/src/export/aggregators/Histogram.ts b/packages/opentelemetry-metrics/src/export/aggregators/Histogram.ts index 1770952177..a44ea9836b 100644 --- a/packages/opentelemetry-metrics/src/export/aggregators/Histogram.ts +++ b/packages/opentelemetry-metrics/src/export/aggregators/Histogram.ts @@ -39,7 +39,7 @@ export class HistogramAggregator implements HistogramAggregatorType { } // we need to an ordered set to be able to correctly compute count for each // boundary since we'll iterate on each in order. - this._boundaries = boundaries.sort(); + this._boundaries = boundaries.sort((a, b) => a - b); this._current = this._newEmptyCheckpoint(); this._lastUpdateTime = hrTime(); } diff --git a/packages/opentelemetry-metrics/test/export/aggregators/Histogram.test.ts b/packages/opentelemetry-metrics/test/export/aggregators/Histogram.test.ts index c2397ae0b8..204b795234 100644 --- a/packages/opentelemetry-metrics/test/export/aggregators/Histogram.test.ts +++ b/packages/opentelemetry-metrics/test/export/aggregators/Histogram.test.ts @@ -27,9 +27,9 @@ describe('HistogramAggregator', () => { }); it('should sort boundaries', () => { - const aggregator = new HistogramAggregator([500, 300, 700]); + const aggregator = new HistogramAggregator([200, 500, 300, 700, 1000, 1500]); const point = aggregator.toPoint().value as Histogram; - assert.deepEqual(point.buckets.boundaries, [300, 500, 700]); + assert.deepEqual(point.buckets.boundaries, [200, 300, 500, 700, 1000, 1500]); }); it('should throw if no boundaries are defined', () => { From d809dd158651a960bc3caebdb06b7d4c1ffae560 Mon Sep 17 00:00:00 2001 From: andrewgrachov Date: Fri, 28 Aug 2020 14:38:33 +0300 Subject: [PATCH 2/2] fix: lint hotfix --- .../test/export/aggregators/Histogram.test.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-metrics/test/export/aggregators/Histogram.test.ts b/packages/opentelemetry-metrics/test/export/aggregators/Histogram.test.ts index 204b795234..f2882858c2 100644 --- a/packages/opentelemetry-metrics/test/export/aggregators/Histogram.test.ts +++ b/packages/opentelemetry-metrics/test/export/aggregators/Histogram.test.ts @@ -27,9 +27,23 @@ describe('HistogramAggregator', () => { }); it('should sort boundaries', () => { - const aggregator = new HistogramAggregator([200, 500, 300, 700, 1000, 1500]); + const aggregator = new HistogramAggregator([ + 200, + 500, + 300, + 700, + 1000, + 1500, + ]); const point = aggregator.toPoint().value as Histogram; - assert.deepEqual(point.buckets.boundaries, [200, 300, 500, 700, 1000, 1500]); + assert.deepEqual(point.buckets.boundaries, [ + 200, + 300, + 500, + 700, + 1000, + 1500, + ]); }); it('should throw if no boundaries are defined', () => {