diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts index 60dbc9e364a..0373bc464b7 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts @@ -16,17 +16,40 @@ import * as api from '@opentelemetry/api'; import * as metrics from '@opentelemetry/api-metrics-wip'; +import { ValueType } from '@opentelemetry/api-metrics-wip'; import { InstrumentDescriptor } from './InstrumentDescriptor'; import { WritableMetricStorage } from './state/WritableMetricStorage'; -export class SyncInstrument { - constructor(private _writableMetricStorage: WritableMetricStorage, private _descriptor: InstrumentDescriptor) { } +class SyncIntInstrument { + private _valueType: ValueType; + + constructor(private _writableMetricStorage: WritableMetricStorage, private _descriptor: InstrumentDescriptor) { + this._valueType = _descriptor.valueType; + } + + getName(): string { + return this._descriptor.name; + } + + protected _record(value: number, attributes: metrics.Attributes = {}, context: api.Context = api.context.active()) { + if (this._valueType === ValueType.INT && !Number.isInteger(value)) { + api.diag.warn( + `INT value type cannot accept a floating-point value for ${this._descriptor.name}, ignoring the fractional digits.` + ); + value = Math.trunc(value); + } + this._writableMetricStorage.record(value, attributes, context); + } +} + +export class SyncDoubleInstrument { + constructor(private _writableMetricStorage: WritableMetricStorage, private _descriptor: InstrumentDescriptor) {} getName(): string { return this._descriptor.name; } - aggregate(value: number, attributes: metrics.Attributes = {}, context: api.Context = api.context.active()) { + protected _record(value: number, attributes: metrics.Attributes = {}, context: api.Context = api.context.active()) { this._writableMetricStorage.record(value, attributes, context); } } @@ -34,19 +57,60 @@ export class SyncInstrument { /** * The class implements {@link metrics.UpDownCounter} interface. */ -export class UpDownCounter extends SyncInstrument implements metrics.UpDownCounter { +export class DoubleUpDownCounter extends SyncDoubleInstrument implements metrics.UpDownCounter { + /** + * Increment value of counter by the input. Inputs may be negative. + */ + add(value: number, attributes?: metrics.Attributes, ctx?: api.Context): void { + this._record(value, attributes, ctx); + } +} + +/** + * The class implements {@link metrics.Counter} interface. + */ +export class DoubleCounter extends SyncDoubleInstrument implements metrics.Counter { + /** + * Increment value of counter by the input. Inputs may not be negative. + */ + add(value: number, attributes?: metrics.Attributes, ctx?: api.Context): void { + if (value < 0) { + api.diag.warn(`negative value provided to counter ${this.getName()}: ${value}`); + return; + } + + this._record(value, attributes, ctx); + } +} + +/** + * The class implements {@link metrics.Histogram} interface. + */ +export class DoubleHistogram extends SyncDoubleInstrument implements metrics.Histogram { + /** + * Records a measurement. Value of the measurement must not be negative. + */ + record(value: number, attributes?: metrics.Attributes, ctx?: api.Context): void { + this._record(value, attributes, ctx); + } +} + +/** + * The class implements {@link metrics.UpDownCounter} interface. + */ + export class IntUpDownCounter extends SyncIntInstrument implements metrics.UpDownCounter { /** * Increment value of counter by the input. Inputs may be negative. */ add(value: number, attributes?: metrics.Attributes, ctx?: api.Context): void { - this.aggregate(value, attributes, ctx); + this._record(value, attributes, ctx); } } /** * The class implements {@link metrics.Counter} interface. */ -export class Counter extends SyncInstrument implements metrics.Counter { +export class IntCounter extends SyncIntInstrument implements metrics.Counter { /** * Increment value of counter by the input. Inputs may not be negative. */ @@ -56,18 +120,18 @@ export class Counter extends SyncInstrument implements metrics.Counter { return; } - this.aggregate(value, attributes, ctx); + this._record(value, attributes, ctx); } } /** * The class implements {@link metrics.Histogram} interface. */ -export class Histogram extends SyncInstrument implements metrics.Histogram { +export class IntHistogram extends SyncIntInstrument implements metrics.Histogram { /** * Records a measurement. Value of the measurement must not be negative. */ record(value: number, attributes?: metrics.Attributes, ctx?: api.Context): void { - this.aggregate(value, attributes, ctx); + this._record(value, attributes, ctx); } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts index cc4c54545c3..8f4c56ad530 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts @@ -17,7 +17,14 @@ import * as metrics from '@opentelemetry/api-metrics-wip'; import { InstrumentationLibrary } from '@opentelemetry/core'; import { createInstrumentDescriptor, InstrumentDescriptor, InstrumentType } from './InstrumentDescriptor'; -import { Counter, Histogram, UpDownCounter } from './Instruments'; +import { + IntCounter, + DoubleCounter, + IntHistogram, + DoubleHistogram, + IntUpDownCounter, + DoubleUpDownCounter, +} from './Instruments'; import { MeterProviderSharedState } from './state/MeterProviderSharedState'; import { MultiMetricStorage } from './state/MultiWritableMetricStorage'; import { SyncMetricStorage } from './state/SyncMetricStorage'; @@ -27,6 +34,7 @@ import { isNotNullish } from './utils'; import { MetricCollectorHandle } from './state/MetricCollector'; import { HrTime } from '@opentelemetry/api'; import { AsyncMetricStorage } from './state/AsyncMetricStorage'; +import { ValueType } from '@opentelemetry/api-metrics-wip'; /** * This class implements the {@link metrics.Meter} interface. @@ -44,7 +52,11 @@ export class Meter implements metrics.Meter { createHistogram(name: string, options?: metrics.HistogramOptions): metrics.Histogram { const descriptor = createInstrumentDescriptor(name, InstrumentType.HISTOGRAM, options); const storage = this._registerMetricStorage(descriptor); - return new Histogram(storage, descriptor); + if (descriptor.valueType === ValueType.INT) { + return new IntHistogram(storage, descriptor); + } else { + return new DoubleHistogram(storage, descriptor); + } } /** @@ -53,7 +65,11 @@ export class Meter implements metrics.Meter { createCounter(name: string, options?: metrics.CounterOptions): metrics.Counter { const descriptor = createInstrumentDescriptor(name, InstrumentType.COUNTER, options); const storage = this._registerMetricStorage(descriptor); - return new Counter(storage, descriptor); + if (descriptor.valueType === ValueType.INT) { + return new IntCounter(storage, descriptor); + } else { + return new DoubleCounter(storage, descriptor); + } } /** @@ -62,7 +78,11 @@ export class Meter implements metrics.Meter { createUpDownCounter(name: string, options?: metrics.UpDownCounterOptions): metrics.UpDownCounter { const descriptor = createInstrumentDescriptor(name, InstrumentType.UP_DOWN_COUNTER, options); const storage = this._registerMetricStorage(descriptor); - return new UpDownCounter(storage, descriptor); + if (descriptor.valueType === ValueType.INT) { + return new IntUpDownCounter(storage, descriptor); + } else { + return new DoubleUpDownCounter(storage, descriptor); + } } /** 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 67c89891f39..97151c5e425 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts @@ -18,33 +18,56 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { InstrumentationLibrary } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; -import { AggregationTemporality, InstrumentDescriptor, MeterProvider, MetricReader, PointData, PointDataType } from '../src'; +import { AggregationTemporality, InstrumentDescriptor, InstrumentType, MeterProvider, MetricReader, PointData, PointDataType } from '../src'; import { TestMetricReader } from './export/TestMetricReader'; import { assertMetricData, assertPointData, commonValues, commonAttributes, defaultResource, defaultInstrumentationLibrary } from './util'; import { Histogram } from '../src/aggregator/types'; -import { ObservableResult } from '@opentelemetry/api-metrics-wip'; +import { ObservableResult, ValueType } from '@opentelemetry/api-metrics-wip'; describe('Instruments', () => { describe('Counter', () => { - it('should add common values and attributes without exceptions', () => { - const { meter } = setup(); - const counter = meter.createCounter('test'); + it('should add common values and attributes without exceptions', async () => { + const { meter, cumulativeReader } = setup(); + const counter = meter.createCounter('test', { + description: 'foobar', + unit: 'kB', + }); for (const values of commonValues) { for (const attributes of commonAttributes) { counter.add(values, attributes); } } + + await validateExport(cumulativeReader, { + instrumentDescriptor: { + name: 'test', + description: 'foobar', + unit: 'kB', + type: InstrumentType.COUNTER, + valueType: ValueType.DOUBLE, + }, + }); }); - it('should add valid values', async () => { + it('should add valid INT values', async () => { const { meter, cumulativeReader } = setup(); - const counter = meter.createCounter('test'); + const counter = meter.createCounter('test', { + valueType: ValueType.INT, + }); counter.add(1); - counter.add(1); + // floating-point value should be trunc-ed. + counter.add(1.1); counter.add(1, { foo: 'bar' }); await validateExport(cumulativeReader, { + instrumentDescriptor: { + name: 'test', + description: '', + unit: '1', + type: InstrumentType.COUNTER, + valueType: ValueType.INT, + }, pointDataType: PointDataType.SINGULAR, pointData: [ { @@ -59,7 +82,7 @@ describe('Instruments', () => { }); // add negative values should not be observable. - counter.add(-1); + counter.add(-1.1); await validateExport(cumulativeReader, { pointDataType: PointDataType.SINGULAR, pointData: [ @@ -74,28 +97,93 @@ describe('Instruments', () => { ], }); }); + + it('should add valid DOUBLE values', async () => { + const { meter, cumulativeReader } = setup(); + const counter = meter.createCounter('test', { + valueType: ValueType.DOUBLE, + }); + + counter.add(1); + // add floating-point value. + counter.add(1.1); + counter.add(1, { foo: 'bar' }); + counter.add(1.2, { foo: 'bar' }); + await validateExport(cumulativeReader, { + pointDataType: PointDataType.SINGULAR, + pointData: [ + { + attributes: {}, + point: 2.1, + }, + { + attributes: { foo: 'bar' }, + point: 2.2, + }, + ], + }); + + // add negative values should not be observable. + counter.add(-1.1); + await validateExport(cumulativeReader, { + pointDataType: PointDataType.SINGULAR, + pointData: [ + { + attributes: {}, + point: 2.1, + }, + { + attributes: { foo: 'bar' }, + point: 2.2, + }, + ], + }); + }); }); describe('UpDownCounter', () => { - it('should add common values and attributes without exceptions', () => { - const { meter } = setup(); - const upDownCounter = meter.createUpDownCounter('test'); + it('should add common values and attributes without exceptions', async () => { + const { meter, cumulativeReader } = setup(); + const upDownCounter = meter.createUpDownCounter('test', { + description: 'foobar', + unit: 'kB', + }); for (const values of commonValues) { for (const attributes of commonAttributes) { upDownCounter.add(values, attributes); } } + + await validateExport(cumulativeReader, { + instrumentDescriptor: { + name: 'test', + description: 'foobar', + unit: 'kB', + type: InstrumentType.UP_DOWN_COUNTER, + valueType: ValueType.DOUBLE, + }, + }); }); - it('should add values', async () => { + it('should add INT values', async () => { const { meter, deltaReader } = setup(); - const upDownCounter = meter.createUpDownCounter('test'); + const upDownCounter = meter.createUpDownCounter('test', { + valueType: ValueType.INT, + }); upDownCounter.add(3); - upDownCounter.add(-1); + upDownCounter.add(-1.1); upDownCounter.add(4, { foo: 'bar' }); + upDownCounter.add(1.1, { foo: 'bar' }); await validateExport(deltaReader, { + instrumentDescriptor: { + name: 'test', + description: '', + unit: '1', + type: InstrumentType.UP_DOWN_COUNTER, + valueType: ValueType.INT, + }, pointDataType: PointDataType.SINGULAR, pointData: [ { @@ -104,7 +192,32 @@ describe('Instruments', () => { }, { attributes: { foo: 'bar' }, - point: 4, + point: 5, + } + ], + }); + }); + + it('should add DOUBLE values', async () => { + const { meter, deltaReader } = setup(); + const upDownCounter = meter.createUpDownCounter('test', { + valueType: ValueType.DOUBLE, + }); + + upDownCounter.add(3); + upDownCounter.add(-1.1); + upDownCounter.add(4, { foo: 'bar' }); + upDownCounter.add(1.1, { foo: 'bar' }); + await validateExport(deltaReader, { + pointDataType: PointDataType.SINGULAR, + pointData: [ + { + attributes: {}, + point: 1.9, + }, + { + attributes: { foo: 'bar' }, + point: 5.1, } ], }); @@ -112,9 +225,12 @@ describe('Instruments', () => { }); describe('Histogram', () => { - it('should record common values and attributes without exceptions', () => { - const { meter } = setup(); - const histogram = meter.createHistogram('test'); + it('should record common values and attributes without exceptions', async () => { + const { meter, cumulativeReader } = setup(); + const histogram = meter.createHistogram('test', { + description: 'foobar', + unit: 'kB', + }); for (const values of commonValues) { for (const attributes of commonAttributes) { @@ -122,16 +238,37 @@ describe('Instruments', () => { histogram.record(values, attributes); } } + + await validateExport(cumulativeReader, { + instrumentDescriptor: { + name: 'test', + description: 'foobar', + unit: 'kB', + type: InstrumentType.HISTOGRAM, + valueType: ValueType.DOUBLE, + }, + }); }); - it('should record values', async () => { + it('should record INT values', async () => { const { meter, deltaReader } = setup(); - const histogram = meter.createHistogram('test'); + const histogram = meter.createHistogram('test', { + valueType: ValueType.INT, + }); histogram.record(10); - histogram.record(-1); + // -0.1 should be trunc-ed to -0 + histogram.record(-0.1); histogram.record(100, { foo: 'bar' }); + histogram.record(-0.1, { foo: 'bar' }); await validateExport(deltaReader, { + instrumentDescriptor: { + name: 'test', + description: '', + unit: '1', + type: InstrumentType.HISTOGRAM, + valueType: ValueType.INT, + }, pointDataType: PointDataType.HISTOGRAM, pointData: [ { @@ -139,10 +276,10 @@ describe('Instruments', () => { point: { 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, + sum: 10, }, }, { @@ -150,15 +287,55 @@ describe('Instruments', () => { point: { buckets: { boundaries: [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000], - counts: [0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0], + counts: [0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0], }, - count: 1, + count: 2, sum: 100, }, }, ], }); }); + + + it('should record DOUBLE values', async () => { + const { meter, deltaReader } = setup(); + const histogram = meter.createHistogram('test', { + valueType: ValueType.DOUBLE, + }); + + histogram.record(10); + histogram.record(-0.1); + histogram.record(100, { foo: 'bar' }); + histogram.record(-0.1, { foo: 'bar' }); + await validateExport(deltaReader, { + pointDataType: PointDataType.HISTOGRAM, + pointData: [ + { + attributes: {}, + point: { + buckets: { + boundaries: [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000], + counts: [1, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0], + }, + count: 2, + sum: 9.9, + }, + }, + { + attributes: { foo: 'bar' }, + point: { + buckets: { + boundaries: [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000], + counts: [1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0], + }, + count: 2, + sum: 99.9, + }, + }, + ], + }); + }); }); describe('ObservableCounter', () => { @@ -361,12 +538,12 @@ interface ValidateMetricData { resource?: Resource; instrumentationLibrary?: InstrumentationLibrary; instrumentDescriptor?: InstrumentDescriptor; - pointDataType: PointDataType, - pointData: Partial>>[]; + pointDataType?: PointDataType, + pointData?: Partial>>[]; } -async function validateExport(deltaReader: MetricReader, expected: ValidateMetricData) { - const metricData = await deltaReader.collect(); +async function validateExport(reader: MetricReader, expected: ValidateMetricData) { + const metricData = await reader.collect(); const metric = metricData[0]; assertMetricData( @@ -376,6 +553,10 @@ async function validateExport(deltaReader: MetricReader, expected: ValidateMetri expected.instrumentationLibrary, expected.resource ); + + if (expected.pointData == null) { + return; + } assert.strictEqual(metric.pointData.length, expected.pointData.length); for (let idx = 0; idx < metric.pointData.length; idx++) { diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/Meter.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/Meter.test.ts index fbb1b3b9917..0d177bf7e8d 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/Meter.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/Meter.test.ts @@ -14,9 +14,16 @@ * limitations under the License. */ -import { ObservableCallback } from '@opentelemetry/api-metrics-wip'; +import { ObservableCallback, ValueType } from '@opentelemetry/api-metrics-wip'; import * as assert from 'assert'; -import { Counter, Histogram, UpDownCounter } from '../src/Instruments'; +import { + IntCounter, + DoubleCounter, + IntHistogram, + DoubleHistogram, + IntUpDownCounter, + DoubleUpDownCounter, +} from '../src/Instruments'; import { Meter } from '../src/Meter'; import { MeterProviderSharedState } from '../src/state/MeterProviderSharedState'; import { defaultInstrumentationLibrary, defaultResource } from './util'; @@ -25,26 +32,56 @@ const noopObservableCallback: ObservableCallback = _observableResult => {}; describe('Meter', () => { describe('createCounter', () => { - it('should create counter', () => { + it('should create INT counter', () => { const meter = new Meter(new MeterProviderSharedState(defaultResource), defaultInstrumentationLibrary); - const counter = meter.createCounter('foobar'); - assert(counter instanceof Counter); + const counter = meter.createCounter('foobar', { + valueType: ValueType.INT, + }); + assert(counter instanceof IntCounter); + }); + + it('should create DOUBLE counter', () => { + const meter = new Meter(new MeterProviderSharedState(defaultResource), defaultInstrumentationLibrary); + const counter = meter.createCounter('foobar', { + valueType: ValueType.DOUBLE, + }); + assert(counter instanceof DoubleCounter); }); }); describe('createUpDownCounter', () => { - it('should create up down counter', () => { + it('should create INT up down counter', () => { const meter = new Meter(new MeterProviderSharedState(defaultResource), defaultInstrumentationLibrary); - const counter = meter.createUpDownCounter('foobar'); - assert(counter instanceof UpDownCounter); + const upDownCounter = meter.createUpDownCounter('foobar', { + valueType: ValueType.INT, + }); + assert(upDownCounter instanceof IntUpDownCounter); + }); + + it('should create DOUBLE up down counter', () => { + const meter = new Meter(new MeterProviderSharedState(defaultResource), defaultInstrumentationLibrary); + const upDownCounter = meter.createUpDownCounter('foobar', { + valueType: ValueType.DOUBLE, + }); + assert(upDownCounter instanceof DoubleUpDownCounter); }); }); describe('createHistogram', () => { - it('should create histogram', () => { + it('should create INT histogram', () => { + const meter = new Meter(new MeterProviderSharedState(defaultResource), defaultInstrumentationLibrary); + const counter = meter.createHistogram('foobar', { + valueType: ValueType.INT, + }); + assert(counter instanceof IntHistogram); + }); + + it('should create DOUBLE histogram', () => { const meter = new Meter(new MeterProviderSharedState(defaultResource), defaultInstrumentationLibrary); - const counter = meter.createHistogram('foobar'); - assert(counter instanceof Histogram); + const counter = meter.createHistogram('foobar', { + valueType: ValueType.DOUBLE, + }); + assert(counter instanceof DoubleHistogram); }); }); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/util.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/util.ts index d3d9e9e9b6c..b50f6ac928a 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/util.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/util.ts @@ -120,6 +120,6 @@ export function assertPartialDeepStrictEqual(actual: unknown, expected: T, me } const ownNames = Object.getOwnPropertyNames(expected); for (const ownName of ownNames) { - assert.deepStrictEqual((actual as any)[ownName], (expected as any)[ownName], message); + assert.deepStrictEqual((actual as any)[ownName], (expected as any)[ownName], `${ownName} not equals: ${message ?? ''}`); } }