refactor(sdk-trace-base): make resource private and remove getActiveSpanProcessor API (#5192)

Author: david-luna

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)
 

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

@@ -262,7 +262,7 @@ describe('Node SDK', () => {
 
       assert.ok(nodeTracerProvider instanceof NodeTracerProvider);
 
-      const spanProcessor = nodeTracerProvider.getActiveSpanProcessor() as any;
+      const spanProcessor = nodeTracerProvider['activeSpanProcessor'] as any;
 
       assert(
         spanProcessor.constructor.name === 'MultiSpanProcessor',
@@ -1117,7 +1117,7 @@ describe('setup exporter from env', () => {
 
     assert(tracerProvider instanceof NodeTracerProvider);
 
-    const activeSpanProcessor = tracerProvider.getActiveSpanProcessor();
+    const activeSpanProcessor = tracerProvider['activeSpanProcessor'];
 
     assert(activeSpanProcessor.constructor.name === 'MultiSpanProcessor');
 

packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts

@@ -68,24 +68,24 @@ export class BasicTracerProvider implements TracerProvider {
 
   private readonly _config: TracerConfig;
   private readonly _tracers: Map<string, Tracer> = new Map();
+  private readonly _resource: IResource;
 
   private activeSpanProcessor: MultiSpanProcessor;
-  readonly resource: IResource;
 
   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[] = [];
@@ -116,7 +116,8 @@ export class BasicTracerProvider implements TracerProvider {
         new Tracer(
           { name, version, schemaUrl: options?.schemaUrl },
           this._config,
-          this
+          this._resource,
+          this.activeSpanProcessor
         )
       );
     }
@@ -125,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

packages/opentelemetry-sdk-trace-base/src/Span.ts

@@ -52,7 +52,6 @@ import { ReadableSpan } from './export/ReadableSpan';
 import { ExceptionEventName } from './enums';
 import { SpanProcessor } from './SpanProcessor';
 import { TimedEvent } from './TimedEvent';
-import { Tracer } from './Tracer';
 import { SpanLimits } from './types';
 
 /**
@@ -61,6 +60,21 @@ import { SpanLimits } from './types';
  */
 export type Span = APISpan & ReadableSpan;
 
+interface SpanOptions {
+  resource: IResource;
+  scope: InstrumentationLibrary;
+  context: Context;
+  spanContext: SpanContext;
+  name: string;
+  kind: SpanKind;
+  parentSpanId?: string;
+  links?: Link[];
+  startTime?: TimeInput;
+  attributes?: Attributes;
+  spanLimits: SpanLimits;
+  spanProcessor: SpanProcessor;
+}
+
 /**
  * This class represents a span.
  */
@@ -99,44 +113,32 @@ export class SpanImpl implements Span {
   /**
    * Constructs a new SpanImpl instance.
    */
-  constructor(
-    parentTracer: Tracer,
-    context: Context,
-    spanName: string,
-    spanContext: SpanContext,
-    kind: SpanKind,
-    parentSpanId?: string,
-    links: Link[] = [],
-    startTime?: TimeInput,
-    _deprecatedClock?: unknown, // keeping this argument even though it is unused to ensure backwards compatibility
-    attributes?: Attributes
-  ) {
-    this.name = spanName;
-    this._spanContext = spanContext;
-    this.parentSpanId = parentSpanId;
-    this.kind = kind;
-    this.links = links;
-
+  constructor(opts: SpanOptions) {
     const now = Date.now();
+
+    this._spanContext = opts.spanContext;
     this._performanceStartTime = otperformance.now();
     this._performanceOffset =
       now - (this._performanceStartTime + getTimeOrigin());
-    this._startTimeProvided = startTime != null;
-
-    this.startTime = this._getTime(startTime ?? now);
-
-    this.resource = parentTracer.resource;
-    this.instrumentationLibrary = parentTracer.instrumentationLibrary;
-    this._spanLimits = parentTracer.getSpanLimits();
+    this._startTimeProvided = opts.startTime != null;
+    this._spanLimits = opts.spanLimits;
     this._attributeValueLengthLimit =
       this._spanLimits.attributeValueLengthLimit || 0;
-
-    if (attributes != null) {
-      this.setAttributes(attributes);
+    this._spanProcessor = opts.spanProcessor;
+
+    this.name = opts.name;
+    this.parentSpanId = opts.parentSpanId;
+    this.kind = opts.kind;
+    this.links = opts.links || [];
+    this.startTime = this._getTime(opts.startTime ?? now);
+    this.resource = opts.resource;
+    this.instrumentationLibrary = opts.scope;
+
+    if (opts.attributes != null) {
+      this.setAttributes(opts.attributes);
     }
 
-    this._spanProcessor = parentTracer.getActiveSpanProcessor();
-    this._spanProcessor.onStart(this, context);
+    this._spanProcessor.onStart(this, opts.context);
   }
 
   spanContext(): SpanContext {

packages/opentelemetry-sdk-trace-base/src/Tracer.ts

@@ -20,15 +20,14 @@ import {
   sanitizeAttributes,
   isTracingSuppressed,
 } from '@opentelemetry/core';
-import { IResource } from '@opentelemetry/resources';
-import { BasicTracerProvider } from './BasicTracerProvider';
 import { SpanImpl } from './Span';
 import { GeneralLimits, SpanLimits, TracerConfig } from './types';
 import { mergeConfig } from './utility';
 import { SpanProcessor } from './SpanProcessor';
 import { Sampler } from './Sampler';
 import { IdGenerator } from './IdGenerator';
 import { RandomIdGenerator } from './platform';
+import { IResource } from '@opentelemetry/resources';
 
 /**
  * This class represents a basic tracer.
@@ -38,23 +37,27 @@ export class Tracer implements api.Tracer {
   private readonly _generalLimits: GeneralLimits;
   private readonly _spanLimits: SpanLimits;
   private readonly _idGenerator: IdGenerator;
-  readonly resource: IResource;
   readonly instrumentationLibrary: InstrumentationLibrary;
 
+  private readonly _resource: IResource;
+  private readonly _spanProcessor: SpanProcessor;
+
   /**
    * Constructs a new Tracer instance.
    */
   constructor(
     instrumentationLibrary: InstrumentationLibrary,
     config: TracerConfig,
-    private _tracerProvider: BasicTracerProvider
+    resource: IResource,
+    spanProcessor: SpanProcessor
   ) {
     const localConfig = mergeConfig(config);
     this._sampler = localConfig.sampler;
     this._generalLimits = localConfig.generalLimits;
     this._spanLimits = localConfig.spanLimits;
     this._idGenerator = config.idGenerator || new RandomIdGenerator();
-    this.resource = _tracerProvider.resource;
+    this._resource = resource;
+    this._spanProcessor = spanProcessor;
     this.instrumentationLibrary = instrumentationLibrary;
   }
 
@@ -138,18 +141,20 @@ export class Tracer implements api.Tracer {
       Object.assign(attributes, samplingResult.attributes)
     );
 
-    const span = new SpanImpl(
-      this,
+    const span = new SpanImpl({
+      resource: this._resource,
+      scope: this.instrumentationLibrary,
       context,
-      name,
       spanContext,
-      spanKind,
-      parentSpanId,
+      name,
+      kind: spanKind,
       links,
-      options.startTime,
-      undefined,
-      initAttributes
-    );
+      parentSpanId,
+      attributes: initAttributes,
+      startTime: options.startTime,
+      spanProcessor: this._spanProcessor,
+      spanLimits: this._spanLimits,
+    });
     return span;
   }
 
@@ -250,8 +255,4 @@ export class Tracer implements api.Tracer {
   getSpanLimits(): SpanLimits {
     return this._spanLimits;
   }
-
-  getActiveSpanProcessor(): SpanProcessor {
-    return this._tracerProvider.getActiveSpanProcessor();
-  }
 }

packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts

@@ -665,8 +665,8 @@ describe('BasicTracerProvider', () => {
       const tracerProvider = new BasicTracerProvider();
       const tracer = tracerProvider.getTracer('default');
       const span = tracer.startSpan('my-span') as Span;
-      assert.strictEqual(tracer.resource, tracerProvider.resource);
-      assert.strictEqual(span.resource, tracerProvider.resource);
+      assert.strictEqual(tracer['_resource'], tracerProvider['_resource']);
+      assert.strictEqual(span.resource, tracerProvider['_resource']);
     });
 
     it('should start a span with name and options', () => {
@@ -929,7 +929,7 @@ describe('BasicTracerProvider', () => {
   describe('.resource', () => {
     it('should use the default resource when no resource is provided', function () {
       const tracerProvider = new BasicTracerProvider();
-      assert.deepStrictEqual(tracerProvider.resource, Resource.default());
+      assert.deepStrictEqual(tracerProvider['_resource'], Resource.default());
     });
 
     it('should not merge with defaults when flag is set to false', function () {
@@ -938,7 +938,7 @@ describe('BasicTracerProvider', () => {
         mergeResourceWithDefaults: false,
         resource: expectedResource,
       });
-      assert.deepStrictEqual(tracerProvider.resource, expectedResource);
+      assert.deepStrictEqual(tracerProvider['_resource'], expectedResource);
     });
 
     it('should merge with defaults when flag is set to true', function () {
@@ -948,7 +948,7 @@ describe('BasicTracerProvider', () => {
         resource: providedResource,
       });
       assert.deepStrictEqual(
-        tracerProvider.resource,
+        tracerProvider['_resource'],
         Resource.default().merge(providedResource)
       );
     });
@@ -958,7 +958,7 @@ describe('BasicTracerProvider', () => {
     it('should trigger shutdown when manually invoked', () => {
       const tracerProvider = new BasicTracerProvider();
       const shutdownStub = sinon.stub(
-        tracerProvider.getActiveSpanProcessor(),
+        tracerProvider['activeSpanProcessor'],
         'shutdown'
       );
       tracerProvider.shutdown();

packages/opentelemetry-sdk-trace-base/test/common/MultiSpanProcessor.test.ts

@@ -63,7 +63,7 @@ describe('MultiSpanProcessor', () => {
     assert.strictEqual(processor1.spans.length, 0);
     span.end();
     assert.strictEqual(processor1.spans.length, 1);
-    tracerProvider.getActiveSpanProcessor().shutdown();
+    tracerProvider['activeSpanProcessor'].shutdown();
   });
 
   it('should handle two span processor', async () => {
@@ -81,9 +81,6 @@ describe('MultiSpanProcessor', () => {
     assert.strictEqual(processor1.spans.length, 1);
     assert.strictEqual(processor1.spans.length, processor2.spans.length);
 
-    // await tracerProvider.getActiveSpanProcessor().shutdown();
-    // assert.strictEqual(processor1.spans.length, 0);
-    // assert.strictEqual(processor1.spans.length, processor2.spans.length);
     tracerProvider.shutdown().then(() => {
       assert.strictEqual(processor1.spans.length, 0);
       assert.strictEqual(processor1.spans.length, processor2.spans.length);