From 58115116e51cc4ef2e61b0f7e195b6de44e68854 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Tue, 24 Nov 2020 21:57:35 +0100 Subject: [PATCH 1/4] feat: renaming batcher to processor, fixing aggregators, adding missing metrics in api --- doc/{batcher-api.md => processor-api.md} | 24 +++---- .../opentelemetry-api/src/metrics/Meter.ts | 26 ++++++++ .../src/metrics/NoopMeter.ts | 29 +++++++++ .../{ExactBatcher.ts => ExactProcessor.ts} | 6 +- .../test/PrometheusExporter.test.ts | 30 ++++----- .../test/PrometheusSerializer.test.ts | 54 ++++++++-------- .../src/BaseObserverMetric.ts | 6 +- .../src/BatchObserverMetric.ts | 6 +- .../src/CounterMetric.ts | 7 +- packages/opentelemetry-metrics/src/Meter.ts | 31 ++++----- .../src/SumObserverMetric.ts | 6 +- .../src/UpDownCounterMetric.ts | 6 +- .../src/UpDownSumObserverMetric.ts | 6 +- .../src/ValueObserverMetric.ts | 6 +- .../src/ValueRecorderMetric.ts | 6 +- .../src/export/Controller.ts | 2 +- .../src/export/{Batcher.ts => Processor.ts} | 19 +++--- packages/opentelemetry-metrics/src/index.ts | 2 +- packages/opentelemetry-metrics/src/types.ts | 6 +- .../opentelemetry-metrics/test/Meter.test.ts | 64 +++++++++---------- .../{Batcher.test.ts => Processor.test.ts} | 6 +- .../test/export/ConsoleMetricExporter.test.ts | 2 +- packages/opentelemetry-sdk-node/README.md | 4 +- packages/opentelemetry-sdk-node/src/sdk.ts | 4 +- packages/opentelemetry-sdk-node/src/types.ts | 2 +- 25 files changed, 209 insertions(+), 151 deletions(-) rename doc/{batcher-api.md => processor-api.md} (75%) rename packages/opentelemetry-exporter-prometheus/test/{ExactBatcher.ts => ExactProcessor.ts} (94%) rename packages/opentelemetry-metrics/src/export/{Batcher.ts => Processor.ts} (88%) rename packages/opentelemetry-metrics/test/{Batcher.test.ts => Processor.test.ts} (91%) diff --git a/doc/batcher-api.md b/doc/processor-api.md similarity index 75% rename from doc/batcher-api.md rename to doc/processor-api.md index 7cb7c9c036..35fe0557b7 100644 --- a/doc/batcher-api.md +++ b/doc/processor-api.md @@ -1,6 +1,6 @@ -# Batcher API Guide +# Processor API Guide -[The batcher](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-metrics/src/export/Batcher.ts?rgh-link-date=2020-05-25T18%3A43%3A57Z) has two responsibilities: choosing which aggregator to choose for a metric instrument and store the last record for each metric ready to be exported. +[The processor](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-metrics/src/export/Processor.ts?rgh-link-date=2020-05-25T18%3A43%3A57Z) has two responsibilities: choosing which aggregator to choose for a metric instrument and store the last record for each metric ready to be exported. ## Selecting a specific aggregator for metrics @@ -41,25 +41,25 @@ export class AverageAggregator implements Aggregator { } ``` -Now we will need to implement our own batcher to configure the sdk to use our new aggregator. To simplify even more, we will just extend the `UngroupedBatcher` (which is the default) to avoid re-implementing the whole `Aggregator` interface. +Now we will need to implement our own processor to configure the sdk to use our new aggregator. To simplify even more, we will just extend the `UngroupedProcessor` (which is the default) to avoid re-implementing the whole `Aggregator` interface. Here the result: ```ts import { - UngroupedBatcher, + UngroupedProcessor, MetricDescriptor, CounterSumAggregator, ObserverAggregator, MeasureExactAggregator, } from '@opentelemetry/metrics'; -export class CustomBatcher extends UngroupedBatcher { +export class CustomProcessor extends UngroupedProcessor { aggregatorFor (metricDescriptor: MetricDescriptor) { if (metricDescriptor.name === 'requests') { return new AverageAggregator(10); } - // this is exactly what the "UngroupedBatcher" does, we will re-use it + // this is exactly what the "UngroupedProcessor" does, we will re-use it // to fallback on the default behavior switch (metricDescriptor.metricKind) { case MetricKind.COUNTER: @@ -73,11 +73,11 @@ export class CustomBatcher extends UngroupedBatcher { } ``` -Finally, we need to specify to the `MeterProvider` to use our `CustomBatcher` when creating new meter: +Finally, we need to specify to the `MeterProvider` to use our `CustomProcessor` when creating new meter: ```ts import { - UngroupedBatcher, + UngroupedProcessor, MetricDescriptor, CounterSumAggregator, ObserverAggregator, @@ -115,12 +115,12 @@ export class AverageAggregator implements Aggregator { } } -export class CustomBatcher extends UngroupedBatcher { +export class CustomProcessor extends UngroupedProcessor { aggregatorFor (metricDescriptor: MetricDescriptor) { if (metricDescriptor.name === 'requests') { return new AverageAggregator(10); } - // this is exactly what the "UngroupedBatcher" does, we will re-use it + // this is exactly what the "UngroupedProcessor" does, we will re-use it // to fallback on the default behavior switch (metricDescriptor.metricKind) { case MetricKind.COUNTER: @@ -134,9 +134,9 @@ export class CustomBatcher extends UngroupedBatcher { } const meter = new MeterProvider({ - batcher: new CustomBatcher(), + processor: new CustomProcessor(), interval: 1000, -}).getMeter('example-custom-batcher'); +}).getMeter('example-custom-processor'); const requestsLatency = meter.createValueRecorder('requests', { monotonic: true, diff --git a/packages/opentelemetry-api/src/metrics/Meter.ts b/packages/opentelemetry-api/src/metrics/Meter.ts index eb570a4802..1f8e63c803 100644 --- a/packages/opentelemetry-api/src/metrics/Meter.ts +++ b/packages/opentelemetry-api/src/metrics/Meter.ts @@ -23,6 +23,8 @@ import { BatchObserver, BatchMetricOptions, UpDownCounter, + SumObserver, + UpDownSumObserver, } from './Metric'; import { ObserverResult } from './ObserverResult'; @@ -81,6 +83,30 @@ export interface Meter { callback?: (observerResult: ObserverResult) => void ): ValueObserver; + /** + * Creates a new `SumObserver` metric. + * @param name the name of the metric. + * @param [options] the metric options. + * @param [callback] the observer callback + */ + createSumObserver( + name: string, + options?: MetricOptions, + callback?: (observerResult: ObserverResult) => void + ): SumObserver; + + /** + * Creates a new `UpDownSumObserver` metric. + * @param name the name of the metric. + * @param [options] the metric options. + * @param [callback] the observer callback + */ + createUpDownSumObserver( + name: string, + options?: MetricOptions, + callback?: (observerResult: ObserverResult) => void + ): UpDownSumObserver; + /** * Creates a new `BatchObserver` metric, can be used to update many metrics * at the same time and when operations needs to be async diff --git a/packages/opentelemetry-api/src/metrics/NoopMeter.ts b/packages/opentelemetry-api/src/metrics/NoopMeter.ts index d59d658b1e..06eabc197d 100644 --- a/packages/opentelemetry-api/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-api/src/metrics/NoopMeter.ts @@ -26,6 +26,7 @@ import { BatchObserver, UpDownCounter, BaseObserver, + UpDownSumObserver, } from './Metric'; import { BoundValueRecorder, @@ -84,6 +85,34 @@ export class NoopMeter implements Meter { return NOOP_VALUE_OBSERVER_METRIC; } + /** + * Returns constant noop sum observer. + * @param name the name of the metric. + * @param [options] the metric options. + * @param [callback] the sum observer callback + */ + createSumObserver( + _name: string, + _options?: MetricOptions, + _callback?: (observerResult: ObserverResult) => void + ): ValueObserver { + return NOOP_SUM_OBSERVER_METRIC; + } + + /** + * Returns constant noop up down sum observer. + * @param name the name of the metric. + * @param [options] the metric options. + * @param [callback] the up down sum observer callback + */ + createUpDownSumObserver( + _name: string, + _options?: MetricOptions, + _callback?: (observerResult: ObserverResult) => void + ): UpDownSumObserver { + return NOOP_UP_DOWN_SUM_OBSERVER_METRIC; + } + /** * Returns constant noop batch observer. * @param name the name of the metric. diff --git a/packages/opentelemetry-exporter-prometheus/test/ExactBatcher.ts b/packages/opentelemetry-exporter-prometheus/test/ExactProcessor.ts similarity index 94% rename from packages/opentelemetry-exporter-prometheus/test/ExactBatcher.ts rename to packages/opentelemetry-exporter-prometheus/test/ExactProcessor.ts index 4d4a6fa972..9e0e45b9f5 100644 --- a/packages/opentelemetry-exporter-prometheus/test/ExactBatcher.ts +++ b/packages/opentelemetry-exporter-prometheus/test/ExactProcessor.ts @@ -14,15 +14,15 @@ * limitations under the License. */ import { - Batcher, - MetricDescriptor, Aggregator, + MetricDescriptor, MetricRecord, + Processor } from '@opentelemetry/metrics'; type Constructor = new (...args: T[]) => R; -export class ExactBatcher extends Batcher { +export class ExactProcessor extends Processor { private readonly args: ConstructorParameters>; public aggregators: R[] = []; diff --git a/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts b/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts index 81af56ad8c..2c9fba9137 100644 --- a/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts @@ -202,13 +202,13 @@ describe('PrometheusExporter', () => { const boundCounter = counter.bind({ key1: 'labelValue1' }); boundCounter.add(10); meter.collect().then(() => { - exporter.export(meter.getBatcher().checkPointSet(), () => { + exporter.export(meter.getProcessor().checkPointSet(), () => { // TODO: Remove this special case once the PR is ready. // This is to test the special case where counters are destroyed // and recreated in the exporter in order to get around prom-client's // aggregation and use ours. boundCounter.add(10); - exporter.export(meter.getBatcher().checkPointSet(), () => { + exporter.export(meter.getProcessor().checkPointSet(), () => { http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { @@ -255,8 +255,8 @@ describe('PrometheusExporter', () => { ); meter.collect().then(() => { - exporter.export(meter.getBatcher().checkPointSet(), () => { - exporter.export(meter.getBatcher().checkPointSet(), () => { + exporter.export(meter.getProcessor().checkPointSet(), () => { + exporter.export(meter.getProcessor().checkPointSet(), () => { http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { @@ -286,7 +286,7 @@ describe('PrometheusExporter', () => { counter.bind({ counterKey1: 'labelValue1' }).add(10); counter.bind({ counterKey1: 'labelValue2' }).add(20); meter.collect().then(() => { - exporter.export(meter.getBatcher().checkPointSet(), () => { + exporter.export(meter.getProcessor().checkPointSet(), () => { http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { @@ -363,7 +363,7 @@ describe('PrometheusExporter', () => { const boundCounter = counter.bind({ key1: 'labelValue1' }); boundCounter.add(10); meter.collect().then(() => { - exporter.export(meter.getBatcher().checkPointSet(), () => { + exporter.export(meter.getProcessor().checkPointSet(), () => { http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { @@ -390,7 +390,7 @@ describe('PrometheusExporter', () => { const boundCounter = counter.bind({ key1: 'labelValue1' }); boundCounter.add(10); meter.collect().then(() => { - exporter.export(meter.getBatcher().checkPointSet(), () => { + exporter.export(meter.getProcessor().checkPointSet(), () => { http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { @@ -419,7 +419,7 @@ describe('PrometheusExporter', () => { counter.bind({ key1: 'labelValue1' }).add(20); meter.collect().then(() => { - exporter.export(meter.getBatcher().checkPointSet(), () => { + exporter.export(meter.getProcessor().checkPointSet(), () => { http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { @@ -456,7 +456,7 @@ describe('PrometheusExporter', () => { ); meter.collect().then(() => { - exporter.export(meter.getBatcher().checkPointSet(), () => { + exporter.export(meter.getProcessor().checkPointSet(), () => { http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { @@ -465,7 +465,7 @@ describe('PrometheusExporter', () => { assert.deepStrictEqual(lines, [ '# HELP sum_observer a test description', - '# TYPE sum_observer counter', + '# TYPE sum_observer gauge', `sum_observer{key1="labelValue1"} 20 ${mockedHrTimeMs}`, '', ]); @@ -496,7 +496,7 @@ describe('PrometheusExporter', () => { ); meter.collect().then(() => { - exporter.export(meter.getBatcher().checkPointSet(), () => { + exporter.export(meter.getProcessor().checkPointSet(), () => { http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { @@ -526,7 +526,7 @@ describe('PrometheusExporter', () => { valueRecorder.bind({ key1: 'labelValue1' }).record(20); meter.collect().then(() => { - exporter.export(meter.getBatcher().checkPointSet(), () => { + exporter.export(meter.getProcessor().checkPointSet(), () => { http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { @@ -579,7 +579,7 @@ describe('PrometheusExporter', () => { }, async () => { await meter.collect(); - exporter!.export(meter.getBatcher().checkPointSet(), () => { + exporter!.export(meter.getProcessor().checkPointSet(), () => { http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { @@ -609,7 +609,7 @@ describe('PrometheusExporter', () => { }, async () => { await meter.collect(); - exporter!.export(meter.getBatcher().checkPointSet(), () => { + exporter!.export(meter.getProcessor().checkPointSet(), () => { http .get('http://localhost:8080/metrics', res => { res.on('data', chunk => { @@ -639,7 +639,7 @@ describe('PrometheusExporter', () => { }, async () => { await meter.collect(); - exporter!.export(meter.getBatcher().checkPointSet(), () => { + exporter!.export(meter.getProcessor().checkPointSet(), () => { http .get('http://localhost:9464/test', res => { res.on('data', chunk => { diff --git a/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts index e2fec6a30d..b4fd48d9cd 100644 --- a/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts @@ -27,7 +27,7 @@ import * as assert from 'assert'; import { Labels } from '@opentelemetry/api'; import { PrometheusSerializer } from '../src/PrometheusSerializer'; import { PrometheusLabelsBatcher } from '../src/PrometheusLabelsBatcher'; -import { ExactBatcher } from './ExactBatcher'; +import { ExactProcessor } from './ExactProcessor'; import { mockedHrTimeMs, mockAggregator } from './util'; const labels = { @@ -51,7 +51,7 @@ describe('PrometheusSerializer', () => { const serializer = new PrometheusSerializer(); const meter = new MeterProvider({ - batcher: new ExactBatcher(SumAggregator), + processor: new ExactProcessor(SumAggregator), }).getMeter('test'); const counter = meter.createCounter('test') as CounterMetric; counter.bind(labels).add(1); @@ -73,7 +73,7 @@ describe('PrometheusSerializer', () => { const serializer = new PrometheusSerializer(undefined, false); const meter = new MeterProvider({ - batcher: new ExactBatcher(SumAggregator), + processor: new ExactProcessor(SumAggregator), }).getMeter('test'); const counter = meter.createCounter('test') as CounterMetric; counter.bind(labels).add(1); @@ -96,7 +96,7 @@ describe('PrometheusSerializer', () => { const serializer = new PrometheusSerializer(); const meter = new MeterProvider({ - batcher: new ExactBatcher(LastValueAggregator), + processor: new ExactProcessor(LastValueAggregator), }).getMeter('test'); const observer = meter.createValueObserver( 'test', @@ -123,7 +123,7 @@ describe('PrometheusSerializer', () => { const serializer = new PrometheusSerializer(undefined, false); const meter = new MeterProvider({ - batcher: new ExactBatcher(LastValueAggregator), + processor: new ExactProcessor(LastValueAggregator), }).getMeter('test'); const observer = meter.createValueObserver( 'test', @@ -150,8 +150,8 @@ describe('PrometheusSerializer', () => { it('should serialize metric record with sum aggregator', async () => { const serializer = new PrometheusSerializer(); - const batcher = new ExactBatcher(HistogramAggregator, [1, 10, 100]); - const meter = new MeterProvider({ batcher }).getMeter('test'); + const processor = new ExactProcessor(HistogramAggregator, [1, 10, 100]); + const meter = new MeterProvider({ processor }).getMeter('test'); const recorder = meter.createValueRecorder('test', { description: 'foobar', }) as ValueRecorderMetric; @@ -178,8 +178,8 @@ describe('PrometheusSerializer', () => { it('serialize metric record with sum aggregator without timestamp', async () => { const serializer = new PrometheusSerializer(undefined, false); - const batcher = new ExactBatcher(HistogramAggregator, [1, 10, 100]); - const meter = new MeterProvider({ batcher }).getMeter('test'); + const processor = new ExactProcessor(HistogramAggregator, [1, 10, 100]); + const meter = new MeterProvider({ processor }).getMeter('test'); const recorder = meter.createValueRecorder('test', { description: 'foobar', }) as ValueRecorderMetric; @@ -213,9 +213,9 @@ describe('PrometheusSerializer', () => { const serializer = new PrometheusSerializer(); const meter = new MeterProvider({ - batcher: new ExactBatcher(SumAggregator), + processor: new ExactProcessor(SumAggregator), }).getMeter('test'); - const batcher = new PrometheusLabelsBatcher(); + const processor = new PrometheusLabelsBatcher(); const counter = meter.createCounter('test', { description: 'foobar', }) as CounterMetric; @@ -223,8 +223,8 @@ describe('PrometheusSerializer', () => { counter.bind({ val: '2' }).add(1); const records = await counter.getMetricRecord(); - records.forEach(it => batcher.process(it)); - const checkPointSet = batcher.checkPointSet(); + records.forEach(it => processor.process(it)); + const checkPointSet = processor.checkPointSet(); const result = serializer.serialize(checkPointSet); assert.strictEqual( @@ -240,9 +240,9 @@ describe('PrometheusSerializer', () => { const serializer = new PrometheusSerializer(undefined, false); const meter = new MeterProvider({ - batcher: new ExactBatcher(SumAggregator), + processor: new ExactProcessor(SumAggregator), }).getMeter('test'); - const batcher = new PrometheusLabelsBatcher(); + const processor = new PrometheusLabelsBatcher(); const counter = meter.createCounter('test', { description: 'foobar', }) as CounterMetric; @@ -250,8 +250,8 @@ describe('PrometheusSerializer', () => { counter.bind({ val: '2' }).add(1); const records = await counter.getMetricRecord(); - records.forEach(it => batcher.process(it)); - const checkPointSet = batcher.checkPointSet(); + records.forEach(it => processor.process(it)); + const checkPointSet = processor.checkPointSet(); const result = serializer.serialize(checkPointSet); assert.strictEqual( @@ -271,9 +271,9 @@ describe('PrometheusSerializer', () => { const serializer = new PrometheusSerializer(); const meter = new MeterProvider({ - batcher: new ExactBatcher(LastValueAggregator), + processor: new ExactProcessor(LastValueAggregator), }).getMeter('test'); - const batcher = new PrometheusLabelsBatcher(); + const processor = new PrometheusLabelsBatcher(); const observer = meter.createValueObserver( 'test', { @@ -285,8 +285,8 @@ describe('PrometheusSerializer', () => { ) as ValueObserverMetric; await meter.collect(); const records = await observer.getMetricRecord(); - records.forEach(it => batcher.process(it)); - const checkPointSet = batcher.checkPointSet(); + records.forEach(it => processor.process(it)); + const checkPointSet = processor.checkPointSet(); const result = serializer.serialize(checkPointSet); assert.strictEqual( @@ -304,8 +304,8 @@ describe('PrometheusSerializer', () => { it('serialize metric record with HistogramAggregator aggregator, cumulative', async () => { const serializer = new PrometheusSerializer(); - const batcher = new ExactBatcher(HistogramAggregator, [1, 10, 100]); - const meter = new MeterProvider({ batcher }).getMeter('test'); + const processor = new ExactProcessor(HistogramAggregator, [1, 10, 100]); + const meter = new MeterProvider({ processor }).getMeter('test'); const recorder = meter.createValueRecorder('test', { description: 'foobar', }) as ValueRecorderMetric; @@ -350,7 +350,7 @@ describe('PrometheusSerializer', () => { const serializer = new PrometheusSerializer(); const meter = new MeterProvider({ - batcher: new ExactBatcher(SumAggregator), + processor: new ExactProcessor(SumAggregator), }).getMeter('test'); const counter = meter.createCounter('test') as CounterMetric; counter.bind({}).add(1); @@ -369,7 +369,7 @@ describe('PrometheusSerializer', () => { const serializer = new PrometheusSerializer(); const meter = new MeterProvider({ - batcher: new ExactBatcher(SumAggregator), + processor: new ExactProcessor(SumAggregator), }).getMeter('test'); const counter = meter.createCounter('test') as CounterMetric; counter @@ -403,7 +403,7 @@ describe('PrometheusSerializer', () => { for (const esac of cases) { const meter = new MeterProvider({ - batcher: new ExactBatcher(SumAggregator), + processor: new ExactProcessor(SumAggregator), }).getMeter('test'); const counter = meter.createUpDownCounter( 'test' @@ -427,7 +427,7 @@ describe('PrometheusSerializer', () => { const serializer = new PrometheusSerializer(); const meter = new MeterProvider({ - batcher: new ExactBatcher(SumAggregator), + processor: new ExactProcessor(SumAggregator), }).getMeter('test'); const counter = meter.createCounter('test') as CounterMetric; counter diff --git a/packages/opentelemetry-metrics/src/BaseObserverMetric.ts b/packages/opentelemetry-metrics/src/BaseObserverMetric.ts index fcb3ec52cb..9eeea46b94 100644 --- a/packages/opentelemetry-metrics/src/BaseObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/BaseObserverMetric.ts @@ -17,7 +17,7 @@ import * as api from '@opentelemetry/api'; import { InstrumentationLibrary } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { BoundObserver } from './BoundInstrument'; -import { Batcher } from './export/Batcher'; +import { Processor } from './export/Processor'; import { MetricKind, MetricRecord } from './export/types'; import { Metric } from './Metric'; import { ObserverResult } from './ObserverResult'; @@ -36,7 +36,7 @@ export abstract class BaseObserverMetric constructor( name: string, options: api.MetricOptions, - private readonly _batcher: Batcher, + private readonly _processor: Processor, resource: Resource, metricKind: MetricKind, instrumentationLibrary: InstrumentationLibrary, @@ -52,7 +52,7 @@ export abstract class BaseObserverMetric this._disabled, this._valueType, this._logger, - this._batcher.aggregatorFor(this._descriptor) + this._processor.aggregatorFor(this._descriptor) ); } diff --git a/packages/opentelemetry-metrics/src/BatchObserverMetric.ts b/packages/opentelemetry-metrics/src/BatchObserverMetric.ts index 241ff0813a..bd51f21327 100644 --- a/packages/opentelemetry-metrics/src/BatchObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/BatchObserverMetric.ts @@ -19,7 +19,7 @@ import { InstrumentationLibrary } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { BatchObserverResult } from './BatchObserverResult'; import { BoundObserver } from './BoundInstrument'; -import { Batcher } from './export/Batcher'; +import { Processor } from './export/Processor'; import { MetricKind, MetricRecord } from './export/types'; import { Metric } from './Metric'; @@ -36,7 +36,7 @@ export class BatchObserverMetric constructor( name: string, options: api.BatchMetricOptions, - private readonly _batcher: Batcher, + private readonly _processor: Processor, resource: Resource, instrumentationLibrary: InstrumentationLibrary, callback?: (observerResult: api.BatchObserverResult) => void @@ -59,7 +59,7 @@ export class BatchObserverMetric this._disabled, this._valueType, this._logger, - this._batcher.aggregatorFor(this._descriptor) + this._processor.aggregatorFor(this._descriptor) ); } diff --git a/packages/opentelemetry-metrics/src/CounterMetric.ts b/packages/opentelemetry-metrics/src/CounterMetric.ts index c3f35fb8bb..5407ffbc6a 100644 --- a/packages/opentelemetry-metrics/src/CounterMetric.ts +++ b/packages/opentelemetry-metrics/src/CounterMetric.ts @@ -18,7 +18,7 @@ import * as api from '@opentelemetry/api'; import { InstrumentationLibrary } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { BoundCounter } from './BoundInstrument'; -import { Batcher } from './export/Batcher'; +import { Processor } from './export/Processor'; import { MetricKind } from './export/types'; import { Metric } from './Metric'; @@ -27,7 +27,7 @@ export class CounterMetric extends Metric implements api.Counter { constructor( name: string, options: api.MetricOptions, - private readonly _batcher: Batcher, + private readonly _processor: Processor, resource: Resource, instrumentationLibrary: InstrumentationLibrary ) { @@ -39,8 +39,7 @@ export class CounterMetric extends Metric implements api.Counter { this._disabled, this._valueType, this._logger, - // @todo: consider to set to CounterSumAggregator always. - this._batcher.aggregatorFor(this._descriptor) + this._processor.aggregatorFor(this._descriptor) ); } diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index d0a54049b3..7d5da2434f 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -19,6 +19,7 @@ import { ConsoleLogger, InstrumentationLibrary } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { BatchObserverMetric } from './BatchObserverMetric'; import { BaseBoundInstrument } from './BoundInstrument'; +import { Processor } from './export/Processor'; import { MetricKind } from './export/types'; import { UpDownCounterMetric } from './UpDownCounterMetric'; import { CounterMetric } from './CounterMetric'; @@ -28,7 +29,7 @@ import { Metric } from './Metric'; import { ValueObserverMetric } from './ValueObserverMetric'; import { SumObserverMetric } from './SumObserverMetric'; import { DEFAULT_METRIC_OPTIONS, DEFAULT_CONFIG, MeterConfig } from './types'; -import { Batcher, UngroupedBatcher } from './export/Batcher'; +import { UngroupedProcessor } from './export/Processor'; import { PushController } from './export/Controller'; import { NoopExporter } from './export/NoopExporter'; @@ -38,7 +39,7 @@ import { NoopExporter } from './export/NoopExporter'; export class Meter implements api.Meter { private readonly _logger: api.Logger; private readonly _metrics = new Map>(); - private readonly _batcher: Batcher; + private readonly _processor: Processor; private readonly _resource: Resource; private readonly _instrumentationLibrary: InstrumentationLibrary; private readonly _controller: PushController; @@ -53,7 +54,7 @@ export class Meter implements api.Meter { config: MeterConfig = DEFAULT_CONFIG ) { this._logger = config.logger || new ConsoleLogger(config.logLevel); - this._batcher = config.batcher ?? new UngroupedBatcher(); + this._processor = config.processor ?? new UngroupedProcessor(); this._resource = config.resource || Resource.createTelemetrySDKResource(); this._instrumentationLibrary = instrumentationLibrary; // start the push controller @@ -86,7 +87,7 @@ export class Meter implements api.Meter { const valueRecorder = new ValueRecorderMetric( name, opt, - this._batcher, + this._processor, this._resource, this._instrumentationLibrary ); @@ -116,7 +117,7 @@ export class Meter implements api.Meter { const counter = new CounterMetric( name, opt, - this._batcher, + this._processor, this._resource, this._instrumentationLibrary ); @@ -152,7 +153,7 @@ export class Meter implements api.Meter { const upDownCounter = new UpDownCounterMetric( name, opt, - this._batcher, + this._processor, this._resource, this._instrumentationLibrary ); @@ -185,7 +186,7 @@ export class Meter implements api.Meter { const valueObserver = new ValueObserverMetric( name, opt, - this._batcher, + this._processor, this._resource, this._instrumentationLibrary, callback @@ -213,7 +214,7 @@ export class Meter implements api.Meter { const sumObserver = new SumObserverMetric( name, opt, - this._batcher, + this._processor, this._resource, this._instrumentationLibrary, callback @@ -247,7 +248,7 @@ export class Meter implements api.Meter { const upDownSumObserver = new UpDownSumObserverMetric( name, opt, - this._batcher, + this._processor, this._resource, this._instrumentationLibrary, callback @@ -281,7 +282,7 @@ export class Meter implements api.Meter { const batchObserver = new BatchObserverMetric( name, opt, - this._batcher, + this._processor, this._resource, this._instrumentationLibrary, callback @@ -293,7 +294,7 @@ export class Meter implements api.Meter { /** * Collects all the metrics created with this `Meter` for export. * - * Utilizes the batcher to create checkpoints of the current values in + * Utilizes the processor to create checkpoints of the current values in * each aggregator belonging to the metrics that were created with this * meter instance. */ @@ -308,7 +309,7 @@ export class Meter implements api.Meter { }); await Promise.all(batchObservers).then(records => { records.forEach(metrics => { - metrics.forEach(metric => this._batcher.process(metric)); + metrics.forEach(metric => this._processor.process(metric)); }); }); @@ -323,13 +324,13 @@ export class Meter implements api.Meter { await Promise.all(metrics).then(records => { records.forEach(metrics => { - metrics.forEach(metric => this._batcher.process(metric)); + metrics.forEach(metric => this._processor.process(metric)); }); }); } - getBatcher(): Batcher { - return this._batcher; + getProcessor(): Processor { + return this._processor; } shutdown(): Promise { diff --git a/packages/opentelemetry-metrics/src/SumObserverMetric.ts b/packages/opentelemetry-metrics/src/SumObserverMetric.ts index 99f9b0d3c6..1eb8bc90bd 100644 --- a/packages/opentelemetry-metrics/src/SumObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/SumObserverMetric.ts @@ -20,7 +20,7 @@ import { Resource } from '@opentelemetry/resources'; import { BaseObserverMetric } from './BaseObserverMetric'; import { ObserverResult } from './ObserverResult'; import { MonotonicObserverResult } from './MonotonicObserverResult'; -import { Batcher } from './export/Batcher'; +import { Processor } from './export/Processor'; import { MetricKind } from './export/types'; /** This is a SDK implementation of SumObserver Metric. */ @@ -30,7 +30,7 @@ export class SumObserverMetric constructor( name: string, options: api.MetricOptions, - batcher: Batcher, + processor: Processor, resource: Resource, instrumentationLibrary: InstrumentationLibrary, callback?: (observerResult: api.ObserverResult) => unknown @@ -38,7 +38,7 @@ export class SumObserverMetric super( name, options, - batcher, + processor, resource, MetricKind.SUM_OBSERVER, instrumentationLibrary, diff --git a/packages/opentelemetry-metrics/src/UpDownCounterMetric.ts b/packages/opentelemetry-metrics/src/UpDownCounterMetric.ts index 19a8d8a0dd..5d019e09c4 100644 --- a/packages/opentelemetry-metrics/src/UpDownCounterMetric.ts +++ b/packages/opentelemetry-metrics/src/UpDownCounterMetric.ts @@ -19,7 +19,7 @@ import { Resource } from '@opentelemetry/resources'; import { InstrumentationLibrary } from '@opentelemetry/core'; import { BoundUpDownCounter } from './BoundInstrument'; import { MetricKind } from './export/types'; -import { Batcher } from './export/Batcher'; +import { Processor } from './export/Processor'; import { Metric } from './Metric'; /** This is a SDK implementation of UpDownCounter Metric. */ @@ -29,7 +29,7 @@ export class UpDownCounterMetric constructor( name: string, options: api.MetricOptions, - private readonly _batcher: Batcher, + private readonly _processor: Processor, resource: Resource, instrumentationLibrary: InstrumentationLibrary ) { @@ -47,7 +47,7 @@ export class UpDownCounterMetric this._disabled, this._valueType, this._logger, - this._batcher.aggregatorFor(this._descriptor) + this._processor.aggregatorFor(this._descriptor) ); } diff --git a/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts b/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts index 8bb0440487..fe8f3ce6e5 100644 --- a/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts @@ -18,7 +18,7 @@ import * as api from '@opentelemetry/api'; import { InstrumentationLibrary } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { BaseObserverMetric } from './BaseObserverMetric'; -import { Batcher } from './export/Batcher'; +import { Processor } from './export/Processor'; import { MetricKind } from './export/types'; /** This is a SDK implementation of UpDownSumObserver Metric. */ @@ -28,7 +28,7 @@ export class UpDownSumObserverMetric constructor( name: string, options: api.MetricOptions, - batcher: Batcher, + processor: Processor, resource: Resource, instrumentationLibrary: InstrumentationLibrary, callback?: (observerResult: api.ObserverResult) => unknown @@ -36,7 +36,7 @@ export class UpDownSumObserverMetric super( name, options, - batcher, + processor, resource, MetricKind.UP_DOWN_SUM_OBSERVER, instrumentationLibrary, diff --git a/packages/opentelemetry-metrics/src/ValueObserverMetric.ts b/packages/opentelemetry-metrics/src/ValueObserverMetric.ts index 8c2547600d..9c20883247 100644 --- a/packages/opentelemetry-metrics/src/ValueObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/ValueObserverMetric.ts @@ -17,7 +17,7 @@ import * as api from '@opentelemetry/api'; import { InstrumentationLibrary } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { BaseObserverMetric } from './BaseObserverMetric'; -import { Batcher } from './export/Batcher'; +import { Processor } from './export/Processor'; import { MetricKind } from './export/types'; /** This is a SDK implementation of Value Observer Metric. */ @@ -27,7 +27,7 @@ export class ValueObserverMetric constructor( name: string, options: api.MetricOptions, - batcher: Batcher, + processor: Processor, resource: Resource, instrumentationLibrary: InstrumentationLibrary, callback?: (observerResult: api.ObserverResult) => unknown @@ -35,7 +35,7 @@ export class ValueObserverMetric super( name, options, - batcher, + processor, resource, MetricKind.VALUE_OBSERVER, instrumentationLibrary, diff --git a/packages/opentelemetry-metrics/src/ValueRecorderMetric.ts b/packages/opentelemetry-metrics/src/ValueRecorderMetric.ts index 8275764105..cb732d1aef 100644 --- a/packages/opentelemetry-metrics/src/ValueRecorderMetric.ts +++ b/packages/opentelemetry-metrics/src/ValueRecorderMetric.ts @@ -18,7 +18,7 @@ import * as api from '@opentelemetry/api'; import { InstrumentationLibrary } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { BoundValueRecorder } from './BoundInstrument'; -import { Batcher } from './export/Batcher'; +import { Processor } from './export/Processor'; import { MetricKind } from './export/types'; import { Metric } from './Metric'; @@ -29,7 +29,7 @@ export class ValueRecorderMetric constructor( name: string, options: api.MetricOptions, - private readonly _batcher: Batcher, + private readonly _processor: Processor, resource: Resource, instrumentationLibrary: InstrumentationLibrary ) { @@ -48,7 +48,7 @@ export class ValueRecorderMetric this._disabled, this._valueType, this._logger, - this._batcher.aggregatorFor(this._descriptor) + this._processor.aggregatorFor(this._descriptor) ); } diff --git a/packages/opentelemetry-metrics/src/export/Controller.ts b/packages/opentelemetry-metrics/src/export/Controller.ts index c1f698edf5..239080a4e2 100644 --- a/packages/opentelemetry-metrics/src/export/Controller.ts +++ b/packages/opentelemetry-metrics/src/export/Controller.ts @@ -51,7 +51,7 @@ export class PushController extends Controller { await this._meter.collect(); return new Promise(resolve => { this._exporter.export( - this._meter.getBatcher().checkPointSet(), + this._meter.getProcessor().checkPointSet(), result => { if (result.code !== ExportResultCode.SUCCESS) { globalErrorHandler( diff --git a/packages/opentelemetry-metrics/src/export/Batcher.ts b/packages/opentelemetry-metrics/src/export/Processor.ts similarity index 88% rename from packages/opentelemetry-metrics/src/export/Batcher.ts rename to packages/opentelemetry-metrics/src/export/Processor.ts index d5baccbcf0..03d2cbcf52 100644 --- a/packages/opentelemetry-metrics/src/export/Batcher.ts +++ b/packages/opentelemetry-metrics/src/export/Processor.ts @@ -23,13 +23,13 @@ import { } from './types'; /** - * Base class for all batcher types. + * Base class for all processor types. * - * The batcher is responsible for storing the aggregators and aggregated + * The processor is responsible for storing the aggregators and aggregated * values received from updates from metrics in the meter. The stored values * will be sent to an exporter for exporting. */ -export abstract class Batcher { +export abstract class Processor { protected readonly _batchMap = new Map(); /** Returns an aggregator based off metric descriptor. */ @@ -44,23 +44,26 @@ export abstract class Batcher { } /** - * Batcher which retains all dimensions/labels. It accepts all records and + * Processor which retains all dimensions/labels. It accepts all records and * passes them for exporting. */ -export class UngroupedBatcher extends Batcher { +export class UngroupedProcessor extends Processor { aggregatorFor(metricDescriptor: MetricDescriptor): Aggregator { switch (metricDescriptor.metricKind) { case MetricKind.COUNTER: case MetricKind.UP_DOWN_COUNTER: + return new aggregators.SumAggregator(); + case MetricKind.SUM_OBSERVER: case MetricKind.UP_DOWN_SUM_OBSERVER: - return new aggregators.SumAggregator(); + case MetricKind.VALUE_OBSERVER: + return new aggregators.LastValueAggregator(); + case MetricKind.VALUE_RECORDER: return new aggregators.HistogramAggregator( metricDescriptor.boundaries || [Infinity] ); - case MetricKind.VALUE_OBSERVER: - return new aggregators.LastValueAggregator(); + default: return new aggregators.LastValueAggregator(); } diff --git a/packages/opentelemetry-metrics/src/index.ts b/packages/opentelemetry-metrics/src/index.ts index 2a0c408d71..afcdeea5b1 100644 --- a/packages/opentelemetry-metrics/src/index.ts +++ b/packages/opentelemetry-metrics/src/index.ts @@ -22,8 +22,8 @@ export * from './MeterProvider'; export * from './Metric'; export * from './ValueObserverMetric'; export * from './export/aggregators'; -export * from './export/Batcher'; export * from './export/ConsoleMetricExporter'; +export * from './export/Processor'; export * from './export/types'; export * from './UpDownCounterMetric'; export { MeterConfig } from './types'; diff --git a/packages/opentelemetry-metrics/src/types.ts b/packages/opentelemetry-metrics/src/types.ts index 1eed57a46e..0ff5610201 100644 --- a/packages/opentelemetry-metrics/src/types.ts +++ b/packages/opentelemetry-metrics/src/types.ts @@ -16,9 +16,9 @@ import { LogLevel, getEnv } from '@opentelemetry/core'; import * as api from '@opentelemetry/api'; +import { Processor } from './export/Processor'; import { MetricExporter } from './export/types'; import { Resource } from '@opentelemetry/resources'; -import { Batcher } from './export/Batcher'; /** MeterConfig provides an interface for configuring a Meter. */ export interface MeterConfig { @@ -37,8 +37,8 @@ export interface MeterConfig { /** Resource associated with metric telemetry */ resource?: Resource; - /** Metric batcher. */ - batcher?: Batcher; + /** Metric Processor. */ + processor?: Processor; } /** Default Meter configuration. */ diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 0940a3057c..fd8d3a6af6 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -41,7 +41,7 @@ import { SumObserverMetric } from '../src/SumObserverMetric'; import { Resource } from '@opentelemetry/resources'; import { UpDownSumObserverMetric } from '../src/UpDownSumObserverMetric'; import { hashLabels } from '../src/Utils'; -import { Batcher } from '../src/export/Batcher'; +import { Processor } from '../src/export/Processor'; import { ValueType } from '@opentelemetry/api'; const nonNumberValues = [ @@ -105,7 +105,7 @@ describe('Meter', () => { const counter = meter.createCounter('name') as CounterMetric; counter.add(10, labels); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 10); const lastTimestamp = record1.aggregator.toPoint().timestamp; @@ -130,7 +130,7 @@ describe('Meter', () => { }); counter.add(1); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 1); }); @@ -162,7 +162,7 @@ describe('Meter', () => { const boundCounter = counter.bind(labels); boundCounter.add(10); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 10); boundCounter.add(10); @@ -181,9 +181,9 @@ describe('Meter', () => { const counter = meter.createCounter('name') as CounterMetric; const boundCounter = counter.bind(labels); boundCounter.add(10); - assert.strictEqual(meter.getBatcher().checkPointSet().length, 0); + assert.strictEqual(meter.getProcessor().checkPointSet().length, 0); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 10); boundCounter.add(-100); @@ -197,7 +197,7 @@ describe('Meter', () => { const boundCounter = counter.bind(labels); boundCounter.add(10); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 0); }); @@ -208,7 +208,7 @@ describe('Meter', () => { const boundCounter1 = counter.bind(labels); boundCounter1.add(10); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 20); assert.strictEqual(boundCounter, boundCounter1); @@ -253,7 +253,7 @@ describe('Meter', () => { counter2.bind(labels).add(500); await meter.collect(); - const record = meter.getBatcher().checkPointSet(); + const record = meter.getProcessor().checkPointSet(); assert.strictEqual(record.length, 1); assert.deepStrictEqual(record[0].descriptor, { @@ -317,7 +317,7 @@ describe('Meter', () => { const upDownCounter = meter.createUpDownCounter('name'); upDownCounter.add(10, labels); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 10); const lastTimestamp = record1.aggregator.toPoint().timestamp; @@ -342,7 +342,7 @@ describe('Meter', () => { }); upDownCounter.add(1); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 1); }); @@ -364,7 +364,7 @@ describe('Meter', () => { const boundCounter = upDownCounter.bind(labels); boundCounter.add(10); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 10); boundCounter.add(-200); @@ -388,7 +388,7 @@ describe('Meter', () => { const boundCounter = upDownCounter.bind(labels); boundCounter.add(10); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 0); }); @@ -399,7 +399,7 @@ describe('Meter', () => { const boundCounter1 = upDownCounter.bind(labels); boundCounter1.add(10); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 20); assert.strictEqual(boundCounter, boundCounter1); @@ -415,7 +415,7 @@ describe('Meter', () => { boundCounter.add(val); }); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 1); }); @@ -430,7 +430,7 @@ describe('Meter', () => { // @ts-expect-error boundCounter.add(val); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 0); }) @@ -448,7 +448,7 @@ describe('Meter', () => { // @ts-expect-error boundCounter.add(val); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.strictEqual(record1.aggregator.toPoint().value, 0); }) @@ -498,7 +498,7 @@ describe('Meter', () => { counter2.bind(labels).add(500); await meter.collect(); - const record = meter.getBatcher().checkPointSet(); + const record = meter.getProcessor().checkPointSet(); assert.strictEqual(record.length, 1); assert.deepStrictEqual(record[0].descriptor, { @@ -622,7 +622,7 @@ describe('Meter', () => { boundValueRecorder.record(10); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.deepStrictEqual( record1.aggregator.toPoint().value as Histogram, { @@ -643,7 +643,7 @@ describe('Meter', () => { boundValueRecorder.record(50); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.deepStrictEqual( record1.aggregator.toPoint().value as Histogram, { @@ -670,7 +670,7 @@ describe('Meter', () => { const boundValueRecorder2 = valueRecorder.bind(labels); boundValueRecorder2.record(100); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.deepStrictEqual( record1.aggregator.toPoint().value as Histogram, { @@ -696,7 +696,7 @@ describe('Meter', () => { // @ts-expect-error boundValueRecorder.record(val); await meter.collect(); - const [record1] = meter.getBatcher().checkPointSet(); + const [record1] = meter.getProcessor().checkPointSet(); assert.deepStrictEqual( record1.aggregator.toPoint().value as Histogram, { @@ -813,7 +813,7 @@ describe('Meter', () => { metricRecords = await sumObserver.getMetricRecord(); assert.strictEqual(metricRecords.length, 1); point = metricRecords[0].aggregator.toPoint(); - assert.strictEqual(point.value, 6); + assert.strictEqual(point.value, 3); }); it('should set callback and observe value when callback returns nothing', async () => { @@ -1030,12 +1030,12 @@ describe('Meter', () => { metricRecords = await upDownSumObserver.getMetricRecord(); assert.strictEqual(metricRecords.length, 1); point = metricRecords[0].aggregator.toPoint(); - assert.strictEqual(point.value, 2); + assert.strictEqual(point.value, -1); metricRecords = await upDownSumObserver.getMetricRecord(); assert.strictEqual(metricRecords.length, 1); point = metricRecords[0].aggregator.toPoint(); - assert.strictEqual(point.value, 5); + assert.strictEqual(point.value, 3); }); it('should set callback and observe value when callback returns nothing', async () => { @@ -1188,7 +1188,7 @@ describe('Meter', () => { ); await meter.collect(); - const records = meter.getBatcher().checkPointSet(); + const records = meter.getProcessor().checkPointSet(); assert.strictEqual(records.length, 8); const metric1 = records[0]; @@ -1290,7 +1290,7 @@ describe('Meter', () => { boundCounter.add(10.45); await meter.collect(); - const record = meter.getBatcher().checkPointSet(); + const record = meter.getProcessor().checkPointSet(); assert.strictEqual(record.length, 1); assert.deepStrictEqual(record[0].descriptor, { @@ -1316,7 +1316,7 @@ describe('Meter', () => { boundCounter.add(10.45); await meter.collect(); - const record = meter.getBatcher().checkPointSet(); + const record = meter.getProcessor().checkPointSet(); assert.strictEqual(record.length, 1); assert.deepStrictEqual(record[0].descriptor, { @@ -1332,9 +1332,9 @@ describe('Meter', () => { }); }); - it('should allow custom batcher', () => { - const customMeter = new MeterProvider().getMeter('custom-batcher', '*', { - batcher: new CustomBatcher(), + it('should allow custom processor', () => { + const customMeter = new MeterProvider().getMeter('custom-processor', '*', { + processor: new CustomProcessor(), }); assert.throws(() => { const valueRecorder = customMeter.createValueRecorder('myValueRecorder'); @@ -1343,7 +1343,7 @@ describe('Meter', () => { }); }); -class CustomBatcher extends Batcher { +class CustomProcessor extends Processor { process(record: MetricRecord): void { throw new Error('process method not implemented.'); } diff --git a/packages/opentelemetry-metrics/test/Batcher.test.ts b/packages/opentelemetry-metrics/test/Processor.test.ts similarity index 91% rename from packages/opentelemetry-metrics/test/Batcher.test.ts rename to packages/opentelemetry-metrics/test/Processor.test.ts index f2d3e2d360..f7d1ce9c11 100644 --- a/packages/opentelemetry-metrics/test/Batcher.test.ts +++ b/packages/opentelemetry-metrics/test/Processor.test.ts @@ -19,7 +19,7 @@ import * as api from '@opentelemetry/api'; import { NoopLogger } from '@opentelemetry/core'; import { Meter, MeterProvider } from '../src'; -describe('Batcher', () => { +describe('Processor', () => { describe('Ungrouped', () => { let meter: Meter; let fooCounter: api.BoundCounter; @@ -30,7 +30,7 @@ describe('Batcher', () => { logger: new NoopLogger(), interval: 10000, }).getMeter('test-meter'); - counter = meter.createCounter('ungrouped-batcher-test'); + counter = meter.createCounter('ungrouped-processor-test'); fooCounter = counter.bind({ key: 'foo' }); barCounter = counter.bind({ key: 'bar' }); }); @@ -40,7 +40,7 @@ describe('Batcher', () => { barCounter.add(1); barCounter.add(2); await meter.collect(); - const checkPointSet = meter.getBatcher().checkPointSet(); + const checkPointSet = meter.getProcessor().checkPointSet(); assert.strictEqual(checkPointSet.length, 2); for (const record of checkPointSet) { switch (record.labels.key) { diff --git a/packages/opentelemetry-metrics/test/export/ConsoleMetricExporter.test.ts b/packages/opentelemetry-metrics/test/export/ConsoleMetricExporter.test.ts index b23a14cd81..a9b516f40c 100644 --- a/packages/opentelemetry-metrics/test/export/ConsoleMetricExporter.test.ts +++ b/packages/opentelemetry-metrics/test/export/ConsoleMetricExporter.test.ts @@ -50,7 +50,7 @@ describe('ConsoleMetricExporter', () => { boundCounter.add(10); await meter.collect(); - consoleExporter.export(meter.getBatcher().checkPointSet(), () => {}); + consoleExporter.export(meter.getProcessor().checkPointSet(), () => {}); assert.strictEqual(spyConsole.args.length, 3); const [descriptor, labels, value] = spyConsole.args; assert.deepStrictEqual(descriptor, [ diff --git a/packages/opentelemetry-sdk-node/README.md b/packages/opentelemetry-sdk-node/README.md index 6bdc5ae7a8..d1bd7203a3 100644 --- a/packages/opentelemetry-sdk-node/README.md +++ b/packages/opentelemetry-sdk-node/README.md @@ -100,9 +100,9 @@ Use a custom logger. Default: Logging disabled Default: [INFO](../opentelemetry-core/src/common/types.ts#L19) -### metricBatcher +### metricProcessor -Use a custom batcher for metrics. Default: [UngroupedBatcher](../opentelemetry-metrics/src/export/Batcher.ts#L50) +Use a custom processor for metrics. Default: [UngroupedProcessor](../opentelemetry-metrics/src/export/Processor.ts#L50) ### metricExporter diff --git a/packages/opentelemetry-sdk-node/src/sdk.ts b/packages/opentelemetry-sdk-node/src/sdk.ts index 54558b802e..a25933998b 100644 --- a/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/packages/opentelemetry-sdk-node/src/sdk.ts @@ -89,8 +89,8 @@ export class NodeSDK { if (configuration.metricExporter) { const meterConfig: MeterConfig = {}; - if (configuration.metricBatcher) { - meterConfig.batcher = configuration.metricBatcher; + if (configuration.metricProcessor) { + meterConfig.processor = configuration.metricProcessor; } if (configuration.metricExporter) { meterConfig.exporter = configuration.metricExporter; diff --git a/packages/opentelemetry-sdk-node/src/types.ts b/packages/opentelemetry-sdk-node/src/types.ts index 4675c88b3d..e3351a4ec1 100644 --- a/packages/opentelemetry-sdk-node/src/types.ts +++ b/packages/opentelemetry-sdk-node/src/types.ts @@ -24,7 +24,7 @@ export interface NodeSDKConfiguration { textMapPropagator: api.TextMapPropagator; logger: api.Logger; logLevel: core.LogLevel; - metricBatcher: metrics.Batcher; + metricProcessor: metrics.Processor; metricExporter: metrics.MetricExporter; metricInterval: number; plugins: node.Plugins; From 13a9d14ecc6e501eda7be6e7c9742cc7d4a0dd4a Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 26 Nov 2020 18:54:53 +0100 Subject: [PATCH 2/4] chore: fixing metrics in exporter collector, updated tests, fixing observer result --- .../src/transformMetrics.ts | 80 ++++----- .../browser/CollectorMetricExporter.test.ts | 34 ++-- .../common/CollectorMetricExporter.test.ts | 21 ++- .../test/common/transformMetrics.test.ts | 118 ++++++++++--- .../test/helper.ts | 165 +++++++++++++++--- .../test/node/CollectorMetricExporter.test.ts | 38 ++-- .../test/ExactProcessor.ts | 2 +- .../src/MonotonicObserverResult.ts | 5 +- .../src/ObserverResult.ts | 4 +- .../opentelemetry-metrics/test/Meter.test.ts | 4 +- 10 files changed, 340 insertions(+), 131 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/transformMetrics.ts b/packages/opentelemetry-exporter-collector/src/transformMetrics.ts index eaf73fc2c9..1128a23260 100644 --- a/packages/opentelemetry-exporter-collector/src/transformMetrics.ts +++ b/packages/opentelemetry-exporter-collector/src/transformMetrics.ts @@ -136,50 +136,42 @@ export function toCollectorMetric( unit: metric.descriptor.unit, }; - switch (metric.aggregator.kind) { - case AggregatorKind.SUM: - { - const result = { - dataPoints: [toDataPoint(metric, startTime)], - isMonotonic: - metric.descriptor.metricKind === MetricKind.COUNTER || - metric.descriptor.metricKind === MetricKind.SUM_OBSERVER, - aggregationTemporality: toAggregationTemporality(metric), - }; - if (metric.descriptor.valueType === api.ValueType.INT) { - metricCollector.intSum = result; - } else { - metricCollector.doubleSum = result; - } - } - break; - - case AggregatorKind.LAST_VALUE: - { - const result = { - dataPoints: [toDataPoint(metric, startTime)], - }; - if (metric.descriptor.valueType === api.ValueType.INT) { - metricCollector.intGauge = result; - } else { - metricCollector.doubleGauge = result; - } - } - break; - - case AggregatorKind.HISTOGRAM: - { - const result = { - dataPoints: [toHistogramPoint(metric, startTime)], - aggregationTemporality: toAggregationTemporality(metric), - }; - if (metric.descriptor.valueType === api.ValueType.INT) { - metricCollector.intHistogram = result; - } else { - metricCollector.doubleHistogram = result; - } - } - break; + if ( + metric.aggregator.kind === AggregatorKind.SUM || + metric.descriptor.metricKind === MetricKind.SUM_OBSERVER || + metric.descriptor.metricKind === MetricKind.UP_DOWN_SUM_OBSERVER + ) { + const result = { + dataPoints: [toDataPoint(metric, startTime)], + isMonotonic: + metric.descriptor.metricKind === MetricKind.COUNTER || + metric.descriptor.metricKind === MetricKind.SUM_OBSERVER, + aggregationTemporality: toAggregationTemporality(metric), + }; + if (metric.descriptor.valueType === api.ValueType.INT) { + metricCollector.intSum = result; + } else { + metricCollector.doubleSum = result; + } + } else if (metric.aggregator.kind === AggregatorKind.LAST_VALUE) { + const result = { + dataPoints: [toDataPoint(metric, startTime)], + }; + if (metric.descriptor.valueType === api.ValueType.INT) { + metricCollector.intGauge = result; + } else { + metricCollector.doubleGauge = result; + } + } else if (metric.aggregator.kind === AggregatorKind.HISTOGRAM) { + const result = { + dataPoints: [toHistogramPoint(metric, startTime)], + aggregationTemporality: toAggregationTemporality(metric), + }; + if (metric.descriptor.valueType === api.ValueType.INT) { + metricCollector.intHistogram = result; + } else { + metricCollector.doubleHistogram = result; + } } return metricCollector; diff --git a/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts index 252e3cd49c..e4f01acb63 100644 --- a/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts @@ -14,13 +14,20 @@ * limitations under the License. */ +import * as api from '@opentelemetry/api'; import { ExportResultCode, NoopLogger } from '@opentelemetry/core'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { CollectorMetricExporter } from '../../src/platform/browser/index'; import { CollectorExporterConfigBase } from '../../src/types'; import * as collectorTypes from '../../src/types'; -import { MetricRecord } from '@opentelemetry/metrics'; +import { + BoundCounter, + BoundObserver, + BoundValueRecorder, + Metric, + MetricRecord, +} from '@opentelemetry/metrics'; import { mockCounter, mockObserver, @@ -48,15 +55,22 @@ describe('CollectorMetricExporter - web', () => { spySend = sinon.stub(XMLHttpRequest.prototype, 'send'); spyBeacon = sinon.stub(navigator, 'sendBeacon'); metrics = []; - metrics.push(await mockCounter()); - metrics.push(await mockObserver()); - metrics.push(await mockValueRecorder()); - - metrics[0].aggregator.update(1); - metrics[1].aggregator.update(3); - metrics[1].aggregator.update(6); - metrics[2].aggregator.update(7); - metrics[2].aggregator.update(14); + const counter: Metric & api.Counter = mockCounter(); + const observer: Metric & api.ValueObserver = mockObserver( + observerResult => { + observerResult.observe(3, {}); + observerResult.observe(6, {}); + } + ); + const recorder: Metric & + api.ValueRecorder = mockValueRecorder(); + counter.add(1); + recorder.record(7); + recorder.record(14); + + metrics.push((await counter.getMetricRecord())[0]); + metrics.push((await observer.getMetricRecord())[0]); + metrics.push((await recorder.getMetricRecord())[0]); }); afterEach(() => { diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts index 2766c0c5f5..4e2384417c 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts @@ -14,12 +14,18 @@ * limitations under the License. */ +import * as api from '@opentelemetry/api'; import { ExportResultCode, NoopLogger } from '@opentelemetry/core'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { CollectorExporterBase } from '../../src/CollectorExporterBase'; import { CollectorExporterConfigBase } from '../../src/types'; -import { MetricRecord } from '@opentelemetry/metrics'; +import { + BoundCounter, + BoundObserver, + Metric, + MetricRecord, +} from '@opentelemetry/metrics'; import { mockCounter, mockObserver } from '../helper'; import * as collectorTypes from '../../src/types'; @@ -63,8 +69,17 @@ describe('CollectorMetricExporter - common', () => { }; collectorExporter = new CollectorMetricExporter(collectorExporterConfig); metrics = []; - metrics.push(await mockCounter()); - metrics.push(await mockObserver()); + const counter: Metric & api.Counter = mockCounter(); + const observer: Metric & api.ValueObserver = mockObserver( + observerResult => { + observerResult.observe(3, {}); + observerResult.observe(6, {}); + } + ); + counter.add(1); + + metrics.push((await counter.getMetricRecord())[0]); + metrics.push((await observer.getMetricRecord())[0]); }); afterEach(() => { diff --git a/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts b/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts index 3b951b2149..ab1f055832 100644 --- a/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts @@ -28,51 +28,107 @@ import { mockedInstrumentationLibraries, multiResourceMetricsGet, multiInstrumentationLibraryMetricsGet, + mockSumObserver, + mockUpDownSumObserver, + ensureSumObserverIsCorrect, + ensureUpDownSumObserverIsCorrect, } from '../helper'; -import { MetricRecord, SumAggregator } from '@opentelemetry/metrics'; +import { + BoundCounter, + BoundObserver, + BoundValueRecorder, + Metric, + SumAggregator, +} from '@opentelemetry/metrics'; import { hrTimeToNanoseconds } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; +import * as api from '@opentelemetry/api'; describe('transformMetrics', () => { describe('toCollectorMetric', async () => { - const counter: MetricRecord = await mockCounter(); - const doubleCounter: MetricRecord = await mockDoubleCounter(); - const observer: MetricRecord = await mockObserver(); - const recorder: MetricRecord = await mockValueRecorder(); + let counter: Metric & api.Counter; + let doubleCounter: Metric & api.Counter; + let observer: Metric & api.ValueObserver; + let sumObserver: Metric & api.SumObserver; + let upDownSumObserver: Metric & api.UpDownSumObserver; + let recorder: Metric & api.ValueRecorder; beforeEach(() => { + counter = mockCounter(); + doubleCounter = mockDoubleCounter(); + observer = mockObserver(observerResult => { + observerResult.observe(3, {}); + observerResult.observe(6, {}); + }); + sumObserver = mockSumObserver(observerResult => { + const labels = {}; + observerResult.observe(3, labels); + observerResult.observe(5, labels); + observerResult.observe(4, labels); + observerResult.observe(-1, labels); + }); + upDownSumObserver = mockUpDownSumObserver(observerResult => { + const labels = {}; + observerResult.observe(3, labels); + observerResult.observe(5, labels); + observerResult.observe(4, labels); + observerResult.observe(-1, labels); + }); + + recorder = mockValueRecorder(); + // Counter - counter.aggregator.update(1); + counter.add(1); // Double Counter - doubleCounter.aggregator.update(8); - - // Observer - observer.aggregator.update(3); - observer.aggregator.update(6); + doubleCounter.add(8); // ValueRecorder - recorder.aggregator.update(7); - recorder.aggregator.update(14); + recorder.record(7); + recorder.record(14); }); - it('should convert metric', () => { + it('should convert metric', async () => { + const counterMetric = (await counter.getMetricRecord())[0]; ensureCounterIsCorrect( - transform.toCollectorMetric(counter, 1592602232694000000), - hrTimeToNanoseconds(counter.aggregator.toPoint().timestamp) + transform.toCollectorMetric(counterMetric, 1592602232694000000), + hrTimeToNanoseconds(await counterMetric.aggregator.toPoint().timestamp) + ); + + const doubleCounterMetric = (await doubleCounter.getMetricRecord())[0]; + ensureDoubleCounterIsCorrect( + transform.toCollectorMetric(doubleCounterMetric, 1592602232694000000), + hrTimeToNanoseconds(doubleCounterMetric.aggregator.toPoint().timestamp) ); + + const observerMetric = (await observer.getMetricRecord())[0]; ensureObserverIsCorrect( - transform.toCollectorMetric(observer, 1592602232694000000), - hrTimeToNanoseconds(observer.aggregator.toPoint().timestamp) + transform.toCollectorMetric(observerMetric, 1592602232694000000), + hrTimeToNanoseconds(observerMetric.aggregator.toPoint().timestamp) ); - ensureValueRecorderIsCorrect( - transform.toCollectorMetric(recorder, 1592602232694000000), - hrTimeToNanoseconds(recorder.aggregator.toPoint().timestamp) + const sumObserverMetric = (await sumObserver.getMetricRecord())[0]; + ensureSumObserverIsCorrect( + transform.toCollectorMetric(sumObserverMetric, 1592602232694000000), + hrTimeToNanoseconds(sumObserverMetric.aggregator.toPoint().timestamp) ); - ensureDoubleCounterIsCorrect( - transform.toCollectorMetric(doubleCounter, 1592602232694000000), - hrTimeToNanoseconds(doubleCounter.aggregator.toPoint().timestamp) + const upDownSumObserverMetric = ( + await upDownSumObserver.getMetricRecord() + )[0]; + ensureUpDownSumObserverIsCorrect( + transform.toCollectorMetric( + upDownSumObserverMetric, + 1592602232694000000 + ), + hrTimeToNanoseconds( + upDownSumObserverMetric.aggregator.toPoint().timestamp + ) + ); + + const recorderMetric = (await recorder.getMetricRecord())[0]; + ensureValueRecorderIsCorrect( + transform.toCollectorMetric(recorderMetric, 1592602232694000000), + hrTimeToNanoseconds(recorderMetric.aggregator.toPoint().timestamp) ); }); @@ -102,7 +158,11 @@ describe('transformMetrics', () => { it('should group by resource', async () => { const [resource1, resource2] = mockedResources; const [library] = mockedInstrumentationLibraries; - const [metric1, metric2, metric3] = await multiResourceMetricsGet(); + const [metric1, metric2, metric3] = multiResourceMetricsGet( + observerResult => { + observerResult.observe(1, {}); + } + ); const expected = new Map([ [resource1, new Map([[library, [metric1, metric3]]])], @@ -110,7 +170,9 @@ describe('transformMetrics', () => { ]); const result = transform.groupMetricsByResourceAndLibrary( - await multiResourceMetricsGet() + multiResourceMetricsGet(observerResult => { + observerResult.observe(1, {}); + }) ); assert.deepStrictEqual(result, expected); @@ -123,7 +185,7 @@ describe('transformMetrics', () => { metric1, metric2, metric3, - ] = await multiInstrumentationLibraryMetricsGet(); + ] = multiInstrumentationLibraryMetricsGet(observerResult => {}); const expected = new Map([ [ resource, @@ -135,7 +197,7 @@ describe('transformMetrics', () => { ]); const result = transform.groupMetricsByResourceAndLibrary( - await multiInstrumentationLibraryMetricsGet() + multiInstrumentationLibraryMetricsGet(observerResult => {}) ); assert.deepStrictEqual(result, expected); diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index f430b441bc..5212177bfc 100644 --- a/packages/opentelemetry-exporter-collector/test/helper.ts +++ b/packages/opentelemetry-exporter-collector/test/helper.ts @@ -14,10 +14,26 @@ * limitations under the License. */ -import { TraceFlags, ValueType, StatusCode } from '@opentelemetry/api'; +import { + TraceFlags, + ValueType, + StatusCode, + SumObserver, + UpDownSumObserver, + ValueObserver, + ValueRecorder, + Counter, + ObserverResult, +} from '@opentelemetry/api'; import { ReadableSpan } from '@opentelemetry/tracing'; import { Resource } from '@opentelemetry/resources'; -import { MetricRecord, MeterProvider } from '@opentelemetry/metrics'; +import { + MeterProvider, + Metric, + BoundCounter, + BoundObserver, + BoundValueRecorder, +} from '@opentelemetry/metrics'; import { hexToBase64, InstrumentationLibrary } from '@opentelemetry/core'; import * as assert from 'assert'; import { opentelemetryProto } from '../src/types'; @@ -42,7 +58,7 @@ if (typeof Buffer === 'undefined') { }; } -export async function mockCounter(): Promise { +export function mockCounter(): Metric & Counter { const name = 'int-counter'; const metric = meter['_metrics'].get(name) || @@ -52,11 +68,10 @@ export async function mockCounter(): Promise { }); metric.clear(); metric.bind({}); - - return (await metric.getMetricRecord())[0]; + return metric; } -export async function mockDoubleCounter(): Promise { +export function mockDoubleCounter(): Metric & Counter { const name = 'double-counter'; const metric = meter['_metrics'].get(name) || @@ -66,25 +81,68 @@ export async function mockDoubleCounter(): Promise { }); metric.clear(); metric.bind({}); - - return (await metric.getMetricRecord())[0]; + return metric; } -export async function mockObserver(): Promise { +export function mockObserver( + callback: (observerResult: ObserverResult) => void +): Metric & ValueObserver { const name = 'double-observer'; const metric = meter['_metrics'].get(name) || - meter.createValueObserver(name, { - description: 'sample observer description', - valueType: ValueType.DOUBLE, - }); + meter.createValueObserver( + name, + { + description: 'sample observer description', + valueType: ValueType.DOUBLE, + }, + callback + ); metric.clear(); metric.bind({}); + return metric; +} - return (await metric.getMetricRecord())[0]; +export function mockSumObserver( + callback: (observerResult: ObserverResult) => void +): Metric & SumObserver { + const name = 'double-sum-observer'; + const metric = + meter['_metrics'].get(name) || + meter.createSumObserver( + name, + { + description: 'sample sum observer description', + valueType: ValueType.DOUBLE, + }, + callback + ); + metric.clear(); + metric.bind({}); + return metric; } -export async function mockValueRecorder(): Promise { +export function mockUpDownSumObserver( + callback: (observerResult: ObserverResult) => void +): Metric & UpDownSumObserver { + const name = 'double-up-down-sum-observer'; + const metric = + meter['_metrics'].get(name) || + meter.createUpDownSumObserver( + name, + { + description: 'sample up down sum observer description', + valueType: ValueType.DOUBLE, + }, + callback + ); + metric.clear(); + metric.bind({}); + return metric; +} + +export function mockValueRecorder(): Metric & + ValueRecorder { const name = 'int-recorder'; const metric = meter['_metrics'].get(name) || @@ -95,8 +153,7 @@ export async function mockValueRecorder(): Promise { }); metric.clear(); metric.bind({}); - - return (await metric.getMetricRecord())[0]; + return metric; } const traceIdHex = '1f1008dc8e270e85c40a0d7c3939b278'; @@ -251,44 +308,44 @@ export const multiResourceTrace: ReadableSpan[] = [ }, ]; -export const multiResourceMetricsGet = async function (): Promise< - MetricRecord[] -> { +export const multiResourceMetricsGet = function ( + callback: (observerResult: ObserverResult) => void +): any[] { return [ { - ...(await mockCounter()), + ...mockCounter(), resource: mockedResources[0], instrumentationLibrary: mockedInstrumentationLibraries[0], }, { - ...(await mockObserver()), + ...mockObserver(callback), resource: mockedResources[1], instrumentationLibrary: mockedInstrumentationLibraries[0], }, { - ...(await mockCounter()), + ...mockCounter(), resource: mockedResources[0], instrumentationLibrary: mockedInstrumentationLibraries[0], }, ]; }; -export const multiInstrumentationLibraryMetricsGet = async function (): Promise< - MetricRecord[] -> { +export const multiInstrumentationLibraryMetricsGet = function ( + callback: (observerResult: ObserverResult) => void +): any[] { return [ { - ...(await mockCounter()), + ...mockCounter(), resource: mockedResources[0], instrumentationLibrary: mockedInstrumentationLibraries[0], }, { - ...(await mockObserver()), + ...mockObserver(callback), resource: mockedResources[0], instrumentationLibrary: mockedInstrumentationLibraries[1], }, { - ...(await mockCounter()), + ...mockCounter(), resource: mockedResources[0], instrumentationLibrary: mockedInstrumentationLibraries[0], }, @@ -574,6 +631,56 @@ export function ensureObserverIsCorrect( }); } +export function ensureSumObserverIsCorrect( + metric: collectorTypes.opentelemetryProto.metrics.v1.Metric, + time: number +) { + assert.deepStrictEqual(metric, { + name: 'double-sum-observer', + description: 'sample sum observer description', + unit: '1', + doubleSum: { + isMonotonic: true, + dataPoints: [ + { + labels: [], + value: 5, + startTimeUnixNano: 1592602232694000000, + timeUnixNano: time, + }, + ], + aggregationTemporality: + collectorTypes.opentelemetryProto.metrics.v1.AggregationTemporality + .AGGREGATION_TEMPORALITY_CUMULATIVE, + }, + }); +} + +export function ensureUpDownSumObserverIsCorrect( + metric: collectorTypes.opentelemetryProto.metrics.v1.Metric, + time: number +) { + assert.deepStrictEqual(metric, { + name: 'double-up-down-sum-observer', + description: 'sample up down sum observer description', + unit: '1', + doubleSum: { + isMonotonic: false, + dataPoints: [ + { + labels: [], + value: 4, + startTimeUnixNano: 1592602232694000000, + timeUnixNano: time, + }, + ], + aggregationTemporality: + collectorTypes.opentelemetryProto.metrics.v1.AggregationTemporality + .AGGREGATION_TEMPORALITY_CUMULATIVE, + }, + }); +} + export function ensureValueRecorderIsCorrect( metric: collectorTypes.opentelemetryProto.metrics.v1.Metric, time: number, diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts index 14d9314c79..5460419d60 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ConsoleLogger, ExportResultCode, LogLevel } from '@opentelemetry/core'; +import * as api from '@opentelemetry/api'; import * as core from '@opentelemetry/core'; import * as http from 'http'; import * as assert from 'assert'; @@ -32,7 +32,13 @@ import { ensureValueRecorderIsCorrect, ensureObserverIsCorrect, } from '../helper'; -import { MetricRecord } from '@opentelemetry/metrics'; +import { + BoundCounter, + BoundObserver, + BoundValueRecorder, + Metric, + MetricRecord, +} from '@opentelemetry/metrics'; const fakeRequest = { end: function () {}, @@ -55,7 +61,7 @@ describe('CollectorMetricExporter - node with json over http', () => { describe('instance', () => { it('should warn about metadata when using json', () => { const metadata = 'foo'; - const logger = new ConsoleLogger(LogLevel.DEBUG); + const logger = new core.ConsoleLogger(core.LogLevel.DEBUG); const spyLoggerWarn = sinon.stub(logger, 'warn'); collectorExporter = new CollectorMetricExporter({ logger, @@ -88,14 +94,22 @@ describe('CollectorMetricExporter - node with json over http', () => { value: 1592602232694000000, }); metrics = []; - metrics.push(await mockCounter()); - metrics.push(await mockObserver()); - metrics.push(await mockValueRecorder()); - metrics[0].aggregator.update(1); - metrics[1].aggregator.update(3); - metrics[1].aggregator.update(6); - metrics[2].aggregator.update(7); - metrics[2].aggregator.update(14); + const counter: Metric & api.Counter = mockCounter(); + const observer: Metric & api.ValueObserver = mockObserver( + observerResult => { + observerResult.observe(3, {}); + observerResult.observe(6, {}); + } + ); + const recorder: Metric & + api.ValueRecorder = mockValueRecorder(); + counter.add(1); + recorder.record(7); + recorder.record(14); + + metrics.push((await counter.getMetricRecord())[0]); + metrics.push((await observer.getMetricRecord())[0]); + metrics.push((await recorder.getMetricRecord())[0]); }); afterEach(() => { @@ -183,7 +197,7 @@ describe('CollectorMetricExporter - node with json over http', () => { assert.strictEqual(spyLoggerError.args.length, 0); assert.strictEqual( responseSpy.args[0][0].code, - ExportResultCode.SUCCESS + core.ExportResultCode.SUCCESS ); done(); }); diff --git a/packages/opentelemetry-exporter-prometheus/test/ExactProcessor.ts b/packages/opentelemetry-exporter-prometheus/test/ExactProcessor.ts index 9e0e45b9f5..5ea8d343b7 100644 --- a/packages/opentelemetry-exporter-prometheus/test/ExactProcessor.ts +++ b/packages/opentelemetry-exporter-prometheus/test/ExactProcessor.ts @@ -17,7 +17,7 @@ import { Aggregator, MetricDescriptor, MetricRecord, - Processor + Processor, } from '@opentelemetry/metrics'; type Constructor = new (...args: T[]) => R; diff --git a/packages/opentelemetry-metrics/src/MonotonicObserverResult.ts b/packages/opentelemetry-metrics/src/MonotonicObserverResult.ts index 480c196169..aee4ad0a55 100644 --- a/packages/opentelemetry-metrics/src/MonotonicObserverResult.ts +++ b/packages/opentelemetry-metrics/src/MonotonicObserverResult.ts @@ -19,7 +19,10 @@ import { ObserverResult } from './ObserverResult'; export class MonotonicObserverResult extends ObserverResult { observe(value: number, labels: Labels): void { if (value >= 0) { - this.values.set(labels, value); + const currentValue = this.values.get(labels) || 0; + if (value >= currentValue) { + this.values.set(labels, value); + } } } } diff --git a/packages/opentelemetry-metrics/src/ObserverResult.ts b/packages/opentelemetry-metrics/src/ObserverResult.ts index 60d555641f..24db9c61a6 100644 --- a/packages/opentelemetry-metrics/src/ObserverResult.ts +++ b/packages/opentelemetry-metrics/src/ObserverResult.ts @@ -26,6 +26,8 @@ export class ObserverResult implements TypeObserverResult { values: Map = new Map(); observe(value: number, labels: Labels): void { - this.values.set(labels, value); + if (value >= 0) { + this.values.set(labels, value); + } } } diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index fd8d3a6af6..78e6a7159b 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -997,7 +997,7 @@ describe('Meter', () => { function getValue() { counter++; if (counter % 2 === 0) { - return -1; + return 2; } return 3; } @@ -1030,7 +1030,7 @@ describe('Meter', () => { metricRecords = await upDownSumObserver.getMetricRecord(); assert.strictEqual(metricRecords.length, 1); point = metricRecords[0].aggregator.toPoint(); - assert.strictEqual(point.value, -1); + assert.strictEqual(point.value, 2); metricRecords = await upDownSumObserver.getMetricRecord(); assert.strictEqual(metricRecords.length, 1); From f67ffe41799fb96652464286fce1458908ce3b8a Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 27 Nov 2020 22:50:12 +0100 Subject: [PATCH 3/4] chore: refactoring tests for rest of collectors --- .../test/CollectorMetricExporter.test.ts | 29 ++++--- .../test/helper.ts | 59 ++++++-------- .../test/CollectorMetricExporter.test.ts | 30 ++++--- .../test/helper.ts | 59 ++++++-------- .../test/helper.ts | 78 ++++++++----------- .../src/xhr.ts | 4 +- packages/opentelemetry-web/src/utils.ts | 4 +- 7 files changed, 122 insertions(+), 141 deletions(-) diff --git a/packages/opentelemetry-exporter-collector-grpc/test/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector-grpc/test/CollectorMetricExporter.test.ts index 37f944b2be..8a80a336ac 100644 --- a/packages/opentelemetry-exporter-collector-grpc/test/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector-grpc/test/CollectorMetricExporter.test.ts @@ -22,7 +22,6 @@ import * as fs from 'fs'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { collectorTypes } from '@opentelemetry/exporter-collector'; -import { MetricRecord } from '@opentelemetry/metrics'; import { CollectorMetricExporter } from '../src'; import { mockCounter, @@ -35,6 +34,8 @@ import { mockValueRecorder, } from './helper'; import { ConsoleLogger, LogLevel } from '@opentelemetry/core'; +import * as api from '@opentelemetry/api'; +import * as metrics from '@opentelemetry/metrics'; const metricsServiceProtoPath = 'opentelemetry/proto/collector/metrics/v1/metrics_service.proto'; @@ -59,7 +60,7 @@ const testCollectorMetricExporter = (params: TestParams) => let exportedData: | collectorTypes.opentelemetryProto.metrics.v1.ResourceMetrics[] | undefined; - let metrics: MetricRecord[]; + let metrics: metrics.MetricRecord[]; let reqMetadata: grpc.Metadata | undefined; before(done => { @@ -134,17 +135,23 @@ const testCollectorMetricExporter = (params: TestParams) => value: 1592602232694000000, }); metrics = []; - metrics.push(await mockCounter()); - metrics.push(await mockObserver()); - metrics.push(await mockValueRecorder()); - - metrics[0].aggregator.update(1); + const counter: metrics.Metric & + api.Counter = mockCounter(); + const observer: metrics.Metric & + api.ValueObserver = mockObserver(observerResult => { + observerResult.observe(3, {}); + observerResult.observe(6, {}); + }); + const recorder: metrics.Metric & + api.ValueRecorder = mockValueRecorder(); - metrics[1].aggregator.update(3); - metrics[1].aggregator.update(6); + counter.add(1); + recorder.record(7); + recorder.record(14); - metrics[2].aggregator.update(7); - metrics[2].aggregator.update(14); + metrics.push((await counter.getMetricRecord())[0]); + metrics.push((await observer.getMetricRecord())[0]); + metrics.push((await recorder.getMetricRecord())[0]); }); afterEach(() => { diff --git a/packages/opentelemetry-exporter-collector-grpc/test/helper.ts b/packages/opentelemetry-exporter-collector-grpc/test/helper.ts index fbab887ba7..a45eecff16 100644 --- a/packages/opentelemetry-exporter-collector-grpc/test/helper.ts +++ b/packages/opentelemetry-exporter-collector-grpc/test/helper.ts @@ -14,15 +14,15 @@ * limitations under the License. */ -import { TraceFlags, ValueType, StatusCode } from '@opentelemetry/api'; +import * as api from '@opentelemetry/api'; +import * as metrics from '@opentelemetry/metrics'; import { ReadableSpan } from '@opentelemetry/tracing'; import { Resource } from '@opentelemetry/resources'; import { collectorTypes } from '@opentelemetry/exporter-collector'; import * as assert from 'assert'; -import { MetricRecord, MeterProvider } from '@opentelemetry/metrics'; import * as grpc from 'grpc'; -const meterProvider = new MeterProvider({ +const meterProvider = new metrics.MeterProvider({ interval: 30000, resource: new Resource({ service: 'ui', @@ -54,61 +54,52 @@ const traceIdArr = [ const spanIdArr = [94, 16, 114, 97, 246, 79, 165, 62]; const parentIdArr = [120, 168, 145, 80, 152, 134, 67, 136]; -export async function mockCounter(): Promise { +export function mockCounter(): metrics.Metric & + api.Counter { const name = 'int-counter'; const metric = meter['_metrics'].get(name) || meter.createCounter(name, { description: 'sample counter description', - valueType: ValueType.INT, + valueType: api.ValueType.INT, }); metric.clear(); metric.bind({}); - - return (await metric.getMetricRecord())[0]; -} - -export async function mockDoubleCounter(): Promise { - const name = 'double-counter'; - const metric = - meter['_metrics'].get(name) || - meter.createCounter(name, { - description: 'sample counter description', - valueType: ValueType.DOUBLE, - }); - metric.clear(); - metric.bind({}); - - return (await metric.getMetricRecord())[0]; + return metric; } -export async function mockObserver(): Promise { +export function mockObserver( + callback: (observerResult: api.ObserverResult) => void +): metrics.Metric & api.ValueObserver { const name = 'double-observer'; const metric = meter['_metrics'].get(name) || - meter.createValueObserver(name, { - description: 'sample observer description', - valueType: ValueType.DOUBLE, - }); + meter.createValueObserver( + name, + { + description: 'sample observer description', + valueType: api.ValueType.DOUBLE, + }, + callback + ); metric.clear(); metric.bind({}); - - return (await metric.getMetricRecord())[0]; + return metric; } -export async function mockValueRecorder(): Promise { +export function mockValueRecorder(): metrics.Metric & + api.ValueRecorder { const name = 'int-recorder'; const metric = meter['_metrics'].get(name) || meter.createValueRecorder(name, { description: 'sample recorder description', - valueType: ValueType.INT, + valueType: api.ValueType.INT, boundaries: [0, 100], }); metric.clear(); metric.bind({}); - - return (await metric.getMetricRecord())[0]; + return metric; } export const mockedReadableSpan: ReadableSpan = { @@ -117,13 +108,13 @@ export const mockedReadableSpan: ReadableSpan = { spanContext: { traceId: '1f1008dc8e270e85c40a0d7c3939b278', spanId: '5e107261f64fa53e', - traceFlags: TraceFlags.SAMPLED, + traceFlags: api.TraceFlags.SAMPLED, }, parentSpanId: '78a8915098864388', startTime: [1574120165, 429803070], endTime: [1574120165, 438688070], ended: true, - status: { code: StatusCode.OK }, + status: { code: api.StatusCode.OK }, attributes: { component: 'document-load' }, links: [ { diff --git a/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts index 109ff576b7..0a943a737f 100644 --- a/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +import * as api from '@opentelemetry/api'; +import * as metrics from '@opentelemetry/metrics'; import { collectorTypes } from '@opentelemetry/exporter-collector'; import * as core from '@opentelemetry/core'; import * as http from 'http'; @@ -32,7 +34,6 @@ import { ensureExportedValueRecorderIsCorrect, MockedResponse, } from './helper'; -import { MetricRecord } from '@opentelemetry/metrics'; import { ExportResult, ExportResultCode } from '@opentelemetry/core'; import { CollectorExporterError } from '@opentelemetry/exporter-collector/build/src/types'; @@ -50,7 +51,7 @@ describe('CollectorMetricExporter - node with proto over http', () => { let collectorExporterConfig: collectorTypes.CollectorExporterConfigBase; let spyRequest: sinon.SinonSpy; let spyWrite: sinon.SinonSpy; - let metrics: MetricRecord[]; + let metrics: metrics.MetricRecord[]; describe('export', () => { beforeEach(async () => { spyRequest = sinon.stub(http, 'request').returns(fakeRequest as any); @@ -71,14 +72,23 @@ describe('CollectorMetricExporter - node with proto over http', () => { value: 1592602232694000000, }); metrics = []; - metrics.push(await mockCounter()); - metrics.push(await mockObserver()); - metrics.push(await mockValueRecorder()); - metrics[0].aggregator.update(1); - metrics[1].aggregator.update(3); - metrics[1].aggregator.update(6); - metrics[2].aggregator.update(7); - metrics[2].aggregator.update(14); + const counter: metrics.Metric & + api.Counter = mockCounter(); + const observer: metrics.Metric & + api.ValueObserver = mockObserver(observerResult => { + observerResult.observe(3, {}); + observerResult.observe(6, {}); + }); + const recorder: metrics.Metric & + api.ValueRecorder = mockValueRecorder(); + + counter.add(1); + recorder.record(7); + recorder.record(14); + + metrics.push((await counter.getMetricRecord())[0]); + metrics.push((await observer.getMetricRecord())[0]); + metrics.push((await recorder.getMetricRecord())[0]); }); afterEach(() => { spyRequest.restore(); diff --git a/packages/opentelemetry-exporter-collector-proto/test/helper.ts b/packages/opentelemetry-exporter-collector-proto/test/helper.ts index b213d0f14f..a2edcfd9f4 100644 --- a/packages/opentelemetry-exporter-collector-proto/test/helper.ts +++ b/packages/opentelemetry-exporter-collector-proto/test/helper.ts @@ -14,16 +14,16 @@ * limitations under the License. */ -import { TraceFlags, ValueType, StatusCode } from '@opentelemetry/api'; +import * as api from '@opentelemetry/api'; +import * as metrics from '@opentelemetry/metrics'; import { hexToBase64 } from '@opentelemetry/core'; import { ReadableSpan } from '@opentelemetry/tracing'; import { Resource } from '@opentelemetry/resources'; import { collectorTypes } from '@opentelemetry/exporter-collector'; import * as assert from 'assert'; -import { MeterProvider, MetricRecord } from '@opentelemetry/metrics'; import { Stream } from 'stream'; -const meterProvider = new MeterProvider({ +const meterProvider = new metrics.MeterProvider({ interval: 30000, resource: new Resource({ service: 'ui', @@ -34,61 +34,52 @@ const meterProvider = new MeterProvider({ const meter = meterProvider.getMeter('default', '0.0.1'); -export async function mockCounter(): Promise { +export function mockCounter(): metrics.Metric & + api.Counter { const name = 'int-counter'; const metric = meter['_metrics'].get(name) || meter.createCounter(name, { description: 'sample counter description', - valueType: ValueType.INT, + valueType: api.ValueType.INT, }); metric.clear(); metric.bind({}); - - return (await metric.getMetricRecord())[0]; -} - -export async function mockDoubleCounter(): Promise { - const name = 'double-counter'; - const metric = - meter['_metrics'].get(name) || - meter.createCounter(name, { - description: 'sample counter description', - valueType: ValueType.DOUBLE, - }); - metric.clear(); - metric.bind({}); - - return (await metric.getMetricRecord())[0]; + return metric; } -export async function mockObserver(): Promise { +export function mockObserver( + callback: (observerResult: api.ObserverResult) => void +): metrics.Metric & api.ValueObserver { const name = 'double-observer'; const metric = meter['_metrics'].get(name) || - meter.createValueObserver(name, { - description: 'sample observer description', - valueType: ValueType.DOUBLE, - }); + meter.createValueObserver( + name, + { + description: 'sample observer description', + valueType: api.ValueType.DOUBLE, + }, + callback + ); metric.clear(); metric.bind({}); - - return (await metric.getMetricRecord())[0]; + return metric; } -export async function mockValueRecorder(): Promise { +export function mockValueRecorder(): metrics.Metric & + api.ValueRecorder { const name = 'int-recorder'; const metric = meter['_metrics'].get(name) || meter.createValueRecorder(name, { description: 'sample recorder description', - valueType: ValueType.INT, + valueType: api.ValueType.INT, boundaries: [0, 100], }); metric.clear(); metric.bind({}); - - return (await metric.getMetricRecord())[0]; + return metric; } const traceIdHex = '1f1008dc8e270e85c40a0d7c3939b278'; @@ -101,13 +92,13 @@ export const mockedReadableSpan: ReadableSpan = { spanContext: { traceId: traceIdHex, spanId: spanIdHex, - traceFlags: TraceFlags.SAMPLED, + traceFlags: api.TraceFlags.SAMPLED, }, parentSpanId: parentIdHex, startTime: [1574120165, 429803070], endTime: [1574120165, 438688070], ended: true, - status: { code: StatusCode.OK }, + status: { code: api.StatusCode.OK }, attributes: { component: 'document-load' }, links: [ { diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index d673a9f786..67ba81f779 100644 --- a/packages/opentelemetry-exporter-collector/test/helper.ts +++ b/packages/opentelemetry-exporter-collector/test/helper.ts @@ -14,32 +14,16 @@ * limitations under the License. */ -import { - TraceFlags, - ValueType, - StatusCode, - SumObserver, - UpDownSumObserver, - ValueObserver, - ValueRecorder, - Counter, - ObserverResult, -} from '@opentelemetry/api'; +import * as api from '@opentelemetry/api'; +import * as metrics from '@opentelemetry/metrics'; import { ReadableSpan } from '@opentelemetry/tracing'; import { Resource } from '@opentelemetry/resources'; -import { - MeterProvider, - Metric, - BoundCounter, - BoundObserver, - BoundValueRecorder, -} from '@opentelemetry/metrics'; import { hexToBase64, InstrumentationLibrary } from '@opentelemetry/core'; import * as assert from 'assert'; import { opentelemetryProto } from '../src/types'; import * as collectorTypes from '../src/types'; -const meterProvider = new MeterProvider({ +const meterProvider = new metrics.MeterProvider({ interval: 30000, resource: new Resource({ service: 'ui', @@ -58,26 +42,28 @@ if (typeof Buffer === 'undefined') { }; } -export function mockCounter(): Metric & Counter { +export function mockCounter(): metrics.Metric & + api.Counter { const name = 'int-counter'; const metric = meter['_metrics'].get(name) || meter.createCounter(name, { description: 'sample counter description', - valueType: ValueType.INT, + valueType: api.ValueType.INT, }); metric.clear(); metric.bind({}); return metric; } -export function mockDoubleCounter(): Metric & Counter { +export function mockDoubleCounter(): metrics.Metric & + api.Counter { const name = 'double-counter'; const metric = meter['_metrics'].get(name) || meter.createCounter(name, { description: 'sample counter description', - valueType: ValueType.DOUBLE, + valueType: api.ValueType.DOUBLE, }); metric.clear(); metric.bind({}); @@ -85,8 +71,8 @@ export function mockDoubleCounter(): Metric & Counter { } export function mockObserver( - callback: (observerResult: ObserverResult) => void -): Metric & ValueObserver { + callback: (observerResult: api.ObserverResult) => unknown +): metrics.Metric & api.ValueObserver { const name = 'double-observer'; const metric = meter['_metrics'].get(name) || @@ -94,7 +80,7 @@ export function mockObserver( name, { description: 'sample observer description', - valueType: ValueType.DOUBLE, + valueType: api.ValueType.DOUBLE, }, callback ); @@ -104,8 +90,8 @@ export function mockObserver( } export function mockSumObserver( - callback: (observerResult: ObserverResult) => void -): Metric & SumObserver { + callback: (observerResult: api.ObserverResult) => unknown +): metrics.Metric & api.SumObserver { const name = 'double-sum-observer'; const metric = meter['_metrics'].get(name) || @@ -113,7 +99,7 @@ export function mockSumObserver( name, { description: 'sample sum observer description', - valueType: ValueType.DOUBLE, + valueType: api.ValueType.DOUBLE, }, callback ); @@ -123,8 +109,8 @@ export function mockSumObserver( } export function mockUpDownSumObserver( - callback: (observerResult: ObserverResult) => void -): Metric & UpDownSumObserver { + callback: (observerResult: api.ObserverResult) => unknown +): metrics.Metric & api.UpDownSumObserver { const name = 'double-up-down-sum-observer'; const metric = meter['_metrics'].get(name) || @@ -132,7 +118,7 @@ export function mockUpDownSumObserver( name, { description: 'sample up down sum observer description', - valueType: ValueType.DOUBLE, + valueType: api.ValueType.DOUBLE, }, callback ); @@ -141,14 +127,14 @@ export function mockUpDownSumObserver( return metric; } -export function mockValueRecorder(): Metric & - ValueRecorder { +export function mockValueRecorder(): metrics.Metric & + api.ValueRecorder { const name = 'int-recorder'; const metric = meter['_metrics'].get(name) || meter.createValueRecorder(name, { description: 'sample recorder description', - valueType: ValueType.INT, + valueType: api.ValueType.INT, boundaries: [0, 100], }); metric.clear(); @@ -166,13 +152,13 @@ export const mockedReadableSpan: ReadableSpan = { spanContext: { traceId: '1f1008dc8e270e85c40a0d7c3939b278', spanId: '5e107261f64fa53e', - traceFlags: TraceFlags.SAMPLED, + traceFlags: api.TraceFlags.SAMPLED, }, parentSpanId: '78a8915098864388', startTime: [1574120165, 429803070], endTime: [1574120165, 438688070], ended: true, - status: { code: StatusCode.OK }, + status: { code: api.StatusCode.OK }, attributes: { component: 'document-load' }, links: [ { @@ -237,13 +223,13 @@ export const basicTrace: ReadableSpan[] = [ spanContext: { traceId: '1f1008dc8e270e85c40a0d7c3939b278', spanId: '5e107261f64fa53e', - traceFlags: TraceFlags.SAMPLED, + traceFlags: api.TraceFlags.SAMPLED, }, parentSpanId: '78a8915098864388', startTime: [1574120165, 429803070], endTime: [1574120165, 438688070], ended: true, - status: { code: StatusCode.OK }, + status: { code: api.StatusCode.OK }, attributes: {}, links: [], events: [], @@ -257,13 +243,13 @@ export const basicTrace: ReadableSpan[] = [ spanContext: { traceId: '1f1008dc8e270e85c40a0d7c3939b278', spanId: 'f64fa53e5e107261', - traceFlags: TraceFlags.SAMPLED, + traceFlags: api.TraceFlags.SAMPLED, }, parentSpanId: '78a8915098864388', startTime: [1575120165, 439803070], endTime: [1575120165, 448688070], ended: true, - status: { code: StatusCode.OK }, + status: { code: api.StatusCode.OK }, attributes: {}, links: [], events: [], @@ -277,13 +263,13 @@ export const basicTrace: ReadableSpan[] = [ spanContext: { traceId: '1f1008dc8e270e85c40a0d7c3939b278', spanId: '07261f64fa53e5e1', - traceFlags: TraceFlags.SAMPLED, + traceFlags: api.TraceFlags.SAMPLED, }, parentSpanId: 'a891578098864388', startTime: [1575120165, 439803070], endTime: [1575120165, 448688070], ended: true, - status: { code: StatusCode.OK }, + status: { code: api.StatusCode.OK }, attributes: {}, links: [], events: [], @@ -309,7 +295,7 @@ export const multiResourceTrace: ReadableSpan[] = [ ]; export const multiResourceMetricsGet = function ( - callback: (observerResult: ObserverResult) => void + callback: (observerResult: api.ObserverResult) => unknown ): any[] { return [ { @@ -331,7 +317,7 @@ export const multiResourceMetricsGet = function ( }; export const multiInstrumentationLibraryMetricsGet = function ( - callback: (observerResult: ObserverResult) => void + callback: (observerResult: api.ObserverResult) => unknown ): any[] { return [ { @@ -521,7 +507,7 @@ export function ensureSpanIsCorrect( assert.strictEqual(span.droppedLinksCount, 0, 'droppedLinksCount is wrong'); assert.deepStrictEqual( span.status, - { code: StatusCode.OK }, + { code: api.StatusCode.OK }, 'status is wrong' ); } diff --git a/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts b/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts index f0ee26468d..70fa9cf5d8 100644 --- a/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts +++ b/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts @@ -68,9 +68,7 @@ export interface XMLHttpRequestInstrumentationConfig /** * This class represents a XMLHttpRequest plugin for auto instrumentation */ -export class XMLHttpRequestInstrumentation extends InstrumentationBase< - XMLHttpRequest -> { +export class XMLHttpRequestInstrumentation extends InstrumentationBase { readonly component: string = 'xml-http-request'; readonly version: string = VERSION; moduleName = this.component; diff --git a/packages/opentelemetry-web/src/utils.ts b/packages/opentelemetry-web/src/utils.ts index 385cb1d05f..6b94959f96 100644 --- a/packages/opentelemetry-web/src/utils.ts +++ b/packages/opentelemetry-web/src/utils.ts @@ -125,9 +125,7 @@ export function getResource( startTimeHR: api.HrTime, endTimeHR: api.HrTime, resources: PerformanceResourceTiming[], - ignoredResources: WeakSet = new WeakSet< - PerformanceResourceTiming - >(), + ignoredResources: WeakSet = new WeakSet(), initiatorType?: string ): PerformanceResourceTimingInfo { // de-relativize the URL before usage (does no harm to absolute URLs) From 1b3e137e200dd646db66e49017d424c5ca25871a Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Wed, 2 Dec 2020 00:31:42 +0100 Subject: [PATCH 4/4] chore: fixing monotonic sum observer, updated test to cover that scenario correctly --- .../browser/CollectorMetricExporter.test.ts | 11 +++-- .../common/CollectorMetricExporter.test.ts | 3 +- .../test/common/transformMetrics.test.ts | 48 +++++++++++++------ .../test/helper.ts | 38 ++++++++------- .../test/node/CollectorMetricExporter.test.ts | 8 ++-- .../src/BaseObserverMetric.ts | 14 +++--- .../src/MonotonicObserverResult.ts | 28 ----------- .../src/ObserverResult.ts | 4 +- .../src/SumObserverMetric.ts | 21 ++++++-- .../opentelemetry-metrics/test/Meter.test.ts | 7 +-- 10 files changed, 99 insertions(+), 83 deletions(-) delete mode 100644 packages/opentelemetry-metrics/src/MonotonicObserverResult.ts diff --git a/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts index 1b1bb8d7c0..934dc72c15 100644 --- a/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts @@ -60,7 +60,8 @@ describe('CollectorMetricExporter - web', () => { observerResult => { observerResult.observe(3, {}); observerResult.observe(6, {}); - } + }, + 'double-observer2' ); const recorder: Metric & api.ValueRecorder = mockValueRecorder(); @@ -125,7 +126,9 @@ describe('CollectorMetricExporter - web', () => { if (metric2) { ensureObserverIsCorrect( metric2, - hrTimeToNanoseconds(metrics[1].aggregator.toPoint().timestamp) + hrTimeToNanoseconds(metrics[1].aggregator.toPoint().timestamp), + 6, + 'double-observer2' ); } @@ -239,7 +242,9 @@ describe('CollectorMetricExporter - web', () => { if (metric2) { ensureObserverIsCorrect( metric2, - hrTimeToNanoseconds(metrics[1].aggregator.toPoint().timestamp) + hrTimeToNanoseconds(metrics[1].aggregator.toPoint().timestamp), + 6, + 'double-observer2' ); } diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts index 4e2384417c..82b8186ba6 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts @@ -74,7 +74,8 @@ describe('CollectorMetricExporter - common', () => { observerResult => { observerResult.observe(3, {}); observerResult.observe(6, {}); - } + }, + 'double-observer3' ); counter.add(1); diff --git a/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts b/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts index b1c7f26513..1b89f89668 100644 --- a/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts @@ -55,23 +55,30 @@ describe('transformMetrics', () => { beforeEach(() => { counter = mockCounter(); doubleCounter = mockDoubleCounter(); + let count1 = 0; + let count2 = 0; + let count3 = 0; + + function getValue(count: number) { + if (count % 2 == 0) { + return 3; + } + return -1; + } + observer = mockObserver(observerResult => { - observerResult.observe(3, {}); - observerResult.observe(6, {}); + count1++; + observerResult.observe(getValue(count1), {}); }); + sumObserver = mockSumObserver(observerResult => { - const labels = {}; - observerResult.observe(3, labels); - observerResult.observe(5, labels); - observerResult.observe(4, labels); - observerResult.observe(-1, labels); + count2++; + observerResult.observe(getValue(count2), {}); }); + upDownSumObserver = mockUpDownSumObserver(observerResult => { - const labels = {}; - observerResult.observe(3, labels); - observerResult.observe(5, labels); - observerResult.observe(4, labels); - observerResult.observe(-1, labels); + count3++; + observerResult.observe(getValue(count3), {}); }); recorder = mockValueRecorder(); @@ -100,18 +107,28 @@ describe('transformMetrics', () => { hrTimeToNanoseconds(doubleCounterMetric.aggregator.toPoint().timestamp) ); + await observer.getMetricRecord(); + await observer.getMetricRecord(); const observerMetric = (await observer.getMetricRecord())[0]; ensureObserverIsCorrect( transform.toCollectorMetric(observerMetric, 1592602232694000000), - hrTimeToNanoseconds(observerMetric.aggregator.toPoint().timestamp) + hrTimeToNanoseconds(observerMetric.aggregator.toPoint().timestamp), + -1 ); + // collect 3 times + await sumObserver.getMetricRecord(); + await sumObserver.getMetricRecord(); const sumObserverMetric = (await sumObserver.getMetricRecord())[0]; ensureSumObserverIsCorrect( transform.toCollectorMetric(sumObserverMetric, 1592602232694000000), - hrTimeToNanoseconds(sumObserverMetric.aggregator.toPoint().timestamp) + hrTimeToNanoseconds(sumObserverMetric.aggregator.toPoint().timestamp), + 3 ); + // collect 3 times + await upDownSumObserver.getMetricRecord(); + await upDownSumObserver.getMetricRecord(); const upDownSumObserverMetric = ( await upDownSumObserver.getMetricRecord() )[0]; @@ -122,7 +139,8 @@ describe('transformMetrics', () => { ), hrTimeToNanoseconds( upDownSumObserverMetric.aggregator.toPoint().timestamp - ) + ), + -1 ); const recorderMetric = (await recorder.getMetricRecord())[0]; diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index 67ba81f779..b451bd1443 100644 --- a/packages/opentelemetry-exporter-collector/test/helper.ts +++ b/packages/opentelemetry-exporter-collector/test/helper.ts @@ -71,9 +71,9 @@ export function mockDoubleCounter(): metrics.Metric & } export function mockObserver( - callback: (observerResult: api.ObserverResult) => unknown -): metrics.Metric & api.ValueObserver { - const name = 'double-observer'; + callback: (observerResult: api.ObserverResult) => unknown, + name = 'double-observer' +): metrics.Metric & api.ValueObserver { const metric = meter['_metrics'].get(name) || meter.createValueObserver( @@ -90,9 +90,9 @@ export function mockObserver( } export function mockSumObserver( - callback: (observerResult: api.ObserverResult) => unknown + callback: (observerResult: api.ObserverResult) => unknown, + name = 'double-sum-observer' ): metrics.Metric & api.SumObserver { - const name = 'double-sum-observer'; const metric = meter['_metrics'].get(name) || meter.createSumObserver( @@ -109,9 +109,9 @@ export function mockSumObserver( } export function mockUpDownSumObserver( - callback: (observerResult: api.ObserverResult) => unknown + callback: (observerResult: api.ObserverResult) => unknown, + name = 'double-up-down-sum-observer' ): metrics.Metric & api.UpDownSumObserver { - const name = 'double-up-down-sum-observer'; const metric = meter['_metrics'].get(name) || meter.createUpDownSumObserver( @@ -598,17 +598,19 @@ export function ensureDoubleCounterIsCorrect( export function ensureObserverIsCorrect( metric: collectorTypes.opentelemetryProto.metrics.v1.Metric, - time: number + time: number, + value: number, + name = 'double-observer' ) { assert.deepStrictEqual(metric, { - name: 'double-observer', + name, description: 'sample observer description', unit: '1', doubleGauge: { dataPoints: [ { labels: [], - value: 6, + value, startTimeUnixNano: 1592602232694000000, timeUnixNano: time, }, @@ -619,10 +621,12 @@ export function ensureObserverIsCorrect( export function ensureSumObserverIsCorrect( metric: collectorTypes.opentelemetryProto.metrics.v1.Metric, - time: number + time: number, + value: number, + name = 'double-sum-observer' ) { assert.deepStrictEqual(metric, { - name: 'double-sum-observer', + name, description: 'sample sum observer description', unit: '1', doubleSum: { @@ -630,7 +634,7 @@ export function ensureSumObserverIsCorrect( dataPoints: [ { labels: [], - value: 5, + value, startTimeUnixNano: 1592602232694000000, timeUnixNano: time, }, @@ -644,10 +648,12 @@ export function ensureSumObserverIsCorrect( export function ensureUpDownSumObserverIsCorrect( metric: collectorTypes.opentelemetryProto.metrics.v1.Metric, - time: number + time: number, + value: number, + name = 'double-up-down-sum-observer' ) { assert.deepStrictEqual(metric, { - name: 'double-up-down-sum-observer', + name, description: 'sample up down sum observer description', unit: '1', doubleSum: { @@ -655,7 +661,7 @@ export function ensureUpDownSumObserverIsCorrect( dataPoints: [ { labels: [], - value: 4, + value, startTimeUnixNano: 1592602232694000000, timeUnixNano: time, }, diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts index 388c244ccd..ecf87d3983 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts @@ -93,9 +93,9 @@ describe('CollectorMetricExporter - node with json over http', () => { const counter: Metric & api.Counter = mockCounter(); const observer: Metric & api.ValueObserver = mockObserver( observerResult => { - observerResult.observe(3, {}); observerResult.observe(6, {}); - } + }, + 'double-observer2' ); const recorder: Metric & api.ValueRecorder = mockValueRecorder(); @@ -161,7 +161,9 @@ describe('CollectorMetricExporter - node with json over http', () => { assert.ok(typeof metric2 !== 'undefined', "observer doesn't exist"); ensureObserverIsCorrect( metric2, - core.hrTimeToNanoseconds(metrics[1].aggregator.toPoint().timestamp) + core.hrTimeToNanoseconds(metrics[1].aggregator.toPoint().timestamp), + 6, + 'double-observer2' ); assert.ok(typeof metric3 !== 'undefined', "histogram doesn't exist"); ensureValueRecorderIsCorrect( diff --git a/packages/opentelemetry-metrics/src/BaseObserverMetric.ts b/packages/opentelemetry-metrics/src/BaseObserverMetric.ts index 9eeea46b94..0b10f60ec3 100644 --- a/packages/opentelemetry-metrics/src/BaseObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/BaseObserverMetric.ts @@ -56,18 +56,20 @@ export abstract class BaseObserverMetric ); } - protected createObserverResult(): ObserverResult { - return new ObserverResult(); - } - async getMetricRecord(): Promise { - const observerResult = this.createObserverResult(); + const observerResult = new ObserverResult(); await this._callback(observerResult); + + this._processResults(observerResult); + + return super.getMetricRecord(); + } + + protected _processResults(observerResult: ObserverResult) { observerResult.values.forEach((value, labels) => { const instrument = this.bind(labels); instrument.update(value); }); - return super.getMetricRecord(); } observation(value: number) { diff --git a/packages/opentelemetry-metrics/src/MonotonicObserverResult.ts b/packages/opentelemetry-metrics/src/MonotonicObserverResult.ts deleted file mode 100644 index aee4ad0a55..0000000000 --- a/packages/opentelemetry-metrics/src/MonotonicObserverResult.ts +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { Labels } from '@opentelemetry/api'; -import { ObserverResult } from './ObserverResult'; -export class MonotonicObserverResult extends ObserverResult { - observe(value: number, labels: Labels): void { - if (value >= 0) { - const currentValue = this.values.get(labels) || 0; - if (value >= currentValue) { - this.values.set(labels, value); - } - } - } -} diff --git a/packages/opentelemetry-metrics/src/ObserverResult.ts b/packages/opentelemetry-metrics/src/ObserverResult.ts index 24db9c61a6..60d555641f 100644 --- a/packages/opentelemetry-metrics/src/ObserverResult.ts +++ b/packages/opentelemetry-metrics/src/ObserverResult.ts @@ -26,8 +26,6 @@ export class ObserverResult implements TypeObserverResult { values: Map = new Map(); observe(value: number, labels: Labels): void { - if (value >= 0) { - this.values.set(labels, value); - } + this.values.set(labels, value); } } diff --git a/packages/opentelemetry-metrics/src/SumObserverMetric.ts b/packages/opentelemetry-metrics/src/SumObserverMetric.ts index 1eb8bc90bd..48a198ff9b 100644 --- a/packages/opentelemetry-metrics/src/SumObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/SumObserverMetric.ts @@ -18,10 +18,9 @@ import * as api from '@opentelemetry/api'; import { InstrumentationLibrary } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { BaseObserverMetric } from './BaseObserverMetric'; -import { ObserverResult } from './ObserverResult'; -import { MonotonicObserverResult } from './MonotonicObserverResult'; import { Processor } from './export/Processor'; -import { MetricKind } from './export/types'; +import { LastValue, MetricKind } from './export/types'; +import { ObserverResult } from './ObserverResult'; /** This is a SDK implementation of SumObserver Metric. */ export class SumObserverMetric @@ -46,7 +45,19 @@ export class SumObserverMetric ); } - protected createObserverResult(): ObserverResult { - return new MonotonicObserverResult(); + protected _processResults(observerResult: ObserverResult) { + observerResult.values.forEach((value, labels) => { + const instrument = this.bind(labels); + // SumObserver is monotonic which means it should only accept values + // greater or equal then previous value + const previous = instrument.getAggregator().toPoint(); + let previousValue = -Infinity; + if (previous.timestamp[0] !== 0 || previous.timestamp[1] !== 0) { + previousValue = previous.value as LastValue; + } + if (value >= previousValue) { + instrument.update(value); + } + }); } } diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 431b1d7136..957d21a497 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -801,10 +801,11 @@ describe('Meter', () => { let counter = 0; function getValue() { + console.log('getting value, counter:', counter); if (++counter % 2 == 0) { - return -1; + return 3; } - return 3; + return -1; } const sumObserver = meter.createSumObserver( @@ -826,7 +827,7 @@ describe('Meter', () => { let metricRecords = await sumObserver.getMetricRecord(); assert.strictEqual(metricRecords.length, 1); let point = metricRecords[0].aggregator.toPoint(); - assert.strictEqual(point.value, 3); + assert.strictEqual(point.value, -1); assert.strictEqual( hashLabels(metricRecords[0].labels), '|#core:1,pid:123'