From 5966a3b4c033f4dcc746d781ef7c910c0e091902 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Wed, 1 Jul 2020 23:18:19 +0200 Subject: [PATCH 01/13] feat: adding new metric: up down sum observer --- .../opentelemetry-api/src/metrics/Metric.ts | 3 + .../src/metrics/NoopMeter.ts | 3 + packages/opentelemetry-metrics/README.md | 69 ++++++++- .../src/BaseObserverMetric.ts | 30 ++-- packages/opentelemetry-metrics/src/Meter.ts | 41 +++++- .../src/UpDownSumObserverMetric.ts | 47 ++++++ .../src/ValueObserverMetric.ts | 4 +- .../opentelemetry-metrics/test/Meter.test.ts | 134 +++++++++++++++++- 8 files changed, 312 insertions(+), 19 deletions(-) create mode 100644 packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts diff --git a/packages/opentelemetry-api/src/metrics/Metric.ts b/packages/opentelemetry-api/src/metrics/Metric.ts index ebf514d6e3..c0896bbc20 100644 --- a/packages/opentelemetry-api/src/metrics/Metric.ts +++ b/packages/opentelemetry-api/src/metrics/Metric.ts @@ -176,6 +176,9 @@ export interface BaseObserver extends UnboundMetric { /** Base interface for the Value Observer metrics. */ export type ValueObserver = BaseObserver; +/** Base interface for the Up Down Sum Observer metrics. */ +export type UpDownSumObserver = BaseObserver; + /** Base interface for the Batch Observer metrics. */ export type BatchObserver = Metric; diff --git a/packages/opentelemetry-api/src/metrics/NoopMeter.ts b/packages/opentelemetry-api/src/metrics/NoopMeter.ts index 9d63f21bb5..5667160372 100644 --- a/packages/opentelemetry-api/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-api/src/metrics/NoopMeter.ts @@ -202,4 +202,7 @@ export const NOOP_BOUND_BASE_OBSERVER = new NoopBoundBaseObserver(); export const NOOP_VALUE_OBSERVER_METRIC = new NoopBaseObserverMetric( NOOP_BOUND_BASE_OBSERVER ); +export const NOOP_UP_DOWN_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 f24668528d..3549401a59 100644 --- a/packages/opentelemetry-metrics/README.md +++ b/packages/opentelemetry-metrics/README.md @@ -76,20 +76,85 @@ boundCounter.add(Math.random() > 0.5 ? 1 : -1); ### Value Observer -Choose this kind of metric when only last value is important without worry about aggregation +Choose this kind of metric when only last value is important without worry about aggregation. +Callback can return either nothing or promise to support async operation ```js const { MeterProvider } = require('@opentelemetry/metrics'); const meter = new MeterProvider().getMeter('your-meter-name'); + +// callback with promise - for operations that needs to wait for value +meter.createValueObserver('cpu_core_usage', { + description: 'Example of a async observer with callback', +}, (observerResult) => { + return new Promise((resolve) => { + getAsyncValue().then((value) => { + observerResult.observe(value, { core: '1' }); + resolve(); + }); + }); +}); + +function getAsyncValue() { + return new Promise((resolve) => { + setTimeout(()=> { + resolve(Math.random()); + }, 100); + }); +} + +// callback without promise in case you don't need to wait for value meter.createValueObserver('cpu_core_usage', { - description: 'Example of a sync observer with callback', + description: 'Example of a async observer with callback', }, (observerResult) => { observerResult.observe(getRandomValue(), { core: '1' }); observerResult.observe(getRandomValue(), { core: '2' }); }); +function getRandomValue() { + return Math.random(); +} +``` + +### Up Down Sum Observer + +Choose this kind of metric when sum is important; +Callback can return either nothing or promise to support async operation +```js +const { MeterProvider } = require('@opentelemetry/metrics'); + +const meter = new MeterProvider().getMeter('your-meter-name'); + +// callback with promise - for operations that needs to wait for value +meter.createUpDownSumObserver('cpu_core_usage', { + description: 'Example of an async observer with callback', +}, (observerResult) => { + return new Promise((resolve) => { + getAsyncValue().then((value) => { + observerResult.observe(value, { core: '1' }); + resolve(); + }); + }); +}); + +function getAsyncValue() { + return new Promise((resolve) => { + setTimeout(()=> { + resolve(Math.random()); + }, 100); + }); +} + +// callback without promise in case you don't need to wait for value +meter.createUpDownSumObserver('cpu_core_usage', { + description: 'Example of an async observer with callback', +}, (observerResult) => { + observerResult.observe(getRandomValue(), { core: '1' }); + resolve(); +}); + function getRandomValue() { return Math.random(); } diff --git a/packages/opentelemetry-metrics/src/BaseObserverMetric.ts b/packages/opentelemetry-metrics/src/BaseObserverMetric.ts index 61167183df..c0b891f81b 100644 --- a/packages/opentelemetry-metrics/src/BaseObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/BaseObserverMetric.ts @@ -30,7 +30,9 @@ const NOOP_CALLBACK = () => {}; */ export abstract class BaseObserverMetric extends Metric implements api.BaseObserver { - protected _callback: (observerResult: api.ObserverResult) => void; + protected _callback: ( + observerResult: api.ObserverResult + ) => Promise | unknown; constructor( name: string, @@ -39,7 +41,9 @@ export abstract class BaseObserverMetric extends Metric resource: Resource, metricKind: MetricKind, instrumentationLibrary: InstrumentationLibrary, - callback?: (observerResult: api.ObserverResult) => void + callback?: ( + observerResult: api.ObserverResult + ) => Promise | unknown ) { super(name, options, metricKind, resource, instrumentationLibrary); this._callback = callback || NOOP_CALLBACK; @@ -56,13 +60,23 @@ export abstract class BaseObserverMetric extends Metric } getMetricRecord(): Promise { - const observerResult = new ObserverResult(); - this._callback(observerResult); - observerResult.values.forEach((value, labels) => { - const instrument = this.bind(labels); - instrument.update(value); + return new Promise((resolve, reject) => { + const observerResult = new ObserverResult(); + Promise.resolve() + .then(() => { + return this._callback(observerResult) || Promise.resolve(); + }) + .then(() => { + observerResult.values.forEach((value, labels) => { + const instrument = this.bind(labels); + instrument.update(value); + }); + super.getMetricRecord().then(resolve, reject); + }) + .catch(e => { + reject(e); + }); }); - return super.getMetricRecord(); } observation(value: number) { diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index 0842eefabd..d99aba5faf 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -22,6 +22,7 @@ import { BaseBoundInstrument } from './BoundInstrument'; import { UpDownCounterMetric } from './UpDownCounterMetric'; import { CounterMetric } from './CounterMetric'; import { MetricRecord } from './export/types'; +import { UpDownSumObserverMetric } from './UpDownSumObserverMetric'; import { ValueRecorderMetric } from './ValueRecorderMetric'; import { Metric } from './Metric'; import { ValueObserverMetric } from './ValueObserverMetric'; @@ -165,7 +166,9 @@ export class Meter implements api.Meter { createValueObserver( name: string, options: api.MetricOptions = {}, - callback?: (observerResult: api.ObserverResult) => void + callback?: ( + observerResult: api.ObserverResult + ) => Promise | unknown ): api.ValueObserver { if (!this._isValidName(name)) { this._logger.warn( @@ -190,6 +193,42 @@ export class Meter implements api.Meter { return valueObserver; } + /** + * Creates a new up down sum observer metric. + * @param name the name of the metric. + * @param [options] the metric options. + * @param [callback] the value observer callback + */ + createUpDownSumObserver( + name: string, + options: api.MetricOptions = {}, + callback?: ( + observerResult: api.ObserverResult + ) => Promise | unknown + ): api.ValueObserver { + if (!this._isValidName(name)) { + this._logger.warn( + `Invalid metric name ${name}. Defaulting to noop metric implementation.` + ); + return api.NOOP_UP_DOWN_SUM_OBSERVER_METRIC; + } + const opt: api.MetricOptions = { + logger: this._logger, + ...DEFAULT_METRIC_OPTIONS, + ...options, + }; + const upDownSumObserver = new UpDownSumObserverMetric( + name, + opt, + this._batcher, + this._resource, + this._instrumentationLibrary, + callback + ); + this._registerMetric(name, upDownSumObserver); + return upDownSumObserver; + } + /** * Creates a new batch observer metric. * @param name the name of the metric. diff --git a/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts b/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts new file mode 100644 index 0000000000..5c7bf9d019 --- /dev/null +++ b/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts @@ -0,0 +1,47 @@ +/* + * 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 * 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 { MetricKind } from './export/types'; + +/** This is a SDK implementation of Up Down Sum Observer Metric. */ +export class UpDownSumObserverMetric extends BaseObserverMetric + implements api.UpDownSumObserver { + constructor( + name: string, + options: api.MetricOptions, + batcher: Batcher, + resource: Resource, + instrumentationLibrary: InstrumentationLibrary, + callback?: ( + observerResult: api.ObserverResult + ) => Promise | unknown + ) { + super( + name, + options, + batcher, + resource, + MetricKind.UP_DOWN_SUM_OBSERVER, + instrumentationLibrary, + callback + ); + } +} diff --git a/packages/opentelemetry-metrics/src/ValueObserverMetric.ts b/packages/opentelemetry-metrics/src/ValueObserverMetric.ts index 14ca5418fe..69606fc130 100644 --- a/packages/opentelemetry-metrics/src/ValueObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/ValueObserverMetric.ts @@ -29,7 +29,9 @@ export class ValueObserverMetric extends BaseObserverMetric batcher: Batcher, resource: Resource, instrumentationLibrary: InstrumentationLibrary, - callback?: (observerResult: api.ObserverResult) => void + callback?: ( + observerResult: api.ObserverResult + ) => Promise | unknown ) { super( name, diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 2cd8c42937..ae7cd610e3 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -35,6 +35,7 @@ import { NoopLogger, hrTime, hrTimeToNanoseconds } from '@opentelemetry/core'; import { BatchObserverResult } from '../src/BatchObserverResult'; import { SumAggregator } from '../src/export/aggregators'; import { Resource } from '@opentelemetry/resources'; +import { UpDownSumObserverMetric } from '../src/UpDownSumObserverMetric'; import { hashLabels } from '../src/Utils'; import { Batcher } from '../src/export/Batcher'; @@ -650,7 +651,7 @@ describe('Meter', () => { }); }); - describe('#valueObserver', () => { + describe('#ValueObserver', () => { it('should create a value observer', () => { const valueObserver = meter.createValueObserver( 'name' @@ -668,16 +669,22 @@ describe('Meter', () => { }); it('should set callback and observe value ', async () => { - const valueRecorder = meter.createValueObserver( + const valueObserver = meter.createValueObserver( 'name', { description: 'desc', }, (observerResult: api.ObserverResult) => { - observerResult.observe(getCpuUsage(), { pid: '123', core: '1' }); - observerResult.observe(getCpuUsage(), { pid: '123', core: '2' }); - observerResult.observe(getCpuUsage(), { pid: '123', core: '3' }); - observerResult.observe(getCpuUsage(), { pid: '123', core: '4' }); + // simulate async + return new Promise(resolve => { + setTimeout(() => { + observerResult.observe(getCpuUsage(), { pid: '123', core: '1' }); + observerResult.observe(getCpuUsage(), { pid: '123', core: '2' }); + observerResult.observe(getCpuUsage(), { pid: '123', core: '3' }); + observerResult.observe(getCpuUsage(), { pid: '123', core: '4' }); + resolve(); + }, 1); + }); } ) as ValueObserverMetric; @@ -685,7 +692,7 @@ describe('Meter', () => { return Math.random(); } - const metricRecords: MetricRecord[] = await valueRecorder.getMetricRecord(); + const metricRecords: MetricRecord[] = await valueObserver.getMetricRecord(); assert.strictEqual(metricRecords.length, 4); const metric1 = metricRecords[0]; @@ -714,6 +721,119 @@ describe('Meter', () => { }); }); + describe('#UpDownSumObserverMetric', () => { + it('should create an UpDownSum observer', () => { + const upDownSumObserver = meter.createUpDownSumObserver( + 'name' + ) as UpDownSumObserverMetric; + assert.ok(upDownSumObserver instanceof Metric); + }); + + it('should create observer with options', () => { + const upDownSumObserver = meter.createUpDownSumObserver('name', { + description: 'desc', + unit: '1', + disabled: false, + }) as ValueObserverMetric; + assert.ok(upDownSumObserver instanceof Metric); + }); + + it('should set callback and observe value ', async () => { + let counter = 0; + function getValue() { + counter++; + if (counter % 2 === 0) { + return -1; + } + return 3; + } + const upDownSumObserver = meter.createUpDownSumObserver( + 'name', + { + description: 'desc', + }, + (observerResult: api.ObserverResult) => { + // simulate async + return new Promise(resolve => { + setTimeout(() => { + observerResult.observe(getValue(), { pid: '123', core: '1' }); + resolve(); + }, 1); + }); + } + ) as UpDownSumObserverMetric; + + let metricRecords = await upDownSumObserver.getMetricRecord(); + assert.strictEqual(metricRecords.length, 1); + let point = metricRecords[0].aggregator.toPoint(); + assert.strictEqual(point.value, 3); + assert.strictEqual( + hashLabels(metricRecords[0].labels), + '|#core:1,pid:123' + ); + + metricRecords = await upDownSumObserver.getMetricRecord(); + assert.strictEqual(metricRecords.length, 1); + point = metricRecords[0].aggregator.toPoint(); + assert.strictEqual(point.value, 2); + + metricRecords = await upDownSumObserver.getMetricRecord(); + assert.strictEqual(metricRecords.length, 1); + point = metricRecords[0].aggregator.toPoint(); + assert.strictEqual(point.value, 5); + }); + + it('should set callback and observe value when callback returns nothing', async () => { + const upDownSumObserver = meter.createUpDownSumObserver( + 'name', + { + description: 'desc', + }, + (observerResult: api.ObserverResult) => { + observerResult.observe(1, { pid: '123', core: '1' }); + } + ) as UpDownSumObserverMetric; + + const metricRecords = await upDownSumObserver.getMetricRecord(); + assert.strictEqual(metricRecords.length, 1); + }); + + it( + 'should set callback and observe value when callback returns anything' + + ' but Promise', + async () => { + const upDownSumObserver = meter.createUpDownSumObserver( + 'name', + { + description: 'desc', + }, + (observerResult: api.ObserverResult) => { + observerResult.observe(1, { pid: '123', core: '1' }); + return '1'; + } + ) as UpDownSumObserverMetric; + + const metricRecords = await upDownSumObserver.getMetricRecord(); + assert.strictEqual(metricRecords.length, 1); + } + ); + + it('should pipe through resource', async () => { + const upDownSumObserver = meter.createUpDownSumObserver( + 'name', + {}, + result => { + result.observe(42, { foo: 'bar' }); + return Promise.resolve(); + } + ) as UpDownSumObserverMetric; + assert.ok(upDownSumObserver.resource instanceof Resource); + + const [record] = await upDownSumObserver.getMetricRecord(); + assert.ok(record.resource instanceof Resource); + }); + }); + describe('#batchObserver', () => { it('should create a batch observer', () => { const measure = meter.createBatchObserver('name', () => {}); From 23390c3a71e782633b8f898e8a3671d10f27d7b4 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Wed, 1 Jul 2020 23:42:51 +0200 Subject: [PATCH 02/13] chore: lint --- packages/opentelemetry-metrics/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/opentelemetry-metrics/README.md b/packages/opentelemetry-metrics/README.md index 3549401a59..20f172d880 100644 --- a/packages/opentelemetry-metrics/README.md +++ b/packages/opentelemetry-metrics/README.md @@ -122,6 +122,7 @@ function getRandomValue() { Choose this kind of metric when sum is important; Callback can return either nothing or promise to support async operation + ```js const { MeterProvider } = require('@opentelemetry/metrics'); From 686f8d39b7baf378e4c2064dfbf9f80e244cf68f Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 2 Jul 2020 00:03:17 +0200 Subject: [PATCH 03/13] chore: improving test cooverage --- .../opentelemetry-metrics/test/Meter.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index ae7cd610e3..133cb355d2 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -818,6 +818,25 @@ describe('Meter', () => { } ); + it('should reject getMetricRecord when callback throws an error', async () => { + const upDownSumObserver = meter.createUpDownSumObserver( + 'name', + { + description: 'desc', + }, + (observerResult: api.ObserverResult) => { + observerResult.observe(1, { pid: '123', core: '1' }); + throw new Error('Boom'); + } + ) as UpDownSumObserverMetric; + await upDownSumObserver + .getMetricRecord() + .then() + .catch(e => { + assert.strictEqual(e.message, 'Boom'); + }); + }); + it('should pipe through resource', async () => { const upDownSumObserver = meter.createUpDownSumObserver( 'name', From 55e62e18f6bd105d026e7d0824eaba18b7485493 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 2 Jul 2020 00:25:53 +0200 Subject: [PATCH 04/13] chore: improving test coverage --- .../opentelemetry-metrics/test/Meter.test.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 133cb355d2..b33adae985 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -15,6 +15,7 @@ */ import * as assert from 'assert'; +import * as sinon from 'sinon'; import { Meter, Metric, @@ -659,6 +660,17 @@ describe('Meter', () => { assert.ok(valueObserver instanceof Metric); }); + it('should return noop observer when name is invalid', () => { + const spy = sinon.stub(meter['_logger'], 'warn'); + const valueObserver = meter.createValueObserver('na me'); + assert.ok(valueObserver === api.NOOP_VALUE_OBSERVER_METRIC); + const args = spy.args[0]; + assert.ok( + args[0], + 'Invalid metric name na me. Defaulting to noop metric implementation.' + ); + }); + it('should create observer with options', () => { const valueObserver = meter.createValueObserver('name', { description: 'desc', @@ -729,6 +741,17 @@ describe('Meter', () => { assert.ok(upDownSumObserver instanceof Metric); }); + it('should return noop observer when name is invalid', () => { + const spy = sinon.stub(meter['_logger'], 'warn'); + const upDownSumObserver = meter.createUpDownSumObserver('na me'); + assert.ok(upDownSumObserver === api.NOOP_UP_DOWN_SUM_OBSERVER_METRIC); + const args = spy.args[0]; + assert.ok( + args[0], + 'Invalid metric name na me. Defaulting to noop metric implementation.' + ); + }); + it('should create observer with options', () => { const upDownSumObserver = meter.createUpDownSumObserver('name', { description: 'desc', From 92f2358b0b147699a49692f5d82b9f69124bb0ff Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 2 Jul 2020 17:25:35 +0200 Subject: [PATCH 05/13] chore: reviews --- packages/opentelemetry-metrics/README.md | 8 ++++---- packages/opentelemetry-metrics/src/Meter.ts | 4 ++-- .../opentelemetry-metrics/src/UpDownSumObserverMetric.ts | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry-metrics/README.md b/packages/opentelemetry-metrics/README.md index 20f172d880..a7d9f7a950 100644 --- a/packages/opentelemetry-metrics/README.md +++ b/packages/opentelemetry-metrics/README.md @@ -77,7 +77,7 @@ boundCounter.add(Math.random() > 0.5 ? 1 : -1); ### Value Observer Choose this kind of metric when only last value is important without worry about aggregation. -Callback can return either nothing or promise to support async operation +The callback can return either nothing or promise to support async operation. ```js const { MeterProvider } = require('@opentelemetry/metrics'); @@ -118,10 +118,10 @@ function getRandomValue() { } ``` -### Up Down Sum Observer +### UpDownSumObserver -Choose this kind of metric when sum is important; -Callback can return either nothing or promise to support async operation +Choose this kind of metric when sum is important. +The callback can return either nothing or promise to support async operation. ```js const { MeterProvider } = require('@opentelemetry/metrics'); diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index d99aba5faf..555e78515d 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -158,7 +158,7 @@ export class Meter implements api.Meter { } /** - * Creates a new value observer metric. + * Creates a new `ValueObserver` metric. * @param name the name of the metric. * @param [options] the metric options. * @param [callback] the value observer callback @@ -194,7 +194,7 @@ export class Meter implements api.Meter { } /** - * Creates a new up down sum observer metric. + * Creates a new `UpDownSumObserver` metric. * @param name the name of the metric. * @param [options] the metric options. * @param [callback] the value observer callback diff --git a/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts b/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts index 5c7bf9d019..4186785336 100644 --- a/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts @@ -21,7 +21,7 @@ import { BaseObserverMetric } from './BaseObserverMetric'; import { Batcher } from './export/Batcher'; import { MetricKind } from './export/types'; -/** This is a SDK implementation of Up Down Sum Observer Metric. */ +/** This is a SDK implementation of UpDownSumObserver Metric. */ export class UpDownSumObserverMetric extends BaseObserverMetric implements api.UpDownSumObserver { constructor( From a4ccd19c88bae56887a9261e43c3223f03bae21e Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 2 Jul 2020 21:11:42 +0200 Subject: [PATCH 06/13] chore: reviews --- packages/opentelemetry-api/src/metrics/Metric.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-api/src/metrics/Metric.ts b/packages/opentelemetry-api/src/metrics/Metric.ts index c0896bbc20..d495762e27 100644 --- a/packages/opentelemetry-api/src/metrics/Metric.ts +++ b/packages/opentelemetry-api/src/metrics/Metric.ts @@ -173,10 +173,10 @@ export interface BaseObserver extends UnboundMetric { }; } -/** Base interface for the Value Observer metrics. */ +/** Base interface for the ValueObserver metrics. */ export type ValueObserver = BaseObserver; -/** Base interface for the Up Down Sum Observer metrics. */ +/** Base interface for the UpDownSumObserver metrics. */ export type UpDownSumObserver = BaseObserver; /** Base interface for the Batch Observer metrics. */ From bc99cc50632691f74d7342d5e01d5054e0c7bbcf Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 2 Jul 2020 22:05:26 +0200 Subject: [PATCH 07/13] chore: fixes / changes after review --- examples/metrics/metrics/observer.js | 5 ++- packages/opentelemetry-metrics/README.md | 41 ++++++++----------- .../src/BaseObserverMetric.ts | 32 ++++----------- packages/opentelemetry-metrics/src/Meter.ts | 8 +--- .../src/UpDownSumObserverMetric.ts | 4 +- .../src/ValueObserverMetric.ts | 4 +- 6 files changed, 33 insertions(+), 61 deletions(-) diff --git a/examples/metrics/metrics/observer.js b/examples/metrics/metrics/observer.js index f455383a05..f8393122c1 100644 --- a/examples/metrics/metrics/observer.js +++ b/examples/metrics/metrics/observer.js @@ -22,7 +22,10 @@ const meter = new MeterProvider({ meter.createValueObserver('cpu_core_usage', { description: 'Example of a sync value observer with callback', -}, (observerResult) => { // this callback is called once per each interval +}, async (observerResult) => { // this callback is called once per each interval + await new Promise((resolve) => { + setTimeout(()=> {resolve()}, 50); + }); observerResult.observe(getRandomValue(), { core: '1' }); observerResult.observe(getRandomValue(), { core: '2' }); }); diff --git a/packages/opentelemetry-metrics/README.md b/packages/opentelemetry-metrics/README.md index a7d9f7a950..70521d7f62 100644 --- a/packages/opentelemetry-metrics/README.md +++ b/packages/opentelemetry-metrics/README.md @@ -77,7 +77,7 @@ boundCounter.add(Math.random() > 0.5 ? 1 : -1); ### Value Observer Choose this kind of metric when only last value is important without worry about aggregation. -The callback can return either nothing or promise to support async operation. +The callback can be sync or async. ```js const { MeterProvider } = require('@opentelemetry/metrics'); @@ -85,16 +85,12 @@ const { MeterProvider } = require('@opentelemetry/metrics'); const meter = new MeterProvider().getMeter('your-meter-name'); -// callback with promise - for operations that needs to wait for value -meter.createValueObserver('cpu_core_usage', { +// async callback - for operations that needs to wait for value +meter.createValueObserver('your_metric_name', { description: 'Example of a async observer with callback', -}, (observerResult) => { - return new Promise((resolve) => { - getAsyncValue().then((value) => { - observerResult.observe(value, { core: '1' }); - resolve(); - }); - }); +}, async (observerResult) => { + const value = getAsyncValue() + observerResult.observe(value, { core: '1' }); }); function getAsyncValue() { @@ -105,8 +101,8 @@ function getAsyncValue() { }); } -// callback without promise in case you don't need to wait for value -meter.createValueObserver('cpu_core_usage', { +// sync callback in case you don't need to wait for value +meter.createValueObserver('your_metric_name', { description: 'Example of a async observer with callback', }, (observerResult) => { observerResult.observe(getRandomValue(), { core: '1' }); @@ -121,23 +117,19 @@ function getRandomValue() { ### UpDownSumObserver Choose this kind of metric when sum is important. -The callback can return either nothing or promise to support async operation. +The callback can be sync or async. ```js const { MeterProvider } = require('@opentelemetry/metrics'); const meter = new MeterProvider().getMeter('your-meter-name'); -// callback with promise - for operations that needs to wait for value -meter.createUpDownSumObserver('cpu_core_usage', { +// async callback - for operations that needs to wait for value +meter.createUpDownSumObserver('your_metric_name', { description: 'Example of an async observer with callback', -}, (observerResult) => { - return new Promise((resolve) => { - getAsyncValue().then((value) => { - observerResult.observe(value, { core: '1' }); - resolve(); - }); - }); +}, async (observerResult) => { + const value = getAsyncValue() + observerResult.observe(value, { core: '1' }); }); function getAsyncValue() { @@ -148,12 +140,11 @@ function getAsyncValue() { }); } -// callback without promise in case you don't need to wait for value -meter.createUpDownSumObserver('cpu_core_usage', { +// sync callback in case you don't need to wait for value +meter.createUpDownSumObserver('your_metric_name', { description: 'Example of an async observer with callback', }, (observerResult) => { observerResult.observe(getRandomValue(), { core: '1' }); - resolve(); }); function getRandomValue() { diff --git a/packages/opentelemetry-metrics/src/BaseObserverMetric.ts b/packages/opentelemetry-metrics/src/BaseObserverMetric.ts index c0b891f81b..3065a34a07 100644 --- a/packages/opentelemetry-metrics/src/BaseObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/BaseObserverMetric.ts @@ -30,9 +30,7 @@ const NOOP_CALLBACK = () => {}; */ export abstract class BaseObserverMetric extends Metric implements api.BaseObserver { - protected _callback: ( - observerResult: api.ObserverResult - ) => Promise | unknown; + protected _callback: (observerResult: api.ObserverResult) => unknown; constructor( name: string, @@ -41,9 +39,7 @@ export abstract class BaseObserverMetric extends Metric resource: Resource, metricKind: MetricKind, instrumentationLibrary: InstrumentationLibrary, - callback?: ( - observerResult: api.ObserverResult - ) => Promise | unknown + callback?: (observerResult: api.ObserverResult) => unknown ) { super(name, options, metricKind, resource, instrumentationLibrary); this._callback = callback || NOOP_CALLBACK; @@ -59,24 +55,14 @@ export abstract class BaseObserverMetric extends Metric ); } - getMetricRecord(): Promise { - return new Promise((resolve, reject) => { - const observerResult = new ObserverResult(); - Promise.resolve() - .then(() => { - return this._callback(observerResult) || Promise.resolve(); - }) - .then(() => { - observerResult.values.forEach((value, labels) => { - const instrument = this.bind(labels); - instrument.update(value); - }); - super.getMetricRecord().then(resolve, reject); - }) - .catch(e => { - reject(e); - }); + async getMetricRecord(): Promise { + const observerResult = new ObserverResult(); + await this._callback(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/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index 555e78515d..32636d3e66 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -166,9 +166,7 @@ export class Meter implements api.Meter { createValueObserver( name: string, options: api.MetricOptions = {}, - callback?: ( - observerResult: api.ObserverResult - ) => Promise | unknown + callback?: (observerResult: api.ObserverResult) => unknown ): api.ValueObserver { if (!this._isValidName(name)) { this._logger.warn( @@ -202,9 +200,7 @@ export class Meter implements api.Meter { createUpDownSumObserver( name: string, options: api.MetricOptions = {}, - callback?: ( - observerResult: api.ObserverResult - ) => Promise | unknown + callback?: (observerResult: api.ObserverResult) => unknown ): api.ValueObserver { if (!this._isValidName(name)) { this._logger.warn( diff --git a/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts b/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts index 4186785336..c96b34d7fc 100644 --- a/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/UpDownSumObserverMetric.ts @@ -30,9 +30,7 @@ export class UpDownSumObserverMetric extends BaseObserverMetric batcher: Batcher, resource: Resource, instrumentationLibrary: InstrumentationLibrary, - callback?: ( - observerResult: api.ObserverResult - ) => Promise | unknown + callback?: (observerResult: api.ObserverResult) => unknown ) { super( name, diff --git a/packages/opentelemetry-metrics/src/ValueObserverMetric.ts b/packages/opentelemetry-metrics/src/ValueObserverMetric.ts index 69606fc130..e71227db43 100644 --- a/packages/opentelemetry-metrics/src/ValueObserverMetric.ts +++ b/packages/opentelemetry-metrics/src/ValueObserverMetric.ts @@ -29,9 +29,7 @@ export class ValueObserverMetric extends BaseObserverMetric batcher: Batcher, resource: Resource, instrumentationLibrary: InstrumentationLibrary, - callback?: ( - observerResult: api.ObserverResult - ) => Promise | unknown + callback?: (observerResult: api.ObserverResult) => unknown ) { super( name, From 81003283c97b9d0f6c2985c68f9d9e77cef8cc34 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 2 Jul 2020 22:11:12 +0200 Subject: [PATCH 08/13] chore: typo --- packages/opentelemetry-metrics/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-metrics/README.md b/packages/opentelemetry-metrics/README.md index 70521d7f62..af81defb58 100644 --- a/packages/opentelemetry-metrics/README.md +++ b/packages/opentelemetry-metrics/README.md @@ -85,7 +85,7 @@ const { MeterProvider } = require('@opentelemetry/metrics'); const meter = new MeterProvider().getMeter('your-meter-name'); -// async callback - for operations that needs to wait for value +// async callback - for operation that needs to wait for value meter.createValueObserver('your_metric_name', { description: 'Example of a async observer with callback', }, async (observerResult) => { @@ -124,7 +124,7 @@ const { MeterProvider } = require('@opentelemetry/metrics'); const meter = new MeterProvider().getMeter('your-meter-name'); -// async callback - for operations that needs to wait for value +// async callback - for operation that needs to wait for value meter.createUpDownSumObserver('your_metric_name', { description: 'Example of an async observer with callback', }, async (observerResult) => { From b5b4f6fd1cce1c4a8ba59c22b00750a872008b6f Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 2 Jul 2020 22:12:29 +0200 Subject: [PATCH 09/13] chore: reviews --- packages/opentelemetry-metrics/README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry-metrics/README.md b/packages/opentelemetry-metrics/README.md index af81defb58..566f5f62f7 100644 --- a/packages/opentelemetry-metrics/README.md +++ b/packages/opentelemetry-metrics/README.md @@ -89,8 +89,8 @@ const meter = new MeterProvider().getMeter('your-meter-name'); meter.createValueObserver('your_metric_name', { description: 'Example of a async observer with callback', }, async (observerResult) => { - const value = getAsyncValue() - observerResult.observe(value, { core: '1' }); + const value = getAsyncValue(); + observerResult.observe(value, { label: '1' }); }); function getAsyncValue() { @@ -105,8 +105,8 @@ function getAsyncValue() { meter.createValueObserver('your_metric_name', { description: 'Example of a async observer with callback', }, (observerResult) => { - observerResult.observe(getRandomValue(), { core: '1' }); - observerResult.observe(getRandomValue(), { core: '2' }); + observerResult.observe(getRandomValue(), { label: '1' }); + observerResult.observe(getRandomValue(), { label: '2' }); }); function getRandomValue() { @@ -128,8 +128,8 @@ const meter = new MeterProvider().getMeter('your-meter-name'); meter.createUpDownSumObserver('your_metric_name', { description: 'Example of an async observer with callback', }, async (observerResult) => { - const value = getAsyncValue() - observerResult.observe(value, { core: '1' }); + const value = getAsyncValue(); + observerResult.observe(value, { label: '1' }); }); function getAsyncValue() { @@ -144,7 +144,7 @@ function getAsyncValue() { meter.createUpDownSumObserver('your_metric_name', { description: 'Example of an async observer with callback', }, (observerResult) => { - observerResult.observe(getRandomValue(), { core: '1' }); + observerResult.observe(getRandomValue(), { label: '1' }); }); function getRandomValue() { From 73f1283b7a34c67b8be806d1e65173f3fef3c552 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 2 Jul 2020 22:56:05 +0200 Subject: [PATCH 10/13] chore: reviews --- packages/opentelemetry-metrics/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-metrics/README.md b/packages/opentelemetry-metrics/README.md index 566f5f62f7..dfcfddd81f 100644 --- a/packages/opentelemetry-metrics/README.md +++ b/packages/opentelemetry-metrics/README.md @@ -89,7 +89,7 @@ const meter = new MeterProvider().getMeter('your-meter-name'); meter.createValueObserver('your_metric_name', { description: 'Example of a async observer with callback', }, async (observerResult) => { - const value = getAsyncValue(); + const value = await getAsyncValue(); observerResult.observe(value, { label: '1' }); }); @@ -116,7 +116,7 @@ function getRandomValue() { ### UpDownSumObserver -Choose this kind of metric when sum is important. +Choose this kind of metric when sum is important and you want to capture any value that starts at zero and rises or falls throughout the process lifetime. The callback can be sync or async. ```js @@ -128,7 +128,7 @@ const meter = new MeterProvider().getMeter('your-meter-name'); meter.createUpDownSumObserver('your_metric_name', { description: 'Example of an async observer with callback', }, async (observerResult) => { - const value = getAsyncValue(); + const value = await getAsyncValue(); observerResult.observe(value, { label: '1' }); }); From d35e6048c0553830e51c0b2cb9ac8477ffd8e69c Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 3 Jul 2020 00:22:53 +0200 Subject: [PATCH 11/13] chore: fixing types --- packages/opentelemetry-metrics/src/Meter.ts | 2 +- packages/opentelemetry-metrics/test/Meter.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index 32636d3e66..f545f444af 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -201,7 +201,7 @@ export class Meter implements api.Meter { name: string, options: api.MetricOptions = {}, callback?: (observerResult: api.ObserverResult) => unknown - ): api.ValueObserver { + ): api.UpDownSumObserver { if (!this._isValidName(name)) { this._logger.warn( `Invalid metric name ${name}. Defaulting to noop metric implementation.` diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index b33adae985..d716832413 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -757,7 +757,7 @@ describe('Meter', () => { description: 'desc', unit: '1', disabled: false, - }) as ValueObserverMetric; + }) as UpDownSumObserverMetric; assert.ok(upDownSumObserver instanceof Metric); }); From 433496462470d9dffbe806a4ca56f6cf6f161ad8 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 9 Jul 2020 13:56:46 +0200 Subject: [PATCH 12/13] chore: fixing copy --- packages/opentelemetry-metrics/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-metrics/README.md b/packages/opentelemetry-metrics/README.md index dfcfddd81f..2e2f1ad393 100644 --- a/packages/opentelemetry-metrics/README.md +++ b/packages/opentelemetry-metrics/README.md @@ -87,7 +87,7 @@ const meter = new MeterProvider().getMeter('your-meter-name'); // async callback - for operation that needs to wait for value meter.createValueObserver('your_metric_name', { - description: 'Example of a async observer with callback', + description: 'Example of an async observer with callback', }, async (observerResult) => { const value = await getAsyncValue(); observerResult.observe(value, { label: '1' }); @@ -103,7 +103,7 @@ function getAsyncValue() { // sync callback in case you don't need to wait for value meter.createValueObserver('your_metric_name', { - description: 'Example of a async observer with callback', + description: 'Example of a sync observer with callback', }, (observerResult) => { observerResult.observe(getRandomValue(), { label: '1' }); observerResult.observe(getRandomValue(), { label: '2' }); @@ -142,7 +142,7 @@ function getAsyncValue() { // sync callback in case you don't need to wait for value meter.createUpDownSumObserver('your_metric_name', { - description: 'Example of an async observer with callback', + description: 'Example of a sync observer with callback', }, (observerResult) => { observerResult.observe(getRandomValue(), { label: '1' }); }); From 6ce2db9cd36f66da1638990a8aca76a707b9ea17 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 9 Jul 2020 14:01:39 +0200 Subject: [PATCH 13/13] chore: reverting protos version to b54688569186e0b862bf7462a983ccf2c50c0547 changed during merge --- .../opentelemetry-exporter-collector/src/platform/node/protos | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/protos b/packages/opentelemetry-exporter-collector/src/platform/node/protos index e6c3c4a74d..b546885691 160000 --- a/packages/opentelemetry-exporter-collector/src/platform/node/protos +++ b/packages/opentelemetry-exporter-collector/src/platform/node/protos @@ -1 +1 @@ -Subproject commit e6c3c4a74d57f870a0d781bada02cb2b2c497d14 +Subproject commit b54688569186e0b862bf7462a983ccf2c50c0547