feat(sdk-metrics): deprecate MeterProvider.addMetricReader() in favor…

… of 'readers' constructor option (open-telemetry#4427) * feat(sdk-metrics): add 'readers' constructor option, deprecate MeterProvider.addMetricReader() * fix(changelog): update changelog entry, fix format
Zirak · Sep 14, 2024 · 79c4edf · 79c4edf
1 parent c2b35ec
commit 79c4edf
Show file tree

Hide file tree

Showing 18 changed files with 141 additions and 106 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,9 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
 
 ### :rocket: (Enhancement)
 
+* feat(sdk-metrics): add constructor option to add metric readers [#4427](https://github.com/open-telemetry/opentelemetry-js/pull/4427) @pichlermarc
+  * deprecates `MeterProvider.addMetricReader()` please use the constructor option `readers` instead.
+
 ### :bug: (Bug Fix)
 
 * fix(sdk-trace-base): ensure attribute value length limit is enforced on span creation [#4417](https://github.com/open-telemetry/opentelemetry-js/pull/4417) @pichlermarc

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,12 @@ export class MeterProvider implements IMeterProvider {
         this._sharedState.viewRegistry.addView(view);
       }
     }
+
+    if (options?.readers != null && options.readers.length > 0) {
+      for (const metricReader of options.readers) {
+        this.addMetricReader(metricReader);
+      }
+    }
   }
 
   /**
@@ -77,7 +84,13 @@ export class MeterProvider implements IMeterProvider {
    * Register a {@link MetricReader} to the meter provider. After the
    * registration, the MetricReader can start metrics collection.
    *
+   * <p> NOTE: {@link MetricReader} instances MUST be added before creating any instruments.
+   * A {@link MetricReader} instance registered later may receive no or incomplete metric data.
+   *
    * @param metricReader the metric reader to be registered.
+   *
+   * @deprecated This method will be removed in SDK 2.0. Please use
+   * {@link MeterProviderOptions.readers} via the {@link MeterProvider} constructor instead
    */
   addMetricReader(metricReader: MetricReader) {
     const collector = new MetricCollector(this._sharedState, metricReader);

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,