Merge branch 'next' into next-update-typescript

open-telemetry · Nov 25, 2024 · c46c11b · c46c11b
2 parents 703f0ab + c28abac
commit c46c11b
Show file tree

Hide file tree

Showing 12 changed files with 816 additions and 530 deletions.
diff --git a/CHANGELOG_NEXT.md b/CHANGELOG_NEXT.md
@@ -15,6 +15,7 @@
 * feat(sdk-metrics)!: drop `View` and `Aggregation` in favor of `ViewOptions` and `AggregationOption` [#4931](https://github.com/open-telemetry/opentelemetry-js/pull/4931) @pichlermarc
 * refactor(sdk-trace-base)!: remove `new Span` constructor in favor of `Tracer.startSpan` API [#5048](https://github.com/open-telemetry/opentelemetry-js/pull/5048) @david-luna
 * refactor(sdk-trace-base)!: remove `BasicTracerProvider.addSpanProcessor` API in favor of constructor options. [#5134](https://github.com/open-telemetry/opentelemetry-js/pull/5134) @david-luna
+* refactor(sdk-trace-base)!: make `resource` property private in `BasicTracerProvider` and remove `getActiveSpanProcessor` API. [#5192](https://github.com/open-telemetry/opentelemetry-js/pull/5192) @david-luna
 
 ### :rocket: (Enhancement)
 
@@ -23,6 +24,7 @@
 ### :house: (Internal)
 
 * chore: remove checks for unsupported node versions [#4341](https://github.com/open-telemetry/opentelemetry-js/pull/4341) @dyladan
+* refactor(sdk-trace-base): remove `BasicTracerProvider._registeredSpanProcessors` private property. [#5134](https://github.com/open-telemetry/opentelemetry-js/pull/5177) @david-luna
 * chore: update typescript to version `^5.6.3` [#5145](https://github.com/open-telemetry/opentelemetry-js/pull/5145) @david-luna
 
 ### :bug: (Bug Fix)
diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
@@ -50,6 +50,7 @@ import {
   NoopSpanProcessor,
   IdGenerator,
   AlwaysOffSampler,
+  SpanProcessor,
 } from '@opentelemetry/sdk-trace-base';
 import * as assert from 'assert';
 import * as semver from 'semver';
@@ -257,13 +258,25 @@ describe('Node SDK', () => {
       );
       const apiTracerProvider =
         trace.getTracerProvider() as ProxyTracerProvider;
-      assert.ok(apiTracerProvider.getDelegate() instanceof NodeTracerProvider);
+      const nodeTracerProvider = apiTracerProvider.getDelegate();
+
+      assert.ok(nodeTracerProvider instanceof NodeTracerProvider);
+
+      const spanProcessor = nodeTracerProvider['activeSpanProcessor'] as any;
+
+      assert(
+        spanProcessor.constructor.name === 'MultiSpanProcessor',
+        'is MultiSpanProcessor'
+      );
 
-      const listOfProcessors =
-        sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+      const listOfProcessors = spanProcessor[
+        '_spanProcessors'
+      ] as SpanProcessor[];
 
-      assert(sdk['_tracerProvider'] instanceof NodeTracerProvider);
-      assert(listOfProcessors.length === 3);
+      assert(
+        listOfProcessors.length === 3,
+        'it has the right amount of processors'
+      );
       assert(listOfProcessors[0] instanceof NoopSpanProcessor);
       assert(listOfProcessors[1] instanceof SimpleSpanProcessor);
       assert(listOfProcessors[2] instanceof BatchSpanProcessor);
@@ -1099,6 +1112,18 @@ describe('Node SDK', () => {
 describe('setup exporter from env', () => {
   let stubLoggerError: Sinon.SinonStub;
 
+  const getSdkSpanProcessors = (sdk: NodeSDK) => {
+    const tracerProvider = sdk['_tracerProvider'];
+
+    assert(tracerProvider instanceof NodeTracerProvider);
+
+    const activeSpanProcessor = tracerProvider['activeSpanProcessor'];
+
+    assert(activeSpanProcessor.constructor.name === 'MultiSpanProcessor');
+
+    return (activeSpanProcessor as any)['_spanProcessors'] as SpanProcessor[];
+  };
+
   beforeEach(() => {
     stubLoggerError = Sinon.stub(diag, 'warn');
   });
@@ -1109,8 +1134,7 @@ describe('setup exporter from env', () => {
   it('should use default exporter when nor env neither SDK config is given', async () => {
     const sdk = new NodeSDK();
     sdk.start();
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 1);
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1124,8 +1148,7 @@ describe('setup exporter from env', () => {
       traceExporter,
     });
     sdk.start();
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 1);
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1140,8 +1163,7 @@ describe('setup exporter from env', () => {
       spanProcessor,
     });
     sdk.start();
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 1);
     assert(listOfProcessors[0] instanceof SimpleSpanProcessor);
@@ -1156,8 +1178,7 @@ describe('setup exporter from env', () => {
       traceExporter,
     });
     sdk.start();
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 1);
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1172,8 +1193,7 @@ describe('setup exporter from env', () => {
       sampler: new AlwaysOffSampler(),
     });
     sdk.start();
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert.ok(
       sdk['_tracerProvider']!['_config']?.sampler instanceof AlwaysOffSampler
@@ -1190,9 +1210,7 @@ describe('setup exporter from env', () => {
     env.OTEL_EXPORTER_OTLP_TRACES_PROTOCOL = 'grpc';
     const sdk = new NodeSDK();
     sdk.start();
-
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 1);
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1208,9 +1226,7 @@ describe('setup exporter from env', () => {
     env.OTEL_EXPORTER_OTLP_TRACES_PROTOCOL = 'grpc';
     const sdk = new NodeSDK();
     sdk.start();
-
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 1);
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1231,12 +1247,10 @@ describe('setup exporter from env', () => {
       'OTEL_TRACES_EXPORTER contains "none". SDK will not be initialized.'
     );
 
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
-    const activeProcessor = sdk['_tracerProvider']?.getActiveSpanProcessor();
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
-    assert(listOfProcessors.length === 0);
-    assert(activeProcessor instanceof NoopSpanProcessor);
+    assert(listOfProcessors.length === 1);
+    assert(listOfProcessors[0] instanceof NoopSpanProcessor);
     delete env.OTEL_TRACES_EXPORTER;
     await sdk.shutdown();
   });
@@ -1246,8 +1260,7 @@ describe('setup exporter from env', () => {
     const sdk = new NodeSDK();
     sdk.start();
 
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 1);
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1267,8 +1280,7 @@ describe('setup exporter from env', () => {
       'OTEL_TRACES_EXPORTER contains "none" along with other exporters. Using default otlp exporter.'
     );
 
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 1);
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1303,8 +1315,7 @@ describe('setup exporter from env', () => {
     const sdk = new NodeSDK();
     sdk.start();
 
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 1);
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1321,8 +1332,7 @@ describe('setup exporter from env', () => {
     const sdk = new NodeSDK();
     sdk.start();
 
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 2);
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1341,8 +1351,7 @@ describe('setup exporter from env', () => {
     const sdk = new NodeSDK();
     sdk.start();
 
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 1);
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1359,8 +1368,7 @@ describe('setup exporter from env', () => {
     const sdk = new NodeSDK();
     sdk.start();
 
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 2);
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1379,8 +1387,7 @@ describe('setup exporter from env', () => {
     const sdk = new NodeSDK();
     sdk.start();
 
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 3);
     assert(listOfProcessors[0] instanceof BatchSpanProcessor);
@@ -1400,8 +1407,7 @@ describe('setup exporter from env', () => {
     const sdk = new NodeSDK();
     sdk.start();
 
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 2);
     assert(listOfProcessors[0] instanceof SimpleSpanProcessor);
@@ -1417,8 +1423,7 @@ describe('setup exporter from env', () => {
     const sdk = new NodeSDK();
     sdk.start();
 
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 1);
     assert(listOfProcessors[0] instanceof SimpleSpanProcessor);
@@ -1433,8 +1438,7 @@ describe('setup exporter from env', () => {
     const sdk = new NodeSDK();
     sdk.start();
 
-    const listOfProcessors =
-      sdk['_tracerProvider']!['_registeredSpanProcessors']!;
+    const listOfProcessors = getSdkSpanProcessors(sdk);
 
     assert(listOfProcessors.length === 1);
     assert(listOfProcessors[0] instanceof SimpleSpanProcessor);

diff --git a/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts b/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts
@@ -67,42 +67,41 @@ export class BasicTracerProvider implements TracerProvider {
   >();
 
   private readonly _config: TracerConfig;
-  private readonly _registeredSpanProcessors: SpanProcessor[] = [];
   private readonly _tracers: Map<string, Tracer> = new Map();
+  private readonly _resource: IResource;
 
-  private activeSpanProcessor: SpanProcessor;
-  readonly resource: IResource;
+  private activeSpanProcessor: MultiSpanProcessor;
 
   constructor(config: TracerConfig = {}) {
     const mergedConfig = merge(
       {},
       loadDefaultConfig(),
       reconfigureLimits(config)
     );
-    this.resource = mergedConfig.resource ?? Resource.empty();
+    this._resource = mergedConfig.resource ?? Resource.empty();
 
     if (mergedConfig.mergeResourceWithDefaults) {
-      this.resource = Resource.default().merge(this.resource);
+      this._resource = Resource.default().merge(this._resource);
     }
 
     this._config = Object.assign({}, mergedConfig, {
-      resource: this.resource,
+      resource: this._resource,
     });
 
+    const spanProcessors: SpanProcessor[] = [];
+
     if (config.spanProcessors?.length) {
-      this._registeredSpanProcessors = [...config.spanProcessors];
-      this.activeSpanProcessor = new MultiSpanProcessor(
-        this._registeredSpanProcessors
-      );
+      spanProcessors.push(...config.spanProcessors);
     } else {
       const defaultExporter = this._buildExporterFromEnv();
-      if (defaultExporter !== undefined) {
-        const batchProcessor = new BatchSpanProcessor(defaultExporter);
-        this.activeSpanProcessor = batchProcessor;
-      } else {
-        this.activeSpanProcessor = new NoopSpanProcessor();
-      }
+      spanProcessors.push(
+        defaultExporter
+          ? new BatchSpanProcessor(defaultExporter)
+          : new NoopSpanProcessor()
+      );
     }
+
+    this.activeSpanProcessor = new MultiSpanProcessor(spanProcessors);
   }
 
   getTracer(
@@ -117,7 +116,8 @@ export class BasicTracerProvider implements TracerProvider {
         new Tracer(
           { name, version, schemaUrl: options?.schemaUrl },
           this._config,
-          this
+          this._resource,
+          this.activeSpanProcessor
         )
       );
     }
@@ -126,10 +126,6 @@ export class BasicTracerProvider implements TracerProvider {
     return this._tracers.get(key)!;
   }
 
-  getActiveSpanProcessor(): SpanProcessor {
-    return this.activeSpanProcessor;
-  }
-
   /**
    * Register this TracerProvider for use with the OpenTelemetry API.
    * Undefined values may be replaced with defaults, and
@@ -154,7 +150,7 @@ export class BasicTracerProvider implements TracerProvider {
 
   forceFlush(): Promise<void> {
     const timeout = this._config.forceFlushTimeoutMillis;
-    const promises = this._registeredSpanProcessors.map(
+    const promises = this.activeSpanProcessor['_spanProcessors'].map(
       (spanProcessor: SpanProcessor) => {
         return new Promise(resolve => {
           let state: ForceFlushState;