feat(sdk-node): remove deprecated methods from NodeSDK (#4609)

Author: pichlermarc

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)

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 =

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 =