diff --git a/packages/opentelemetry-api/src/metrics/BoundInstrument.ts b/packages/opentelemetry-api/src/metrics/BoundInstrument.ts index 4ffbd143c7..0d5554771e 100644 --- a/packages/opentelemetry-api/src/metrics/BoundInstrument.ts +++ b/packages/opentelemetry-api/src/metrics/BoundInstrument.ts @@ -14,9 +14,6 @@ * limitations under the License. */ -import { CorrelationContext } from '../correlation_context/CorrelationContext'; -import { SpanContext } from '../trace/span_context'; - /** An Instrument for Counter Metric. */ export interface BoundCounter { /** @@ -31,18 +28,8 @@ export interface BoundValueRecorder { /** * Records the given value to this value recorder. * @param value to record. - * @param correlationContext the correlationContext associated with the - * values. - * @param spanContext the {@link SpanContext} that identifies the {@link Span} - * which the values are associated with. */ record(value: number): void; - record(value: number, correlationContext: CorrelationContext): void; - record( - value: number, - correlationContext: CorrelationContext, - spanContext: SpanContext - ): void; } /** An Instrument for Base Observer */ diff --git a/packages/opentelemetry-api/src/metrics/Metric.ts b/packages/opentelemetry-api/src/metrics/Metric.ts index ee7fd29ab4..0022c2a5b6 100644 --- a/packages/opentelemetry-api/src/metrics/Metric.ts +++ b/packages/opentelemetry-api/src/metrics/Metric.ts @@ -14,8 +14,6 @@ * limitations under the License. */ -import { CorrelationContext } from '../correlation_context/CorrelationContext'; -import { SpanContext } from '../trace/span_context'; import { BoundBaseObserver, BoundCounter, @@ -51,12 +49,6 @@ export interface MetricOptions { */ disabled?: boolean; - /** - * (Measure only, default true) Asserts that this metric will only accept - * non-negative values (e.g. disk usage). - */ - absolute?: boolean; - /** * Indicates the type of the recorded value. * @default {@link ValueType.DOUBLE} @@ -148,19 +140,6 @@ export interface ValueRecorder extends UnboundMetric { * Records the given value to this value recorder. */ record(value: number, labels?: Labels): void; - - record( - value: number, - labels: Labels, - correlationContext: CorrelationContext - ): void; - - record( - value: number, - labels: Labels, - correlationContext: CorrelationContext, - spanContext: SpanContext - ): void; } /** Base interface for the Observer metrics. */ diff --git a/packages/opentelemetry-api/src/metrics/NoopMeter.ts b/packages/opentelemetry-api/src/metrics/NoopMeter.ts index d09a6ba742..765a0fd8ae 100644 --- a/packages/opentelemetry-api/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-api/src/metrics/NoopMeter.ts @@ -142,19 +142,8 @@ export class NoopCounterMetric export class NoopValueRecorderMetric extends NoopMetric implements ValueRecorder { - record( - value: number, - labels: Labels, - correlationContext?: CorrelationContext, - spanContext?: SpanContext - ) { - if (typeof correlationContext === 'undefined') { - this.bind(labels).record(value); - } else if (typeof spanContext === 'undefined') { - this.bind(labels).record(value, correlationContext); - } else { - this.bind(labels).record(value, correlationContext, spanContext); - } + record(value: number, labels: Labels) { + this.bind(labels).record(value); } } diff --git a/packages/opentelemetry-metrics/src/BoundInstrument.ts b/packages/opentelemetry-metrics/src/BoundInstrument.ts index 087c6312bd..e84bb72487 100644 --- a/packages/opentelemetry-metrics/src/BoundInstrument.ts +++ b/packages/opentelemetry-metrics/src/BoundInstrument.ts @@ -126,34 +126,17 @@ export class BoundUpDownCounter export class BoundValueRecorder extends BaseBoundInstrument implements api.BoundValueRecorder { - private readonly _absolute: boolean; - constructor( labels: api.Labels, disabled: boolean, - absolute: boolean, valueType: api.ValueType, logger: api.Logger, aggregator: Aggregator ) { super(labels, logger, disabled, valueType, aggregator); - this._absolute = absolute; } - record( - value: number, - correlationContext?: api.CorrelationContext, - spanContext?: api.SpanContext - ): void { - if (this._absolute && value < 0) { - this._logger.error( - `Absolute ValueRecorder cannot contain negative values for $${Object.values( - this._labels - )}` - ); - return; - } - + record(value: number): void { this.update(value); } } @@ -161,7 +144,9 @@ export class BoundValueRecorder /** * BoundObserver is an implementation of the {@link BoundObserver} interface. */ -export class BoundObserver extends BaseBoundInstrument { +export class BoundObserver + extends BaseBoundInstrument + implements api.BoundBaseObserver { constructor( labels: api.Labels, disabled: boolean, diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index 61261fea01..06a11f8142 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -77,7 +77,6 @@ export class Meter implements api.Meter { const opt: api.MetricOptions = { logger: this._logger, ...DEFAULT_METRIC_OPTIONS, - absolute: true, // value recorders are defined as absolute by default ...options, }; diff --git a/packages/opentelemetry-metrics/src/ValueRecorderMetric.ts b/packages/opentelemetry-metrics/src/ValueRecorderMetric.ts index a65581dadc..8275764105 100644 --- a/packages/opentelemetry-metrics/src/ValueRecorderMetric.ts +++ b/packages/opentelemetry-metrics/src/ValueRecorderMetric.ts @@ -26,8 +26,6 @@ import { Metric } from './Metric'; export class ValueRecorderMetric extends Metric implements api.ValueRecorder { - protected readonly _absolute: boolean; - constructor( name: string, options: api.MetricOptions, @@ -42,14 +40,12 @@ export class ValueRecorderMetric resource, instrumentationLibrary ); - - this._absolute = options.absolute !== undefined ? options.absolute : true; // Absolute default is true } + protected _makeInstrument(labels: api.Labels): BoundValueRecorder { return new BoundValueRecorder( labels, this._disabled, - this._absolute, this._valueType, this._logger, this._batcher.aggregatorFor(this._descriptor) diff --git a/packages/opentelemetry-metrics/src/types.ts b/packages/opentelemetry-metrics/src/types.ts index 87e07df10f..d8e5768a48 100644 --- a/packages/opentelemetry-metrics/src/types.ts +++ b/packages/opentelemetry-metrics/src/types.ts @@ -53,7 +53,6 @@ export const DEFAULT_CONFIG = { /** The default metric creation options value. */ export const DEFAULT_METRIC_OPTIONS = { disabled: false, - absolute: false, description: '', unit: '1', valueType: api.ValueType.DOUBLE, diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 19960fcb9f..f137b7fba7 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -555,31 +555,6 @@ describe('Meter', () => { assert.ok(valueRecorder instanceof Metric); }); - it('should be absolute by default', () => { - const valueRecorder = meter.createValueRecorder('name', { - description: 'desc', - unit: '1', - disabled: false, - }); - assert.strictEqual( - (valueRecorder as ValueRecorderMetric)['_absolute'], - true - ); - }); - - it('should be able to set absolute to false', () => { - const valueRecorder = meter.createValueRecorder('name', { - description: 'desc', - unit: '1', - disabled: false, - absolute: false, - }); - assert.strictEqual( - (valueRecorder as ValueRecorderMetric)['_absolute'], - false - ); - }); - it('should pipe through resource', async () => { const valueRecorder = meter.createValueRecorder( 'name' @@ -638,10 +613,12 @@ describe('Meter', () => { assert.doesNotThrow(() => boundValueRecorder.record(10)); }); - it('should not accept negative values by default', async () => { - const valueRecorder = meter.createValueRecorder('name'); + it('should not set the instrument data when disabled', async () => { + const valueRecorder = meter.createValueRecorder('name', { + disabled: true, + }) as ValueRecorderMetric; const boundValueRecorder = valueRecorder.bind(labels); - boundValueRecorder.record(-10); + boundValueRecorder.record(10); await meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); @@ -657,57 +634,30 @@ describe('Meter', () => { ); }); - it('should not set the instrument data when disabled', async () => { - const valueRecorder = meter.createValueRecorder('name', { - disabled: true, - }) as ValueRecorderMetric; + it('should accept negative (and positive) values', async () => { + const valueRecorder = meter.createValueRecorder('name'); const boundValueRecorder = valueRecorder.bind(labels); - boundValueRecorder.record(10); + boundValueRecorder.record(-10); + boundValueRecorder.record(50); await meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); assert.deepStrictEqual( record1.aggregator.toPoint().value as Distribution, { - count: 0, - last: 0, - max: -Infinity, - min: Infinity, - sum: 0, + count: 2, + last: 50, + max: 50, + min: -10, + sum: 40, } ); + assert.ok( + hrTimeToNanoseconds(record1.aggregator.toPoint().timestamp) > + hrTimeToNanoseconds(performanceTimeOrigin) + ); }); - it( - 'should accept negative (and positive) values when absolute is set' + - ' to false', - async () => { - const valueRecorder = meter.createValueRecorder('name', { - absolute: false, - }); - const boundValueRecorder = valueRecorder.bind(labels); - boundValueRecorder.record(-10); - boundValueRecorder.record(50); - - await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); - assert.deepStrictEqual( - record1.aggregator.toPoint().value as Distribution, - { - count: 2, - last: 50, - max: 50, - min: -10, - sum: 40, - } - ); - assert.ok( - hrTimeToNanoseconds(record1.aggregator.toPoint().timestamp) > - hrTimeToNanoseconds(performanceTimeOrigin) - ); - } - ); - it('should return same instrument on same label values', async () => { const valueRecorder = meter.createValueRecorder( 'name'