From 184de79d6ae6297add2f764e36aebfc11d082209 Mon Sep 17 00:00:00 2001 From: Nidhi Singh Date: Tue, 26 Aug 2025 20:32:25 +0530 Subject: [PATCH] feat(node-sdk): add support for multiple metric readers (#5777) Co-authored-by: Marc Pichler --- experimental/CHANGELOG.md | 21 ++- .../opentelemetry-sdk-node/src/sdk.ts | 24 +++- .../opentelemetry-sdk-node/src/types.ts | 2 + .../opentelemetry-sdk-node/test/sdk.test.ts | 121 +++++++++++++++++- 4 files changed, 156 insertions(+), 12 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index ed3e22ec3..4254c9e04 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -16,8 +16,23 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2 ### :rocket: Features -*feat(opentelemetry-configuration): creation of basic ConfigProvider [#5809](https://github.com/open-telemetry/opentelemetry-js/pull/5809) @maryliag -*feat(opentelemetry-configuration): creation of basic FileConfigProvider [#5863](https://github.com/open-telemetry/opentelemetry-js/pull/5863) @maryliag +* feat(opentelemetry-configuration): creation of basic ConfigProvider [#5809](https://github.com/open-telemetry/opentelemetry-js/pull/5809) @maryliag +* feat(opentelemetry-configuration): creation of basic FileConfigProvider [#5863](https://github.com/open-telemetry/opentelemetry-js/pull/5863) @maryliag +* feat(sdk-node): Add support for multiple metric readers via the new `metricReaders` option in NodeSDK configuration. Users can now register multiple metric readers (e.g., Console, Prometheus) directly through the NodeSDK constructor. The old `metricReader` (singular) option is now deprecated and will show a warning if used, but remains supported for backward compatibility. Comprehensive tests and documentation have been added. [#5760](https://github.com/open-telemetry/opentelemetry-js/issues/5760) + * **Migration:** + - Before: + + ```js + const sdk = new NodeSDK({ metricReader: myMetricReader }); + ``` + + - After: + + ```js + const sdk = new NodeSDK({ metricReaders: [myMetricReader] }); + ``` + + * Users should migrate to the new `metricReaders` array option for future compatibility. The old option will be removed in an upcoming experimental version. ### :bug: Bug Fixes @@ -138,7 +153,7 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2 ### :house: (Internal) -* chore(instrumentation-grpc): remove unused findIndex() function [#5372](https://github.com/open-telemetry/opentelemetry-js/pull/5372) @cjihrig +* refactor(instrumentation-grpc): remove unused findIndex() function [#5372](https://github.com/open-telemetry/opentelemetry-js/pull/5372) @cjihrig * refactor(otlp-exporter-base): remove unnecessary isNaN() checks [#5374](https://github.com/open-telemetry/opentelemetry-js/pull/5374) @cjihrig * refactor(exporter-prometheus): remove unnecessary isNaN() check [#5377](https://github.com/open-telemetry/opentelemetry-js/pull/5377) @cjihrig * refactor(sdk-node): move code to auto-instantiate propagators into utils [#5355](https://github.com/open-telemetry/opentelemetry-js/pull/5355) @pichlermarc diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index e653b55dc..8ff8e2d6c 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -85,9 +85,9 @@ import { export type MeterProviderConfig = { /** - * Reference to the MetricReader instance by the NodeSDK + * Reference to the MetricReader instances by the NodeSDK */ - reader?: IMetricReader; + readers?: IMetricReader[]; /** * List of {@link ViewOptions}s that should be passed to the MeterProvider */ @@ -312,10 +312,20 @@ export class NodeSDK { this.configureLoggerProviderFromEnv(); } - if (configuration.metricReader || configuration.views) { + if ( + configuration.metricReaders || + configuration.metricReader || + configuration.views + ) { const meterProviderConfig: MeterProviderConfig = {}; - if (configuration.metricReader) { - meterProviderConfig.reader = configuration.metricReader; + + if (configuration.metricReaders) { + meterProviderConfig.readers = configuration.metricReaders; + } else if (configuration.metricReader) { + meterProviderConfig.readers = [configuration.metricReader]; + diag.warn( + "The 'metricReader' option is deprecated. Please use 'metricReaders' instead." + ); } if (configuration.views) { @@ -395,8 +405,8 @@ export class NodeSDK { configureMetricProviderFromEnv(); if (this._meterProviderConfig || metricReadersFromEnv.length > 0) { const readers: IMetricReader[] = []; - if (this._meterProviderConfig?.reader) { - readers.push(this._meterProviderConfig.reader); + if (this._meterProviderConfig?.readers) { + readers.push(...this._meterProviderConfig.readers); } if (readers.length === 0) { diff --git a/experimental/packages/opentelemetry-sdk-node/src/types.ts b/experimental/packages/opentelemetry-sdk-node/src/types.ts index ff400de8f..48b363a18 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/types.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/types.ts @@ -35,7 +35,9 @@ export interface NodeSDKConfiguration { /** @deprecated use logRecordProcessors instead*/ logRecordProcessor: LogRecordProcessor; logRecordProcessors?: LogRecordProcessor[]; + /** @deprecated use metricReaders instead*/ metricReader: IMetricReader; + metricReaders?: IMetricReader[]; views: ViewOptions[]; instrumentations: (Instrumentation | Instrumentation[])[]; resource: Resource; diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts index dbb896e6b..55394ea42 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -322,6 +322,123 @@ describe('Node SDK', () => { delete env.OTEL_TRACES_EXPORTER; }); + it('should register a meter provider if multiple readers are provided', async () => { + // need to set OTEL_TRACES_EXPORTER to none since default value is otlp + // which sets up an exporter and affects the context manager + env.OTEL_TRACES_EXPORTER = 'none'; + const consoleExporter = new ConsoleMetricExporter(); + const inMemoryExporter = new InMemoryMetricExporter( + AggregationTemporality.CUMULATIVE + ); + const metricReader1 = new PeriodicExportingMetricReader({ + exporter: consoleExporter, + exportIntervalMillis: 100, + exportTimeoutMillis: 100, + }); + const metricReader2 = new PeriodicExportingMetricReader({ + exporter: inMemoryExporter, + exportIntervalMillis: 100, + exportTimeoutMillis: 100, + }); + + const sdk = new NodeSDK({ + metricReaders: [metricReader1, metricReader2], + autoDetectResources: false, + }); + + sdk.start(); + + assert.strictEqual( + context['_getContextManager'](), + ctxManager, + 'context manager should not change' + ); + assert.strictEqual( + propagation['_getGlobalPropagator'](), + propagator, + 'propagator should not change' + ); + assert.strictEqual( + (trace.getTracerProvider() as ProxyTracerProvider).getDelegate(), + delegate, + 'tracer provider should not have changed' + ); + + const meterProvider = metrics.getMeterProvider() as MeterProvider; + assert.ok(meterProvider instanceof MeterProvider); + + // Verify that both metric readers are registered + const sharedState = (meterProvider as any)['_sharedState']; + assert.strictEqual(sharedState.metricCollectors.length, 2); + + await sdk.shutdown(); + delete env.OTEL_TRACES_EXPORTER; + }); + + it('should show deprecation warning when using metricReader option', async () => { + // need to set OTEL_TRACES_EXPORTER to none since default value is otlp + // which sets up an exporter and affects the context manager + env.OTEL_TRACES_EXPORTER = 'none'; + const exporter = new ConsoleMetricExporter(); + const metricReader = new PeriodicExportingMetricReader({ + exporter: exporter, + exportIntervalMillis: 100, + exportTimeoutMillis: 100, + }); + + const warnSpy = Sinon.spy(diag, 'warn'); + + const sdk = new NodeSDK({ + metricReader: metricReader, + autoDetectResources: false, + }); + + sdk.start(); + + // Verify deprecation warning was shown + Sinon.assert.calledWith( + warnSpy, + "The 'metricReader' option is deprecated. Please use 'metricReaders' instead." + ); + + assert.ok(metrics.getMeterProvider() instanceof MeterProvider); + + await sdk.shutdown(); + delete env.OTEL_TRACES_EXPORTER; + }); + + it('should not show deprecation warning when using metricReaders option', async () => { + // need to set OTEL_TRACES_EXPORTER to none since default value is otlp + // which sets up an exporter and affects the context manager + env.OTEL_TRACES_EXPORTER = 'none'; + const exporter = new ConsoleMetricExporter(); + const metricReader = new PeriodicExportingMetricReader({ + exporter: exporter, + exportIntervalMillis: 100, + exportTimeoutMillis: 100, + }); + + const warnSpy = Sinon.spy(diag, 'warn'); + + const sdk = new NodeSDK({ + metricReaders: [metricReader], + autoDetectResources: false, + }); + + sdk.start(); + + // Verify no metricReader deprecation warning was shown + Sinon.assert.neverCalledWith( + warnSpy, + "The 'metricReader' option is deprecated. Please use 'metricReaders' instead." + ); + + assert.ok(metrics.getMeterProvider() instanceof MeterProvider); + + await sdk.shutdown(); + delete env.OTEL_TRACES_EXPORTER; + }); + it('should register a logger provider if a log record processor is provided', async () => { env.OTEL_TRACES_EXPORTER = 'none'; const logRecordExporter = new InMemoryLogRecordExporter(); @@ -617,7 +734,7 @@ describe('Node SDK', () => { // Local functions to test if a mocked method is ever called with a specific argument or regex matching for an argument. // Needed because of race condition with parallel detectors. const callArgsContains = ( - mockedFunction: sinon.SinonSpy, + mockedFunction: Sinon.SinonSpy, arg: any ): boolean => { return mockedFunction.getCalls().some(call => { @@ -625,7 +742,7 @@ describe('Node SDK', () => { }); }; const callArgsMatches = ( - mockedFunction: sinon.SinonSpy, + mockedFunction: Sinon.SinonSpy, regex: RegExp ): boolean => { return mockedFunction.getCalls().some(call => {