feat(basic-tracer): make scope mamnager required

packages/opentelemetry-basic-tracer/src/BasicTracer.ts

@@ -26,6 +26,7 @@ import {
   NEVER_SAMPLER,
 } from '@opentelemetry/core';
 import { BasicTracerConfig } from '../src/types';
+import { BinaryFormat, HttpTextFormat } from '@opentelemetry/types';
 
 /**
  * This class represents a basic tracer.
@@ -38,9 +39,7 @@ export class BasicTracer implements types.Tracer {
   private _binaryFormat: types.BinaryFormat;
   private _httpTextFormat: types.HttpTextFormat;
   private _sampler: types.Sampler;
-  // TODO: discuss default ScopeManager (undefined, NoopScopeManager etc.)
-  // TODO: discuss behaviour without scope manager / with NoopScopeManager: log, throw, swallow etc.
-  private _scopeManager?: ScopeManager;
+  private _scopeManager: ScopeManager;
 
   // TODO: consume invalid span context from `SpanContext.INVALID`
   static defaultSpan = new NoopSpan({
@@ -53,7 +52,7 @@ export class BasicTracer implements types.Tracer {
   /**
    * Constructs a new Tracer instance.
    */
-  constructor(config: BasicTracerConfig = {}) {
+  constructor(config: BasicTracerConfig) {
     this._binaryFormat = config.binaryFormat || new BinaryTraceContext();
     this._defaultAttributes = config.defaultAttributes || {};
     this._httpTextFormat = config.httpTextFormat || new HttpTraceContext();
@@ -108,14 +107,6 @@ export class BasicTracer implements types.Tracer {
    * Returns the current Span from the current context.
    */
   getCurrentSpan(): types.Span {
-    // Return with defaultSpan if no scope manager provided.
-    if (!this._scopeManager) {
-      this._logger.warn(
-        'getCurrentSpan() returns an invalid default span without a scopeManager'
-      );
-      return BasicTracer.defaultSpan;
-    }
-
     // Get the current Span from the context.
     return this._scopeManager.active() as types.Span;
   }
@@ -127,11 +118,6 @@ export class BasicTracer implements types.Tracer {
     span: types.Span,
     fn: T
   ): ReturnType<T> {
-    if (!this._scopeManager) {
-      this._logger.warn('withSpan(...) has no effect without a scopeManager');
-      return fn();
-    }
-
     // Set given span to context.
     return this._scopeManager.with(span, fn);
   }
@@ -146,14 +132,14 @@ export class BasicTracer implements types.Tracer {
   /**
    * Returns the binary format interface which can serialize/deserialize Spans.
    */
-  getBinaryFormat(): unknown {
+  getBinaryFormat(): BinaryFormat {
     return this._binaryFormat;
   }
 
   /**
    * Returns the HTTP text format interface which can inject/extract Spans.
    */
-  getHttpTextFormat(): unknown {
+  getHttpTextFormat(): HttpTextFormat {
     return this._httpTextFormat;
   }
 }

packages/opentelemetry-basic-tracer/src/types.ts

@@ -56,5 +56,5 @@ export interface BasicTracerConfig {
   /**
    * Scope manager keeps context across in-process operations.
    */
-  scopeManager?: ScopeManager;
+  scopeManager: ScopeManager;
 }

packages/opentelemetry-basic-tracer/test/BasicTracer.test.ts

@@ -23,40 +23,49 @@ import {
   NEVER_SAMPLER,
   NoopLogger,
 } from '@opentelemetry/core';
+import { NoopScopeManager } from '@opentelemetry/scope-base';
 
 describe('BasicTracer', () => {
   describe('constructor', () => {
-    it('should construct an instance without options', () => {
-      const tracer = new BasicTracer();
+    it('should construct an instance with required only options', () => {
+      const tracer = new BasicTracer({
+        scopeManager: new NoopScopeManager(),
+      });
       assert.ok(tracer instanceof BasicTracer);
     });
 
     it('should construct an instance with binary format', () => {
       const tracer = new BasicTracer({
         binaryFormat: new BinaryTraceContext(),
+        scopeManager: new NoopScopeManager(),
       });
       assert.ok(tracer instanceof BasicTracer);
     });
 
     it('should construct an instance with http text format', () => {
       const tracer = new BasicTracer({
         httpTextFormat: new HttpTraceContext(),
+        scopeManager: new NoopScopeManager(),
       });
       assert.ok(tracer instanceof BasicTracer);
     });
 
     it('should construct an instance with logger', () => {
-      const tracer = new BasicTracer({ logger: new NoopLogger() });
+      const tracer = new BasicTracer({
+        logger: new NoopLogger(),
+        scopeManager: new NoopScopeManager(),
+      });
       assert.ok(tracer instanceof BasicTracer);
     });
 
     it('should construct an instance with sampler', () => {
-      const tracer = new BasicTracer({ sampler: ALWAYS_SAMPLER });
+      const tracer = new BasicTracer({
+        scopeManager: new NoopScopeManager(),
+        sampler: ALWAYS_SAMPLER,
+      });
       assert.ok(tracer instanceof BasicTracer);
     });
 
-    it('should construct an instance with scope manager');
-
     it('should construct an instance with propagation');
 
     it('should construct an instance with default attributes', () => {
@@ -65,26 +74,34 @@ describe('BasicTracer', () => {
           region: 'eu-west',
           asg: 'my-asg',
         },
+        scopeManager: new NoopScopeManager(),
       });
       assert.ok(tracer instanceof BasicTracer);
     });
   });
 
   describe('#startSpan', () => {
     it('should start a span with name only', () => {
-      const tracer = new BasicTracer();
+      const tracer = new BasicTracer({
+        scopeManager: new NoopScopeManager(),
+      });
       const span = tracer.startSpan('my-span');
       assert.ok(span);
     });
 
     it('should start a span with name and options', () => {
-      const tracer = new BasicTracer();
+      const tracer = new BasicTracer({
+        scopeManager: new NoopScopeManager(),
+      });
       const span = tracer.startSpan('my-span', {});
       assert.ok(span);
     });
 
     it('should return a default span with no sampling', () => {
-      const tracer = new BasicTracer({ sampler: NEVER_SAMPLER });
+      const tracer = new BasicTracer({
+        sampler: NEVER_SAMPLER,
+        scopeManager: new NoopScopeManager(),
+      });
       const span = tracer.startSpan('my-span');
       assert.deepStrictEqual(span, BasicTracer.defaultSpan);
     });
@@ -95,25 +112,25 @@ describe('BasicTracer', () => {
   });
 
   describe('#getCurrentSpan', () => {
-    it('should return default span without scope manager', () => {
-      const tracer = new BasicTracer();
+    it('should return null with NoopScopeManager', () => {
+      const tracer = new BasicTracer({
+        scopeManager: new NoopScopeManager(),
+      });
       const currentSpan = tracer.getCurrentSpan();
-      assert.deepStrictEqual(currentSpan, BasicTracer.defaultSpan);
+      assert.deepStrictEqual(currentSpan, null);
     });
-
-    it('should return a span with scope manager');
   });
 
   describe('#withSpan', () => {
-    it('should run scope without scope manager', done => {
-      const tracer = new BasicTracer();
+    it('should run scope with NoopScopeManager scope manager', done => {
+      const tracer = new BasicTracer({
+        scopeManager: new NoopScopeManager(),
+      });
       const span = tracer.startSpan('my-span');
       tracer.withSpan(span, () => {
         return done();
       });
     });
-
-    it('should run and set scope with scope manager');
   });
 
   describe('#recordSpanData', () => {
@@ -122,14 +139,18 @@ describe('BasicTracer', () => {
 
   describe('#getBinaryFormat', () => {
     it('should get default binary formatter', () => {
-      const tracer = new BasicTracer();
+      const tracer = new BasicTracer({
+        scopeManager: new NoopScopeManager(),
+      });
       assert.ok(tracer.getBinaryFormat() instanceof BinaryTraceContext);
     });
   });
 
   describe('#getHttpTextFormat', () => {
     it('should get default HTTP text formatter', () => {
-      const tracer = new BasicTracer();
+      const tracer = new BasicTracer({
+        scopeManager: new NoopScopeManager(),
+      });
       assert.ok(tracer.getHttpTextFormat() instanceof HttpTraceContext);
     });
   });

packages/opentelemetry-core/src/index.ts

@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
+export * from './common/NoopLogger';
 export * from './context/propagation/BinaryTraceContext';
 export * from './context/propagation/HttpTraceContext';
-export * from './common/NoopLogger';
 export * from './platform';
 export * from './resources/Resource';
 export * from './trace/NoopSpan';