From df291115633f43840b655dc0530c4351dab13db2 Mon Sep 17 00:00:00 2001 From: legendecas Date: Tue, 2 Aug 2022 18:04:19 +0800 Subject: [PATCH] feat(sdk-metrics-base): per metric-reader aggregation --- experimental/CHANGELOG.md | 1 + .../src/OTLPMetricExporter.ts | 8 +- .../test/OTLPMetricExporter.test.ts | 18 +- .../test/metricsHelper.ts | 5 + .../src/AggregationSelector.ts | 64 +++++++ .../src/OTLPMetricExporterBase.ts | 40 +---- .../src/index.ts | 1 - .../platform/browser/OTLPMetricExporter.ts | 7 +- .../src/platform/node/OTLPMetricExporter.ts | 7 +- .../browser/CollectorMetricExporter.test.ts | 62 ++++--- .../test/metricsHelper.ts | 5 + .../test/node/CollectorMetricExporter.test.ts | 32 ++-- .../src/OTLPMetricExporter.ts | 10 +- .../test/OTLPMetricExporter.test.ts | 17 +- .../test/metricsHelper.ts | 5 + .../src/PrometheusExporter.ts | 6 +- .../test/PrometheusExporter.test.ts | 29 ++- .../test/PrometheusSerializer.test.ts | 5 + .../src/export/AggregationSelector.ts | 29 +++ .../src/export/AggregationTemporality.ts | 4 - .../src/export/ConsoleMetricExporter.ts | 6 - .../src/export/InMemoryMetricExporter.ts | 5 - .../src/export/MetricExporter.ts | 8 - .../src/export/MetricReader.ts | 7 + .../export/PeriodicExportingMetricReader.ts | 49 ++++- .../src/index.ts | 1 + .../src/state/MeterProviderSharedState.ts | 11 +- .../src/state/MeterSharedState.ts | 71 +++++--- .../src/state/MetricCollector.ts | 6 +- .../src/state/MetricStorage.ts | 2 +- .../src/state/MetricStorageRegistry.ts | 85 +++++++-- .../src/view/ViewRegistry.ts | 6 - .../test/Instruments.test.ts | 7 +- .../test/export/MetricReader.test.ts | 51 +++++- .../PeriodicExportingMetricReader.test.ts | 64 +------ .../test/export/TestMetricExporter.ts | 12 +- .../test/export/TestMetricProducer.ts} | 18 +- .../test/export/TestMetricReader.ts | 26 ++- .../test/state/MeterSharedState.test.ts | 119 +++++++++++- .../test/state/MetricCollector.test.ts | 32 ++-- .../test/state/MetricStorageRegistry.test.ts | 169 ++++++++++++++---- .../test/view/ViewRegistry.test.ts | 7 - .../src/utils/environment.ts | 8 +- tsconfig.json | 45 +++-- 44 files changed, 771 insertions(+), 399 deletions(-) create mode 100644 experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/AggregationSelector.ts create mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationSelector.ts rename experimental/packages/{opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterOptions.ts => opentelemetry-sdk-metrics-base/test/export/TestMetricProducer.ts} (55%) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index ca329878e31..98f1f473c78 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -10,6 +10,7 @@ All notable changes to experimental packages in this project will be documented * feature(add-console-metrics-exporter): add ConsoleMetricExporter [#3120](https://github.com/open-telemetry/opentelemetry-js/pull/3120) @weyert * feature(prometheus-serialiser): export the unit block when unit is set in metric descriptor [#3066](https://github.com/open-telemetry/opentelemetry-js/pull/3041) @weyert +* feat(sdk-metrics-base): add per metric-reader aggregation support [#3153](https://github.com/open-telemetry/opentelemetry-js/pull/3153) @legendecas ### :bug: (Bug Fix) diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/OTLPMetricExporter.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/OTLPMetricExporter.ts index 90c6ceb92b8..c20c664486a 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/OTLPMetricExporter.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/OTLPMetricExporter.ts @@ -15,9 +15,7 @@ */ import { - defaultOptions, OTLPMetricExporterBase, - OTLPMetricExporterOptions } from '@opentelemetry/exporter-metrics-otlp-http'; import { ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; import { @@ -33,7 +31,7 @@ import { createExportMetricsServiceRequest, IExportMetricsServiceRequest } from class OTLPMetricExporterProxy extends OTLPGRPCExporterNodeBase { - constructor(config: OTLPGRPCExporterConfigNode & OTLPMetricExporterOptions= defaultOptions) { + constructor(config: OTLPGRPCExporterConfigNode = {}) { super(config); const headers = baggageUtils.parseKeyPairsIntoRecord(getEnv().OTEL_EXPORTER_OTLP_METRICS_HEADERS); this.metadata ||= new Metadata(); @@ -73,7 +71,7 @@ class OTLPMetricExporterProxy extends OTLPGRPCExporterNodeBase{ - constructor(config: OTLPGRPCExporterConfigNode & OTLPMetricExporterOptions = defaultOptions) { - super(new OTLPMetricExporterProxy(config), config); + constructor(config: OTLPGRPCExporterConfigNode = {}) { + super(new OTLPMetricExporterProxy(config)); } } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts index 3792c3e8dac..c5cdbf5fde8 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts @@ -33,7 +33,7 @@ import { mockHistogram, mockObservableGauge, setUp, shutdown, } from './metricsHelper'; -import { AggregationTemporality, ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; +import { ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; import { IExportMetricsServiceRequest, IResourceMetrics } from '@opentelemetry/otlp-transformer'; const metricsServiceProtoPath = @@ -128,7 +128,6 @@ const testOTLPMetricExporter = (params: TestParams) => url: 'https://' + address, credentials, metadata: params.metadata, - temporalityPreference: AggregationTemporality.CUMULATIVE }); setUp(); @@ -184,7 +183,6 @@ const testOTLPMetricExporter = (params: TestParams) => headers: { foo: 'bar', }, - temporalityPreference: AggregationTemporality.CUMULATIVE }); const args = warnStub.args[0]; assert.strictEqual(args[0], 'Headers cannot be set when using grpc'); @@ -192,7 +190,6 @@ const testOTLPMetricExporter = (params: TestParams) => it('should warn about path in url', () => { collectorExporter = new OTLPMetricExporter({ url: `http://${address}/v1/metrics`, - temporalityPreference: AggregationTemporality.CUMULATIVE }); const args = warnStub.args[0]; assert.strictEqual( @@ -214,13 +211,18 @@ const testOTLPMetricExporter = (params: TestParams) => assert.ok(exportedData, 'exportedData does not exist'); + // The order of the metrics is not guaranteed. + const counterIndex = exportedData[0].scopeMetrics[0].metrics.findIndex(it => it.name === 'int-counter'); + const observableIndex = exportedData[0].scopeMetrics[0].metrics.findIndex(it => it.name === 'double-observable-gauge'); + const histogramIndex = exportedData[0].scopeMetrics[0].metrics.findIndex(it => it.name === 'int-histogram'); + const resource = exportedData[0].resource; const counter = - exportedData[0].scopeMetrics[0].metrics[0]; + exportedData[0].scopeMetrics[0].metrics[counterIndex]; const observableGauge = - exportedData[0].scopeMetrics[0].metrics[1]; + exportedData[0].scopeMetrics[0].metrics[observableIndex]; const histogram = - exportedData[0].scopeMetrics[0].metrics[2]; + exportedData[0].scopeMetrics[0].metrics[histogramIndex]; ensureExportedCounterIsCorrect( counter, counter.sum?.dataPoints[0].timeUnixNano, @@ -264,7 +266,6 @@ describe('OTLPMetricExporter - node (getDefaultUrl)', () => { const url = 'http://foo.bar.com'; const collectorExporter = new OTLPMetricExporter({ url, - temporalityPreference: AggregationTemporality.CUMULATIVE }); setTimeout(() => { assert.strictEqual(collectorExporter._otlpExporter.url, 'foo.bar.com'); @@ -309,7 +310,6 @@ describe('when configuring via environment', () => { envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = 'foo=boo'; const collectorExporter = new OTLPMetricExporter({ metadata, - temporalityPreference: AggregationTemporality.CUMULATIVE }); assert.deepStrictEqual(collectorExporter._otlpExporter.metadata?.get('foo'), ['boo']); assert.deepStrictEqual(collectorExporter._otlpExporter.metadata?.get('bar'), ['foo']); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts index 62376db7bae..476bf51766d 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts @@ -20,6 +20,7 @@ import * as assert from 'assert'; import * as grpc from '@grpc/grpc-js'; import { VERSION } from '@opentelemetry/core'; import { + Aggregation, AggregationTemporality, ExplicitBucketHistogramAggregation, MeterProvider, @@ -29,6 +30,10 @@ import { import { IKeyValue, IMetric, IResource } from '@opentelemetry/otlp-transformer'; class TestMetricReader extends MetricReader { + selectAggregation() { + return Aggregation.Default(); + } + selectAggregationTemporality() { return AggregationTemporality.CUMULATIVE; } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/AggregationSelector.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/AggregationSelector.ts new file mode 100644 index 00000000000..93a53250d20 --- /dev/null +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/AggregationSelector.ts @@ -0,0 +1,64 @@ +/* + * 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 { getEnv } from '@opentelemetry/core'; +import { Aggregation, AggregationSelector, AggregationTemporality, AggregationTemporalitySelector, InstrumentType } from '@opentelemetry/sdk-metrics-base'; + +/** + * Default {@link AggregationTemporalitySelector} when the OTLP Metrics Exporter is configured with environ `OTEL_METRICS_EXPORTER`. + */ +const CumulativeTemporalitySelector: AggregationTemporalitySelector = () => AggregationTemporality.CUMULATIVE; + +/** + * Default {@link AggregationSelector} when the OTLP Metrics Exporter is configured with environ `OTEL_METRICS_EXPORTER`. + */ +const DeltaTemporalitySelector: AggregationTemporalitySelector = (instrumentType: InstrumentType) => { + switch (instrumentType) { + case InstrumentType.COUNTER: + case InstrumentType.OBSERVABLE_COUNTER: + case InstrumentType.HISTOGRAM: + case InstrumentType.OBSERVABLE_GAUGE: + return AggregationTemporality.DELTA; + case InstrumentType.UP_DOWN_COUNTER: + case InstrumentType.OBSERVABLE_UP_DOWN_COUNTER: + return AggregationTemporality.CUMULATIVE; + } +}; + +function chooseTemporalitySelector(temporalityPreference?: AggregationTemporality): AggregationTemporalitySelector { + if (temporalityPreference === AggregationTemporality.DELTA) { + return DeltaTemporalitySelector; + } + + return CumulativeTemporalitySelector; +} + +/** + * Get the default {@link AggregationTemporalitySelector} based on the environ `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE`. + */ +export function getDefaultAggregationTemporalitySelector(): AggregationTemporalitySelector { + const env = getEnv(); + const temporalityPreference = env.OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE !== 'cumulative' ? AggregationTemporality.DELTA : AggregationTemporality.CUMULATIVE; + return chooseTemporalitySelector(temporalityPreference); +} + +/** + * Get the default {@link AggregationSelector} based on the environ `OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION`. + */ +export function getDefaultAggregationSelector(): AggregationSelector { + // TODO(legendecas): exponential bucket histogram support. + return Aggregation.Default; +} diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts index 059d44eeca6..ea0c394cc3a 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts @@ -16,50 +16,20 @@ import { ExportResult } from '@opentelemetry/core'; import { - AggregationTemporality, - AggregationTemporalitySelector, - InstrumentType, PushMetricExporter, ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; -import { defaultOptions, OTLPMetricExporterOptions } from './OTLPMetricExporterOptions'; -import { OTLPExporterBase } from '@opentelemetry/otlp-exporter-base'; +import { OTLPExporterBase, OTLPExporterConfigBase } from '@opentelemetry/otlp-exporter-base'; import { IExportMetricsServiceRequest } from '@opentelemetry/otlp-transformer'; -export const CumulativeTemporalitySelector: AggregationTemporalitySelector = () => AggregationTemporality.CUMULATIVE; - -export const DeltaTemporalitySelector: AggregationTemporalitySelector = (instrumentType: InstrumentType) => { - switch (instrumentType) { - case InstrumentType.COUNTER: - case InstrumentType.OBSERVABLE_COUNTER: - case InstrumentType.HISTOGRAM: - case InstrumentType.OBSERVABLE_GAUGE: - return AggregationTemporality.DELTA; - case InstrumentType.UP_DOWN_COUNTER: - case InstrumentType.OBSERVABLE_UP_DOWN_COUNTER: - return AggregationTemporality.CUMULATIVE; - } -}; - -function chooseTemporalitySelector(temporalityPreference?: AggregationTemporality): AggregationTemporalitySelector { - if (temporalityPreference === AggregationTemporality.DELTA) { - return DeltaTemporalitySelector; - } - - return CumulativeTemporalitySelector; -} - -export class OTLPMetricExporterBase> implements PushMetricExporter { public _otlpExporter: T; - protected _aggregationTemporalitySelector: AggregationTemporalitySelector; - constructor(exporter: T, - config: OTLPMetricExporterOptions = defaultOptions) { + constructor(exporter: T) { this._otlpExporter = exporter; - this._aggregationTemporalitySelector = chooseTemporalitySelector(config.temporalityPreference); } export(metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void): void { @@ -73,8 +43,4 @@ implements PushMetricExporter { forceFlush(): Promise { return Promise.resolve(); } - - selectAggregationTemporality(instrumentType: InstrumentType): AggregationTemporality { - return this._aggregationTemporalitySelector(instrumentType); - } } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/index.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/index.ts index ee1b1574a63..71a643b4f81 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/index.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/index.ts @@ -15,5 +15,4 @@ */ export * from './platform'; -export * from './OTLPMetricExporterOptions'; export * from './OTLPMetricExporterBase'; diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/browser/OTLPMetricExporter.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/browser/OTLPMetricExporter.ts index e7aedd8d994..e5af892edd1 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/browser/OTLPMetricExporter.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/browser/OTLPMetricExporter.ts @@ -16,7 +16,6 @@ import { ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; import { baggageUtils, getEnv } from '@opentelemetry/core'; -import { defaultOptions, OTLPMetricExporterOptions } from '../../OTLPMetricExporterOptions'; import { OTLPMetricExporterBase } from '../../OTLPMetricExporterBase'; import { OTLPExporterBrowserBase, @@ -31,7 +30,7 @@ const DEFAULT_COLLECTOR_URL = `http://localhost:4318/${DEFAULT_COLLECTOR_RESOURC class OTLPExporterBrowserProxy extends OTLPExporterBrowserBase { - constructor(config: OTLPMetricExporterOptions & OTLPExporterConfigBase = defaultOptions) { + constructor(config: OTLPExporterConfigBase) { super(config); this._headers = Object.assign( this._headers, @@ -60,7 +59,7 @@ class OTLPExporterBrowserProxy extends OTLPExporterBrowserBase { - constructor(config: OTLPExporterConfigBase & OTLPMetricExporterOptions = defaultOptions) { - super(new OTLPExporterBrowserProxy(config), config); + constructor(config: OTLPExporterConfigBase = {}) { + super(new OTLPExporterBrowserProxy(config)); } } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/node/OTLPMetricExporter.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/node/OTLPMetricExporter.ts index 4652764f7d3..f3b074ff6f5 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/node/OTLPMetricExporter.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/node/OTLPMetricExporter.ts @@ -16,7 +16,6 @@ import { ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; import { getEnv, baggageUtils} from '@opentelemetry/core'; -import { defaultOptions, OTLPMetricExporterOptions } from '../../OTLPMetricExporterOptions'; import { OTLPMetricExporterBase } from '../../OTLPMetricExporterBase'; import { OTLPExporterNodeBase, @@ -31,7 +30,7 @@ const DEFAULT_COLLECTOR_URL = `http://localhost:4318/${DEFAULT_COLLECTOR_RESOURC class OTLPExporterNodeProxy extends OTLPExporterNodeBase { - constructor(config: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions = defaultOptions) { + constructor(config: OTLPExporterNodeConfigBase) { super(config); this.headers = Object.assign( this.headers, @@ -60,7 +59,7 @@ class OTLPExporterNodeProxy extends OTLPExporterNodeBase { - constructor(config: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions = defaultOptions) { - super(new OTLPExporterNodeProxy(config), config); + constructor(config: OTLPExporterNodeConfigBase = {}) { + super(new OTLPExporterNodeProxy(config)); } } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts index c76ca3ebdcd..b48791d3c6c 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts @@ -17,7 +17,7 @@ import { diag, DiagLogger, DiagLogLevel } from '@opentelemetry/api'; import { Counter, Histogram, } from '@opentelemetry/api-metrics'; import { ExportResultCode, hrTimeToNanoseconds } from '@opentelemetry/core'; -import { AggregationTemporality, ResourceMetrics, } from '@opentelemetry/sdk-metrics-base'; +import { ResourceMetrics, } from '@opentelemetry/sdk-metrics-base'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { OTLPMetricExporter } from '../../src/platform/browser'; @@ -34,7 +34,6 @@ import { setUp, shutdown, } from '../metricsHelper'; -import { OTLPMetricExporterOptions } from '../../src'; import { OTLPExporterConfigBase } from '@opentelemetry/otlp-exporter-base'; import { IExportMetricsServiceRequest } from '@opentelemetry/otlp-transformer'; @@ -96,9 +95,9 @@ describe('OTLPMetricExporter - web', () => { beforeEach(() => { collectorExporter = new OTLPMetricExporter({ url: 'http://foo.bar.com', - temporalityPreference: AggregationTemporality.CUMULATIVE }); }); + it('should successfully send metrics using sendBeacon', done => { collectorExporter.export(metrics, () => { }); @@ -109,16 +108,22 @@ describe('OTLPMetricExporter - web', () => { const blob: Blob = args[1]; const body = await blob.text(); const json = JSON.parse(body) as IExportMetricsServiceRequest; - const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[0]; - const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[1]; - const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[2]; + + // The order of the metrics is not guaranteed. + const counterIndex = metrics.scopeMetrics[0].metrics.findIndex(it => it.descriptor.name === 'int-counter'); + const observableIndex = metrics.scopeMetrics[0].metrics.findIndex(it => it.descriptor.name === 'double-observable-gauge2'); + const histogramIndex = metrics.scopeMetrics[0].metrics.findIndex(it => it.descriptor.name === 'int-histogram'); + + const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[counterIndex]; + const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[observableIndex]; + const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[histogramIndex]; assert.ok(typeof metric1 !== 'undefined', "metric doesn't exist"); ensureCounterIsCorrect( metric1, - hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[0].dataPoints[0].endTime), - hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[0].dataPoints[0].startTime) + hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0].endTime), + hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0].startTime) ); @@ -128,8 +133,8 @@ describe('OTLPMetricExporter - web', () => { ); ensureObservableGaugeIsCorrect( metric2, - hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[1].dataPoints[0].endTime), - hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[1].dataPoints[0].startTime), + hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0].endTime), + hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0].startTime), 6, 'double-observable-gauge2' ); @@ -140,8 +145,8 @@ describe('OTLPMetricExporter - web', () => { ); ensureHistogramIsCorrect( metric3, - hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[2].dataPoints[0].endTime), - hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[2].dataPoints[0].startTime), + hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0].endTime), + hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0].startTime), [0, 100], [0, 2, 0] ); @@ -193,7 +198,6 @@ describe('OTLPMetricExporter - web', () => { (window.navigator as any).sendBeacon = false; collectorExporter = new OTLPMetricExporter({ url: 'http://foo.bar.com', - temporalityPreference: AggregationTemporality.CUMULATIVE }); // Overwrites the start time to make tests consistent Object.defineProperty(collectorExporter, '_startTime', { @@ -216,15 +220,20 @@ describe('OTLPMetricExporter - web', () => { const body = request.requestBody; const json = JSON.parse(body) as IExportMetricsServiceRequest; - const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[0]; - const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[1]; - const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[2]; + // The order of the metrics is not guaranteed. + const counterIndex = metrics.scopeMetrics[0].metrics.findIndex(it => it.descriptor.name === 'int-counter'); + const observableIndex = metrics.scopeMetrics[0].metrics.findIndex(it => it.descriptor.name === 'double-observable-gauge2'); + const histogramIndex = metrics.scopeMetrics[0].metrics.findIndex(it => it.descriptor.name === 'int-histogram'); + + const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[counterIndex]; + const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[observableIndex]; + const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[histogramIndex]; assert.ok(typeof metric1 !== 'undefined', "metric doesn't exist"); ensureCounterIsCorrect( metric1, - hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[0].dataPoints[0].endTime), - hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[0].dataPoints[0].startTime) + hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0].endTime), + hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0].startTime) ); assert.ok( @@ -233,8 +242,8 @@ describe('OTLPMetricExporter - web', () => { ); ensureObservableGaugeIsCorrect( metric2, - hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[1].dataPoints[0].endTime), - hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[1].dataPoints[0].startTime), + hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0].endTime), + hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0].startTime), 6, 'double-observable-gauge2' ); @@ -245,8 +254,8 @@ describe('OTLPMetricExporter - web', () => { ); ensureHistogramIsCorrect( metric3, - hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[2].dataPoints[0].endTime), - hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[2].dataPoints[0].startTime), + hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0].endTime), + hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0].startTime), [0, 100], [0, 2, 0] ); @@ -313,12 +322,11 @@ describe('OTLPMetricExporter - web', () => { foo: 'bar', bar: 'baz', }; - let collectorExporterConfig: (OTLPExporterConfigBase & OTLPMetricExporterOptions) | undefined; + let collectorExporterConfig: OTLPExporterConfigBase; beforeEach(() => { collectorExporterConfig = { headers: customHeaders, - temporalityPreference: AggregationTemporality.CUMULATIVE }; server = sinon.fakeServer.create(); }); @@ -454,8 +462,7 @@ describe('when configuring via environment', () => { it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const collectorExporter = new OTLPMetricExporter({ - headers: {}, - temporalityPreference: AggregationTemporality.CUMULATIVE + headers: {} }); assert.strictEqual(collectorExporter['_otlpExporter']['_headers'].foo, 'bar'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; @@ -464,8 +471,7 @@ describe('when configuring via environment', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = 'foo=boo'; const collectorExporter = new OTLPMetricExporter({ - headers: {}, - temporalityPreference: AggregationTemporality.CUMULATIVE + headers: {} }); assert.strictEqual(collectorExporter['_otlpExporter']['_headers'].foo, 'boo'); assert.strictEqual(collectorExporter['_otlpExporter']['_headers'].bar, 'foo'); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/metricsHelper.ts index 90e4a39cd6d..3ab828510d4 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/metricsHelper.ts @@ -27,6 +27,7 @@ import { Resource } from '@opentelemetry/resources'; import * as assert from 'assert'; import { InstrumentationScope, VERSION } from '@opentelemetry/core'; import { + Aggregation, AggregationTemporality, ExplicitBucketHistogramAggregation, MeterProvider, @@ -57,6 +58,10 @@ class TestMetricReader extends MetricReader { return Promise.resolve(undefined); } + selectAggregation() { + return Aggregation.Default(); + } + selectAggregationTemporality() { return AggregationTemporality.CUMULATIVE; } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts index ddd4780b15c..d287d5676bf 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts @@ -20,9 +20,6 @@ import * as core from '@opentelemetry/core'; import * as assert from 'assert'; import * as http from 'http'; import * as sinon from 'sinon'; -import { - OTLPMetricExporterOptions -} from '../../src'; import { OTLPMetricExporter @@ -41,7 +38,7 @@ import { HISTOGRAM_AGGREGATION_VIEW, } from '../metricsHelper'; import { MockedResponse } from './nodeHelpers'; -import { AggregationTemporality, ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; +import { ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; import { Stream, PassThrough } from 'stream'; import { OTLPExporterError, OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base'; import { IExportMetricsServiceRequest } from '@opentelemetry/otlp-transformer'; @@ -52,7 +49,7 @@ const address = 'localhost:1501'; describe('OTLPMetricExporter - node with json over http', () => { let collectorExporter: OTLPMetricExporter; - let collectorExporterConfig: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions; + let collectorExporterConfig: OTLPExporterNodeConfigBase; let stubRequest: sinon.SinonStub; let metrics: ResourceMetrics; @@ -203,7 +200,6 @@ describe('OTLPMetricExporter - node with json over http', () => { url: 'http://foo.bar.com', keepAlive: true, httpAgentOptions: { keepAliveMsecs: 2000 }, - temporalityPreference: AggregationTemporality.CUMULATIVE }; collectorExporter = new OTLPMetricExporter(collectorExporterConfig); @@ -288,29 +284,34 @@ describe('OTLPMetricExporter - node with json over http', () => { const responseBody = buff.toString(); const json = JSON.parse(responseBody) as IExportMetricsServiceRequest; - const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[0]; - const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[1]; - const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[2]; + // The order of the metrics is not guaranteed. + const counterIndex = metrics.scopeMetrics[0].metrics.findIndex(it => it.descriptor.name === 'int-counter'); + const observableIndex = metrics.scopeMetrics[0].metrics.findIndex(it => it.descriptor.name === 'double-observable-gauge2'); + const histogramIndex = metrics.scopeMetrics[0].metrics.findIndex(it => it.descriptor.name === 'int-histogram'); + + const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[counterIndex]; + const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[observableIndex]; + const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[histogramIndex]; assert.ok(typeof metric1 !== 'undefined', "counter doesn't exist"); ensureCounterIsCorrect( metric1, - core.hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[0].dataPoints[0].endTime), - core.hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[0].dataPoints[0].startTime) + core.hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0].endTime), + core.hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0].startTime) ); assert.ok(typeof metric2 !== 'undefined', "observable gauge doesn't exist"); ensureObservableGaugeIsCorrect( metric2, - core.hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[1].dataPoints[0].endTime), - core.hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[1].dataPoints[0].startTime), + core.hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0].endTime), + core.hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0].startTime), 6, 'double-observable-gauge2' ); assert.ok(typeof metric3 !== 'undefined', "histogram doesn't exist"); ensureHistogramIsCorrect( metric3, - core.hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[2].dataPoints[0].endTime), - core.hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[2].dataPoints[0].startTime), + core.hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0].endTime), + core.hrTimeToNanoseconds(metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0].startTime), [0, 100], [0, 2, 0] ); @@ -397,7 +398,6 @@ describe('OTLPMetricExporter - node with json over http', () => { const url = 'http://foo.bar.com'; const collectorExporter = new OTLPMetricExporter({ url, - temporalityPreference: AggregationTemporality.CUMULATIVE }); setTimeout(() => { assert.strictEqual(collectorExporter._otlpExporter.url, url); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts index 925be74611f..a453e456add 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts @@ -14,10 +14,6 @@ * limitations under the License. */ -import { - defaultOptions, - OTLPMetricExporterOptions -} from '@opentelemetry/exporter-metrics-otlp-http'; import { ServiceClientType, OTLPProtoExporterNodeBase } from '@opentelemetry/otlp-proto-exporter-base'; import { getEnv, baggageUtils} from '@opentelemetry/core'; import { ResourceMetrics} from '@opentelemetry/sdk-metrics-base'; @@ -34,7 +30,7 @@ const DEFAULT_COLLECTOR_URL = `http://localhost:4318/${DEFAULT_COLLECTOR_RESOURC class OTLPMetricExporterNodeProxy extends OTLPProtoExporterNodeBase { - constructor(config: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions = defaultOptions) { + constructor(config: OTLPExporterNodeConfigBase = {}) { super(config); this.headers = Object.assign( this.headers, @@ -64,7 +60,7 @@ class OTLPMetricExporterNodeProxy extends OTLPProtoExporterNodeBase { - constructor(config: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions = defaultOptions) { - super(new OTLPMetricExporterNodeProxy(config), config); + constructor(config: OTLPExporterNodeConfigBase = {}) { + super(new OTLPMetricExporterNodeProxy(config)); } } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts index ad81bb5e6a7..f01ba3067dc 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts @@ -32,8 +32,7 @@ import { mockObservableGauge, mockHistogram, collect, setUp, shutdown, } from './metricsHelper'; -import { AggregationTemporality, ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; -import { OTLPMetricExporterOptions } from '@opentelemetry/exporter-metrics-otlp-http'; +import { ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; import { Stream, PassThrough } from 'stream'; import { OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base'; import { IExportMetricsServiceRequest } from '@opentelemetry/otlp-transformer'; @@ -42,7 +41,7 @@ let fakeRequest: PassThrough; describe('OTLPMetricExporter - node with proto over http', () => { let collectorExporter: OTLPMetricExporter; - let collectorExporterConfig: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions; + let collectorExporterConfig: OTLPExporterNodeConfigBase; let metrics: ResourceMetrics; afterEach(() => { @@ -153,7 +152,6 @@ describe('OTLPMetricExporter - node with proto over http', () => { url: 'http://foo.bar.com', keepAlive: true, httpAgentOptions: { keepAliveMsecs: 2000 }, - temporalityPreference: AggregationTemporality.CUMULATIVE }; collectorExporter = new OTLPMetricExporter(collectorExporterConfig); setUp(); @@ -240,9 +238,14 @@ describe('OTLPMetricExporter - node with proto over http', () => { const data = ExportTraceServiceRequestProto.decode(buff); const json = data?.toJSON() as IExportMetricsServiceRequest; - const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[0]; - const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[1]; - const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[2]; + // The order of the metrics is not guaranteed. + const counterIndex = metrics.scopeMetrics[0].metrics.findIndex(it => it.descriptor.name === 'int-counter'); + const observableIndex = metrics.scopeMetrics[0].metrics.findIndex(it => it.descriptor.name === 'double-observable-gauge'); + const histogramIndex = metrics.scopeMetrics[0].metrics.findIndex(it => it.descriptor.name === 'int-histogram'); + + const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[counterIndex]; + const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[observableIndex]; + const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[histogramIndex]; assert.ok(typeof metric1 !== 'undefined', "counter doesn't exist"); ensureExportedCounterIsCorrect( diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts index 06527896898..98daa011840 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts @@ -24,6 +24,7 @@ import { import { Resource } from '@opentelemetry/resources'; import * as assert from 'assert'; import { + Aggregation, AggregationTemporality, ExplicitBucketHistogramAggregation, MeterProvider, @@ -34,6 +35,10 @@ import { IExportMetricsServiceRequest, IKeyValue, IMetric } from '@opentelemetry import { Stream } from 'stream'; export class TestMetricReader extends MetricReader { + selectAggregation() { + return Aggregation.Default(); + } + selectAggregationTemporality() { return AggregationTemporality.CUMULATIVE; } diff --git a/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts b/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts index dd00bfee770..928a4a30db9 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts @@ -18,7 +18,7 @@ import { diag } from '@opentelemetry/api'; import { globalErrorHandler, } from '@opentelemetry/core'; -import { AggregationTemporality, MetricReader } from '@opentelemetry/sdk-metrics-base'; +import { Aggregation, AggregationTemporality, MetricReader } from '@opentelemetry/sdk-metrics-base'; import { createServer, IncomingMessage, Server, ServerResponse } from 'http'; import { ExporterConfig } from './export/types'; import { PrometheusSerializer } from './PrometheusSerializer'; @@ -90,6 +90,10 @@ export class PrometheusExporter extends MetricReader { } } + selectAggregation(): Aggregation { + return Aggregation.Default(); + } + selectAggregationTemporality(): AggregationTemporality { return AggregationTemporality.CUMULATIVE; } diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts index 6439b57c2e7..78b22a490bd 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts @@ -15,9 +15,7 @@ */ import { Meter, ObservableResult } from '@opentelemetry/api-metrics'; -import { - MeterProvider, -} from '@opentelemetry/sdk-metrics-base'; +import { MeterProvider } from '@opentelemetry/sdk-metrics-base'; import * as assert from 'assert'; import * as sinon from 'sinon'; import * as http from 'http'; @@ -480,22 +478,19 @@ describe('PrometheusExporter', () => { let meter: Meter; let meterProvider: MeterProvider; let counter: Counter; - let exporter: PrometheusExporter | undefined; + let exporter: PrometheusExporter; - beforeEach(() => { + function setup(reader: PrometheusExporter) { meterProvider = new MeterProvider(); + meterProvider.addMetricReader(reader); + meter = meterProvider.getMeter('test-prometheus'); counter = meter.createCounter('counter'); counter.add(10, { key1: 'attributeValue1' }); - }); + } - afterEach(done => { - if (exporter) { - exporter.shutdown().then(done); - exporter = undefined; - } else { - done(); - } + afterEach(async () => { + await exporter.shutdown(); }); it('should use a configured name prefix', done => { @@ -504,7 +499,7 @@ describe('PrometheusExporter', () => { prefix: 'test_prefix', }, async () => { - meterProvider.addMetricReader(exporter!); + setup(exporter); http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { @@ -532,7 +527,7 @@ describe('PrometheusExporter', () => { port: 8080, }, async () => { - meterProvider.addMetricReader(exporter!); + setup(exporter); http .get('http://localhost:8080/metrics', res => { res.on('data', chunk => { @@ -560,7 +555,7 @@ describe('PrometheusExporter', () => { endpoint: '/test', }, async () => { - meterProvider.addMetricReader(exporter!); + setup(exporter); http .get('http://localhost:9464/test', res => { res.on('data', chunk => { @@ -588,7 +583,7 @@ describe('PrometheusExporter', () => { appendTimestamp: false, }, async () => { - meterProvider.addMetricReader(exporter!); + setup(exporter); http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts index b0fc47ce2db..476538c5b1d 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts @@ -17,6 +17,7 @@ import * as assert from 'assert'; import { MetricAttributes, UpDownCounter } from '@opentelemetry/api-metrics'; import { + Aggregation, AggregationTemporality, DataPoint, DataPointType, @@ -46,6 +47,10 @@ class TestMetricReader extends MetricReader { return AggregationTemporality.CUMULATIVE; } + selectAggregation() { + return Aggregation.Default(); + } + async onForceFlush() {} async onShutdown() {} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationSelector.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationSelector.ts new file mode 100644 index 00000000000..8a372d0e74d --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationSelector.ts @@ -0,0 +1,29 @@ +/* + * 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 '../InstrumentDescriptor'; +import { Aggregation } from '../view/Aggregation'; +import { AggregationTemporality } from './AggregationTemporality'; + +/** + * Aggregation selector based on metric instrument types. + */ +export type AggregationSelector = (instrumentType: InstrumentType) => Aggregation; + +/** + * Aggregation temporality selector based on metric instrument types. + */ +export type AggregationTemporalitySelector = (instrumentType: InstrumentType) => AggregationTemporality; diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationTemporality.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationTemporality.ts index 0b936714722..6cc6d1231bf 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationTemporality.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/AggregationTemporality.ts @@ -14,8 +14,6 @@ * limitations under the License. */ -import { InstrumentType } from '../InstrumentDescriptor'; - /** * AggregationTemporality indicates the way additive quantities are expressed. */ @@ -23,5 +21,3 @@ export enum AggregationTemporality { DELTA, CUMULATIVE, } - -export type AggregationTemporalitySelector = (instrumentType: InstrumentType) => AggregationTemporality; diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/ConsoleMetricExporter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/ConsoleMetricExporter.ts index 39b268f1930..a798ccde063 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/ConsoleMetricExporter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/ConsoleMetricExporter.ts @@ -14,8 +14,6 @@ * limitations under the License. */ import { ExportResult, ExportResultCode } from '@opentelemetry/core'; -import { InstrumentType } from '../InstrumentDescriptor'; -import { AggregationTemporality } from './AggregationTemporality'; import { ResourceMetrics } from './MetricData'; import { PushMetricExporter } from './MetricExporter'; @@ -37,10 +35,6 @@ export class ConsoleMetricExporter implements PushMetricExporter { return Promise.resolve(); } - selectAggregationTemporality(_instrumentType: InstrumentType): AggregationTemporality { - return AggregationTemporality.CUMULATIVE; - } - shutdown(): Promise { this._shutdown = true; return Promise.resolve(); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/InMemoryMetricExporter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/InMemoryMetricExporter.ts index c0ec67e74ab..091d82f33a2 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/InMemoryMetricExporter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/InMemoryMetricExporter.ts @@ -16,7 +16,6 @@ import { ExportResultCode } from '@opentelemetry/core'; import { ExportResult } from '@opentelemetry/core'; -import { InstrumentType } from '../InstrumentDescriptor'; import { AggregationTemporality } from './AggregationTemporality'; import { ResourceMetrics } from './MetricData'; import { PushMetricExporter } from './MetricExporter'; @@ -65,10 +64,6 @@ export class InMemoryMetricExporter implements PushMetricExporter { this._metrics = []; } - selectAggregationTemporality(_instrumentType: InstrumentType): AggregationTemporality { - return this._aggregationTemporality; - } - shutdown(): Promise { this._shutdown = true; return Promise.resolve(); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts index 782a39ccb8a..9d175604e38 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricExporter.ts @@ -14,12 +14,10 @@ * limitations under the License. */ -import { AggregationTemporality } from './AggregationTemporality'; import { ResourceMetrics } from './MetricData'; import { ExportResult, } from '@opentelemetry/core'; -import { InstrumentType } from '../InstrumentDescriptor'; /** * An interface that allows different metric services to export recorded data @@ -40,12 +38,6 @@ export interface PushMetricExporter { */ forceFlush(): Promise; - /** - * Select the {@link AggregationTemporality} for the given - * {@link InstrumentType} for this exporter. - */ - selectAggregationTemporality(instrumentType: InstrumentType): AggregationTemporality; - /** * Returns a promise which resolves when the last exportation is completed. * Further calls to {@link PushMetricExporter.export} may not export the diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts index 35d86c5d943..3037384fcf9 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricReader.ts @@ -21,6 +21,7 @@ import { CollectionResult } from './MetricData'; import { callWithTimeout } from '../utils'; import { InstrumentType } from '../InstrumentDescriptor'; import { CollectionOptions, ForceFlushOptions, ShutdownOptions } from '../types'; +import { Aggregation } from '../view/Aggregation'; /** * A registered reader of metrics that, when linked to a {@link MetricProducer}, offers global @@ -46,6 +47,12 @@ export abstract class MetricReader { this.onInitialized(); } + /** + * Select the {@link Aggregation} for the given {@link InstrumentType} for this + * reader. + */ + abstract selectAggregation(instrumentType: InstrumentType): Aggregation; + /** * Select the {@link AggregationTemporality} for the given * {@link InstrumentType} for this reader. diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts index 3e188c02931..215e4de18dc 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/PeriodicExportingMetricReader.ts @@ -25,25 +25,51 @@ import { AggregationTemporality } from './AggregationTemporality'; import { InstrumentType } from '../InstrumentDescriptor'; import { PushMetricExporter } from './MetricExporter'; import { callWithTimeout, TimeoutError } from '../utils'; +import { Aggregation } from '../view/Aggregation'; +import { AggregationSelector, AggregationTemporalitySelector } from './AggregationSelector'; export type PeriodicExportingMetricReaderOptions = { - exporter: PushMetricExporter - exportIntervalMillis?: number, - exportTimeoutMillis?: number + /** + * Aggregation selector based on metric instrument types. If no views are + * configured for a metric instrument, a per-metric-reader aggregation is + * selected with this selector. + */ + aggregationSelector?: AggregationSelector; + /** + * Aggregation temporality selector based on metric instrument types. A + * per-metric-reader aggregation temporality is selected for each metric + * instrument type with this selector. + */ + aggregationTemporalitySelector?: AggregationTemporalitySelector; + /** + * The backing exporter for the metric reader. + */ + exporter: PushMetricExporter; + /** + * An internal milliseconds for the metric reader to initiate metric + * collection. + */ + exportIntervalMillis?: number; + /** + * Milliseconds for the async observable callback to timeout. + */ + exportTimeoutMillis?: number; }; +const DEFAULT_AGGREGATION_SELECTOR: AggregationSelector = Aggregation.Default; +const DEFAULT_AGGREGATION_TEMPORALITY_SELECTOR: AggregationTemporalitySelector = () => AggregationTemporality.CUMULATIVE; + /** * {@link MetricReader} which collects metrics based on a user-configurable time interval, and passes the metrics to * the configured {@link MetricExporter} */ export class PeriodicExportingMetricReader extends MetricReader { private _interval?: ReturnType; - private _exporter: PushMetricExporter; - private readonly _exportInterval: number; - private readonly _exportTimeout: number; + private readonly _aggregationSelector: AggregationSelector; + private readonly _aggregationTemporalitySelector: AggregationTemporalitySelector; constructor(options: PeriodicExportingMetricReaderOptions) { super(); @@ -65,6 +91,8 @@ export class PeriodicExportingMetricReader extends MetricReader { this._exportInterval = options.exportIntervalMillis ?? 60000; this._exportTimeout = options.exportTimeoutMillis ?? 30000; this._exporter = options.exporter; + this._aggregationSelector = options.aggregationSelector ?? DEFAULT_AGGREGATION_SELECTOR; + this._aggregationTemporalitySelector = options.aggregationTemporalitySelector ?? DEFAULT_AGGREGATION_TEMPORALITY_SELECTOR; } private async _runOnce(): Promise { @@ -119,10 +147,17 @@ export class PeriodicExportingMetricReader extends MetricReader { await this._exporter.shutdown(); } + /** + * @inheritdoc + */ + selectAggregation(instrumentType: InstrumentType): Aggregation { + return this._aggregationSelector(instrumentType); + } + /** * @inheritdoc */ selectAggregationTemporality(instrumentType: InstrumentType): AggregationTemporality { - return this._exporter.selectAggregationTemporality(instrumentType); + return this._aggregationTemporalitySelector(instrumentType); } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/index.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/index.ts index e9ad1ef4b94..e5254bb7618 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/index.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/index.ts @@ -15,6 +15,7 @@ */ export { Sum, LastValue, Histogram } from './aggregator/types'; +export * from './export/AggregationSelector'; export * from './export/AggregationTemporality'; export * from './export/MetricData'; export * from './export/MetricExporter'; diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MeterProviderSharedState.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MeterProviderSharedState.ts index 87fe540ee56..14f8bd86561 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MeterProviderSharedState.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MeterProviderSharedState.ts @@ -16,10 +16,11 @@ import { InstrumentationScope } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; +import { Aggregation, InstrumentType } from '..'; import { instrumentationScopeId } from '../utils'; import { ViewRegistry } from '../view/ViewRegistry'; import { MeterSharedState } from './MeterSharedState'; -import { MetricCollector } from './MetricCollector'; +import { MetricCollector, MetricCollectorHandle } from './MetricCollector'; /** * An internal record for shared meter provider states. @@ -42,4 +43,12 @@ export class MeterProviderSharedState { } return meterSharedState; } + + selectAggregators(instrumentType: InstrumentType) { + const result: [MetricCollectorHandle, Aggregation][] = []; + for (const collector of this.metricCollectors) { + result.push([collector, collector.selectAggregation(instrumentType)]); + } + return result; + } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MeterSharedState.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MeterSharedState.ts index ed96aca444a..f30b469b0e0 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MeterSharedState.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MeterSharedState.ts @@ -20,7 +20,7 @@ import { MetricCollectOptions } from '../export/MetricProducer'; import { ScopeMetrics } from '../export/MetricData'; import { createInstrumentDescriptorWithView, InstrumentDescriptor } from '../InstrumentDescriptor'; import { Meter } from '../Meter'; -import { isNotNullish } from '../utils'; +import { isNotNullish, Maybe } from '../utils'; import { AsyncMetricStorage } from './AsyncMetricStorage'; import { MeterProviderSharedState } from './MeterProviderSharedState'; import { MetricCollectorHandle } from './MetricCollector'; @@ -28,12 +28,15 @@ import { MetricStorageRegistry } from './MetricStorageRegistry'; import { MultiMetricStorage } from './MultiWritableMetricStorage'; import { ObservableRegistry } from './ObservableRegistry'; import { SyncMetricStorage } from './SyncMetricStorage'; +import { Accumulation, Aggregator } from '../aggregator/types'; +import { AttributesProcessor } from '../view/AttributesProcessor'; +import { MetricStorage } from './MetricStorage'; /** * An internal record for shared meter provider states. */ export class MeterSharedState { - private _metricStorageRegistry = new MetricStorageRegistry(); + metricStorageRegistry = new MetricStorageRegistry(); observableRegistry = new ObservableRegistry(); meter: Meter; @@ -42,15 +45,8 @@ export class MeterSharedState { } registerMetricStorage(descriptor: InstrumentDescriptor) { - const views = this._meterProviderSharedState.viewRegistry.findViews(descriptor, this._instrumentationScope); - const storages = views - .map(view => { - const viewDescriptor = createInstrumentDescriptorWithView(view, descriptor); - const aggregator = view.aggregation.createAggregator(viewDescriptor); - const storage = new SyncMetricStorage(viewDescriptor, aggregator, view.attributesProcessor); - return this._metricStorageRegistry.register(storage); - }) - .filter(isNotNullish); + const storages = this._registerMetricStorage(descriptor, SyncMetricStorage); + if (storages.length === 1) { return storages[0]; } @@ -58,15 +54,8 @@ export class MeterSharedState { } registerAsyncMetricStorage(descriptor: InstrumentDescriptor) { - const views = this._meterProviderSharedState.viewRegistry.findViews(descriptor, this._instrumentationScope); - const storages = views - .map(view => { - const viewDescriptor = createInstrumentDescriptorWithView(view, descriptor); - const aggregator = view.aggregation.createAggregator(viewDescriptor); - const viewStorage = new AsyncMetricStorage(viewDescriptor, aggregator, view.attributesProcessor); - return this._metricStorageRegistry.register(viewStorage); - }) - .filter(isNotNullish); + const storages = this._registerMetricStorage(descriptor, AsyncMetricStorage); + return storages; } @@ -81,7 +70,7 @@ export class MeterSharedState { * 2. Collect metric result for the collector. */ const errors = await this.observableRegistry.observe(collectionTime, options?.timeoutMillis); - const metricDataList = Array.from(this._metricStorageRegistry.getStorages()) + const metricDataList = Array.from(this.metricStorageRegistry.getStorages(collector)) .map(metricStorage => { return metricStorage.collect( collector, @@ -98,9 +87,49 @@ export class MeterSharedState { errors, }; } + + private _registerMetricStorage>(descriptor: InstrumentDescriptor, MetricStorageType: MetricStorageType): R[] { + const views = this._meterProviderSharedState.viewRegistry.findViews(descriptor, this._instrumentationScope); + let storages = views + .map(view => { + const viewDescriptor = createInstrumentDescriptorWithView(view, descriptor); + const compatibleStorage = this.metricStorageRegistry.findOrUpdateCompatibleStorage(viewDescriptor); + if (compatibleStorage != null) { + return compatibleStorage; + } + const aggregator = view.aggregation.createAggregator(viewDescriptor); + const viewStorage = new MetricStorageType(viewDescriptor, aggregator, view.attributesProcessor) as R; + this.metricStorageRegistry.register(viewStorage); + return viewStorage; + }); + + // Fallback to the per-collector aggregations if no view is configured for the instrument. + if (storages.length === 0) { + const perCollectorAggregations = this._meterProviderSharedState.selectAggregators(descriptor.type); + const collectorStorages = perCollectorAggregations.map(([collector, aggregation]) => { + const compatibleStorage = this.metricStorageRegistry.findOrUpdateCompatibleCollectorStorage(collector, descriptor); + if (compatibleStorage != null) { + return compatibleStorage; + } + const aggregator = aggregation.createAggregator(descriptor); + const storage = new MetricStorageType(descriptor, aggregator, AttributesProcessor.Noop()) as R; + this.metricStorageRegistry.registerForCollector(collector, storage); + return storage; + }); + storages = storages.concat(collectorStorages); + } + + return storages; + } } interface ScopeMetricsResult { scopeMetrics: ScopeMetrics; errors: unknown[]; } + +interface MetricStorageConstructor { + new (instrumentDescriptor: InstrumentDescriptor, + aggregator: Aggregator>, + attributesProcessor: AttributesProcessor): MetricStorage; +} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts index 066106e47d4..fbffc2f0605 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts @@ -15,7 +15,7 @@ */ import { hrTime } from '@opentelemetry/core'; -import { AggregationTemporalitySelector } from '../export/AggregationTemporality'; +import { AggregationTemporalitySelector } from '../export/AggregationSelector'; import { CollectionResult } from '../export/MetricData'; import { MetricProducer, MetricCollectOptions } from '../export/MetricProducer'; import { MetricReader } from '../export/MetricReader'; @@ -65,6 +65,10 @@ export class MetricCollector implements MetricProducer { selectAggregationTemporality(instrumentType: InstrumentType) { return this._metricReader.selectAggregationTemporality(instrumentType); } + + selectAggregation(instrumentType: InstrumentType) { + return this._metricReader.selectAggregation(instrumentType); + } } /** diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorage.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorage.ts index 8e24da668ab..f69a00daa45 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorage.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorage.ts @@ -41,7 +41,7 @@ export abstract class MetricStorage { collectionTime: HrTime, ): Maybe; - getInstrumentDescriptor(): InstrumentDescriptor{ + getInstrumentDescriptor(): Readonly { return this._instrumentDescriptor; } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorageRegistry.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorageRegistry.ts index 1a930cacb05..c65a1dd7b2a 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorageRegistry.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorageRegistry.ts @@ -15,40 +15,93 @@ */ import { MetricStorage } from './MetricStorage'; -import { isDescriptorCompatibleWith } from '../InstrumentDescriptor'; +import { InstrumentDescriptor, isDescriptorCompatibleWith } from '../InstrumentDescriptor'; import * as api from '@opentelemetry/api'; -import { Maybe } from '../utils'; import { getConflictResolutionRecipe, getIncompatibilityDetails } from '../view/RegistrationConflicts'; +import { MetricCollectorHandle } from './MetricCollector'; + +type StorageMap = Map; /** * Internal class for storing {@link MetricStorage} */ export class MetricStorageRegistry { - private readonly _metricStorageRegistry = new Map(); + private readonly _sharedRegistry: StorageMap = new Map(); + private readonly _perCollectorRegistry = new Map(); static create(){ return new MetricStorageRegistry(); } - getStorages(): MetricStorage[] { + getStorages(collector: MetricCollectorHandle): MetricStorage[] { let storages: MetricStorage[] = []; - for (const metricStorages of this._metricStorageRegistry.values()) { + for (const metricStorages of this._sharedRegistry.values()) { storages = storages.concat(metricStorages); } + const perCollectorStorages = this._perCollectorRegistry.get(collector); + if (perCollectorStorages != null) { + for (const metricStorages of perCollectorStorages.values()) { + storages = storages.concat(metricStorages); + } + } + return storages; } - register(storage: T): Maybe { - const expectedDescriptor = storage.getInstrumentDescriptor(); - const existingStorages = this._metricStorageRegistry.get(expectedDescriptor.name); + register(storage: MetricStorage) { + this._registerStorage(storage, this._sharedRegistry); + } + + registerForCollector(collector: MetricCollectorHandle, storage: MetricStorage) { + let storageMap = this._perCollectorRegistry.get(collector); + if (storageMap == null) { + storageMap = new Map(); + this._perCollectorRegistry.set(collector, storageMap); + } + this._registerStorage(storage, storageMap); + } + + findOrUpdateCompatibleStorage(expectedDescriptor: InstrumentDescriptor): T | null { + const storages = this._sharedRegistry.get(expectedDescriptor.name); + if (storages === undefined) { + return null; + } + + // If the descriptor is compatible, the type of their metric storage + // (either SyncMetricStorage or AsyncMetricStorage) must be compatible. + return this._findOrUpdateCompatibleStorage(expectedDescriptor, storages); + } - // Add storage if it does not exist. - if (existingStorages === undefined) { - this._metricStorageRegistry.set(expectedDescriptor.name, [storage]); - return storage; + findOrUpdateCompatibleCollectorStorage(collector: MetricCollectorHandle, expectedDescriptor: InstrumentDescriptor): T | null { + const storageMap = this._perCollectorRegistry.get(collector); + if (storageMap === undefined) { + return null; } + const storages = this._sharedRegistry.get(expectedDescriptor.name); + if (storages === undefined) { + return null; + } + + // If the descriptor is compatible, the type of their metric storage + // (either SyncMetricStorage or AsyncMetricStorage) must be compatible. + return this._findOrUpdateCompatibleStorage(expectedDescriptor, storages); + } + + private _registerStorage(storage: MetricStorage, storageMap: StorageMap) { + const descriptor = storage.getInstrumentDescriptor(); + const storages = storageMap.get(descriptor.name); + + if (storages === undefined) { + storageMap.set(descriptor.name, [storage]); + return; + } + + storages.push(storage); + } + + private _findOrUpdateCompatibleStorage(expectedDescriptor: InstrumentDescriptor, existingStorages: MetricStorage[]): T | null { let compatibleStorage = null; for (const existingStorage of existingStorages) { @@ -84,12 +137,6 @@ export class MetricStorageRegistry { } } - if (compatibleStorage != null) { - return compatibleStorage; - } - - // None of the storages were compatible, add the current one to the list. - existingStorages.push(storage); - return storage; + return compatibleStorage; } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/view/ViewRegistry.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/view/ViewRegistry.ts index 1a26387d10a..1dcaf7d22f2 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/view/ViewRegistry.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/view/ViewRegistry.ts @@ -21,9 +21,6 @@ import { MeterSelector } from './MeterSelector'; import { View } from './View'; export class ViewRegistry { - private static DEFAULT_VIEW = new View({ - instrumentName: '*' - }); private _registeredViews: View[] = []; addView(view: View) { @@ -37,9 +34,6 @@ export class ViewRegistry { this._matchMeter(registeredView.meterSelector, meter); }); - if (views.length === 0) { - return [ViewRegistry.DEFAULT_VIEW]; - } return views; } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts index 2bb47be3b2d..772e268fa4a 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts @@ -19,7 +19,6 @@ import * as sinon from 'sinon'; import { InstrumentationScope } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { - AggregationTemporality, InstrumentDescriptor, InstrumentType, MeterProvider, @@ -28,7 +27,7 @@ import { DataPointType, Histogram } from '../src'; -import { TestMetricReader } from './export/TestMetricReader'; +import { TestDeltaMetricReader, TestMetricReader } from './export/TestMetricReader'; import { assertMetricData, assertDataPoint, @@ -654,9 +653,9 @@ function setup() { const meter = meterProvider.getMeter(defaultInstrumentationScope.name, defaultInstrumentationScope.version, { schemaUrl: defaultInstrumentationScope.schemaUrl, }); - const deltaReader = new TestMetricReader(() => AggregationTemporality.DELTA); + const deltaReader = new TestDeltaMetricReader(); meterProvider.addMetricReader(deltaReader); - const cumulativeReader = new TestMetricReader(() => AggregationTemporality.CUMULATIVE); + const cumulativeReader = new TestMetricReader(); meterProvider.addMetricReader(cumulativeReader); return { diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/MetricReader.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/MetricReader.test.ts index 1ee88df5d1a..27a54d867ac 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/MetricReader.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/MetricReader.test.ts @@ -15,10 +15,12 @@ */ import * as assert from 'assert'; +import * as sinon from 'sinon'; import { MeterProvider } from '../../src/MeterProvider'; +import { assertRejects } from '../test-utils'; +import { emptyResourceMetrics, TestMetricProducer } from './TestMetricProducer'; import { TestMetricReader } from './TestMetricReader'; - describe('MetricReader', () => { describe('setMetricProducer', () => { it('The SDK MUST NOT allow a MetricReader instance to be registered on more than one MeterProvider instance', () => { @@ -31,4 +33,51 @@ describe('MetricReader', () => { assert.throws(() => meterProvider2.addMetricReader(reader), /MetricReader can not be bound to a MeterProvider again/); }); }); + + describe('setMetricProducer', () => { + it('should initialize the metric reader', async () => { + const reader = new TestMetricReader(); + + reader.setMetricProducer(new TestMetricProducer()); + const result = await reader.collect(); + + assert.deepStrictEqual(result, { + resourceMetrics: emptyResourceMetrics, + errors: [], + }); + await reader.shutdown(); + }); + }); + + describe('collect', () => { + it('should throw on non-initialized instance', async () => { + const reader = new TestMetricReader(); + + await assertRejects(() => reader.collect(), /MetricReader is not bound to a MetricProducer/); + }); + + it('should return empty on shut-down instance', async () => { + const reader = new TestMetricReader(); + + reader.setMetricProducer(new TestMetricProducer()); + + await reader.shutdown(); + assertRejects(reader.collect(), /MetricReader is shutdown/); + }); + + it('should call MetricProduce.collect with timeout', async () => { + const reader = new TestMetricReader(); + const producer = new TestMetricProducer(); + reader.setMetricProducer(producer); + + const collectStub = sinon.stub(producer, 'collect'); + + await reader.collect({ timeoutMillis: 20 }); + assert(collectStub.calledOnce); + const args = collectStub.args[0]; + assert.deepStrictEqual(args, [{ timeoutMillis: 20 }]); + + await reader.shutdown(); + }); + }); }); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/PeriodicExportingMetricReader.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/PeriodicExportingMetricReader.test.ts index 21890024bf5..77489f1e233 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/PeriodicExportingMetricReader.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/PeriodicExportingMetricReader.test.ts @@ -17,14 +17,13 @@ import { PeriodicExportingMetricReader } from '../../src/export/PeriodicExportingMetricReader'; import { AggregationTemporality } from '../../src/export/AggregationTemporality'; import { InstrumentType, PushMetricExporter } from '../../src'; -import { CollectionResult, ResourceMetrics } from '../../src/export/MetricData'; +import { ResourceMetrics } from '../../src/export/MetricData'; import * as assert from 'assert'; import * as sinon from 'sinon'; -import { MetricProducer } from '../../src/export/MetricProducer'; import { TimeoutError } from '../../src/utils'; import { ExportResult, ExportResultCode } from '@opentelemetry/core'; import { assertRejects } from '../test-utils'; -import { defaultResource } from '../util'; +import { emptyResourceMetrics, TestMetricProducer } from './TestMetricProducer'; const MAX_32_BIT_INT = 2 ** 31 - 1; @@ -88,17 +87,6 @@ class TestDeltaMetricExporter extends TestMetricExporter { } } -const emptyResourceMetrics = { resource: defaultResource, scopeMetrics: [] }; - -class TestMetricProducer implements MetricProducer { - async collect(): Promise { - return { - resourceMetrics: { resource: defaultResource, scopeMetrics: [] }, - errors: [], - }; - } -} - describe('PeriodicExportingMetricReader', () => { afterEach(() => { sinon.restore(); @@ -364,53 +352,5 @@ describe('PeriodicExportingMetricReader', () => { await assertRejects(() => reader.shutdown(), /Error during forceFlush/); }); - }) - ; - - describe('collect', () => { - it('should throw on non-initialized instance', async () => { - const exporter = new TestMetricExporter(); - const reader = new PeriodicExportingMetricReader({ - exporter: exporter, - exportIntervalMillis: MAX_32_BIT_INT, - exportTimeoutMillis: 80, - }); - - await assertRejects(() => reader.collect(), /MetricReader is not bound to a MetricProducer/); - }); - - it('should return empty on shut-down instance', async () => { - const exporter = new TestMetricExporter(); - const reader = new PeriodicExportingMetricReader({ - exporter: exporter, - exportIntervalMillis: MAX_32_BIT_INT, - exportTimeoutMillis: 80, - }); - - reader.setMetricProducer(new TestMetricProducer()); - - await reader.shutdown(); - assertRejects(reader.collect(), /MetricReader is shutdown/); - }); - - it('should call MetricProduce.collect with timeout', async () => { - const exporter = new TestMetricExporter(); - const reader = new PeriodicExportingMetricReader({ - exporter: exporter, - exportIntervalMillis: MAX_32_BIT_INT, - exportTimeoutMillis: 80, - }); - const producer = new TestMetricProducer(); - reader.setMetricProducer(producer); - - const collectStub = sinon.stub(producer, 'collect'); - - await reader.collect({ timeoutMillis: 20 }); - assert(collectStub.calledOnce); - const args = collectStub.args[0]; - assert.deepStrictEqual(args, [{ timeoutMillis: 20 }]); - - await reader.shutdown(); - }); }); }); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricExporter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricExporter.ts index 67f33a898d5..44aa5057f1a 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricExporter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricExporter.ts @@ -15,7 +15,7 @@ */ import { ExportResult, ExportResultCode } from '@opentelemetry/core'; -import { AggregationTemporality, PushMetricExporter, ResourceMetrics } from '../../src'; +import { PushMetricExporter, ResourceMetrics } from '../../src'; export class TestMetricExporter implements PushMetricExporter { resourceMetricsList: ResourceMetrics[] = []; @@ -26,14 +26,4 @@ export class TestMetricExporter implements PushMetricExporter { async forceFlush(): Promise {} async shutdown(): Promise {} - - selectAggregationTemporality(): AggregationTemporality { - return AggregationTemporality.CUMULATIVE; - } -} - -export class TestDeltaMetricExporter extends TestMetricExporter { - override selectAggregationTemporality(): AggregationTemporality { - return AggregationTemporality.DELTA; - } } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterOptions.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricProducer.ts similarity index 55% rename from experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterOptions.ts rename to experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricProducer.ts index a9f601992ff..cb1247ed00d 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterOptions.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricProducer.ts @@ -14,11 +14,17 @@ * limitations under the License. */ -import { AggregationTemporality } from '@opentelemetry/sdk-metrics-base'; -import { OTLPExporterConfigBase } from '@opentelemetry/otlp-exporter-base'; +import { CollectionResult } from '../../src/export/MetricData'; +import { MetricProducer } from '../../src/export/MetricProducer'; +import { defaultResource } from '../util'; -export interface OTLPMetricExporterOptions extends OTLPExporterConfigBase { - temporalityPreference?: AggregationTemporality +export const emptyResourceMetrics = { resource: defaultResource, scopeMetrics: [] }; + +export class TestMetricProducer implements MetricProducer { + async collect(): Promise { + return { + resourceMetrics: { resource: defaultResource, scopeMetrics: [] }, + errors: [], + }; + } } -export const defaultExporterTemporality = AggregationTemporality.CUMULATIVE; -export const defaultOptions = {temporalityPreference: defaultExporterTemporality}; diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts index 4b02562c943..e708e1d2e00 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts @@ -15,6 +15,8 @@ */ import { + Aggregation, + AggregationSelector, AggregationTemporality, AggregationTemporalitySelector, InstrumentType, @@ -22,15 +24,22 @@ import { } from '../../src'; import { MetricCollector } from '../../src/state/MetricCollector'; +export interface TestMetricReaderOptions { + aggregationTemporalitySelector?: AggregationTemporalitySelector; + aggregationSelector?: AggregationSelector; +} + /** * A test metric reader that implements no-op onForceFlush() and onShutdown() handlers. */ export class TestMetricReader extends MetricReader { private _aggregationTemporalitySelector: AggregationTemporalitySelector; + private _aggregationSelector: AggregationSelector; - constructor(aggregationTemporalitySelector?: AggregationTemporalitySelector) { + constructor(options?: TestMetricReaderOptions) { super(); - this._aggregationTemporalitySelector = aggregationTemporalitySelector ?? (() => AggregationTemporality.CUMULATIVE); + this._aggregationTemporalitySelector = options?.aggregationTemporalitySelector ?? (() => AggregationTemporality.CUMULATIVE); + this._aggregationSelector = options?.aggregationSelector ?? Aggregation.Default; } protected onForceFlush(): Promise { @@ -45,7 +54,20 @@ export class TestMetricReader extends MetricReader { return this._aggregationTemporalitySelector(instrumentType); } + selectAggregation(instrumentType: InstrumentType) { + return this._aggregationSelector(instrumentType); + } + getMetricCollector(): MetricCollector { return this['_metricProducer'] as MetricCollector; } } + +export class TestDeltaMetricReader extends TestMetricReader { + constructor(options: TestMetricReaderOptions = {}) { + super({ + ...options, + aggregationTemporalitySelector: () => AggregationTemporality.DELTA, + }); + } +} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MeterSharedState.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MeterSharedState.test.ts index bacbe87e3f5..a362b452c71 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MeterSharedState.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MeterSharedState.test.ts @@ -17,15 +17,17 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { - AggregationTemporality, Meter, MeterProvider, DataPointType, CollectionResult, - View + View, + Aggregation, + MetricReader, + InstrumentType } from '../../src'; import { assertMetricData, defaultInstrumentationScope, defaultResource, sleep } from '../util'; -import { TestMetricReader } from '../export/TestMetricReader'; +import { TestDeltaMetricReader, TestMetricReader } from '../export/TestMetricReader'; import { MeterSharedState } from '../../src/state/MeterSharedState'; describe('MeterSharedState', () => { @@ -33,15 +35,122 @@ describe('MeterSharedState', () => { sinon.restore(); }); + describe('registerMetricStorage', () => { + function setupMeter(views?: View[], readers?: MetricReader[]) { + const meterProvider = new MeterProvider({ + resource: defaultResource, + views, + }); + readers?.forEach(reader => meterProvider.addMetricReader(reader)); + + const meter = meterProvider.getMeter('test-meter'); + + return { + meter, + meterSharedState: meterProvider['_sharedState'].getMeterSharedState({ name: 'test-meter' }), + collectors: Array.from(meterProvider['_sharedState'].metricCollectors), + }; + } + + it('should register metric storages with views', () => { + const reader = new TestMetricReader({ + aggregationSelector: () => { + throw new Error('should not be called'); + }, + }); + const { meter, meterSharedState, collectors } = setupMeter( + [ new View({ instrumentName: 'test-counter' }) ], + [reader], + ); + + meter.createCounter('test-counter'); + const metricStorages = meterSharedState.metricStorageRegistry.getStorages(collectors[0]); + + assert.strictEqual(metricStorages.length, 1); + assert.strictEqual(metricStorages[0].getInstrumentDescriptor().name, 'test-counter'); + }); + + it('should register metric storages with views', () => { + const reader = new TestMetricReader({ + aggregationSelector: () => { + throw new Error('should not be called'); + }, + }); + const { meter, meterSharedState, collectors } = setupMeter( + [ new View({ instrumentName: 'test-counter' }) ], + [reader], + ); + + meter.createCounter('test-counter'); + const metricStorages = meterSharedState.metricStorageRegistry.getStorages(collectors[0]); + + assert.strictEqual(metricStorages.length, 1); + assert.strictEqual(metricStorages[0].getInstrumentDescriptor().name, 'test-counter'); + }); + + it('should register metric storages with the collector', () => { + const reader = new TestMetricReader({ + aggregationSelector: (instrumentType: InstrumentType) => { + return Aggregation.Drop(); + }, + }); + const readerAggregationSelectorSpy = sinon.spy(reader, 'selectAggregation'); + + const { meter, meterSharedState, collectors } = setupMeter( + [], /** no views registered */ + [reader], + ); + + meter.createCounter('test-counter'); + const metricStorages = meterSharedState.metricStorageRegistry.getStorages(collectors[0]); + + // Should select aggregation with the metric reader. + assert.strictEqual(readerAggregationSelectorSpy.callCount, 1); + assert.strictEqual(metricStorages.length, 1); + assert.strictEqual(metricStorages[0].getInstrumentDescriptor().name, 'test-counter'); + }); + + it('should register metric storages with collectors', () => { + const reader = new TestMetricReader({ + aggregationSelector: (instrumentType: InstrumentType) => { + return Aggregation.Drop(); + }, + }); + const reader2 = new TestMetricReader({ + aggregationSelector: (instrumentType: InstrumentType) => { + return Aggregation.LastValue(); + }, + }); + + const { meter, meterSharedState, collectors } = setupMeter( + [], /** no views registered */ + [reader, reader2], + ); + + meter.createCounter('test-counter'); + const metricStorages = meterSharedState.metricStorageRegistry.getStorages(collectors[0]); + const metricStorages2 = meterSharedState.metricStorageRegistry.getStorages(collectors[1]); + + // Should select aggregation with the metric reader. + assert.strictEqual(metricStorages.length, 1); + assert.strictEqual(metricStorages[0].getInstrumentDescriptor().name, 'test-counter'); + + assert.strictEqual(metricStorages2.length, 1); + assert.strictEqual(metricStorages2[0].getInstrumentDescriptor().name, 'test-counter'); + + assert.notStrictEqual(metricStorages[0], metricStorages2[0], 'should create a distinct metric storage for each metric reader'); + }); + }); + describe('collect', () => { function setupInstruments(views?: View[]) { const meterProvider = new MeterProvider({ resource: defaultResource, views: views }); - const cumulativeReader = new TestMetricReader(() => AggregationTemporality.CUMULATIVE); + const cumulativeReader = new TestMetricReader(); meterProvider.addMetricReader(cumulativeReader); const cumulativeCollector = cumulativeReader.getMetricCollector(); - const deltaReader = new TestMetricReader(() => AggregationTemporality.DELTA); + const deltaReader = new TestDeltaMetricReader(); meterProvider.addMetricReader(deltaReader); const deltaCollector = deltaReader.getMetricCollector(); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts index 77047d9f52b..07bf1fb68a0 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts @@ -18,7 +18,6 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { MeterProvider, TimeoutError } from '../../src'; import { DataPointType } from '../../src/export/MetricData'; -import { PushMetricExporter } from '../../src/export/MetricExporter'; import { MeterProviderSharedState } from '../../src/state/MeterProviderSharedState'; import { MetricCollector } from '../../src/state/MetricCollector'; import { @@ -29,8 +28,7 @@ import { ObservableCallbackDelegate, BatchObservableCallbackDelegate, } from '../util'; -import { TestMetricReader } from '../export/TestMetricReader'; -import { TestDeltaMetricExporter, TestMetricExporter } from '../export/TestMetricExporter'; +import { TestDeltaMetricReader, TestMetricReader } from '../export/TestMetricReader'; describe('MetricCollector', () => { afterEach(() => { @@ -40,20 +38,18 @@ describe('MetricCollector', () => { describe('constructor', () => { it('should construct MetricCollector without exceptions', () => { const meterProviderSharedState = new MeterProviderSharedState(defaultResource); - const exporters = [ new TestMetricExporter(), new TestDeltaMetricExporter() ]; - for (const exporter of exporters) { - const reader = new TestMetricReader(exporter.selectAggregationTemporality); + const readers = [ new TestMetricReader(), new TestDeltaMetricReader() ]; + for (const reader of readers) { assert.doesNotThrow(() => new MetricCollector(meterProviderSharedState, reader)); } }); }); describe('collect', () => { - - function setupInstruments(exporter: PushMetricExporter) { + function setupInstruments() { const meterProvider = new MeterProvider({ resource: defaultResource }); - const reader = new TestMetricReader(exporter.selectAggregationTemporality); + const reader = new TestMetricReader(); meterProvider.addMetricReader(reader); const metricCollector = reader.getMetricCollector(); @@ -66,8 +62,7 @@ describe('MetricCollector', () => { it('should collect sync metrics', async () => { /** preparing test instrumentations */ - const exporter = new TestMetricExporter(); - const { metricCollector, meter } = setupInstruments(exporter); + const { metricCollector, meter } = setupInstruments(); /** creating metric events */ const counter = meter.createCounter('counter1'); @@ -104,8 +99,7 @@ describe('MetricCollector', () => { it('should collect async metrics', async () => { /** preparing test instrumentations */ - const exporter = new TestMetricExporter(); - const { metricCollector, meter } = setupInstruments(exporter); + const { metricCollector, meter } = setupInstruments(); /** creating metric events */ /** observable */ @@ -163,8 +157,7 @@ describe('MetricCollector', () => { it('should collect observer metrics with timeout', async () => { sinon.useFakeTimers(); /** preparing test instrumentations */ - const exporter = new TestMetricExporter(); - const { metricCollector, meter } = setupInstruments(exporter); + const { metricCollector, meter } = setupInstruments(); /** creating metric events */ @@ -246,8 +239,7 @@ describe('MetricCollector', () => { it('should collect with throwing observable callbacks', async () => { /** preparing test instrumentations */ - const exporter = new TestMetricExporter(); - const { metricCollector, meter } = setupInstruments(exporter); + const { metricCollector, meter } = setupInstruments(); /** creating metric events */ const counter = meter.createCounter('counter1'); @@ -283,8 +275,7 @@ describe('MetricCollector', () => { it('should collect batch observer metrics with timeout', async () => { sinon.useFakeTimers(); /** preparing test instrumentations */ - const exporter = new TestMetricExporter(); - const { metricCollector, meter } = setupInstruments(exporter); + const { metricCollector, meter } = setupInstruments(); /** creating metric events */ @@ -366,8 +357,7 @@ describe('MetricCollector', () => { it('should collect with throwing batch observable callbacks', async () => { /** preparing test instrumentations */ - const exporter = new TestMetricExporter(); - const { metricCollector, meter } = setupInstruments(exporter); + const { metricCollector, meter } = setupInstruments(); /** creating metric events */ const counter = meter.createCounter('counter1'); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricStorageRegistry.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricStorageRegistry.test.ts index e3defd2c924..fa8424b992b 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricStorageRegistry.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricStorageRegistry.test.ts @@ -31,7 +31,8 @@ import { } from '../../src/view/RegistrationConflicts'; class TestMetricStorage extends MetricStorage { - collect(collector: MetricCollectorHandle, + collect( + collector: MetricCollectorHandle, collectors: MetricCollectorHandle[], collectionTime: HrTime, ): Maybe { @@ -50,8 +51,19 @@ describe('MetricStorageRegistry', () => { sinon.restore(); }); + const collectorHandle: MetricCollectorHandle = { + selectAggregationTemporality: () => { + throw new Error('should not be invoked'); + }, + }; + const collectorHandle2: MetricCollectorHandle = { + selectAggregationTemporality: () => { + throw new Error('should not be invoked'); + }, + }; + describe('register', () => { - it('should register MetricStorage if it does not exist', () => { + it('should register MetricStorage', () => { const registry = new MetricStorageRegistry(); const storage = new TestMetricStorage({ name: 'instrument', @@ -61,34 +73,65 @@ describe('MetricStorageRegistry', () => { valueType: ValueType.DOUBLE }); - const registeredStorage = registry.register(storage); - const registeredStorages = registry.getStorages(); + registry.register(storage); + const registeredStorages = registry.getStorages(collectorHandle); - // returned the same storage - assert.strictEqual(registeredStorage, storage); - // registered the actual storage + // registered the storage. assert.deepStrictEqual([storage], registeredStorages); - // no warning logs written - assert.strictEqual(spyLoggerWarn.args.length, 0); }); + }); - function testConflictingRegistration(existingDescriptor: InstrumentDescriptor, + describe('registerForCollector', () => { + it('should register MetricStorage for each collector', () => { + const registry = new MetricStorageRegistry(); + const storage = new TestMetricStorage({ + name: 'instrument', + type: InstrumentType.COUNTER, + description: 'description', + unit: '1', + valueType: ValueType.DOUBLE + }); + const storage2 = new TestMetricStorage({ + name: 'instrument2', + type: InstrumentType.COUNTER, + description: 'description', + unit: '1', + valueType: ValueType.DOUBLE + }); + + registry.registerForCollector(collectorHandle, storage); + registry.registerForCollector(collectorHandle2, storage); + registry.registerForCollector(collectorHandle2, storage2); + + assert.deepStrictEqual(registry.getStorages(collectorHandle), [storage]); + assert.deepStrictEqual(registry.getStorages(collectorHandle2), [storage, storage2]); + }); + }); + + describe('findOrUpdateCompatibleStorage', () => { + function testConflictingRegistration( + existingDescriptor: InstrumentDescriptor, otherDescriptor: InstrumentDescriptor, - expectedLog: string) { + expectedLog: string + ) { const registry = new MetricStorageRegistry(); const storage = new TestMetricStorage(existingDescriptor); const otherStorage = new TestMetricStorage(otherDescriptor); - assert.strictEqual(registry.register(storage), storage); - assert.strictEqual(registry.register(otherStorage), otherStorage); - const registeredStorages = registry.getStorages(); + assert.strictEqual(registry.findOrUpdateCompatibleStorage(existingDescriptor), null); + registry.register(storage); + assertLogNotCalled(); - // registered both storages - assert.deepStrictEqual([storage, otherStorage], registeredStorages); + assert.strictEqual(registry.findOrUpdateCompatibleStorage(otherDescriptor), null); // warned assertLogCalledOnce(); assertFirstLogContains(expectedLog); + registry.register(otherStorage); + + // registered both storages + const registeredStorages = registry.getStorages(collectorHandle); + assert.deepStrictEqual([storage, otherStorage], registeredStorages); } it('warn when instrument with same name and different type is already registered', () => { @@ -179,17 +222,14 @@ describe('MetricStorageRegistry', () => { const storage = new TestMetricStorage(existingDescriptor); const otherStorage = new TestMetricStorage(otherDescriptor); - // returns the first registered storage. - assert.strictEqual(registry.register(storage), storage); - // returns the original storage - assert.strictEqual(registry.register(otherStorage), storage); + // register the first storage. + assert.strictEqual(registry.findOrUpdateCompatibleStorage(existingDescriptor), null); + registry.register(storage); + // register the second storage. + assert.strictEqual(registry.findOrUpdateCompatibleStorage(otherDescriptor), storage); // original storage now has the updated (longer) description. assert.strictEqual(otherStorage.getInstrumentDescriptor().description, otherDescriptor.description); - const registeredStorages = registry.getStorages(); - - // only the original storage has been added - assert.deepStrictEqual([storage], registeredStorages); // log called exactly once assertLogCalledOnce(); // added resolution recipe to the log @@ -207,14 +247,9 @@ describe('MetricStorageRegistry', () => { }; const storage = new TestMetricStorage(descriptor); - const otherStorage = new TestMetricStorage(descriptor); - assert.strictEqual(registry.register(storage), storage); - assert.strictEqual(registry.register(otherStorage), storage); - const registeredStorages = registry.getStorages(); - - // registered the actual storage, but not more than that. - assert.deepStrictEqual([storage], registeredStorages); + registry.register(storage); + assert.strictEqual(registry.findOrUpdateCompatibleStorage(descriptor), storage); }); it('should return the existing instrument if a compatible sync instrument is already registered', () => { @@ -228,18 +263,15 @@ describe('MetricStorageRegistry', () => { }; const storage = new TestMetricStorage(descriptor); - const otherStorage = new TestMetricStorage(descriptor); registry.register(storage); - const previouslyRegisteredStorage = registry.register(otherStorage); - const registeredStorages = registry.getStorages(); - - // returned undefined - assert.strictEqual(previouslyRegisteredStorage, storage); - // registered the actual storage, but not more than that. - assert.deepStrictEqual([storage], registeredStorages); + assert.strictEqual(registry.findOrUpdateCompatibleStorage(descriptor), storage); }); + function assertLogNotCalled() { + assert.strictEqual(spyLoggerWarn.args.length, 0); + } + function assertLogCalledOnce() { assert.strictEqual(spyLoggerWarn.args.length, 1); } @@ -248,4 +280,63 @@ describe('MetricStorageRegistry', () => { assert.ok(spyLoggerWarn.args[0].includes(expectedString), 'Logs did not include: ' + expectedString); } }); + + describe('findOrUpdateCompatibleCollectorStorage', () => { + it('register conflicting metric storages for collector', () => { + const existingDescriptor = { + name: 'instrument', + type: InstrumentType.COUNTER, + description: 'description', + unit: '1', + valueType: ValueType.DOUBLE + }; + + const otherDescriptor = { + name: 'instrument', + type: InstrumentType.UP_DOWN_COUNTER, + description: 'description', + unit: '1', + valueType: ValueType.DOUBLE + }; + + const registry = new MetricStorageRegistry(); + + const storage = new TestMetricStorage(existingDescriptor); + const otherStorage = new TestMetricStorage(otherDescriptor); + + assert.strictEqual(registry.findOrUpdateCompatibleCollectorStorage(collectorHandle, existingDescriptor), null); + registry.registerForCollector(collectorHandle, storage); + + // Should not return an existing metric storage. + assert.strictEqual(registry.findOrUpdateCompatibleCollectorStorage(collectorHandle, otherDescriptor), null); + registry.registerForCollector(collectorHandle, otherStorage); + + // registered both storages + const registeredStorages = registry.getStorages(collectorHandle); + assert.deepStrictEqual([storage, otherStorage], registeredStorages); + }); + + it('register the same metric storage for each collector', () => { + const descriptor = { + name: 'instrument', + type: InstrumentType.COUNTER, + description: 'description', + unit: '1', + valueType: ValueType.DOUBLE + }; + const registry = new MetricStorageRegistry(); + + const storage = new TestMetricStorage(descriptor); + + assert.strictEqual(registry.findOrUpdateCompatibleCollectorStorage(collectorHandle, descriptor), null); + registry.registerForCollector(collectorHandle, storage); + + assert.strictEqual(registry.findOrUpdateCompatibleCollectorStorage(collectorHandle2, descriptor), null); + registry.registerForCollector(collectorHandle2, storage); + + // registered the storage for each collector + assert.deepStrictEqual(registry.getStorages(collectorHandle), [storage]); + assert.deepStrictEqual(registry.getStorages(collectorHandle2), [storage]); + }); + }); }); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/view/ViewRegistry.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/view/ViewRegistry.test.ts index a56d7b65e06..68f9f270723 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/view/ViewRegistry.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/view/ViewRegistry.test.ts @@ -23,13 +23,6 @@ import { View } from '../../src'; describe('ViewRegistry', () => { describe('findViews', () => { - it('should return default view if no view registered', () => { - const registry = new ViewRegistry(); - const views = registry.findViews(defaultInstrumentDescriptor, defaultInstrumentationScope); - assert.strictEqual(views.length, 1); - assert.strictEqual(views[0], ViewRegistry['DEFAULT_VIEW']); - }); - describe('InstrumentSelector', () => { it('should match view with instrument name', () => { const registry = new ViewRegistry(); diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts index 05ba058f0fa..95588ac6ba4 100644 --- a/packages/opentelemetry-core/src/utils/environment.ts +++ b/packages/opentelemetry-core/src/utils/environment.ts @@ -102,7 +102,9 @@ export type ENVIRONMENT = { OTEL_EXPORTER_OTLP_METRICS_CLIENT_KEY?: string, OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE?: string, OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE?: string, - OTEL_EXPORTER_OTLP_METRICS_CLIENT_CERTIFICATE?: string + OTEL_EXPORTER_OTLP_METRICS_CLIENT_CERTIFICATE?: string; + OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE?: string; + OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION?: string; } & ENVIRONMENT_NUMBERS & ENVIRONMENT_LISTS; @@ -171,7 +173,9 @@ export const DEFAULT_ENVIRONMENT: Required = { OTEL_EXPORTER_OTLP_METRICS_CLIENT_KEY: '', OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE: '', OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE: '', - OTEL_EXPORTER_OTLP_METRICS_CLIENT_CERTIFICATE: '' + OTEL_EXPORTER_OTLP_METRICS_CLIENT_CERTIFICATE: '', + OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE: 'cumulative', + OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION: 'explicit_bucket_histogram', }; /** diff --git a/tsconfig.json b/tsconfig.json index d5872dfc602..99c79816554 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -47,64 +47,61 @@ "path": "packages/opentelemetry-context-async-hooks" }, { - "path": "packages/opentelemetry-context-zone-peer-dep" + "path": "packages/opentelemetry-context-zone-peer-dep/tsconfig.all.json" }, { - "path": "packages/opentelemetry-context-zone" + "path": "packages/opentelemetry-context-zone/tsconfig.all.json" }, { - "path": "packages/opentelemetry-core" + "path": "packages/opentelemetry-core/tsconfig.all.json" }, { "path": "packages/opentelemetry-exporter-jaeger" }, { - "path": "packages/opentelemetry-exporter-zipkin" + "path": "packages/opentelemetry-exporter-zipkin/tsconfig.all.json" }, { - "path": "packages/opentelemetry-propagator-b3" + "path": "packages/opentelemetry-propagator-b3/tsconfig.all.json" }, { - "path": "packages/opentelemetry-propagator-jaeger" + "path": "packages/opentelemetry-propagator-jaeger/tsconfig.all.json" }, { - "path": "packages/opentelemetry-resources" + "path": "packages/opentelemetry-resources/tsconfig.all.json" }, { - "path": "packages/opentelemetry-sdk-trace-base" + "path": "packages/opentelemetry-sdk-trace-base/tsconfig.all.json" }, { "path": "packages/opentelemetry-sdk-trace-node" }, { - "path": "packages/opentelemetry-sdk-trace-web" + "path": "packages/opentelemetry-sdk-trace-web/tsconfig.all.json" }, { - "path": "packages/opentelemetry-semantic-conventions" + "path": "packages/opentelemetry-semantic-conventions/tsconfig.all.json" }, { "path": "packages/opentelemetry-shim-opentracing" }, - { - "path": "packages/template" - }, { "path": "experimental/packages/exporter-trace-otlp-grpc" }, { - "path": "experimental/packages/exporter-trace-otlp-http" + "path": "experimental/packages/exporter-trace-otlp-http/tsconfig.all.json" }, { "path": "experimental/packages/exporter-trace-otlp-proto" }, { - "path": "experimental/packages/opentelemetry-api-metrics" + "path": "experimental/packages/opentelemetry-api-metrics/tsconfig.all.json" }, { "path": "experimental/packages/opentelemetry-exporter-metrics-otlp-grpc" }, { - "path": "experimental/packages/opentelemetry-exporter-metrics-otlp-http" + "path": "experimental/packages/opentelemetry-exporter-metrics-otlp-http/tsconfig.all.json" }, { "path": "experimental/packages/opentelemetry-exporter-metrics-otlp-proto" @@ -113,7 +110,7 @@ "path": "experimental/packages/opentelemetry-exporter-prometheus" }, { - "path": "experimental/packages/opentelemetry-instrumentation-fetch" + "path": "experimental/packages/opentelemetry-instrumentation-fetch/tsconfig.all.json" }, { "path": "experimental/packages/opentelemetry-instrumentation-grpc" @@ -122,28 +119,28 @@ "path": "experimental/packages/opentelemetry-instrumentation-http" }, { - "path": "experimental/packages/opentelemetry-instrumentation-xml-http-request" + "path": "experimental/packages/opentelemetry-instrumentation-xml-http-request/tsconfig.all.json" }, { - "path": "experimental/packages/opentelemetry-instrumentation" + "path": "experimental/packages/opentelemetry-instrumentation/tsconfig.all.json" }, { - "path": "experimental/packages/opentelemetry-sdk-metrics-base" + "path": "experimental/packages/opentelemetry-sdk-metrics-base/tsconfig.all.json" }, { "path": "experimental/packages/opentelemetry-sdk-node" }, { - "path": "experimental/packages/otlp-exporter-base" + "path": "experimental/packages/otlp-exporter-base/tsconfig.all.json" }, { - "path": "experimental/packages/otlp-grpc-exporter-base" + "path": "experimental/packages/otlp-grpc-exporter-base/tsconfig.all.json" }, { - "path": "experimental/packages/otlp-proto-exporter-base" + "path": "experimental/packages/otlp-proto-exporter-base/tsconfig.all.json" }, { - "path": "experimental/packages/otlp-transformer" + "path": "experimental/packages/otlp-transformer/tsconfig.all.json" }, { "path": "experimental/backwards-compatability/node14"