From 7fb673cc59bffa37710bbeef5b1ce3174b72340e Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Wed, 10 Apr 2024 13:31:24 +0200 Subject: [PATCH] feat(sdk-node): remove deprecated methods from NodeSDK (#4609) --- experimental/CHANGELOG.md | 7 + .../opentelemetry-sdk-node/src/sdk.ts | 136 ++---------------- .../opentelemetry-sdk-node/test/sdk.test.ts | 101 ------------- 3 files changed, 21 insertions(+), 223 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index c8d631e6c1..3d0f124c9d 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -6,6 +6,13 @@ All notable changes to experimental packages in this project will be documented ### :boom: Breaking Change +* feat(sdk-node)!: remove long deprecated methods in favor of constructor options [#4606](https://github.com/open-telemetry/opentelemetry-js/pull/4606) @pichlermarc + * `NodeSDK.configureTracerProvider()`, please use constructor options instead + * `NodeSDK.configureMeterProvider()`, please use constructor options instead + * `NodeSDK.configureLoggerProvider()`, please use constructor options instead + * `NodeSDK.addResource()`, please use constructor options instead + * `NodeSDK.detectResources()`, this is not necessary anymore, resources are now auto-detected on `NodeSDK.start()` if the constructor option `autoDetectResources` is unset, `undefined` or `true`. + ### :rocket: (Enhancement) * feat(otlp-transformer): consolidate scope/resource creation in transformer [#4600](https://github.com/open-telemetry/opentelemetry-js/pull/4600) diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index 5e7cd3676a..c5f38d63a9 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -161,19 +161,18 @@ export class NodeSDK { const spanProcessors = configuration.spanProcessors ?? [spanProcessor]; - this.configureTracerProvider( - tracerProviderConfig, + this._tracerProviderConfig = { + tracerConfig: tracerProviderConfig, spanProcessors, - configuration.contextManager, - configuration.textMapPropagator - ); + contextManager: configuration.contextManager, + textMapPropagator: configuration.textMapPropagator, + }; } if (configuration.logRecordProcessor) { - const loggerProviderConfig: LoggerProviderConfig = { + this._loggerProviderConfig = { logRecordProcessor: configuration.logRecordProcessor, }; - this.configureLoggerProvider(loggerProviderConfig); } if (configuration.metricReader || configuration.views) { @@ -186,7 +185,7 @@ export class NodeSDK { meterProviderConfig.views = configuration.views; } - this.configureMeterProvider(meterProviderConfig); + this._meterProviderConfig = meterProviderConfig; } let instrumentations: InstrumentationOption[] = []; @@ -196,119 +195,6 @@ export class NodeSDK { this._instrumentations = instrumentations; } - /** - * - * @deprecated Please pass {@code sampler}, {@code generalLimits}, {@code spanLimits}, {@code resource}, - * {@code IdGenerator}, {@code spanProcessor}, {@code contextManager} and {@code textMapPropagator}, - * to the constructor options instead. - * - * Set configurations needed to register a TracerProvider - */ - public configureTracerProvider( - tracerConfig: NodeTracerConfig, - spanProcessors: SpanProcessor[], - contextManager?: ContextManager, - textMapPropagator?: TextMapPropagator - ): void { - this._tracerProviderConfig = { - tracerConfig, - spanProcessors, - contextManager, - textMapPropagator, - }; - } - - /** - * @deprecated Please pass {@code logRecordProcessor} to the constructor options instead. - * - * Set configurations needed to register a LoggerProvider - */ - public configureLoggerProvider(config: LoggerProviderConfig): void { - // nothing is set yet, we can set config and then return - if (this._loggerProviderConfig == null) { - this._loggerProviderConfig = config; - return; - } - - // make sure we do not override existing logRecordProcessor with other logRecordProcessors. - if ( - this._loggerProviderConfig.logRecordProcessor != null && - config.logRecordProcessor != null - ) { - throw new Error( - 'LogRecordProcessor passed but LogRecordProcessor has already been configured.' - ); - } - - // set logRecordProcessor, but make sure we do not override existing logRecordProcessors with null/undefined. - if (config.logRecordProcessor != null) { - this._loggerProviderConfig.logRecordProcessor = config.logRecordProcessor; - } - } - - /** - * @deprecated Please pass {@code views} and {@code reader} to the constructor options instead. - * - * Set configurations needed to register a MeterProvider - */ - public configureMeterProvider(config: MeterProviderConfig): void { - // nothing is set yet, we can set config and return. - if (this._meterProviderConfig == null) { - this._meterProviderConfig = config; - return; - } - - // make sure we do not override existing views with other views. - if (this._meterProviderConfig.views != null && config.views != null) { - throw new Error('Views passed but Views have already been configured.'); - } - - // set views, but make sure we do not override existing views with null/undefined. - if (config.views != null) { - this._meterProviderConfig.views = config.views; - } - - // make sure we do not override existing reader with another reader. - if (this._meterProviderConfig.reader != null && config.reader != null) { - throw new Error( - 'MetricReader passed but MetricReader has already been configured.' - ); - } - - // set reader, but make sure we do not override existing reader with null/undefined. - if (config.reader != null) { - this._meterProviderConfig.reader = config.reader; - } - } - - /** - * @deprecated Resources are detected automatically on {@link NodeSDK.start()}, when the {@code autoDetectResources} - * constructor option is set to {@code true} or left {@code undefined}. - * - * Detect resource attributes - */ - public detectResources(): void { - if (this._disabled) { - return; - } - - const internalConfig: ResourceDetectionConfig = { - detectors: this._resourceDetectors, - }; - - this.addResource(detectResourcesSync(internalConfig)); - } - - /** - * @deprecated Please pre-merge resources and pass them to the constructor - * - * Manually add a Resource - * @param resource - */ - public addResource(resource: IResource): void { - this._resource = this._resource.merge(resource); - } - /** * Call this method to construct SDK components and register them with the OpenTelemetry API. */ @@ -322,7 +208,13 @@ export class NodeSDK { }); if (this._autoDetectResources) { - this.detectResources(); + const internalConfig: ResourceDetectionConfig = { + detectors: this._resourceDetectors, + }; + + this._resource = this._resource.merge( + detectResourcesSync(internalConfig) + ); } this._resource = diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts index b2d7bdb909..d76534450f 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -409,107 +409,6 @@ describe('Node SDK', () => { delete env.OTEL_TRACES_EXPORTER; }); - it('should throw error when calling configureMeterProvider when views are already configured', () => { - const exporter = new InMemoryMetricExporter( - AggregationTemporality.CUMULATIVE - ); - const metricReader = new PeriodicExportingMetricReader({ - exporter: exporter, - exportIntervalMillis: 100, - exportTimeoutMillis: 100, - }); - - const sdk = new NodeSDK({ - metricReader: metricReader, - views: [ - new View({ - name: 'test-view', - instrumentName: 'test_counter', - instrumentType: InstrumentType.COUNTER, - }), - ], - autoDetectResources: false, - }); - - assert.throws( - () => { - sdk.configureMeterProvider({ - reader: metricReader, - views: [ - new View({ - name: 'test-view', - instrumentName: 'test_counter', - instrumentType: InstrumentType.COUNTER, - }), - ], - }); - }, - (error: Error) => { - return error.message.includes( - 'Views passed but Views have already been configured' - ); - } - ); - }); - - it('should throw error when calling configureMeterProvider when metricReader is already configured', () => { - const exporter = new InMemoryMetricExporter( - AggregationTemporality.CUMULATIVE - ); - const metricReader = new PeriodicExportingMetricReader({ - exporter: exporter, - exportIntervalMillis: 100, - exportTimeoutMillis: 100, - }); - - const sdk = new NodeSDK({ - metricReader: metricReader, - views: [ - new View({ - name: 'test-view', - instrumentName: 'test_counter', - instrumentType: InstrumentType.COUNTER, - }), - ], - autoDetectResources: false, - }); - - assert.throws( - () => { - sdk.configureMeterProvider({ - reader: metricReader, - }); - }, - (error: Error) => { - return error.message.includes( - 'MetricReader passed but MetricReader has already been configured.' - ); - } - ); - }); - - it('should throw error when calling configureLoggerProvider when logRecordProcessor is already configured', () => { - const logRecordExporter = new InMemoryLogRecordExporter(); - const logRecordProcessor = new SimpleLogRecordProcessor(logRecordExporter); - const sdk = new NodeSDK({ - logRecordProcessor: logRecordProcessor, - autoDetectResources: false, - }); - - assert.throws( - () => { - sdk.configureLoggerProvider({ - logRecordProcessor: logRecordProcessor, - }); - }, - (error: Error) => { - return error.message.includes( - 'LogRecordProcessor passed but LogRecordProcessor has already been configured.' - ); - } - ); - }); - describe('detectResources', async () => { beforeEach(() => { process.env.OTEL_RESOURCE_ATTRIBUTES =