feat(sdk-trace-base)!: drop ability to instantiate propagators beyond defaults

Author: pichlermarc

experimental/packages/opentelemetry-sdk-node/src/sdk.ts

@@ -74,6 +74,7 @@ import {
   getResourceDetectorsFromEnv,
   getSpanProcessorsFromEnv,
   filterBlanksAndNulls,
+  getPropgagatorFromEnv,
 } from './utils';
 
 /** This class represents everything needed to register a fully configured OpenTelemetry Node.js SDK */
@@ -376,7 +377,9 @@ export class NodeSDK {
           this._tracerProviderConfig?.contextManager ??
           // _tracerProviderConfig may be undefined if trace-specific settings are not provided - fall back to raw config
           this._configuration?.contextManager,
-        propagator: this._tracerProviderConfig?.textMapPropagator,
+        propagator:
+          this._tracerProviderConfig?.textMapPropagator ??
+          getPropgagatorFromEnv(),
       });
     }
 

experimental/packages/opentelemetry-sdk-node/src/utils.ts

@@ -14,8 +14,13 @@
  * limitations under the License.
  */
 
-import { diag } from '@opentelemetry/api';
-import { getEnv, getEnvWithoutDefaults } from '@opentelemetry/core';
+import { diag, TextMapPropagator } from '@opentelemetry/api';
+import {
+  CompositePropagator,
+  getEnv,
+  getEnvWithoutDefaults,
+  W3CTraceContextPropagator,
+} from '@opentelemetry/core';
 import { OTLPTraceExporter as OTLPProtoTraceExporter } from '@opentelemetry/exporter-trace-otlp-proto';
 import { OTLPTraceExporter as OTLPHttpTraceExporter } from '@opentelemetry/exporter-trace-otlp-http';
 import { OTLPTraceExporter as OTLPGrpcTraceExporter } from '@opentelemetry/exporter-trace-otlp-grpc';
@@ -35,6 +40,8 @@ import {
   SpanExporter,
   SpanProcessor,
 } from '@opentelemetry/sdk-trace-base';
+import { B3InjectEncoding, B3Propagator } from '@opentelemetry/propagator-b3';
+import { JaegerPropagator } from '@opentelemetry/propagator-jaeger';
 
 const RESOURCE_DETECTOR_ENVIRONMENT = 'env';
 const RESOURCE_DETECTOR_HOST = 'host';
@@ -180,3 +187,66 @@ export function getSpanProcessorsFromEnv(): SpanProcessor[] {
 
   return processors;
 }
+
+/**
+ * Get a propagator as defined by environment variables
+ */
+export function getPropgagatorFromEnv(): TextMapPropagator | null | undefined {
+  // Empty and undefined MUST be treated equal.
+  if (
+    process.env.OTEL_PROPAGATORS === undefined ||
+    process.env.OTEL_PROPAGATORS?.trim() === ''
+  ) {
+    // return undefined to fall back to default
+    return undefined;
+  }
+
+  // Implementation note: this only contains specification required propagators that are actually hosted in this repo.
+  // Any other propagators (like aws, aws-lambda, should go into `@opentelemetry/auto-configuration-propagators` instead).
+  const propagatorsFactory = new Map<string, () => TextMapPropagator>([
+    ['tracecontext', () => new W3CTraceContextPropagator()],
+    ['baggage', () => new W3CTraceContextPropagator()],
+    ['b3', () => new B3Propagator()],
+    [
+      'b3multi',
+      () => new B3Propagator({ injectEncoding: B3InjectEncoding.MULTI_HEADER }),
+    ],
+    ['jaeger', () => new JaegerPropagator()],
+  ]);
+
+  // Values MUST be deduplicated in order to register a Propagator only once.
+  const uniquePropagatorNames = Array.from(new Set(getEnv().OTEL_PROPAGATORS));
+
+  const propagators = uniquePropagatorNames.map(name => {
+    const propagator = propagatorsFactory.get(name)?.();
+    if (!propagator) {
+      diag.warn(
+        `Propagator "${name}" requested through environment variable is unavailable.`
+      );
+      return undefined;
+    }
+
+    return propagator;
+  });
+
+  const validPropagators = propagators.reduce<TextMapPropagator[]>(
+    (list, item) => {
+      if (item) {
+        list.push(item);
+      }
+      return list;
+    },
+    []
+  );
+
+  if (validPropagators.length === 0) {
+    // null to signal that the default should **not** be used in its place.
+    return null;
+  } else if (uniquePropagatorNames.length === 1) {
+    return validPropagators[0];
+  } else {
+    return new CompositePropagator({
+      propagators: validPropagators,
+    });
+  }
+}

experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts

@@ -414,6 +414,20 @@ describe('Node SDK', () => {
       assert.equal(actualContextManager, expectedContextManager);
       await sdk.shutdown();
     });
+
+    it('should register a propagators as defined in OTEL_PROPAGATORS if trace SDK is configured', async () => {
+      process.env.OTEL_PROPAGATORS = 'b3';
+      const sdk = new NodeSDK({
+        traceExporter: new ConsoleSpanExporter(),
+        autoDetectResources: false,
+      });
+
+      sdk.start();
+
+      assert.deepStrictEqual(propagation.fields(), ['b3']);
+
+      await sdk.shutdown();
+    });
   });
 
   async function waitForNumberOfMetrics(

experimental/packages/opentelemetry-sdk-node/test/utils.test.ts

@@ -0,0 +1,103 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { getPropgagatorFromEnv } from '../src/utils';
+import * as assert from 'assert';
+import * as sinon from 'sinon';
+import { diag } from '@opentelemetry/api';
+
+describe('getPropagatorFromEnv', function () {
+  afterEach(() => {
+    delete process.env.OTEL_PROPAGATORS;
+    sinon.restore();
+  });
+
+  describe('should default to undefined', function () {
+    it('when not defined', function () {
+      delete process.env.OTEL_PROPAGATORS;
+
+      const propagator = getPropgagatorFromEnv();
+
+      assert.deepStrictEqual(propagator, undefined);
+    });
+
+    it('on empty string', function () {
+      (process.env as any).OTEL_PROPAGATORS = '';
+
+      const propagator = getPropgagatorFromEnv();
+
+      assert.deepStrictEqual(propagator, undefined);
+    });
+
+    it('on space-only string', function () {
+      (process.env as any).OTEL_PROPAGATORS = '   ';
+
+      const propagator = getPropgagatorFromEnv();
+
+      assert.deepStrictEqual(propagator, undefined);
+    });
+  });
+
+  it('should return the selected propagator when one is in the list', () => {
+    process.env.OTEL_PROPAGATORS = 'tracecontext';
+    assert.deepStrictEqual(getPropgagatorFromEnv()?.fields(), [
+      'traceparent',
+      'tracestate',
+    ]);
+  });
+
+  it('should return the selected propagators when multiple are in the list', () => {
+    process.env.OTEL_PROPAGATORS = 'b3,jaeger';
+    assert.deepStrictEqual(getPropgagatorFromEnv()?.fields(), [
+      'b3',
+      'uber-trace-id',
+    ]);
+  });
+
+  it('should return the selected propagators when multiple are in the list', () => {
+    process.env.OTEL_PROPAGATORS = 'b3,jaeger';
+    assert.deepStrictEqual(getPropgagatorFromEnv()?.fields(), [
+      'b3',
+      'uber-trace-id',
+    ]);
+  });
+
+  it('should return null and warn if propagators are unknown', () => {
+    const warnStub = sinon.stub(diag, 'warn');
+
+    process.env.OTEL_PROPAGATORS = 'my, unknown, propagators';
+    assert.deepStrictEqual(getPropgagatorFromEnv(), null);
+    sinon.assert.calledWithExactly(
+      warnStub,
+      'Propagator "my" requested through environment variable is unavailable.'
+    );
+    sinon.assert.calledWithExactly(
+      warnStub,
+      'Propagator "unknown" requested through environment variable is unavailable.'
+    );
+    sinon.assert.calledWithExactly(
+      warnStub,
+      'Propagator "propagators" requested through environment variable is unavailable.'
+    );
+    sinon.assert.calledThrice(warnStub);
+  });
+
+  it('should return null if only "none" is selected', () => {
+    process.env.OTEL_PROPAGATORS = 'none';
+
+    assert.deepStrictEqual(getPropgagatorFromEnv(), null);
+  });
+});

package-lock.json

@@ -16,7 +16,6 @@
 
 import {
   context,
-  diag,
   propagation,
   TextMapPropagator,
   trace,
@@ -26,7 +25,6 @@ import {
   CompositePropagator,
   W3CBaggagePropagator,
   W3CTraceContextPropagator,
-  getEnv,
   merge,
 } from '@opentelemetry/core';
 import { IResource, Resource } from '@opentelemetry/resources';
@@ -48,18 +46,14 @@ export enum ForceFlushState {
   'unresolved',
 }
 
+function getDefaultPropagators(): TextMapPropagator[] {
+  return [new W3CTraceContextPropagator(), new W3CBaggagePropagator()];
+}
+
 /**
  * This class represents a basic tracer provider which platform libraries can extend
  */
 export class BasicTracerProvider implements TracerProvider {
-  protected static readonly _registeredPropagators = new Map<
-    string,
-    PROPAGATOR_FACTORY
-  >([
-    ['tracecontext', () => new W3CTraceContextPropagator()],
-    ['baggage', () => new W3CBaggagePropagator()],
-  ]);
-
   private readonly _config: TracerConfig;
   private readonly _tracers: Map<string, Tracer> = new Map();
   private readonly _resource: IResource;
@@ -117,16 +111,19 @@ export class BasicTracerProvider implements TracerProvider {
    */
   register(config: SDKRegistrationConfig = {}): void {
     trace.setGlobalTracerProvider(this);
-    if (config.propagator === undefined) {
-      config.propagator = this._buildPropagatorFromEnv();
-    }
 
     if (config.contextManager) {
       context.setGlobalContextManager(config.contextManager);
     }
 
-    if (config.propagator) {
-      propagation.setGlobalPropagator(config.propagator);
+    // undefined means "unset", null means don't register propagator
+    if (config.propagator !== null) {
+      propagation.setGlobalPropagator(
+        config.propagator ??
+          new CompositePropagator({
+            propagators: getDefaultPropagators(),
+          })
+      );
     }
   }
 
@@ -182,54 +179,4 @@ export class BasicTracerProvider implements TracerProvider {
   shutdown(): Promise<void> {
     return this._activeSpanProcessor.shutdown();
   }
-
-  /**
-   * TS cannot yet infer the type of this.constructor:
-   * https://github.com/Microsoft/TypeScript/issues/3841#issuecomment-337560146
-   * There is no need to override either of the getters in your child class.
-   * The type of the registered component maps should be the same across all
-   * classes in the inheritance tree.
-   */
-  protected _getPropagator(name: string): TextMapPropagator | undefined {
-    return (
-      this.constructor as typeof BasicTracerProvider
-    )._registeredPropagators.get(name)?.();
-  }
-
-  protected _buildPropagatorFromEnv(): TextMapPropagator | undefined {
-    // per spec, propagators from env must be deduplicated
-    const uniquePropagatorNames = Array.from(
-      new Set(getEnv().OTEL_PROPAGATORS)
-    );
-
-    const propagators = uniquePropagatorNames.map(name => {
-      const propagator = this._getPropagator(name);
-      if (!propagator) {
-        diag.warn(
-          `Propagator "${name}" requested through environment variable is unavailable.`
-        );
-      }
-
-      return propagator;
-    });
-    const validPropagators = propagators.reduce<TextMapPropagator[]>(
-      (list, item) => {
-        if (item) {
-          list.push(item);
-        }
-        return list;
-      },
-      []
-    );
-
-    if (validPropagators.length === 0) {
-      return;
-    } else if (uniquePropagatorNames.length === 1) {
-      return validPropagators[0];
-    } else {
-      return new CompositePropagator({
-        propagators: validPropagators,
-      });
-    }
-  }
 }