From d046310cc96d2f58e7ef8c0ecdbb400875a62cdc Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 1 Feb 2024 16:07:05 +0100 Subject: [PATCH] fix(sdk-metrics): ignore in accumulation instead --- packages/sdk-metrics/src/Instruments.ts | 6 ------ packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts | 6 ++++++ packages/sdk-metrics/src/aggregator/Histogram.ts | 6 ++++++ packages/sdk-metrics/test/Instruments.test.ts | 2 -- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/sdk-metrics/src/Instruments.ts b/packages/sdk-metrics/src/Instruments.ts index 78ccd75171f..f665952f059 100644 --- a/packages/sdk-metrics/src/Instruments.ts +++ b/packages/sdk-metrics/src/Instruments.ts @@ -54,12 +54,6 @@ export class SyncInstrument { ); return; } - if (Number.isNaN(value)) { - diag.warn( - `NaN provided to metric ${this._descriptor.name}, ignoring to preserve the integrity of the metrics stream` - ); - return; - } if ( this._descriptor.valueType === ValueType.INT && !Number.isInteger(value) diff --git a/packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts b/packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts index 93565802059..4f71549535d 100644 --- a/packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts +++ b/packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts @@ -193,6 +193,12 @@ export class ExponentialHistogramAccumulation implements Accumulation { * @param increment */ updateByIncrement(value: number, increment: number) { + // NaN does not fall into any bucket, is not zero and should not be counted, + // NaN is never greater than max nor less than min, therefore return as there's nothing for us to do. + if (Number.isNaN(value)) { + return; + } + if (value > this._max) { this._max = value; } diff --git a/packages/sdk-metrics/src/aggregator/Histogram.ts b/packages/sdk-metrics/src/aggregator/Histogram.ts index 60e5e8df05d..94cf9060393 100644 --- a/packages/sdk-metrics/src/aggregator/Histogram.ts +++ b/packages/sdk-metrics/src/aggregator/Histogram.ts @@ -72,6 +72,12 @@ export class HistogramAccumulation implements Accumulation { ) {} record(value: number): void { + // NaN does not fall into any bucket, is not zero and should not be counted, + // NaN is never greater than max nor less than min, therefore return as there's nothing for us to do. + if (Number.isNaN(value)) { + return; + } + this._current.count += 1; this._current.sum += value; diff --git a/packages/sdk-metrics/test/Instruments.test.ts b/packages/sdk-metrics/test/Instruments.test.ts index 16b4fe2ad9f..4b354518310 100644 --- a/packages/sdk-metrics/test/Instruments.test.ts +++ b/packages/sdk-metrics/test/Instruments.test.ts @@ -135,7 +135,6 @@ describe('Instruments', () => { counter.add(1.2, { foo: 'bar' }); // non-number values should be ignored. counter.add('1' as any); - counter.add(NaN); await validateExport(cumulativeReader, { dataPointType: DataPointType.SUM, @@ -248,7 +247,6 @@ describe('Instruments', () => { upDownCounter.add(1.1, { foo: 'bar' }); // non-number values should be ignored. upDownCounter.add('1' as any); - upDownCounter.add(NaN); await validateExport(deltaReader, { dataPointType: DataPointType.SUM,