From 99497ac7c3184e9458b17b2b89d395e73b4377cb Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Tue, 16 Jan 2024 11:28:28 +0100 Subject: [PATCH 1/2] feat(sdk-metrics)!: remove MeterProvider.addMetricReader() in favor of constructor option --- .../test/metricsHelper.ts | 11 ++-- .../test/metricsHelper.ts | 13 +++-- .../test/metricsHelper.ts | 11 ++-- .../test/PrometheusExporter.test.ts | 10 ++-- .../test/PrometheusSerializer.test.ts | 18 +++---- .../test/functionals/http-metrics.test.ts | 3 +- .../opentelemetry-sdk-node/src/sdk.ts | 9 ++-- packages/sdk-metrics/src/MeterProvider.ts | 21 ++++---- packages/sdk-metrics/test/Instruments.test.ts | 11 ++-- .../sdk-metrics/test/MeterProvider.test.ts | 54 +++++++++---------- .../test/export/ConsoleMetricExporter.test.ts | 14 ++--- .../export/InMemoryMetricExporter.test.ts | 20 +++---- .../test/export/MetricReader.test.ts | 25 +++++---- .../cumulative-exponential-histogram.test.ts | 5 +- ...wo-metric-readers-async-instrument.test.ts | 7 ++- .../test/state/MeterSharedState.test.ts | 12 ++--- .../test/state/MetricCollector.test.ts | 8 +-- 17 files changed, 134 insertions(+), 118 deletions(-) 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 14c8f3a3df..7c9fbfd99c 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts @@ -55,10 +55,11 @@ const testResource = new Resource({ cost: 112.12, }); -let meterProvider = new MeterProvider({ resource: testResource }); - let reader = new TestMetricReader(); -meterProvider.addMetricReader(reader); +let meterProvider = new MeterProvider({ + resource: testResource, + readers: [reader], +}); let meter = meterProvider.getMeter('default', '0.0.1'); @@ -67,6 +68,7 @@ export async function collect() { } export function setUp() { + reader = new TestMetricReader(); meterProvider = new MeterProvider({ resource: testResource, views: [ @@ -75,9 +77,8 @@ export function setUp() { instrumentName: 'int-histogram', }), ], + readers: [reader], }); - reader = new TestMetricReader(); - meterProvider.addMetricReader(reader); meter = meterProvider.getMeter('default', '0.0.1'); } 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 5605a69b28..850cdae4d0 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/metricsHelper.ts @@ -72,9 +72,11 @@ const defaultResource = Resource.default().merge( }) ); -let meterProvider = new MeterProvider({ resource: defaultResource }); let reader = new TestMetricReader(); -meterProvider.addMetricReader(reader); +let meterProvider = new MeterProvider({ + resource: defaultResource, + readers: [reader], +}); let meter = meterProvider.getMeter('default', '0.0.1'); export async function collect() { @@ -82,9 +84,12 @@ export async function collect() { } export function setUp(views?: View[]) { - meterProvider = new MeterProvider({ resource: defaultResource, views }); reader = new TestMetricReader(); - meterProvider.addMetricReader(reader); + meterProvider = new MeterProvider({ + resource: defaultResource, + views, + readers: [reader], + }); meter = meterProvider.getMeter('default', '0.0.1'); } 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 5c9c29abee..ef70a75c05 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts @@ -54,10 +54,11 @@ const testResource = new Resource({ cost: 112.12, }); -let meterProvider = new MeterProvider({ resource: testResource }); - let reader = new TestMetricReader(); -meterProvider.addMetricReader(reader); +let meterProvider = new MeterProvider({ + resource: testResource, + readers: [reader], +}); let meter = meterProvider.getMeter('default', '0.0.1'); @@ -66,6 +67,7 @@ export async function collect() { } export function setUp() { + reader = new TestMetricReader(); meterProvider = new MeterProvider({ resource: testResource, views: [ @@ -74,9 +76,8 @@ export function setUp() { instrumentName: 'int-histogram', }), ], + readers: [reader], }); - reader = new TestMetricReader(); - meterProvider.addMetricReader(reader); meter = meterProvider.getMeter('default', '0.0.1'); } diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts index 1030203a2c..d512240677 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts @@ -261,8 +261,9 @@ describe('PrometheusExporter', () => { beforeEach(done => { exporter = new PrometheusExporter({}, () => { - meterProvider = new MeterProvider(); - meterProvider.addMetricReader(exporter); + meterProvider = new MeterProvider({ + readers: [exporter], + }); meter = meterProvider.getMeter('test-prometheus', '1'); done(); }); @@ -533,8 +534,9 @@ describe('PrometheusExporter', () => { let exporter: PrometheusExporter; function setup(reader: PrometheusExporter) { - meterProvider = new MeterProvider(); - meterProvider.addMetricReader(reader); + meterProvider = new MeterProvider({ + readers: [exporter], + }); meter = meterProvider.getMeter('test-prometheus'); counter = meter.createCounter('counter'); diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts index 1a39aae003..b2d9e36f8f 100644 --- a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts +++ b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts @@ -91,8 +91,8 @@ describe('PrometheusSerializer', () => { instrumentName: '*', }), ], + readers: [reader], }); - meterProvider.addMetricReader(reader); const meter = meterProvider.getMeter('test'); const counter = meter.createCounter('test_total'); @@ -141,8 +141,8 @@ describe('PrometheusSerializer', () => { instrumentName: '*', }), ], + readers: [reader], }); - meterProvider.addMetricReader(reader); const meter = meterProvider.getMeter('test'); const histogram = meter.createHistogram('test'); @@ -206,8 +206,8 @@ describe('PrometheusSerializer', () => { instrumentName: '*', }), ], + readers: [reader], }); - meterProvider.addMetricReader(reader); const meter = meterProvider.getMeter('test'); const counter = meter.createCounter('test_total', { @@ -261,8 +261,8 @@ describe('PrometheusSerializer', () => { instrumentName: '*', }), ], + readers: [reader], }); - meterProvider.addMetricReader(reader); const meter = meterProvider.getMeter('test'); const counter = meter.createUpDownCounter('test_total', { @@ -315,8 +315,8 @@ describe('PrometheusSerializer', () => { instrumentName: '*', }), ], + readers: [reader], }); - meterProvider.addMetricReader(reader); const meter = meterProvider.getMeter('test'); const counter = meter.createUpDownCounter('test_total', { @@ -369,8 +369,8 @@ describe('PrometheusSerializer', () => { instrumentName: '*', }), ], + readers: [reader], }); - meterProvider.addMetricReader(reader); const meter = meterProvider.getMeter('test'); const histogram = meter.createHistogram('test', { @@ -424,8 +424,8 @@ describe('PrometheusSerializer', () => { instrumentName: '*', }), ], + readers: [reader], }); - meterProvider.addMetricReader(reader); const meter = meterProvider.getMeter('test'); const upDownCounter = meter.createUpDownCounter('test', { @@ -474,8 +474,8 @@ describe('PrometheusSerializer', () => { views: [ new View({ aggregation: new SumAggregation(), instrumentName: '*' }), ], + readers: [reader], }); - meterProvider.addMetricReader(reader); const meter = meterProvider.getMeter('test'); const { unit, exportAll = false } = options; @@ -563,8 +563,8 @@ describe('PrometheusSerializer', () => { views: [ new View({ aggregation: new SumAggregation(), instrumentName: '*' }), ], + readers: [reader], }); - meterProvider.addMetricReader(reader); const meter = meterProvider.getMeter('test'); const counter = meter.createUpDownCounter(name); diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts index d7f55ed846..0f484f08bf 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts @@ -38,13 +38,12 @@ const protocol = 'http'; const hostname = 'localhost'; const pathname = '/test'; const tracerProvider = new NodeTracerProvider(); -const meterProvider = new MeterProvider(); const metricsMemoryExporter = new InMemoryMetricExporter( AggregationTemporality.DELTA ); const metricReader = new TestMetricReader(metricsMemoryExporter); +const meterProvider = new MeterProvider({ readers: [metricReader] }); -meterProvider.addMetricReader(metricReader); instrumentation.setTracerProvider(tracerProvider); instrumentation.setMeterProvider(meterProvider); diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index 30a1bea790..6c8cedb296 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -350,15 +350,16 @@ export class NodeSDK { } if (this._meterProviderConfig) { + const readers: MetricReader[] = []; + if (this._meterProviderConfig.reader) { + readers.push(this._meterProviderConfig.reader); + } const meterProvider = new MeterProvider({ resource: this._resource, views: this._meterProviderConfig?.views ?? [], + readers: readers, }); - if (this._meterProviderConfig.reader) { - meterProvider.addMetricReader(this._meterProviderConfig.reader); - } - this._meterProvider = meterProvider; metrics.setGlobalMeterProvider(meterProvider); diff --git a/packages/sdk-metrics/src/MeterProvider.ts b/packages/sdk-metrics/src/MeterProvider.ts index f10cf42b9b..f4668d27cd 100644 --- a/packages/sdk-metrics/src/MeterProvider.ts +++ b/packages/sdk-metrics/src/MeterProvider.ts @@ -35,6 +35,7 @@ export interface MeterProviderOptions { /** Resource associated with metric telemetry */ resource?: IResource; views?: View[]; + readers?: MetricReader[]; } /** @@ -54,6 +55,14 @@ export class MeterProvider implements IMeterProvider { this._sharedState.viewRegistry.addView(view); } } + + if (options?.readers != null && options.readers.length > 0) { + for (const metricReader of options.readers) { + const collector = new MetricCollector(this._sharedState, metricReader); + metricReader.setMetricProducer(collector); + this._sharedState.metricCollectors.push(collector); + } + } } /** @@ -73,18 +82,6 @@ export class MeterProvider implements IMeterProvider { }).meter; } - /** - * Register a {@link MetricReader} to the meter provider. After the - * registration, the MetricReader can start metrics collection. - * - * @param metricReader the metric reader to be registered. - */ - addMetricReader(metricReader: MetricReader) { - const collector = new MetricCollector(this._sharedState, metricReader); - metricReader.setMetricProducer(collector); - this._sharedState.metricCollectors.push(collector); - } - /** * Flush all buffered data and shut down the MeterProvider and all registered * MetricReaders. diff --git a/packages/sdk-metrics/test/Instruments.test.ts b/packages/sdk-metrics/test/Instruments.test.ts index 56ecf03af6..ab466c295a 100644 --- a/packages/sdk-metrics/test/Instruments.test.ts +++ b/packages/sdk-metrics/test/Instruments.test.ts @@ -751,7 +751,12 @@ describe('Instruments', () => { }); function setup() { - const meterProvider = new MeterProvider({ resource: defaultResource }); + const deltaReader = new TestDeltaMetricReader(); + const cumulativeReader = new TestMetricReader(); + const meterProvider = new MeterProvider({ + resource: defaultResource, + readers: [deltaReader, cumulativeReader], + }); const meter = meterProvider.getMeter( defaultInstrumentationScope.name, defaultInstrumentationScope.version, @@ -759,10 +764,6 @@ function setup() { schemaUrl: defaultInstrumentationScope.schemaUrl, } ); - const deltaReader = new TestDeltaMetricReader(); - meterProvider.addMetricReader(deltaReader); - const cumulativeReader = new TestMetricReader(); - meterProvider.addMetricReader(cumulativeReader); return { meterProvider, diff --git a/packages/sdk-metrics/test/MeterProvider.test.ts b/packages/sdk-metrics/test/MeterProvider.test.ts index be075f0fa6..615fcffab3 100644 --- a/packages/sdk-metrics/test/MeterProvider.test.ts +++ b/packages/sdk-metrics/test/MeterProvider.test.ts @@ -73,9 +73,11 @@ describe('MeterProvider', () => { }); it('get meter with same identity', async () => { - const meterProvider = new MeterProvider({ resource: defaultResource }); const reader = new TestMetricReader(); - meterProvider.addMetricReader(reader); + const meterProvider = new MeterProvider({ + resource: defaultResource, + readers: [reader], + }); // Create meter and instrument, needs observation on instrument, otherwise the scope will not be reported. // name+version pair 1 @@ -131,6 +133,7 @@ describe('MeterProvider', () => { describe('addView', () => { it('with existing instrument should rename', async () => { + const reader = new TestMetricReader(); const meterProvider = new MeterProvider({ resource: defaultResource, // Add view to rename 'non-renamed-instrument' to 'renamed-instrument' @@ -141,11 +144,9 @@ describe('MeterProvider', () => { instrumentName: 'non-renamed-instrument', }), ], + readers: [reader], }); - const reader = new TestMetricReader(); - meterProvider.addMetricReader(reader); - // Create meter and instrument. const myMeter = meterProvider.getMeter('meter1', 'v1.0.0'); const counter = myMeter.createCounter('non-renamed-instrument'); @@ -200,6 +201,8 @@ describe('MeterProvider', () => { }); it('with attributeKeys should drop non-listed attributes', async () => { + const reader = new TestMetricReader(); + // Add view to drop all attributes except 'attrib1' const meterProvider = new MeterProvider({ resource: defaultResource, @@ -209,11 +212,9 @@ describe('MeterProvider', () => { instrumentName: 'non-renamed-instrument', }), ], + readers: [reader], }); - const reader = new TestMetricReader(); - meterProvider.addMetricReader(reader); - // Create meter and instrument. const myMeter = meterProvider.getMeter('meter1', 'v1.0.0'); const counter = myMeter.createCounter('non-renamed-instrument'); @@ -266,6 +267,8 @@ describe('MeterProvider', () => { }); it('with no meter name should apply view to instruments of all meters', async () => { + const reader = new TestMetricReader(); + // Add view that renames 'test-counter' to 'renamed-instrument' const meterProvider = new MeterProvider({ resource: defaultResource, @@ -275,11 +278,9 @@ describe('MeterProvider', () => { instrumentName: 'test-counter', }), ], + readers: [reader], }); - const reader = new TestMetricReader(); - meterProvider.addMetricReader(reader); - // Create two meters. const meter1 = meterProvider.getMeter('meter1', 'v1.0.0'); const meter2 = meterProvider.getMeter('meter2', 'v1.0.0'); @@ -339,6 +340,7 @@ describe('MeterProvider', () => { }); it('with meter name should apply view to only the selected meter', async () => { + const reader = new TestMetricReader(); const meterProvider = new MeterProvider({ resource: defaultResource, views: [ @@ -349,11 +351,9 @@ describe('MeterProvider', () => { meterName: 'meter1', }), ], + readers: [reader], }); - const reader = new TestMetricReader(); - meterProvider.addMetricReader(reader); - // Create two meters. const meter1 = meterProvider.getMeter('meter1', 'v1.0.0'); const meter2 = meterProvider.getMeter('meter2', 'v1.0.0'); @@ -413,6 +413,7 @@ describe('MeterProvider', () => { }); it('with different instrument types does not throw', async () => { + const reader = new TestMetricReader(); const meterProvider = new MeterProvider({ resource: defaultResource, // Add Views to rename both instruments (of different types) to the same name. @@ -428,9 +429,8 @@ describe('MeterProvider', () => { meterName: 'meter1', }), ], + readers: [reader], }); - const reader = new TestMetricReader(); - meterProvider.addMetricReader(reader); // Create meter and instruments. const meter = meterProvider.getMeter('meter1', 'v1.0.0'); @@ -481,6 +481,8 @@ describe('MeterProvider', () => { const msBoundaries = [0, 1, 2, 3, 4, 5]; const sBoundaries = [10, 50, 250, 1000]; + const reader = new TestMetricReader(); + const meterProvider = new MeterProvider({ resource: defaultResource, views: [ @@ -493,11 +495,9 @@ describe('MeterProvider', () => { aggregation: new ExplicitBucketHistogramAggregation(sBoundaries), }), ], + readers: [reader], }); - const reader = new TestMetricReader(); - meterProvider.addMetricReader(reader); - // Create meter and histograms, with different units. const meter = meterProvider.getMeter('meter1', 'v1.0.0'); const histogram1 = meter.createHistogram('test-histogram-ms', { @@ -543,14 +543,14 @@ describe('MeterProvider', () => { describe('shutdown', () => { it('should shutdown all registered metric readers', async () => { - const meterProvider = new MeterProvider({ resource: defaultResource }); const reader1 = new TestMetricReader(); const reader2 = new TestMetricReader(); const reader1ShutdownSpy = sinon.spy(reader1, 'shutdown'); const reader2ShutdownSpy = sinon.spy(reader2, 'shutdown'); - - meterProvider.addMetricReader(reader1); - meterProvider.addMetricReader(reader2); + const meterProvider = new MeterProvider({ + resource: defaultResource, + readers: [reader1, reader2], + }); await meterProvider.shutdown({ timeoutMillis: 1234 }); await meterProvider.shutdown(); @@ -569,14 +569,14 @@ describe('MeterProvider', () => { describe('forceFlush', () => { it('should forceFlush all registered metric readers', async () => { - const meterProvider = new MeterProvider({ resource: defaultResource }); const reader1 = new TestMetricReader(); const reader2 = new TestMetricReader(); const reader1ForceFlushSpy = sinon.spy(reader1, 'forceFlush'); const reader2ForceFlushSpy = sinon.spy(reader2, 'forceFlush'); - - meterProvider.addMetricReader(reader1); - meterProvider.addMetricReader(reader2); + const meterProvider = new MeterProvider({ + resource: defaultResource, + readers: [reader1, reader2], + }); await meterProvider.forceFlush({ timeoutMillis: 1234 }); await meterProvider.forceFlush({ timeoutMillis: 5678 }); diff --git a/packages/sdk-metrics/test/export/ConsoleMetricExporter.test.ts b/packages/sdk-metrics/test/export/ConsoleMetricExporter.test.ts index fe46fa9f71..6409f6aa04 100644 --- a/packages/sdk-metrics/test/export/ConsoleMetricExporter.test.ts +++ b/packages/sdk-metrics/test/export/ConsoleMetricExporter.test.ts @@ -50,7 +50,7 @@ describe('ConsoleMetricExporter', () => { let previousConsoleDir: any; let exporter: ConsoleMetricExporter; let meterProvider: MeterProvider; - let meterReader: PeriodicExportingMetricReader; + let metricReader: PeriodicExportingMetricReader; let meter: metrics.Meter; beforeEach(() => { @@ -58,20 +58,22 @@ describe('ConsoleMetricExporter', () => { console.dir = () => {}; exporter = new ConsoleMetricExporter(); - meterProvider = new MeterProvider({ resource: defaultResource }); - meter = meterProvider.getMeter('ConsoleMetricExporter', '1.0.0'); - meterReader = new PeriodicExportingMetricReader({ + metricReader = new PeriodicExportingMetricReader({ exporter: exporter, exportIntervalMillis: 100, exportTimeoutMillis: 100, }); - meterProvider.addMetricReader(meterReader); + meterProvider = new MeterProvider({ + resource: defaultResource, + readers: [metricReader], + }); + meter = meterProvider.getMeter('ConsoleMetricExporter', '1.0.0'); }); afterEach(async () => { console.dir = previousConsoleDir; - await meterReader.shutdown(); + await metricReader.shutdown(); }); it('should export information about metric', async () => { diff --git a/packages/sdk-metrics/test/export/InMemoryMetricExporter.test.ts b/packages/sdk-metrics/test/export/InMemoryMetricExporter.test.ts index 38a3f98586..1e14a8ed5e 100644 --- a/packages/sdk-metrics/test/export/InMemoryMetricExporter.test.ts +++ b/packages/sdk-metrics/test/export/InMemoryMetricExporter.test.ts @@ -45,24 +45,26 @@ async function waitForNumberOfExports( describe('InMemoryMetricExporter', () => { let exporter: InMemoryMetricExporter; let meterProvider: MeterProvider; - let meterReader: PeriodicExportingMetricReader; + let metricReader: PeriodicExportingMetricReader; let meter: metrics.Meter; beforeEach(() => { exporter = new InMemoryMetricExporter(AggregationTemporality.CUMULATIVE); - meterProvider = new MeterProvider({ resource: defaultResource }); - meter = meterProvider.getMeter('InMemoryMetricExporter', '1.0.0'); - meterReader = new PeriodicExportingMetricReader({ + metricReader = new PeriodicExportingMetricReader({ exporter: exporter, exportIntervalMillis: 100, exportTimeoutMillis: 100, }); - meterProvider.addMetricReader(meterReader); + meterProvider = new MeterProvider({ + resource: defaultResource, + readers: [metricReader], + }); + meter = meterProvider.getMeter('InMemoryMetricExporter', '1.0.0'); }); afterEach(async () => { await exporter.shutdown(); - await meterReader.shutdown(); + await metricReader.shutdown(); }); it('should return failed result code', done => { @@ -85,7 +87,7 @@ describe('InMemoryMetricExporter', () => { }; exporter.export(resourceMetrics, result => { assert.ok(result.code === ExportResultCode.FAILED); - meterReader.shutdown().then(() => { + metricReader.shutdown().then(() => { done(); }); }); @@ -108,7 +110,7 @@ describe('InMemoryMetricExporter', () => { assert.ok(otherMetrics.length === 0); await exporter.shutdown(); - await meterReader.shutdown(); + await metricReader.shutdown(); }); it('should be able to access metric', async () => { @@ -146,6 +148,6 @@ describe('InMemoryMetricExporter', () => { const histogramDataPoint = histogramMetric.dataPoints.shift(); assert.ok(histogramDataPoint); - await meterReader.shutdown(); + await metricReader.shutdown(); }); }); diff --git a/packages/sdk-metrics/test/export/MetricReader.test.ts b/packages/sdk-metrics/test/export/MetricReader.test.ts index c0643a60da..b08cde5e38 100644 --- a/packages/sdk-metrics/test/export/MetricReader.test.ts +++ b/packages/sdk-metrics/test/export/MetricReader.test.ts @@ -73,16 +73,18 @@ describe('MetricReader', () => { describe('setMetricProducer', () => { it('The SDK MUST NOT allow a MetricReader instance to be registered on more than one MeterProvider instance', () => { const reader = new TestMetricReader(); - const meterProvider1 = new MeterProvider(); - const meterProvider2 = new MeterProvider(); - - meterProvider1.addMetricReader(reader); assert.throws( - () => meterProvider1.addMetricReader(reader), + () => + new MeterProvider({ + readers: [reader, reader], + }), /MetricReader can not be bound to a MeterProvider again/ ); assert.throws( - () => meterProvider2.addMetricReader(reader), + () => + new MeterProvider({ + readers: [reader], + }), /MetricReader can not be bound to a MeterProvider again/ ); }); @@ -138,7 +140,6 @@ describe('MetricReader', () => { }); it('should collect metrics from the SDK and the additional metricProducers', async () => { - const meterProvider = new MeterProvider({ resource: defaultResource }); const additionalProducer = new TestMetricProducer({ resourceMetrics: { resource: new Resource({ @@ -150,7 +151,10 @@ describe('MetricReader', () => { const reader = new TestMetricReader({ metricProducers: [additionalProducer], }); - meterProvider.addMetricReader(reader); + const meterProvider = new MeterProvider({ + resource: defaultResource, + readers: [reader], + }); // Make a measurement meterProvider @@ -182,14 +186,15 @@ describe('MetricReader', () => { }); it('should merge the errors from the SDK and all metricProducers', async () => { - const meterProvider = new MeterProvider(); const reader = new TestMetricReader({ metricProducers: [ new TestMetricProducer({ errors: ['err1'] }), new TestMetricProducer({ errors: ['err2'] }), ], }); - meterProvider.addMetricReader(reader); + const meterProvider = new MeterProvider({ + readers: [reader], + }); // Provide a callback throwing an error too meterProvider diff --git a/packages/sdk-metrics/test/regression/cumulative-exponential-histogram.test.ts b/packages/sdk-metrics/test/regression/cumulative-exponential-histogram.test.ts index 79dcfc434f..2462ddf5c8 100644 --- a/packages/sdk-metrics/test/regression/cumulative-exponential-histogram.test.ts +++ b/packages/sdk-metrics/test/regression/cumulative-exponential-histogram.test.ts @@ -46,7 +46,6 @@ describe('cumulative-exponential-histogram', () => { }); const doTest = async (histogramAggregation: Aggregation) => { - const meterProvider = new MeterProvider(); const reader = new TestMetricReader({ aggregationTemporalitySelector() { return AggregationTemporality.CUMULATIVE; @@ -57,8 +56,10 @@ describe('cumulative-exponential-histogram', () => { : Aggregation.Default(); }, }); + const meterProvider = new MeterProvider({ + readers: [reader], + }); - meterProvider.addMetricReader(reader); const meter = meterProvider.getMeter('my-meter'); const hist = meter.createHistogram('testhist'); diff --git a/packages/sdk-metrics/test/regression/two-metric-readers-async-instrument.test.ts b/packages/sdk-metrics/test/regression/two-metric-readers-async-instrument.test.ts index a1301be0ec..74913f9476 100644 --- a/packages/sdk-metrics/test/regression/two-metric-readers-async-instrument.test.ts +++ b/packages/sdk-metrics/test/regression/two-metric-readers-async-instrument.test.ts @@ -23,12 +23,11 @@ import { assertDataPoint, assertMetricData } from '../util'; describe('two-metric-readers-async-instrument', () => { it('both metric readers should collect metrics', async () => { - const meterProvider = new MeterProvider(); const reader1 = new TestDeltaMetricReader(); const reader2 = new TestDeltaMetricReader(); - - meterProvider.addMetricReader(reader1); - meterProvider.addMetricReader(reader2); + const meterProvider = new MeterProvider({ + readers: [reader1, reader2], + }); const meter = meterProvider.getMeter('my-meter'); diff --git a/packages/sdk-metrics/test/state/MeterSharedState.test.ts b/packages/sdk-metrics/test/state/MeterSharedState.test.ts index 55f7fe5386..9e814a4b43 100644 --- a/packages/sdk-metrics/test/state/MeterSharedState.test.ts +++ b/packages/sdk-metrics/test/state/MeterSharedState.test.ts @@ -48,8 +48,8 @@ describe('MeterSharedState', () => { const meterProvider = new MeterProvider({ resource: defaultResource, views, + readers: readers, }); - readers?.forEach(reader => meterProvider.addMetricReader(reader)); const meter = meterProvider.getMeter('test-meter'); @@ -185,19 +185,17 @@ describe('MeterSharedState', () => { describe('collect', () => { function setupInstruments(views?: View[]) { + const cumulativeReader = new TestMetricReader(); + const deltaReader = new TestDeltaMetricReader(); + const meterProvider = new MeterProvider({ resource: defaultResource, views: views, + readers: [cumulativeReader, deltaReader], }); - const cumulativeReader = new TestMetricReader(); - meterProvider.addMetricReader(cumulativeReader); const cumulativeCollector = cumulativeReader.getMetricCollector(); - - const deltaReader = new TestDeltaMetricReader(); - meterProvider.addMetricReader(deltaReader); const deltaCollector = deltaReader.getMetricCollector(); - const metricCollectors = [cumulativeCollector, deltaCollector]; const meter = meterProvider.getMeter( diff --git a/packages/sdk-metrics/test/state/MetricCollector.test.ts b/packages/sdk-metrics/test/state/MetricCollector.test.ts index 8466a5a95c..f173a48446 100644 --- a/packages/sdk-metrics/test/state/MetricCollector.test.ts +++ b/packages/sdk-metrics/test/state/MetricCollector.test.ts @@ -55,10 +55,12 @@ describe('MetricCollector', () => { describe('collect', () => { function setupInstruments() { - const meterProvider = new MeterProvider({ resource: defaultResource }); - const reader = new TestMetricReader(); - meterProvider.addMetricReader(reader); + const meterProvider = new MeterProvider({ + resource: defaultResource, + readers: [reader], + }); + const metricCollector = reader.getMetricCollector(); const meter = meterProvider.getMeter( From 686331c55ac60599155ea5ed5db538483b5b4eb1 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Tue, 16 Jan 2024 13:03:19 +0100 Subject: [PATCH 2/2] fix(changelog): add changelog entry --- CHANGELOG_NEXT.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG_NEXT.md b/CHANGELOG_NEXT.md index 1ecdcaf9eb..677ff7e46c 100644 --- a/CHANGELOG_NEXT.md +++ b/CHANGELOG_NEXT.md @@ -2,6 +2,8 @@ ### :boom: Breaking Change +* feat(sdk-metrics)!: remove MeterProvider.addMetricReader() in favor of constructor option [#4419](https://github.com/open-telemetry/opentelemetry-js/pull/4419) @pichlermarc + ### :rocket: (Enhancement) ### :books: (Refine Doc)