diff --git a/examples/metrics/metrics/observer.js b/examples/metrics/metrics/observer.js index f8393122c18..aaece51489e 100644 --- a/examples/metrics/metrics/observer.js +++ b/examples/metrics/metrics/observer.js @@ -40,7 +40,7 @@ const cpuUsageMetric = meter.createValueObserver('cpu_usage_per_app', { description: 'Example of sync value observer used with async batch observer', }); -meter.createBatchObserver('metric_batch_observer', (observerBatchResult) => { +meter.createBatchObserver((observerBatchResult) => { Promise.all([ someAsyncMetrics(), // simulate waiting diff --git a/packages/opentelemetry-api/src/metrics/Meter.ts b/packages/opentelemetry-api/src/metrics/Meter.ts index eb570a48027..22fe74834b1 100644 --- a/packages/opentelemetry-api/src/metrics/Meter.ts +++ b/packages/opentelemetry-api/src/metrics/Meter.ts @@ -20,7 +20,6 @@ import { Counter, ValueRecorder, ValueObserver, - BatchObserver, BatchMetricOptions, UpDownCounter, } from './Metric'; @@ -82,15 +81,13 @@ export interface Meter { ): ValueObserver; /** - * Creates a new `BatchObserver` metric, can be used to update many metrics + * Creates a new `BatchObserver`, can be used to update many observer metrics * at the same time and when operations needs to be async - * @param name the name of the metric. * @param callback the batch observer callback * @param [options] the metric batch options. */ createBatchObserver( - name: string, callback: (batchObserverResult: BatchObserverResult) => void, options?: BatchMetricOptions - ): BatchObserver; + ): void; } diff --git a/packages/opentelemetry-api/src/metrics/Metric.ts b/packages/opentelemetry-api/src/metrics/Metric.ts index 083055fbed2..b0e2e1c9c31 100644 --- a/packages/opentelemetry-api/src/metrics/Metric.ts +++ b/packages/opentelemetry-api/src/metrics/Metric.ts @@ -166,9 +166,6 @@ export type UpDownSumObserver = BaseObserver; /** Base interface for the SumObserver metrics. */ export type SumObserver = BaseObserver; -/** Base interface for the Batch Observer metrics. */ -export type BatchObserver = Metric; - /** * key-value pairs passed by the user. */ diff --git a/packages/opentelemetry-api/src/metrics/NoopMeter.ts b/packages/opentelemetry-api/src/metrics/NoopMeter.ts index d59d658b1e3..cc73a1ccffa 100644 --- a/packages/opentelemetry-api/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-api/src/metrics/NoopMeter.ts @@ -23,7 +23,6 @@ import { Counter, ValueRecorder, ValueObserver, - BatchObserver, UpDownCounter, BaseObserver, } from './Metric'; @@ -90,10 +89,9 @@ export class NoopMeter implements Meter { * @param callback the batch observer callback */ createBatchObserver( - _name: string, _callback: (batchObserverResult: BatchObserverResult) => void - ): BatchObserver { - return NOOP_BATCH_OBSERVER_METRIC; + ): void { + return; } } @@ -131,24 +129,21 @@ export class NoopMetric implements UnboundMetric { } } -export class NoopCounterMetric - extends NoopMetric +export class NoopCounterMetric extends NoopMetric implements Counter { add(value: number, labels: Labels) { this.bind(labels).add(value); } } -export class NoopValueRecorderMetric - extends NoopMetric +export class NoopValueRecorderMetric extends NoopMetric implements ValueRecorder { record(value: number, labels: Labels) { this.bind(labels).record(value); } } -export class NoopBaseObserverMetric - extends NoopMetric +export class NoopBaseObserverMetric extends NoopMetric implements BaseObserver { observation() { return { @@ -158,10 +153,6 @@ export class NoopBaseObserverMetric } } -export class NoopBatchObserverMetric - extends NoopMetric - implements BatchObserver {} - export class NoopBoundCounter implements BoundCounter { add(_value: number): void { return; @@ -203,5 +194,3 @@ export const NOOP_UP_DOWN_SUM_OBSERVER_METRIC = new NoopBaseObserverMetric( export const NOOP_SUM_OBSERVER_METRIC = new NoopBaseObserverMetric( NOOP_BOUND_BASE_OBSERVER ); - -export const NOOP_BATCH_OBSERVER_METRIC = new NoopBatchObserverMetric(); diff --git a/packages/opentelemetry-metrics/README.md b/packages/opentelemetry-metrics/README.md index 7f200f9418e..1216aca0c52 100644 --- a/packages/opentelemetry-metrics/README.md +++ b/packages/opentelemetry-metrics/README.md @@ -222,7 +222,7 @@ const MemUsageMetric = meter.createValueObserver('mem_usage_per_app', { description: 'Memory', }); -meter.createBatchObserver('metric_batch_observer', (observerBatchResult) => { +meter.createBatchObserver((observerBatchResult) => { getSomeAsyncMetrics().then(metrics => { observerBatchResult.observe({ app: 'myApp' }, [ cpuUsageMetric.observation(metrics.value1), diff --git a/packages/opentelemetry-metrics/src/BatchObserverMetric.ts b/packages/opentelemetry-metrics/src/BatchObserver.ts similarity index 65% rename from packages/opentelemetry-metrics/src/BatchObserverMetric.ts rename to packages/opentelemetry-metrics/src/BatchObserver.ts index 241ff0813a0..81c68cfeba1 100644 --- a/packages/opentelemetry-metrics/src/BatchObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/BatchObserver.ts @@ -15,57 +15,32 @@ */ import * as api from '@opentelemetry/api'; -import { InstrumentationLibrary } from '@opentelemetry/core'; -import { Resource } from '@opentelemetry/resources'; +import { Logger, NoopLogger } from '@opentelemetry/api'; import { BatchObserverResult } from './BatchObserverResult'; -import { BoundObserver } from './BoundInstrument'; -import { Batcher } from './export/Batcher'; -import { MetricKind, MetricRecord } from './export/types'; -import { Metric } from './Metric'; +import { MetricRecord } from './export/types'; const NOOP_CALLBACK = () => {}; const MAX_TIMEOUT_UPDATE_MS = 500; /** This is a SDK implementation of Batch Observer Metric. */ -export class BatchObserverMetric - extends Metric - implements api.BatchObserver { +export class BatchObserver { private _callback: (observerResult: api.BatchObserverResult) => void; private _maxTimeoutUpdateMS: number; + private _logger: Logger; constructor( - name: string, options: api.BatchMetricOptions, - private readonly _batcher: Batcher, - resource: Resource, - instrumentationLibrary: InstrumentationLibrary, callback?: (observerResult: api.BatchObserverResult) => void ) { - super( - name, - options, - MetricKind.BATCH_OBSERVER, - resource, - instrumentationLibrary - ); + this._logger = options.logger ?? new NoopLogger(); this._maxTimeoutUpdateMS = options.maxTimeoutUpdateMS ?? MAX_TIMEOUT_UPDATE_MS; this._callback = callback || NOOP_CALLBACK; } - protected _makeInstrument(labels: api.Labels): BoundObserver { - return new BoundObserver( - labels, - this._disabled, - this._valueType, - this._logger, - this._batcher.aggregatorFor(this._descriptor) - ); - } - - getMetricRecord(): Promise { + collect(): Promise { this._logger.debug('getMetricRecord - start'); - return new Promise((resolve, reject) => { + return new Promise(resolve => { const observerResult = new BatchObserverResult(); // cancels after MAX_TIMEOUT_MS - no more waiting for results @@ -74,14 +49,14 @@ export class BatchObserverMetric // remove callback to prevent user from updating the values later if // for any reason the observerBatchResult will be referenced observerResult.onObserveCalled(); - super.getMetricRecord().then(resolve, reject); + resolve(); this._logger.debug('getMetricRecord - timeout'); }, this._maxTimeoutUpdateMS); // sets callback for each "observe" method observerResult.onObserveCalled(() => { clearTimeout(timer); - super.getMetricRecord().then(resolve, reject); + resolve(); this._logger.debug('getMetricRecord - end'); }); diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index d0a54049b3e..2f8bd6b756a 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -17,9 +17,8 @@ import * as api from '@opentelemetry/api'; import { ConsoleLogger, InstrumentationLibrary } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; -import { BatchObserverMetric } from './BatchObserverMetric'; +import { BatchObserver } from './BatchObserver'; import { BaseBoundInstrument } from './BoundInstrument'; -import { MetricKind } from './export/types'; import { UpDownCounterMetric } from './UpDownCounterMetric'; import { CounterMetric } from './CounterMetric'; import { UpDownSumObserverMetric } from './UpDownSumObserverMetric'; @@ -37,6 +36,7 @@ import { NoopExporter } from './export/NoopExporter'; */ export class Meter implements api.Meter { private readonly _logger: api.Logger; + private readonly _batchObservers: BatchObserver[] = []; private readonly _metrics = new Map>(); private readonly _batcher: Batcher; private readonly _resource: Resource; @@ -258,35 +258,20 @@ export class Meter implements api.Meter { /** * Creates a new batch observer metric. - * @param name the name of the metric. * @param callback the batch observer callback * @param [options] the metric batch options. */ createBatchObserver( - name: string, callback: (observerResult: api.BatchObserverResult) => void, options: api.BatchMetricOptions = {} - ): api.BatchObserver { - if (!this._isValidName(name)) { - this._logger.warn( - `Invalid metric name ${name}. Defaulting to noop metric implementation.` - ); - return api.NOOP_BATCH_OBSERVER_METRIC; - } + ) { const opt: api.BatchMetricOptions = { logger: this._logger, ...DEFAULT_METRIC_OPTIONS, ...options, }; - const batchObserver = new BatchObserverMetric( - name, - opt, - this._batcher, - this._resource, - this._instrumentationLibrary, - callback - ); - this._registerMetric(name, batchObserver); + const batchObserver = new BatchObserver(opt, callback); + this._batchObservers.push(batchObserver); return batchObserver; } @@ -299,27 +284,17 @@ export class Meter implements api.Meter { */ async collect(): Promise { // call batch observers first - const batchObservers = Array.from(this._metrics.values()) - .filter(metric => { - return metric.getKind() === MetricKind.BATCH_OBSERVER; - }) - .map(metric => { - return metric.getMetricRecord(); - }); - await Promise.all(batchObservers).then(records => { - records.forEach(metrics => { - metrics.forEach(metric => this._batcher.process(metric)); - }); - }); + const observations = Array.from(this._batchObservers.values()).map( + observer => { + return observer.collect(); + } + ); + await Promise.all(observations); // after this all remaining metrics can be run - const metrics = Array.from(this._metrics.values()) - .filter(metric => { - return metric.getKind() !== MetricKind.BATCH_OBSERVER; - }) - .map(metric => { - return metric.getMetricRecord(); - }); + const metrics = Array.from(this._metrics.values()).map(metric => { + return metric.getMetricRecord(); + }); await Promise.all(metrics).then(records => { records.forEach(metrics => { diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 6c6a995796f..07f8de437a8 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -43,6 +43,7 @@ import { UpDownSumObserverMetric } from '../src/UpDownSumObserverMetric'; import { hashLabels } from '../src/Utils'; import { Batcher } from '../src/export/Batcher'; import { ValueType } from '@opentelemetry/api'; +import { BatchObserver } from '../src/BatchObserver'; const nonNumberValues = [ // type undefined @@ -1137,18 +1138,18 @@ describe('Meter', () => { describe('#batchObserver', () => { it('should create a batch observer', () => { - const measure = meter.createBatchObserver('name', () => {}); - assert.ok(measure instanceof Metric); + const measure = meter.createBatchObserver(() => {}); + assert.ok(measure instanceof BatchObserver); }); it('should create batch observer with options', () => { - const measure = meter.createBatchObserver('name', () => {}, { + const measure = meter.createBatchObserver(() => {}, { description: 'desc', unit: '1', disabled: false, maxTimeoutUpdateMS: 100, }); - assert.ok(measure instanceof Metric); + assert.ok(measure instanceof BatchObserver); }); it('should use callback to observe values ', async () => { @@ -1160,59 +1161,56 @@ describe('Meter', () => { description: 'desc', }) as ValueObserverMetric; - meter.createBatchObserver( - 'metric_batch_observer', - observerBatchResult => { - interface StatItem { - usage: number; - temp: number; - } - - interface Stat { - name: string; - core1: StatItem; - core2: StatItem; - } + meter.createBatchObserver(observerBatchResult => { + interface StatItem { + usage: number; + temp: number; + } - function someAsyncMetrics() { - return new Promise(resolve => { - const stats: Stat[] = [ - { - name: 'app1', - core1: { usage: 2.1, temp: 67 }, - core2: { usage: 3.1, temp: 69 }, - }, - { - name: 'app2', - core1: { usage: 1.2, temp: 67 }, - core2: { usage: 4.5, temp: 69 }, - }, - ]; - resolve(stats); - }); - } + interface Stat { + name: string; + core1: StatItem; + core2: StatItem; + } - Promise.all([ - someAsyncMetrics(), - // simulate waiting - new Promise((resolve, reject) => { - setTimeout(resolve, 1); - }), - ]).then((stats: unknown[]) => { - const apps = (stats[0] as unknown) as Stat[]; - apps.forEach(app => { - observerBatchResult.observe({ app: app.name, core: '1' }, [ - tempMetric.observation(app.core1.temp), - cpuUsageMetric.observation(app.core1.usage), - ]); - observerBatchResult.observe({ app: app.name, core: '2' }, [ - tempMetric.observation(app.core2.temp), - cpuUsageMetric.observation(app.core2.usage), - ]); - }); + function someAsyncMetrics() { + return new Promise(resolve => { + const stats: Stat[] = [ + { + name: 'app1', + core1: { usage: 2.1, temp: 67 }, + core2: { usage: 3.1, temp: 69 }, + }, + { + name: 'app2', + core1: { usage: 1.2, temp: 67 }, + core2: { usage: 4.5, temp: 69 }, + }, + ]; + resolve(stats); }); } - ); + + Promise.all([ + someAsyncMetrics(), + // simulate waiting + new Promise((resolve, reject) => { + setTimeout(resolve, 1); + }), + ]).then((stats: unknown[]) => { + const apps = (stats[0] as unknown) as Stat[]; + apps.forEach(app => { + observerBatchResult.observe({ app: app.name, core: '1' }, [ + tempMetric.observation(app.core1.temp), + cpuUsageMetric.observation(app.core1.usage), + ]); + observerBatchResult.observe({ app: app.name, core: '2' }, [ + tempMetric.observation(app.core2.temp), + cpuUsageMetric.observation(app.core2.usage), + ]); + }); + }); + }); await meter.collect(); const records = meter.getBatcher().checkPointSet(); @@ -1253,7 +1251,6 @@ describe('Meter', () => { }) as ValueObserverMetric; meter.createBatchObserver( - 'metric_batch_observer', observerBatchResult => { Promise.all([ // simulate waiting 11ms