fix(sdk-node): Allow Defining Sampler with Exporter in Env (#4394)

* fix(sdk-node): Allow tracerProvider to be created when exporter is defined in the env. * fix(node-sdk): Update to not accept when exporter is set to none. * fix(sdk-node): Update Changelog. * fix(sdk-node): Fix Changelog. * fix(sdk-node): cleanup changelog. * fix(sdk-node): lint fix * fix(sdk-node): Fix logic for creating tracerProviders. * Fix lint. * Update experimental/CHANGELOG.md Co-authored-by: Marc Pichler <[email protected]> * Fix manual sampler and environment exporter case. * Update logic to check for a defined traceExporter on the config before using the NodeTracerProvider. * Fix equality check. * Update env exporter configuration logic and add tests. * Update experimental/CHANGELOG.md Co-authored-by: Marc Pichler <[email protected]> * Fix changelog issues. * Clean up tracerProvider logic. * Update sdk.ts * Update sdk.ts * Update experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts Co-authored-by: Marc Pichler <[email protected]> * Update experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts Co-authored-by: Marc Pichler <[email protected]> * Update experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts Co-authored-by: Marc Pichler <[email protected]> --------- Co-authored-by: Marc Pichler <[email protected]>
open-telemetry · Jan 31, 2024 · efa6307 · efa6307
1 parent ac75b70
commit efa6307
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 2 deletions.
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -12,6 +12,8 @@ All notable changes to experimental packages in this project will be documented
 
 ### :bug: (Bug Fix)
 
+* fix(sdk-node): allow using samplers when the exporter is defined in the environment [#4394](https://github.com/open-telemetry/opentelemetry-js/pull/4394) @JacksonWeber
+
 ### :books: (Refine Doc)
 
 ### :house: (Internal)

diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts
@@ -92,6 +92,7 @@ export class NodeSDK {
   private _loggerProvider?: LoggerProvider;
   private _meterProvider?: MeterProvider;
   private _serviceName?: string;
+  private _configuration?: Partial<NodeSDKConfiguration>;
 
   private _disabled?: boolean;
 
@@ -116,6 +117,8 @@ export class NodeSDK {
       });
     }
 
+    this._configuration = configuration;
+
     this._resource = configuration.resource ?? new Resource({});
     this._resourceDetectors = configuration.resourceDetectors ?? [
       envDetector,
@@ -126,7 +129,8 @@ export class NodeSDK {
 
     this._autoDetectResources = configuration.autoDetectResources ?? true;
 
-    if (configuration.spanProcessor || configuration.traceExporter) {
+    // If a tracer provider can be created from manual configuration, create it
+    if (configuration.traceExporter || configuration.spanProcessor) {
       const tracerProviderConfig: NodeTracerConfig = {};
 
       if (configuration.sampler) {
@@ -316,12 +320,14 @@ export class NodeSDK {
             })
           );
 
+    // if there is a tracerProviderConfig (traceExporter/spanProcessor was set manually) or the traceExporter is set manually, use NodeTracerProvider
     const Provider = this._tracerProviderConfig
       ? NodeTracerProvider
       : TracerProviderWithEnvExporters;
 
+    // If the Provider is configured with Env Exporters, we need to check if the SDK had any manual configurations and set them here
     const tracerProvider = new Provider({
-      ...this._tracerProviderConfig?.tracerConfig,
+      ...this._configuration,
       resource: this._resource,
     });
 

diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
@@ -46,6 +46,7 @@ import {
   BatchSpanProcessor,
   NoopSpanProcessor,
   IdGenerator,
+  AlwaysOffSampler,
 } from '@opentelemetry/sdk-trace-base';
 import * as assert from 'assert';
 import * as semver from 'semver';
@@ -176,6 +177,29 @@ describe('Node SDK', () => {
       assert.ok(apiTracerProvider.getDelegate() instanceof NodeTracerProvider);
     });
 
+    it('should register a tracer provider if an exporter is provided via env', async () => {
+      env.OTEL_TRACES_EXPORTER = 'console';
+      const sdk = new NodeSDK({
+        autoDetectResources: false,
+      });
+
+      sdk.start();
+
+      assert.ok(!(metrics.getMeterProvider() instanceof MeterProvider));
+
+      assert.ok(
+        context['_getContextManager']().constructor.name ===
+          DefaultContextManager.name
+      );
+      assert.ok(
+        propagation['_getGlobalPropagator']() instanceof CompositePropagator
+      );
+      const apiTracerProvider =
+        trace.getTracerProvider() as ProxyTracerProvider;
+      assert.ok(apiTracerProvider.getDelegate() instanceof NodeTracerProvider);
+      delete env.OTEL_TRACES_EXPORTER;
+    });
+
     it('should register a tracer provider if a span processor is provided', async () => {
       const exporter = new ConsoleSpanExporter();
       const spanProcessor = new SimpleSpanProcessor(exporter);
@@ -873,6 +897,21 @@ describe('setup exporter from env', () => {
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
     delete env.OTEL_TRACES_EXPORTER;
   });
+  it('should only create one span processor when configured using env vars and config', async () => {
+    env.OTEL_TRACES_EXPORTER = 'console';
+    const sdk = new NodeSDK({
+      sampler: new AlwaysOffSampler(),
+    });
+    sdk.start();
+    const listOfProcessors =
+      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    assert.ok(sdk['_tracerProvider'] instanceof TracerProviderWithEnvExporters);
+    assert.ok(
+      sdk['_tracerProvider']!['_config']?.sampler instanceof AlwaysOffSampler
+    );
+    assert.strictEqual(listOfProcessors.length, 1);
+    delete env.OTEL_TRACES_EXPORTER;
+  });
   it('use otlp exporter and defined exporter protocol env value', async () => {
     env.OTEL_TRACES_EXPORTER = 'otlp';
     env.OTEL_EXPORTER_OTLP_TRACES_PROTOCOL = 'grpc';