From 178538f884862e897eae6f38b0ea08500da91ca7 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 11 Nov 2021 12:11:23 -0500 Subject: [PATCH 01/14] feat: add WIP metrics SDK --- .../src/Aggregation.ts | 25 +++ .../src/Instruments.ts | 24 +++ .../src/Measurement.ts | 24 +++ .../src/Meter.ts | 60 +++++++ .../src/MeterProvider.ts | 166 ++++++++++++++++++ .../src/Metric.ts | 74 ++++++++ .../src/MetricExporter.ts | 42 +++++ .../src/MetricReader.ts | 42 +++++ .../src/View.ts | 112 ++++++++++++ .../src/index.ts | 4 +- 10 files changed, 572 insertions(+), 1 deletion(-) create mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/src/Aggregation.ts create mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts create mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts create mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts create mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts create mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/src/Metric.ts create mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts create mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/src/MetricReader.ts create mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/src/View.ts diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Aggregation.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Aggregation.ts new file mode 100644 index 00000000000..fac07ef4c20 --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Aggregation.ts @@ -0,0 +1,25 @@ +/* + * 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 { Measurement } from './Measurement'; + +// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#aggregation + +export interface Aggregator { + aggregate(measurement: Measurement): void; +} + +// TODO define actual aggregator classes diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts new file mode 100644 index 00000000000..63288df0f80 --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts @@ -0,0 +1,24 @@ +/* + * 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. + */ + +export enum InstrumentType { + COUNTER = 'COUNTER', + HISTOGRAM = 'HISTOGRAM', + UP_DOWN_COUNTER = 'UP_DOWN_COUNTER', + OBSERVABLE_COUNTER = 'OBSERVABLE_COUNTER', + OBSERVABLE_GAUGE = 'OBSERVABLE_GAUGE', + OBSERVABLE_UP_DOWN_COUNTER = 'OBSERVABLE_UP_DOWN_COUNTER', +} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts new file mode 100644 index 00000000000..17b6cee8f05 --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts @@ -0,0 +1,24 @@ +/* + * 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' + +export type Measurement = { + value: number; + // TODO use common attributes + attributes: api.SpanAttributes; + context?: api.Context; +} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts new file mode 100644 index 00000000000..0ee0096ba24 --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts @@ -0,0 +1,60 @@ +/* + * 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 metrics from '@opentelemetry/api-metrics'; +import { InstrumentationLibrary } from '@opentelemetry/core'; +import { Measurement } from './Measurement'; +import { MeterProvider } from './MeterProvider'; +import { Histogram, Metric } from './Metric'; + +export class Meter implements metrics.Meter { + // instrumentation library required by spec to be on meter + // spec requires provider config changes to apply to previously created meters, achieved by holding a reference to the provider + constructor(private _provider: MeterProvider, private _instrumentationLibrary: InstrumentationLibrary, private _schemaUrl?: string) { } + + /** this exists just to prevent ts errors from unused variables and may be removed */ + getSchemaUrl(): string | undefined { + return this._schemaUrl; + } + + /** this exists just to prevent ts errors from unused variables and may be removed */ + getInstrumentationLibrary(): InstrumentationLibrary { + return this._instrumentationLibrary; + } + + createHistogram(_name: string, _options?: metrics.MetricOptions): Histogram { + throw new Error('Method not implemented.'); + } + createCounter(_name: string, _options?: metrics.MetricOptions): metrics.Counter { + throw new Error('Method not implemented.'); + } + createUpDownCounter(_name: string, _options?: metrics.MetricOptions): metrics.UpDownCounter { + throw new Error('Method not implemented.'); + } + createObservableGauge(_name: string, _options?: metrics.MetricOptions, _callback?: (observableResult: metrics.ObservableResult) => void): metrics.ObservableBase { + throw new Error('Method not implemented.'); + } + createObservableCounter(_name: string, _options?: metrics.MetricOptions, _callback?: (observableResult: metrics.ObservableResult) => void): metrics.ObservableBase { + throw new Error('Method not implemented.'); + } + createObservableUpDownCounter(_name: string, _options?: metrics.MetricOptions, _callback?: (observableResult: metrics.ObservableResult) => void): metrics.ObservableBase { + throw new Error('Method not implemented.'); + } + + public aggregate(metric: Metric, measurement: Measurement) { + this._provider.aggregate(this, metric, measurement); + } +} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts new file mode 100644 index 00000000000..9780ef2d66f --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts @@ -0,0 +1,166 @@ +/* + * 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 * as metrics from '@opentelemetry/api-metrics'; +import { Resource } from '@opentelemetry/resources'; +import { Measurement } from './Measurement'; +import { Meter } from './Meter'; +import { Metric } from './Metric'; +import { MetricExporter } from './MetricExporter'; +import { MetricReader } from './MetricReader'; +import { View } from './View'; + +// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#meterprovider + +export type MeterProviderOptions = { + resource?: Resource; +} + +export class MeterProvider { + private _resource: Resource; + private _shutdown = false; + private _metricReaders: MetricReader[] = []; + private _metricExporters: MetricExporter[] = []; + private _views: View[] = []; + + constructor(options: MeterProviderOptions) { + this._resource = options.resource ?? Resource.empty(); + } + + /** + * **Unstable** + * + * This method is only here to prevent typescript from complaining and may be removed. + */ + getResource() { + return this._resource; + } + + getMeter(name: string, version = '', options: metrics.MeterOptions = {}): metrics.Meter { + // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#meter-creation + if (this._shutdown) { + api.diag.warn('A shutdown MeterProvider cannot provide a Meter') + return metrics.NOOP_METER; + } + + // Spec leaves it unspecified if creating a meter with duplicate + // name/version returns the same meter. We create a new one here + // for simplicity. This may change in the future. + return new Meter(this, { name, version }, options.schemaUrl); + } + + addMetricReader(metricReader: MetricReader) { + this._metricReaders.push(metricReader); + } + + addView(view: View) { + // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#view + this._views.push(view); + } + + async shutdown(): Promise { + // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#shutdown + + // TODO add a timeout - spec leaves it up the the SDK if this is configurable + this._shutdown = true; + + // Shut down all exporters and readers. + // Throw the first error and log all others. + let err: unknown; + for (const exporter of this._metricExporters) { + try { + await exporter.shutdown(); + } catch (e) { + if (e instanceof Error) { + api.diag.error(`Error shutting down: ${e.message}`) + } + err = err || e; + } + } + + for (const reader of this._metricReaders) { + try { + await reader.shutdown(); + } catch (e) { + if (e instanceof Error) { + api.diag.error(`Error shutting down: ${e.message}`) + } + err = err || e; + } + } + + if (err != null) { + throw err; + } + } + + async forceFlush(): Promise { + // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#forceflush + + // TODO add a timeout - spec leaves it up the the SDK if this is configurable + + // do not flush after shutdown + if (this._shutdown) { + return; + } + + // Flush all exporters and readers. + // Throw the first error and log all others. + let err: unknown; + for (const exporter of this._metricExporters) { + try { + await exporter.forceFlush(); + } catch (e) { + if (e instanceof Error) { + api.diag.error(`Error force flushing: ${e.message}`) + } + err = err || e; + } + } + + for (const reader of this._metricReaders) { + try { + await reader.forceFlush(); + } catch (e) { + if (e instanceof Error) { + api.diag.error(`Error force flushing: ${e.message}`) + } + err = err || e; + } + } + + if (err != null) { + throw err; + } + } + + public aggregate(_meter: Meter, _metric: Metric, _measurement: Measurement) { + // TODO actually aggregate + + /** + * if there are no views: + * apply the default configuration + * else: + * for each view: + * if view matches: + * apply view configuration + * if no view matched: + * if user has not disabled default fallback: + * apply default configuration + */ + } +} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Metric.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Metric.ts new file mode 100644 index 00000000000..aceef7fd6ad --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Metric.ts @@ -0,0 +1,74 @@ +/* + * 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 * as metrics from '@opentelemetry/api-metrics'; +import { Measurement } from './Measurement'; +import { Meter } from './Meter'; + +export class Metric { + constructor(private _meter: Meter, private _name: string, private _version?: string) { } + + getName(): string { + return this._name; + } + + /** + * This only exists to stop typescript complaining about unused variable and may be removed. + */ + getVersion(): string | undefined { + return this._version; + } + + aggregate(measurement: Measurement) { + this._meter.aggregate(this, measurement); + } +} + +export class UpDownCounter extends Metric implements metrics.Counter { + add(value: number, attributes?: api.SpanAttributes, ctx?: api.Context): void { + if (typeof value != 'number') { + api.diag.warn(`invalid type value provided to counter ${this.getName()}: ${typeof value}`); + return; + } + + attributes = attributes ?? {}; + ctx = ctx ?? api.context.active(); + + this.aggregate({ + value, + attributes, + context: ctx, + }); + } +} + +export class Counter extends UpDownCounter implements metrics.Counter { + override add(value: number, attributes?: api.SpanAttributes, ctx?: api.Context): void { + if (value < 0) { + api.diag.warn(`negative value provided to counter ${this.getName()}: ${value}`); + return; + } + + return super.add(value, attributes, ctx); + } +} + +export class Histogram extends Metric implements metrics.Histogram { + record(_measurement: number, _labels?: metrics.Labels, _context?: api.Context): void { + throw new Error('Method not implemented.'); + } +} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts new file mode 100644 index 00000000000..bdbbf708491 --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts @@ -0,0 +1,42 @@ +/* + * 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. + */ + +export abstract class MetricExporter { + private _shutdown = false; + + async shutdown(): Promise { + try { + await this.forceFlush(); + } finally { + this._shutdown = true; + } + } + + abstract forceFlush(): Promise; + + isShutdown() { + return this._shutdown; + } +} + +export class ConsoleMetricExporter extends MetricExporter { + export() { + throw new Error('Method not implemented'); + } + + // nothing to do + async forceFlush() {} +} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricReader.ts new file mode 100644 index 00000000000..7b4e0e2a5c2 --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricReader.ts @@ -0,0 +1,42 @@ +/* + * 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 { MetricExporter } from '.'; + +export class MetricReader { + private _shutdown = false; + + constructor(private _exporter: MetricExporter) {} + + async shutdown(): Promise { + if (this._shutdown) { + return; + } + + this._shutdown = true; + // errors thrown to caller + await this._exporter.shutdown(); + } + + async forceFlush(): Promise { + if (this._shutdown) { + return; + } + + // errors thrown to caller + await this._exporter.forceFlush(); + } +} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/View.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/View.ts new file mode 100644 index 00000000000..ef5870c18d4 --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/View.ts @@ -0,0 +1,112 @@ +/* + * 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 { InstrumentType } from './Instruments'; + +// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#view + +/** + * A metric view selects a stream of metrics from a MeterProvider and applies + * a configuration to that stream. If no configuration is provided, the default + * configuration is used. + */ +export class View { + private _selector: Partial; + + /** + * Construct a metric view + * + * @param options a required object which describes the view selector and configuration + */ + constructor(options: ViewOptions) { + if (typeof options.selector == null) { + throw new Error('Missing required view selector') + } + + if ( + options.selector.instrumentType == null && + options.selector.instrumentName == null && + options.selector.meterName == null && + options.selector.meterVersion == null && + options.selector.meterSchemaUrl == null + ) { + // It is recommended by the SDK specification to fail fast when invalid options are provided + throw new Error('Cannot create a view which selects no options'); + } + + this._selector = options.selector; + } + + /** + * Given a metric selector, determine if all of this view's metric selectors match. + * + * @param selector selector to match + * @returns boolean + */ + public match(selector: ViewMetricSelector) { + return this._matchSelectorProperty('instrumentType', selector.instrumentType) && + this._matchInstrumentName(selector.instrumentName) && + this._matchSelectorProperty('meterName', selector.meterName) && + this._matchSelectorProperty('meterVersion', selector.meterVersion) && + this._matchSelectorProperty('meterSchemaUrl', selector.meterSchemaUrl); + } + + /** + * Match instrument name against the configured selector metric name, which may include wildcards + */ + private _matchInstrumentName(name: string) { + if (this._selector.instrumentName == null) { + return true; + } + + return this._selector.instrumentName === name; + } + + private _matchSelectorProperty(property: Prop, metricProperty: ViewMetricSelector[Prop]): boolean { + if (this._selector[property] == null) { + return true; + } + + if (this._selector[property] === metricProperty) { + return true; + } + + return false; + } +} + +export type ViewMetricSelector = { + instrumentType: InstrumentType; + instrumentName: string; + meterName: string; + meterVersion?: string; + meterSchemaUrl?: string; +} + +export type ViewOptions = { + name?: string; + selector: Partial; + streamConfig?: ViewStreamConfig; +} + +export type ViewStreamConfig = { + description: string; + attributeKeys?: string[]; + + // TODO use these types when they are defined + aggregation?: unknown; + exemplarReservoir?: unknown; +} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/index.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/index.ts index f4fa9c64729..ef6f9b17d23 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/index.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/index.ts @@ -14,4 +14,6 @@ * limitations under the License. */ -export * from './version'; +export { MeterProvider, MeterProviderOptions } from './MeterProvider'; +export * from './MetricExporter'; +export * from './MetricReader'; From 9ac4f10d5c9ffa513315b84e685f48e25a4530fc Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 11 Nov 2021 12:15:13 -0500 Subject: [PATCH 02/14] chore: update ts references --- .../opentelemetry-exporter-metrics-otlp-grpc/tsconfig.json | 3 --- .../opentelemetry-exporter-metrics-otlp-http/tsconfig.esm.json | 3 --- .../opentelemetry-exporter-metrics-otlp-http/tsconfig.json | 3 --- .../opentelemetry-exporter-metrics-otlp-proto/tsconfig.json | 3 --- .../packages/opentelemetry-exporter-prometheus/tsconfig.json | 3 --- experimental/packages/opentelemetry-sdk-node/tsconfig.json | 3 --- 6 files changed, 18 deletions(-) diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/tsconfig.json b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/tsconfig.json index b3ff4e84578..64ff561763b 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/tsconfig.json +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/tsconfig.json @@ -20,9 +20,6 @@ }, { "path": "../opentelemetry-exporter-trace-otlp-http" - }, - { - "path": "../opentelemetry-sdk-metrics-base" } ] } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/tsconfig.esm.json b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/tsconfig.esm.json index 484461f470a..063825713f5 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/tsconfig.esm.json +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/tsconfig.esm.json @@ -14,9 +14,6 @@ }, { "path": "../opentelemetry-exporter-trace-otlp-http/tsconfig.esm.json" - }, - { - "path": "../opentelemetry-sdk-metrics-base/tsconfig.esm.json" } ] } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/tsconfig.json b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/tsconfig.json index 13dc2e7744d..d9595b8a987 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/tsconfig.json +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/tsconfig.json @@ -14,9 +14,6 @@ }, { "path": "../opentelemetry-exporter-trace-otlp-http" - }, - { - "path": "../opentelemetry-sdk-metrics-base" } ] } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/tsconfig.json b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/tsconfig.json index 35c3f6d9c73..50c20b23e2e 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/tsconfig.json +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/tsconfig.json @@ -20,9 +20,6 @@ }, { "path": "../opentelemetry-exporter-trace-otlp-proto" - }, - { - "path": "../opentelemetry-sdk-metrics-base" } ] } diff --git a/experimental/packages/opentelemetry-exporter-prometheus/tsconfig.json b/experimental/packages/opentelemetry-exporter-prometheus/tsconfig.json index 3c062d3feb2..948abef3ceb 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/tsconfig.json +++ b/experimental/packages/opentelemetry-exporter-prometheus/tsconfig.json @@ -11,9 +11,6 @@ "references": [ { "path": "../opentelemetry-api-metrics" - }, - { - "path": "../opentelemetry-sdk-metrics-base" } ] } diff --git a/experimental/packages/opentelemetry-sdk-node/tsconfig.json b/experimental/packages/opentelemetry-sdk-node/tsconfig.json index 36c71e90d8d..b0e33797f42 100644 --- a/experimental/packages/opentelemetry-sdk-node/tsconfig.json +++ b/experimental/packages/opentelemetry-sdk-node/tsconfig.json @@ -14,9 +14,6 @@ }, { "path": "../opentelemetry-instrumentation" - }, - { - "path": "../opentelemetry-sdk-metrics-base" } ] } From 769d16fb76449f7aaa6a005d2eba7c24c9504563 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 11 Nov 2021 12:34:20 -0500 Subject: [PATCH 03/14] chore: fix versions in wip metrics --- .../packages/opentelemetry-sdk-metrics-base/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/package.json b/experimental/packages/opentelemetry-sdk-metrics-base/package.json index b03868fd5fd..30ef6073e2e 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/package.json +++ b/experimental/packages/opentelemetry-sdk-metrics-base/package.json @@ -1,6 +1,6 @@ { "name": "@opentelemetry/sdk-metrics-base-wip", - "version": "0.26.0", + "version": "0.27.0", "private": true, "description": "Work in progress OpenTelemetry metrics SDK", "main": "build/src/index.js", @@ -64,7 +64,7 @@ "@opentelemetry/api": "^1.0.0" }, "dependencies": { - "@opentelemetry/api-metrics": "0.26.0", + "@opentelemetry/api-metrics": "0.27.0", "@opentelemetry/core": "1.0.0", "@opentelemetry/resources": "1.0.0", "lodash.merge": "^4.6.2" From e04ea5937ae97a10049ae804e14cc368184d8235 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 11 Nov 2021 12:34:44 -0500 Subject: [PATCH 04/14] chore: rename metric to instrument --- .../src/Instruments.ts | 59 +++++++++++++++ .../src/Meter.ts | 4 +- .../src/MeterProvider.ts | 4 +- .../src/Metric.ts | 74 ------------------- 4 files changed, 63 insertions(+), 78 deletions(-) delete mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/src/Metric.ts diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts index 63288df0f80..5d91ced65f7 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts @@ -14,6 +14,11 @@ * limitations under the License. */ +import * as api from '@opentelemetry/api'; +import * as metrics from '@opentelemetry/api-metrics'; +import { Measurement } from './Measurement'; +import { Meter } from './Meter'; + export enum InstrumentType { COUNTER = 'COUNTER', HISTOGRAM = 'HISTOGRAM', @@ -22,3 +27,57 @@ export enum InstrumentType { OBSERVABLE_GAUGE = 'OBSERVABLE_GAUGE', OBSERVABLE_UP_DOWN_COUNTER = 'OBSERVABLE_UP_DOWN_COUNTER', } + +export class Instrument { + constructor(private _meter: Meter, private _name: string, private _version?: string) { } + + getName(): string { + return this._name; + } + + /** + * This only exists to stop typescript complaining about unused variable and may be removed. + */ + getVersion(): string | undefined { + return this._version; + } + + aggregate(measurement: Measurement) { + this._meter.aggregate(this, measurement); + } +} + +export class UpDownCounter extends Instrument implements metrics.Counter { + add(value: number, attributes?: api.SpanAttributes, ctx?: api.Context): void { + if (typeof value != 'number') { + api.diag.warn(`invalid type value provided to counter ${this.getName()}: ${typeof value}`); + return; + } + + attributes = attributes ?? {}; + ctx = ctx ?? api.context.active(); + + this.aggregate({ + value, + attributes, + context: ctx, + }); + } +} + +export class Counter extends UpDownCounter implements metrics.Counter { + override add(value: number, attributes?: api.SpanAttributes, ctx?: api.Context): void { + if (value < 0) { + api.diag.warn(`negative value provided to counter ${this.getName()}: ${value}`); + return; + } + + return super.add(value, attributes, ctx); + } +} + +export class Histogram extends Instrument implements metrics.Histogram { + record(_measurement: number, _labels?: metrics.Attributes, _context?: api.Context): void { + throw new Error('Method not implemented.'); + } +} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts index 0ee0096ba24..f7b5a9b5206 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts @@ -18,7 +18,7 @@ import * as metrics from '@opentelemetry/api-metrics'; import { InstrumentationLibrary } from '@opentelemetry/core'; import { Measurement } from './Measurement'; import { MeterProvider } from './MeterProvider'; -import { Histogram, Metric } from './Metric'; +import { Histogram, Instrument } from './Instruments'; export class Meter implements metrics.Meter { // instrumentation library required by spec to be on meter @@ -54,7 +54,7 @@ export class Meter implements metrics.Meter { throw new Error('Method not implemented.'); } - public aggregate(metric: Metric, measurement: Measurement) { + public aggregate(metric: Instrument, measurement: Measurement) { this._provider.aggregate(this, metric, measurement); } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts index 9780ef2d66f..718599005b5 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts @@ -17,9 +17,9 @@ import * as api from '@opentelemetry/api'; import * as metrics from '@opentelemetry/api-metrics'; import { Resource } from '@opentelemetry/resources'; +import { Instrument } from './Instruments'; import { Measurement } from './Measurement'; import { Meter } from './Meter'; -import { Metric } from './Metric'; import { MetricExporter } from './MetricExporter'; import { MetricReader } from './MetricReader'; import { View } from './View'; @@ -148,7 +148,7 @@ export class MeterProvider { } } - public aggregate(_meter: Meter, _metric: Metric, _measurement: Measurement) { + public aggregate(_meter: Meter, _metric: Instrument, _measurement: Measurement) { // TODO actually aggregate /** diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Metric.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Metric.ts deleted file mode 100644 index aceef7fd6ad..00000000000 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Metric.ts +++ /dev/null @@ -1,74 +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 * as api from '@opentelemetry/api'; -import * as metrics from '@opentelemetry/api-metrics'; -import { Measurement } from './Measurement'; -import { Meter } from './Meter'; - -export class Metric { - constructor(private _meter: Meter, private _name: string, private _version?: string) { } - - getName(): string { - return this._name; - } - - /** - * This only exists to stop typescript complaining about unused variable and may be removed. - */ - getVersion(): string | undefined { - return this._version; - } - - aggregate(measurement: Measurement) { - this._meter.aggregate(this, measurement); - } -} - -export class UpDownCounter extends Metric implements metrics.Counter { - add(value: number, attributes?: api.SpanAttributes, ctx?: api.Context): void { - if (typeof value != 'number') { - api.diag.warn(`invalid type value provided to counter ${this.getName()}: ${typeof value}`); - return; - } - - attributes = attributes ?? {}; - ctx = ctx ?? api.context.active(); - - this.aggregate({ - value, - attributes, - context: ctx, - }); - } -} - -export class Counter extends UpDownCounter implements metrics.Counter { - override add(value: number, attributes?: api.SpanAttributes, ctx?: api.Context): void { - if (value < 0) { - api.diag.warn(`negative value provided to counter ${this.getName()}: ${value}`); - return; - } - - return super.add(value, attributes, ctx); - } -} - -export class Histogram extends Metric implements metrics.Histogram { - record(_measurement: number, _labels?: metrics.Labels, _context?: api.Context): void { - throw new Error('Method not implemented.'); - } -} From 42a329846a2ee07ff350317eded9d2dc63d77dec Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 11 Nov 2021 13:00:48 -0500 Subject: [PATCH 05/14] chore: simplify counter inheritance --- .../src/Instruments.ts | 36 ++++++++----------- .../src/Measurement.ts | 3 +- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts index 5d91ced65f7..c257cf05480 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts @@ -16,7 +16,7 @@ import * as api from '@opentelemetry/api'; import * as metrics from '@opentelemetry/api-metrics'; -import { Measurement } from './Measurement'; +import { Attributes } from '@opentelemetry/api-metrics'; import { Meter } from './Meter'; export enum InstrumentType { @@ -42,22 +42,8 @@ export class Instrument { return this._version; } - aggregate(measurement: Measurement) { - this._meter.aggregate(this, measurement); - } -} - -export class UpDownCounter extends Instrument implements metrics.Counter { - add(value: number, attributes?: api.SpanAttributes, ctx?: api.Context): void { - if (typeof value != 'number') { - api.diag.warn(`invalid type value provided to counter ${this.getName()}: ${typeof value}`); - return; - } - - attributes = attributes ?? {}; - ctx = ctx ?? api.context.active(); - - this.aggregate({ + aggregate(value: number, attributes: Attributes = {}, ctx: api.Context = api.context.active()) { + this._meter.aggregate(this, { value, attributes, context: ctx, @@ -65,19 +51,25 @@ export class UpDownCounter extends Instrument implements metrics.Counter { } } -export class Counter extends UpDownCounter implements metrics.Counter { - override add(value: number, attributes?: api.SpanAttributes, ctx?: api.Context): void { +export class UpDownCounter extends Instrument implements metrics.Counter { + add(value: number, attributes?: Attributes, ctx?: api.Context): void { + this.aggregate(value, attributes, ctx); + } +} + +export class Counter extends Instrument implements metrics.Counter { + add(value: number, attributes?: Attributes, ctx?: api.Context): void { if (value < 0) { api.diag.warn(`negative value provided to counter ${this.getName()}: ${value}`); return; } - return super.add(value, attributes, ctx); + this.aggregate(value, attributes, ctx); } } export class Histogram extends Instrument implements metrics.Histogram { - record(_measurement: number, _labels?: metrics.Attributes, _context?: api.Context): void { - throw new Error('Method not implemented.'); + record(value: number, attributes?: Attributes, ctx?: api.Context): void { + this.aggregate(value, attributes, ctx); } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts index 17b6cee8f05..1d93945ca0c 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts @@ -15,10 +15,11 @@ */ import * as api from '@opentelemetry/api' +import { Attributes } from '@opentelemetry/api-metrics' export type Measurement = { value: number; // TODO use common attributes - attributes: api.SpanAttributes; + attributes: Attributes context?: api.Context; } From d80eb916924f40a75d563a7f92f6e3a49ad418a6 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Thu, 11 Nov 2021 13:03:44 -0500 Subject: [PATCH 06/14] chore: fix double import --- .../opentelemetry-sdk-metrics-base/src/Instruments.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts index c257cf05480..f964a069b8e 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts @@ -16,7 +16,6 @@ import * as api from '@opentelemetry/api'; import * as metrics from '@opentelemetry/api-metrics'; -import { Attributes } from '@opentelemetry/api-metrics'; import { Meter } from './Meter'; export enum InstrumentType { @@ -42,7 +41,7 @@ export class Instrument { return this._version; } - aggregate(value: number, attributes: Attributes = {}, ctx: api.Context = api.context.active()) { + aggregate(value: number, attributes: metrics.Attributes = {}, ctx: api.Context = api.context.active()) { this._meter.aggregate(this, { value, attributes, @@ -52,13 +51,13 @@ export class Instrument { } export class UpDownCounter extends Instrument implements metrics.Counter { - add(value: number, attributes?: Attributes, ctx?: api.Context): void { + add(value: number, attributes?: metrics.Attributes, ctx?: api.Context): void { this.aggregate(value, attributes, ctx); } } export class Counter extends Instrument implements metrics.Counter { - add(value: number, attributes?: Attributes, ctx?: api.Context): void { + add(value: number, attributes?: metrics.Attributes, ctx?: api.Context): void { if (value < 0) { api.diag.warn(`negative value provided to counter ${this.getName()}: ${value}`); return; @@ -69,7 +68,7 @@ export class Counter extends Instrument implements metrics.Counter { } export class Histogram extends Instrument implements metrics.Histogram { - record(value: number, attributes?: Attributes, ctx?: api.Context): void { + record(value: number, attributes?: metrics.Attributes, ctx?: api.Context): void { this.aggregate(value, attributes, ctx); } } From fa3254dbca5bdeb26dd0cbb73339141070226be3 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 12 Nov 2021 08:26:37 -0500 Subject: [PATCH 07/14] chore: implement basic sync instruments --- .../src/Instruments.ts | 18 ++++++------------ .../src/Meter.ts | 13 ++++++++----- .../src/MeterProvider.ts | 3 +-- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts index f964a069b8e..8015af9cb89 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts @@ -27,20 +27,14 @@ export enum InstrumentType { OBSERVABLE_UP_DOWN_COUNTER = 'OBSERVABLE_UP_DOWN_COUNTER', } -export class Instrument { - constructor(private _meter: Meter, private _name: string, private _version?: string) { } +export class SyncInstrument { + constructor(private _meter: Meter, private _name: string) { } getName(): string { return this._name; } - /** - * This only exists to stop typescript complaining about unused variable and may be removed. - */ - getVersion(): string | undefined { - return this._version; - } - + aggregate(value: number, attributes: metrics.Attributes = {}, ctx: api.Context = api.context.active()) { this._meter.aggregate(this, { value, @@ -50,13 +44,13 @@ export class Instrument { } } -export class UpDownCounter extends Instrument implements metrics.Counter { +export class UpDownCounter extends SyncInstrument implements metrics.Counter { add(value: number, attributes?: metrics.Attributes, ctx?: api.Context): void { this.aggregate(value, attributes, ctx); } } -export class Counter extends Instrument implements metrics.Counter { +export class Counter extends SyncInstrument implements metrics.Counter { add(value: number, attributes?: metrics.Attributes, ctx?: api.Context): void { if (value < 0) { api.diag.warn(`negative value provided to counter ${this.getName()}: ${value}`); @@ -67,7 +61,7 @@ export class Counter extends Instrument implements metrics.Counter { } } -export class Histogram extends Instrument implements metrics.Histogram { +export class Histogram extends SyncInstrument implements metrics.Histogram { record(value: number, attributes?: metrics.Attributes, ctx?: api.Context): void { this.aggregate(value, attributes, ctx); } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts index f7b5a9b5206..2c4fd1283ac 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts @@ -16,9 +16,9 @@ import * as metrics from '@opentelemetry/api-metrics'; import { InstrumentationLibrary } from '@opentelemetry/core'; +import { Counter, Histogram, UpDownCounter } from './Instruments'; import { Measurement } from './Measurement'; import { MeterProvider } from './MeterProvider'; -import { Histogram, Instrument } from './Instruments'; export class Meter implements metrics.Meter { // instrumentation library required by spec to be on meter @@ -36,14 +36,17 @@ export class Meter implements metrics.Meter { } createHistogram(_name: string, _options?: metrics.MetricOptions): Histogram { - throw new Error('Method not implemented.'); + return new Histogram(this, _name); } + createCounter(_name: string, _options?: metrics.MetricOptions): metrics.Counter { - throw new Error('Method not implemented.'); + return new Counter(this, _name); } + createUpDownCounter(_name: string, _options?: metrics.MetricOptions): metrics.UpDownCounter { - throw new Error('Method not implemented.'); + return new UpDownCounter(this, _name); } + createObservableGauge(_name: string, _options?: metrics.MetricOptions, _callback?: (observableResult: metrics.ObservableResult) => void): metrics.ObservableBase { throw new Error('Method not implemented.'); } @@ -54,7 +57,7 @@ export class Meter implements metrics.Meter { throw new Error('Method not implemented.'); } - public aggregate(metric: Instrument, measurement: Measurement) { + public aggregate(metric: unknown, measurement: Measurement) { this._provider.aggregate(this, metric, measurement); } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts index 718599005b5..7ceb752ff97 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts @@ -17,7 +17,6 @@ import * as api from '@opentelemetry/api'; import * as metrics from '@opentelemetry/api-metrics'; import { Resource } from '@opentelemetry/resources'; -import { Instrument } from './Instruments'; import { Measurement } from './Measurement'; import { Meter } from './Meter'; import { MetricExporter } from './MetricExporter'; @@ -148,7 +147,7 @@ export class MeterProvider { } } - public aggregate(_meter: Meter, _metric: Instrument, _measurement: Measurement) { + public aggregate(_meter: Meter, _metric: unknown, _measurement: Measurement) { // TODO actually aggregate /** From 3ce902db800d72a18059406fa3c915ad86a9a083 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 12 Nov 2021 08:27:42 -0500 Subject: [PATCH 08/14] chore: check for shutdown in metric exporter --- .../src/MetricExporter.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts index bdbbf708491..0b197fdd6f2 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts @@ -14,15 +14,22 @@ * limitations under the License. */ +// TODO should this just be an interface and exporters can implement their own shutdown? export abstract class MetricExporter { - private _shutdown = false; + protected _shutdown = false; + + // TODO: define the methods that actually export - must allow for push and pull exporters async shutdown(): Promise { - try { - await this.forceFlush(); - } finally { - this._shutdown = true; + if (this._shutdown) { + return; } + + // Setting _shutdown before flushing might prevent some exporters from flushing + // Waiting until flushing is complete might allow another flush to occur during shutdown + const flushPromise = this.forceFlush(); + this._shutdown = true; + await flushPromise; } abstract forceFlush(): Promise; @@ -33,7 +40,7 @@ export abstract class MetricExporter { } export class ConsoleMetricExporter extends MetricExporter { - export() { + async export() { throw new Error('Method not implemented'); } From 6d95c402f31de9673486496be5363fff485b55b1 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 12 Nov 2021 08:30:48 -0500 Subject: [PATCH 09/14] chore: add wildcard support todo --- experimental/packages/opentelemetry-sdk-metrics-base/src/View.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/View.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/View.ts index ef5870c18d4..2abbf8760f3 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/View.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/View.ts @@ -72,6 +72,7 @@ export class View { return true; } + // TODO wildcard support return this._selector.instrumentName === name; } From 8d4223d2db1fd72fd80446c58dc2569cee08f091 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 12 Nov 2021 08:35:49 -0500 Subject: [PATCH 10/14] chore: guard multi-shutdown on meter provider --- .../opentelemetry-sdk-metrics-base/src/MeterProvider.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts index 7ceb752ff97..b71989ae770 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts @@ -74,6 +74,11 @@ export class MeterProvider { async shutdown(): Promise { // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#shutdown + if (this._shutdown) { + api.diag.warn('shutdown may only be called once per MeterProvider'); + return; + } + // TODO add a timeout - spec leaves it up the the SDK if this is configurable this._shutdown = true; From 25a55d816b8f6df7c806bdc4efbededaa84e5875 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 12 Nov 2021 08:35:49 -0500 Subject: [PATCH 11/14] chore: guard against multiple shutdown --- .../opentelemetry-sdk-metrics-base/src/MeterProvider.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts index 7ceb752ff97..b71989ae770 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts @@ -74,6 +74,11 @@ export class MeterProvider { async shutdown(): Promise { // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#shutdown + if (this._shutdown) { + api.diag.warn('shutdown may only be called once per MeterProvider'); + return; + } + // TODO add a timeout - spec leaves it up the the SDK if this is configurable this._shutdown = true; From b286dbd83a8bc841d5419f7779867b92463fdedb Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 12 Nov 2021 11:29:30 -0500 Subject: [PATCH 12/14] chore: add throw todo --- .../opentelemetry-sdk-metrics-base/src/MeterProvider.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts index b71989ae770..de0b435fea7 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts @@ -59,6 +59,7 @@ export class MeterProvider { // Spec leaves it unspecified if creating a meter with duplicate // name/version returns the same meter. We create a new one here // for simplicity. This may change in the future. + // TODO: consider returning the same meter if the same name/version is used return new Meter(this, { name, version }, options.schemaUrl); } @@ -84,6 +85,7 @@ export class MeterProvider { // Shut down all exporters and readers. // Throw the first error and log all others. + // TODO make sure it is acceptable to throw here let err: unknown; for (const exporter of this._metricExporters) { try { From 753d0f926924d3f0609764ed23ee289ec136a2ee Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 12 Nov 2021 15:18:06 -0500 Subject: [PATCH 13/14] chore: do not throw in shutdown/flush --- .../src/MeterProvider.ts | 71 ++++++------------- 1 file changed, 21 insertions(+), 50 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts index de0b435fea7..e8fe86ef027 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts @@ -72,6 +72,10 @@ export class MeterProvider { this._views.push(view); } + /** + * Flush all buffered data and shut down the MeterProvider and all exporters and metric readers. + * Returns a promise which is resolved when all flushes are complete. + */ async shutdown(): Promise { // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#shutdown @@ -83,37 +87,13 @@ export class MeterProvider { // TODO add a timeout - spec leaves it up the the SDK if this is configurable this._shutdown = true; - // Shut down all exporters and readers. - // Throw the first error and log all others. - // TODO make sure it is acceptable to throw here - let err: unknown; - for (const exporter of this._metricExporters) { - try { - await exporter.shutdown(); - } catch (e) { - if (e instanceof Error) { - api.diag.error(`Error shutting down: ${e.message}`) - } - err = err || e; - } - } - - for (const reader of this._metricReaders) { - try { - await reader.shutdown(); - } catch (e) { - if (e instanceof Error) { - api.diag.error(`Error shutting down: ${e.message}`) - } - err = err || e; - } - } - - if (err != null) { - throw err; - } + await this._forceFlush(); } + /** + * Notifies all exporters and metric readers to flush any buffered data. + * Returns a promise which is resolved when all flushes are complete. + */ async forceFlush(): Promise { // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#forceflush @@ -121,37 +101,28 @@ export class MeterProvider { // do not flush after shutdown if (this._shutdown) { + api.diag.warn('invalid attempt to force flush after shutdown') return; } - // Flush all exporters and readers. - // Throw the first error and log all others. - let err: unknown; - for (const exporter of this._metricExporters) { - try { - await exporter.forceFlush(); - } catch (e) { - if (e instanceof Error) { - api.diag.error(`Error force flushing: ${e.message}`) - } - err = err || e; - } - } + await this._forceFlush(); + } - for (const reader of this._metricReaders) { + /** + * A private implementation of force flush which doesn't check if the function is shut down + */ + private async _forceFlush() { + // Shut down all exporters and readers. + // Catch and log all errors + for (const exporter of [...this._metricExporters, ...this._metricReaders]) { try { - await reader.forceFlush(); + await exporter.shutdown(); } catch (e) { if (e instanceof Error) { - api.diag.error(`Error force flushing: ${e.message}`) + api.diag.error(`Error shutting down: ${e.message}`) } - err = err || e; } } - - if (err != null) { - throw err; - } } public aggregate(_meter: Meter, _metric: unknown, _measurement: Measurement) { From 892c92ba98a3d14af1d6a99359f30b1caeb7507a Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Mon, 15 Nov 2021 08:42:08 -0500 Subject: [PATCH 14/14] chore: add links to spec Co-authored-by: Georg Pirklbauer --- .../packages/opentelemetry-sdk-metrics-base/src/Instruments.ts | 2 ++ .../packages/opentelemetry-sdk-metrics-base/src/Measurement.ts | 2 ++ .../packages/opentelemetry-sdk-metrics-base/src/Meter.ts | 2 ++ .../opentelemetry-sdk-metrics-base/src/MetricExporter.ts | 2 ++ .../packages/opentelemetry-sdk-metrics-base/src/MetricReader.ts | 2 ++ 5 files changed, 10 insertions(+) diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts index 8015af9cb89..8c5d1212d37 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts @@ -18,6 +18,8 @@ import * as api from '@opentelemetry/api'; import * as metrics from '@opentelemetry/api-metrics'; import { Meter } from './Meter'; +// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument + export enum InstrumentType { COUNTER = 'COUNTER', HISTOGRAM = 'HISTOGRAM', diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts index 1d93945ca0c..215426f9d76 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts @@ -17,6 +17,8 @@ import * as api from '@opentelemetry/api' import { Attributes } from '@opentelemetry/api-metrics' +// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#measurement + export type Measurement = { value: number; // TODO use common attributes diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts index 2c4fd1283ac..c7e16406ba3 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts @@ -20,6 +20,8 @@ import { Counter, Histogram, UpDownCounter } from './Instruments'; import { Measurement } from './Measurement'; import { MeterProvider } from './MeterProvider'; +// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#meter + export class Meter implements metrics.Meter { // instrumentation library required by spec to be on meter // spec requires provider config changes to apply to previously created meters, achieved by holding a reference to the provider diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts index 0b197fdd6f2..4046f041364 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts @@ -13,6 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + +// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#metricexporter // TODO should this just be an interface and exporters can implement their own shutdown? export abstract class MetricExporter { diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricReader.ts index 7b4e0e2a5c2..9f20299a4f2 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricReader.ts @@ -16,6 +16,8 @@ import { MetricExporter } from '.'; +// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#metricreader + export class MetricReader { private _shutdown = false;