diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index f24fbe0ea0..20581c1183 100644 --- 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(sdk-metrics-base): only record non-negative histogram values #3002 @pichlermarc + ### :books: (Refine Doc) ### :house: (Internal) diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts index 44467cc7d8..3cd1f0f15b 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts @@ -72,6 +72,10 @@ export class HistogramInstrument extends SyncInstrument implements metrics.Histo * Records a measurement. Value of the measurement must not be negative. */ record(value: number, attributes?: metrics.MetricAttributes, ctx?: api.Context): void { + if (value < 0) { + api.diag.warn(`negative value provided to histogram ${this._descriptor.name}: ${value}`); + return; + } this._record(value, attributes, ctx); } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts index c0d7d77dfb..b6301f5c64 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts @@ -18,9 +18,24 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { InstrumentationScope } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; -import { AggregationTemporality, InstrumentDescriptor, InstrumentType, MeterProvider, MetricReader, DataPoint, DataPointType } from '../src'; +import { + AggregationTemporality, + InstrumentDescriptor, + InstrumentType, + MeterProvider, + MetricReader, + DataPoint, + DataPointType +} from '../src'; import { TestMetricReader } from './export/TestMetricReader'; -import { assertMetricData, assertDataPoint, commonValues, commonAttributes, defaultResource, defaultInstrumentationScope } from './util'; +import { + assertMetricData, + assertDataPoint, + commonValues, + commonAttributes, + defaultResource, + defaultInstrumentationScope +} from './util'; import { Histogram } from '../src/aggregator/types'; import { ObservableResult, ValueType } from '@opentelemetry/api-metrics'; @@ -257,10 +272,10 @@ describe('Instruments', () => { }); histogram.record(10); - // -0.1 should be trunc-ed to -0 - histogram.record(-0.1); + // 0.1 should be trunc-ed to 0 + histogram.record(0.1); histogram.record(100, { foo: 'bar' }); - histogram.record(-0.1, { foo: 'bar' }); + histogram.record(0.1, { foo: 'bar' }); await validateExport(deltaReader, { descriptor: { name: 'test', @@ -297,6 +312,18 @@ describe('Instruments', () => { }); }); + it('should not record negative INT values', async () => { + const { meter, deltaReader } = setup(); + const histogram = meter.createHistogram('test', { + valueType: ValueType.DOUBLE, + }); + + histogram.record(-1, { foo: 'bar' }); + await validateExport(deltaReader, { + dataPointType: DataPointType.HISTOGRAM, + dataPoints: [], + }); + }); it('should record DOUBLE values', async () => { const { meter, deltaReader } = setup(); @@ -305,9 +332,9 @@ describe('Instruments', () => { }); histogram.record(10); - histogram.record(-0.1); + histogram.record(0.1); histogram.record(100, { foo: 'bar' }); - histogram.record(-0.1, { foo: 'bar' }); + histogram.record(0.1, { foo: 'bar' }); await validateExport(deltaReader, { dataPointType: DataPointType.HISTOGRAM, dataPoints: [ @@ -316,10 +343,10 @@ describe('Instruments', () => { value: { buckets: { boundaries: [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000], - counts: [1, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0], + counts: [0, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0], }, count: 2, - sum: 9.9, + sum: 10.1, }, }, { @@ -327,15 +354,28 @@ describe('Instruments', () => { value: { buckets: { boundaries: [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000], - counts: [1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0], + counts: [0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0], }, count: 2, - sum: 99.9, + sum: 100.1, }, }, ], }); }); + + it('should not record negative DOUBLE values', async () => { + const { meter, deltaReader } = setup(); + const histogram = meter.createHistogram('test', { + valueType: ValueType.DOUBLE, + }); + + histogram.record(-0.5, { foo: 'bar' }); + await validateExport(deltaReader, { + dataPointType: DataPointType.HISTOGRAM, + dataPoints: [], + }); + }); }); describe('ObservableCounter', () => {