From dd22372a09236a62cfff6edb93527643af99a937 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Thu, 1 Sep 2022 13:57:05 +0800 Subject: [PATCH] feat(sdk-metrics-base): per metric-reader aggregation (#3153) --- experimental/CHANGELOG.md | 1 + .../test/OTLPMetricExporter.test.ts | 11 +- .../test/metricsHelper.ts | 5 + .../browser/CollectorMetricExporter.test.ts | 48 +++-- .../test/metricsHelper.ts | 5 + .../test/node/CollectorMetricExporter.test.ts | 23 ++- .../test/OTLPMetricExporter.test.ts | 11 +- .../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/MetricReader.ts | 7 + .../export/PeriodicExportingMetricReader.ts | 38 +++- .../opentelemetry-sdk-metrics/src/index.ts | 6 +- .../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/TestMetricProducer.ts | 30 ++++ .../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 - 31 files changed, 667 insertions(+), 252 deletions(-) create mode 100644 experimental/packages/opentelemetry-sdk-metrics/src/export/AggregationSelector.ts create mode 100644 experimental/packages/opentelemetry-sdk-metrics/test/export/TestMetricProducer.ts diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index f420b3f4686..8d5b3025619 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -36,6 +36,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: support latest `@opentelemetry/api` [#3177](https://github.com/open-telemetry/opentelemetry-js/pull/3177) @dyladan +* 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/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts index e6d4ced3698..08a30aca3e0 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 @@ -214,13 +214,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, 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 88d91a872be..0ca7df3226f 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/test/browser/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts index b0b95ce0be9..166dbc4a81c 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 @@ -99,6 +99,7 @@ describe('OTLPMetricExporter - web', () => { temporalityPreference: AggregationTemporality.CUMULATIVE }); }); + it('should successfully send metrics using sendBeacon', done => { collectorExporter.export(metrics, () => { }); @@ -109,16 +110,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 +135,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 +147,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] ); @@ -216,15 +223,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 +245,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 +257,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] ); 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 40a08775bf8..8cb3811f810 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 9cdbaed7e0b..77c50231536 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 @@ -288,29 +288,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] ); 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 38ea5db9184..8ff43fed7cc 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 @@ -240,9 +240,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 0beb770ed8f..3a69427f90e 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 c548b906dc6..675472ec566 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'; +import { Aggregation, AggregationTemporality, MetricReader } from '@opentelemetry/sdk-metrics'; 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 2a722e573ad..080ac599e3f 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'; +import { MeterProvider } from '@opentelemetry/sdk-metrics'; 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 c05f94f81c0..00e082a8612 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/src/export/AggregationSelector.ts b/experimental/packages/opentelemetry-sdk-metrics/src/export/AggregationSelector.ts new file mode 100644 index 00000000000..8a372d0e74d --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics/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/src/export/AggregationTemporality.ts b/experimental/packages/opentelemetry-sdk-metrics/src/export/AggregationTemporality.ts index 0b936714722..6cc6d1231bf 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/src/export/AggregationTemporality.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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/src/export/MetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics/src/export/MetricReader.ts index 35d86c5d943..3037384fcf9 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/src/export/MetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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/src/export/PeriodicExportingMetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics/src/export/PeriodicExportingMetricReader.ts index 3e188c02931..4d2206cda00 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/src/export/PeriodicExportingMetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/src/export/PeriodicExportingMetricReader.ts @@ -25,25 +25,43 @@ import { AggregationTemporality } from './AggregationTemporality'; import { InstrumentType } from '../InstrumentDescriptor'; import { PushMetricExporter } from './MetricExporter'; import { callWithTimeout, TimeoutError } from '../utils'; +import { Aggregation } from '../view/Aggregation'; +import { AggregationSelector } 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; + /** + * 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; + /** * {@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; constructor(options: PeriodicExportingMetricReaderOptions) { super(); @@ -65,6 +83,7 @@ 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; } private async _runOnce(): Promise { @@ -119,6 +138,13 @@ export class PeriodicExportingMetricReader extends MetricReader { await this._exporter.shutdown(); } + /** + * @inheritdoc + */ + selectAggregation(instrumentType: InstrumentType): Aggregation { + return this._aggregationSelector(instrumentType); + } + /** * @inheritdoc */ diff --git a/experimental/packages/opentelemetry-sdk-metrics/src/index.ts b/experimental/packages/opentelemetry-sdk-metrics/src/index.ts index 3ce97504de1..e5e1ab6ed2a 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/src/index.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/src/index.ts @@ -21,8 +21,12 @@ export { } from './aggregator/types'; export { - AggregationTemporality, + AggregationSelector, AggregationTemporalitySelector, +} from './export/AggregationSelector'; + +export { + AggregationTemporality, } from './export/AggregationTemporality'; export { diff --git a/experimental/packages/opentelemetry-sdk-metrics/src/state/MeterProviderSharedState.ts b/experimental/packages/opentelemetry-sdk-metrics/src/state/MeterProviderSharedState.ts index 87fe540ee56..a63f53d51dc 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/src/state/MeterProviderSharedState.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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; } + + selectAggregations(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/src/state/MeterSharedState.ts b/experimental/packages/opentelemetry-sdk-metrics/src/state/MeterSharedState.ts index ed96aca444a..4ddc0b89328 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/src/state/MeterSharedState.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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.selectAggregations(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/src/state/MetricCollector.ts b/experimental/packages/opentelemetry-sdk-metrics/src/state/MetricCollector.ts index 066106e47d4..fbffc2f0605 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/src/state/MetricCollector.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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/src/state/MetricStorage.ts b/experimental/packages/opentelemetry-sdk-metrics/src/state/MetricStorage.ts index 8e24da668ab..f69a00daa45 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/src/state/MetricStorage.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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/src/state/MetricStorageRegistry.ts b/experimental/packages/opentelemetry-sdk-metrics/src/state/MetricStorageRegistry.ts index 1a930cacb05..c65a1dd7b2a 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/src/state/MetricStorageRegistry.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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/src/view/ViewRegistry.ts b/experimental/packages/opentelemetry-sdk-metrics/src/view/ViewRegistry.ts index 1a26387d10a..1dcaf7d22f2 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/src/view/ViewRegistry.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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/test/Instruments.test.ts b/experimental/packages/opentelemetry-sdk-metrics/test/Instruments.test.ts index 2bb47be3b2d..772e268fa4a 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/test/Instruments.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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/test/export/MetricReader.test.ts b/experimental/packages/opentelemetry-sdk-metrics/test/export/MetricReader.test.ts index 1ee88df5d1a..27a54d867ac 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/test/export/MetricReader.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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/test/export/PeriodicExportingMetricReader.test.ts b/experimental/packages/opentelemetry-sdk-metrics/test/export/PeriodicExportingMetricReader.test.ts index 21890024bf5..77489f1e233 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/test/export/PeriodicExportingMetricReader.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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/test/export/TestMetricProducer.ts b/experimental/packages/opentelemetry-sdk-metrics/test/export/TestMetricProducer.ts new file mode 100644 index 00000000000..cb1247ed00d --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics/test/export/TestMetricProducer.ts @@ -0,0 +1,30 @@ +/* + * 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 { CollectionResult } from '../../src/export/MetricData'; +import { MetricProducer } from '../../src/export/MetricProducer'; +import { defaultResource } from '../util'; + +export const emptyResourceMetrics = { resource: defaultResource, scopeMetrics: [] }; + +export class TestMetricProducer implements MetricProducer { + async collect(): Promise { + return { + resourceMetrics: { resource: defaultResource, scopeMetrics: [] }, + errors: [], + }; + } +} diff --git a/experimental/packages/opentelemetry-sdk-metrics/test/export/TestMetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics/test/export/TestMetricReader.ts index 4b02562c943..e708e1d2e00 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/test/export/TestMetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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/test/state/MeterSharedState.test.ts b/experimental/packages/opentelemetry-sdk-metrics/test/state/MeterSharedState.test.ts index 0a4b504b9ff..dc7651f83aa 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/test/state/MeterSharedState.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/test/state/MeterSharedState.test.ts @@ -17,14 +17,16 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { - AggregationTemporality, Meter, MeterProvider, DataPointType, - 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'; import { CollectionResult } from '../../src/export/MetricData'; @@ -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/test/state/MetricCollector.test.ts b/experimental/packages/opentelemetry-sdk-metrics/test/state/MetricCollector.test.ts index ad65f1c027e..6f316a11edb 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/test/state/MetricCollector.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/test/state/MetricCollector.test.ts @@ -19,7 +19,6 @@ import * as sinon from 'sinon'; import { MeterProvider } from '../../src'; import { TimeoutError } from '../../src/utils'; 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 { @@ -30,8 +29,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(() => { @@ -41,20 +39,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(); @@ -67,8 +63,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'); @@ -105,8 +100,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 */ @@ -164,8 +158,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 */ @@ -247,8 +240,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'); @@ -284,8 +276,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 */ @@ -367,8 +358,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/test/state/MetricStorageRegistry.test.ts b/experimental/packages/opentelemetry-sdk-metrics/test/state/MetricStorageRegistry.test.ts index e3defd2c924..fa8424b992b 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/test/state/MetricStorageRegistry.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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/test/view/ViewRegistry.test.ts b/experimental/packages/opentelemetry-sdk-metrics/test/view/ViewRegistry.test.ts index a56d7b65e06..68f9f270723 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/test/view/ViewRegistry.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/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();