feat(sdk-metrics)!: remove MeterProvider.addMetricReader() in favor o…

…f constructor option (#4419) * feat(sdk-metrics)!: remove MeterProvider.addMetricReader() in favor of constructor option * fix(changelog): add changelog entry
open-telemetry · Jan 24, 2024 · a9af247 · a9af247
1 parent 0aba75c
commit a9af247
Show file tree

Hide file tree

Showing 18 changed files with 135 additions and 118 deletions.
diff --git a/CHANGELOG_NEXT.md b/CHANGELOG_NEXT.md
@@ -4,6 +4,7 @@
 
 * chore(shim-opentracing): replace deprecated SpanAttributes [#4430](https://github.com/open-telemetry/opentelemetry-js/pull/4430) @JamieDanielson
 * chore(otel-core): replace deprecated SpanAttributes [#4408](https://github.com/open-telemetry/opentelemetry-js/pull/4408) @JamieDanielson
+* feat(sdk-metrics)!: remove MeterProvider.addMetricReader() in favor of constructor option [#4419](https://github.com/open-telemetry/opentelemetry-js/pull/4419) @pichlermarc
 * chore(otel-resources): replace deprecated SpanAttributes [#4428](https://github.com/open-telemetry/opentelemetry-js/pull/4428) @JamieDanielson
 
 ### :rocket: (Enhancement)

diff --git 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
@@ -72,19 +72,24 @@ 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() {
   return (await reader.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
@@ -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
@@ -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
@@ -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/...imental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts b/...imental/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
@@ -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
@@ -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
@@ -751,18 +751,19 @@ 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,
     {
       schemaUrl: defaultInstrumentationScope.schemaUrl,
     }
   );
-  const deltaReader = new TestDeltaMetricReader();
-  meterProvider.addMetricReader(deltaReader);
-  const cumulativeReader = new TestMetricReader();
-  meterProvider.addMetricReader(cumulativeReader);
 
   return {
     meterProvider,