chore: allow parent span to be null (#569)

* chore: allow parent span to be null * chore: revert nullish operator * chore: return undefined for missing span * test: undefined spans * test: undefined spans
open-telemetry · Nov 27, 2019 · b3332e3 · b3332e3
1 parent 5a52eda
commit b3332e3
Show file tree

Hide file tree

Showing 12 changed files with 24 additions and 29 deletions.
diff --git a/packages/opentelemetry-core/src/trace/TracerDelegate.ts b/packages/opentelemetry-core/src/trace/TracerDelegate.ts
@@ -48,7 +48,7 @@ export class TracerDelegate implements types.Tracer {
 
   // -- Tracer interface implementation below -- //
 
-  getCurrentSpan(): types.Span | null {
+  getCurrentSpan(): types.Span | undefined {
     return this._currentTracer.getCurrentSpan.apply(
       this._currentTracer,
       // tslint:disable-next-line:no-any

diff --git a/packages/opentelemetry-node/test/NodeTracer.test.ts b/packages/opentelemetry-node/test/NodeTracer.test.ts
@@ -167,9 +167,9 @@ describe('NodeTracer', () => {
   });
 
   describe('.getCurrentSpan()', () => {
-    it('should return null with AsyncHooksScopeManager when no span started', () => {
+    it('should return undefined with AsyncHooksScopeManager when no span started', () => {
       tracer = new NodeTracer({});
-      assert.deepStrictEqual(tracer.getCurrentSpan(), null);
+      assert.deepStrictEqual(tracer.getCurrentSpan(), undefined);
     });
   });
 
@@ -181,7 +181,8 @@ describe('NodeTracer', () => {
         assert.deepStrictEqual(tracer.getCurrentSpan(), span);
         return done();
       });
-      assert.deepStrictEqual(tracer.getCurrentSpan(), null);
+      // @todo: below check is not running.a
+      assert.deepStrictEqual(tracer.getCurrentSpan(), undefined);
     });
 
     it('should run scope with AsyncHooksScopeManager scope manager with multiple spans', done => {
@@ -202,7 +203,7 @@ describe('NodeTracer', () => {
       });
       // when span ended.
       // @todo: below check is not running.
-      assert.deepStrictEqual(tracer.getCurrentSpan(), null);
+      assert.deepStrictEqual(tracer.getCurrentSpan(), undefined);
     });
 
     it('should find correct scope with promises', done => {
@@ -216,7 +217,7 @@ describe('NodeTracer', () => {
         }
         return done();
       });
-      assert.deepStrictEqual(tracer.getCurrentSpan(), null);
+      assert.deepStrictEqual(tracer.getCurrentSpan(), undefined);
     });
   });
 

diff --git a/packages/opentelemetry-plugin-dns/src/dns.ts b/packages/opentelemetry-plugin-dns/src/dns.ts
@@ -116,7 +116,7 @@ export class DnsPlugin extends BasePlugin<Dns> {
       plugin._logger.debug('wrap lookup callback function and starts span');
       const name = utils.getOperationName('lookup');
       const span = plugin._startDnsSpan(name, {
-        parent: plugin._tracer.getCurrentSpan() || undefined,
+        parent: plugin._tracer.getCurrentSpan(),
         attributes: {
           [AttributeNames.PEER_HOSTNAME]: hostname,
         },

diff --git a/packages/opentelemetry-plugin-document-load/src/documentLoad.ts b/packages/opentelemetry-plugin-document-load/src/documentLoad.ts
@@ -121,16 +121,14 @@ export class DocumentLoad extends BasePlugin<unknown> {
     const metaElement = [...document.getElementsByTagName('meta')].find(
       e => e.getAttribute('name') === TRACE_PARENT_HEADER
     );
-    const serverContext =
-      parseTraceParent((metaElement && metaElement.content) || '') || undefined;
 
     const entries = this._getEntries();
 
     const rootSpan = this._startSpan(
       AttributeNames.DOCUMENT_LOAD,
       PTN.FETCH_START,
       entries,
-      { parent: serverContext }
+      { parent: parseTraceParent((metaElement && metaElement.content) || '') }
     );
     if (!rootSpan) {
       return;

diff --git a/packages/opentelemetry-plugin-grpc/src/grpc.ts b/packages/opentelemetry-plugin-grpc/src/grpc.ts
@@ -172,10 +172,9 @@ export class GrpcPlugin extends BasePlugin<grpc> {
               const self = this;
 
               const spanName = `grpc.${name.replace('/', '')}`;
-              const parentSpan = plugin._getSpanContext(call.metadata);
               const spanOptions: SpanOptions = {
                 kind: SpanKind.SERVER,
-                parent: parentSpan || undefined,
+                parent: plugin._getSpanContext(call.metadata),
               };
 
               plugin._logger.debug(
@@ -347,11 +346,10 @@ export class GrpcPlugin extends BasePlugin<grpc> {
       return function clientMethodTrace(this: grpcTypes.Client) {
         const name = `grpc.${original.path.replace('/', '')}`;
         const args = Array.prototype.slice.call(arguments);
-        const currentSpan = plugin._tracer.getCurrentSpan();
         const span = plugin._tracer
           .startSpan(name, {
             kind: SpanKind.CLIENT,
-            parent: currentSpan || undefined,
+            parent: plugin._tracer.getCurrentSpan(),
           })
           .setAttribute(AttributeNames.COMPONENT, GrpcPlugin.component);
         return plugin._makeGrpcClientRemoteCall(

diff --git a/packages/opentelemetry-plugin-postgres/opentelemetry-plugin-pg/src/utils.ts b/packages/opentelemetry-plugin-postgres/opentelemetry-plugin-pg/src/utils.ts
@@ -52,7 +52,7 @@ function pgStartSpan(
   const jdbcString = getJDBCString(client.connectionParameters);
   return tracer.startSpan(name, {
     kind: SpanKind.CLIENT,
-    parent: tracer.getCurrentSpan() || undefined,
+    parent: tracer.getCurrentSpan(),
     attributes: {
       [AttributeNames.COMPONENT]: PostgresPlugin.COMPONENT, // required
       [AttributeNames.DB_INSTANCE]: client.connectionParameters.database, // required

diff --git a/packages/opentelemetry-plugin-postgres/opentelemetry-plugin-pg/test/pg.test.ts b/packages/opentelemetry-plugin-postgres/opentelemetry-plugin-pg/test/pg.test.ts
@@ -396,7 +396,7 @@ describe('[email protected]', () => {
 
     it('should preserve correct context even when using the same callback in client.query()', done => {
       const spans = [tracer.startSpan('span 1'), tracer.startSpan('span 2')];
-      const currentSpans: (Span | null)[] = [];
+      const currentSpans: (Span | undefined)[] = [];
       const queryHandler = () => {
         currentSpans.push(tracer.getCurrentSpan());
         if (currentSpans.length === 2) {
@@ -415,7 +415,7 @@ describe('[email protected]', () => {
 
     it('should preserve correct context even when using the same promise resolver in client.query()', done => {
       const spans = [tracer.startSpan('span 1'), tracer.startSpan('span 2')];
-      const currentSpans: (Span | null)[] = [];
+      const currentSpans: (Span | undefined)[] = [];
       const queryHandler = () => {
         currentSpans.push(tracer.getCurrentSpan());
         if (currentSpans.length === 2) {

diff --git a/packages/opentelemetry-plugin-redis/src/utils.ts b/packages/opentelemetry-plugin-redis/src/utils.ts
@@ -72,14 +72,12 @@ export const getTracedInternalSendCommand = (
     this: redisTypes.RedisClient & RedisPluginClientTypes,
     cmd?: RedisCommand
   ) {
-    const parentSpan = tracer.getCurrentSpan();
-
     // New versions of redis (2.4+) use a single options object
     // instead of named arguments
     if (arguments.length === 1 && typeof cmd === 'object') {
       const span = tracer.startSpan(`${RedisPlugin.COMPONENT}-${cmd.command}`, {
         kind: SpanKind.CLIENT,
-        parent: parentSpan || undefined,
+        parent: tracer.getCurrentSpan(),
         attributes: {
           [AttributeNames.COMPONENT]: RedisPlugin.COMPONENT,
           [AttributeNames.DB_STATEMENT]: cmd.command,

diff --git a/packages/opentelemetry-tracing/src/BasicTracer.ts b/packages/opentelemetry-tracing/src/BasicTracer.ts
@@ -115,11 +115,11 @@ export class BasicTracer implements types.Tracer {
    *
    * If there is no Span associated with the current context, null is returned.
    */
-  getCurrentSpan(): types.Span | null {
+  getCurrentSpan(): types.Span | undefined {
     // Get the current Span from the context or null if none found.
     const current = this._scopeManager.active();
     if (current === null || current === undefined) {
-      return null;
+      return;
     } else {
       return current as types.Span;
     }
@@ -174,7 +174,7 @@ export class BasicTracer implements types.Tracer {
   }
 
   private _getParentSpanContext(
-    parent: types.Span | types.SpanContext | undefined
+    parent?: types.Span | types.SpanContext | null
   ): types.SpanContext | undefined {
     if (!parent) return undefined;
 

diff --git a/packages/opentelemetry-tracing/test/BasicTracer.test.ts b/packages/opentelemetry-tracing/test/BasicTracer.test.ts
@@ -306,10 +306,10 @@ describe('BasicTracer', () => {
   });
 
   describe('.getCurrentSpan()', () => {
-    it('should return null with NoopScopeManager', () => {
+    it('should return undefined with NoopScopeManager', () => {
       const tracer = new BasicTracer();
       const currentSpan = tracer.getCurrentSpan();
-      assert.deepStrictEqual(currentSpan, null);
+      assert.deepStrictEqual(currentSpan, undefined);
     });
 
     it('should return current span when it exists', () => {
@@ -327,7 +327,7 @@ describe('BasicTracer', () => {
       const tracer = new BasicTracer();
       const span = tracer.startSpan('my-span');
       tracer.withSpan(span, () => {
-        assert.deepStrictEqual(tracer.getCurrentSpan(), null);
+        assert.deepStrictEqual(tracer.getCurrentSpan(), undefined);
         return done();
       });
     });
@@ -338,7 +338,7 @@ describe('BasicTracer', () => {
       const tracer = new BasicTracer();
       const span = tracer.startSpan('my-span');
       const fn = () => {
-        assert.deepStrictEqual(tracer.getCurrentSpan(), null);
+        assert.deepStrictEqual(tracer.getCurrentSpan(), undefined);
         return done();
       };
       const patchedFn = tracer.bind(fn, span);

diff --git a/packages/opentelemetry-types/src/trace/SpanOptions.ts b/packages/opentelemetry-types/src/trace/SpanOptions.ts
@@ -46,7 +46,7 @@ export interface SpanOptions {
    * A parent `SpanContext` (or `Span`, for convenience) that the newly-started
    * span will be the child of.
    */
-  parent?: Span | SpanContext;
+  parent?: Span | SpanContext | null;
 
   /** A manually specified start time for the created `Span` object. */
   startTime?: number;

diff --git a/packages/opentelemetry-types/src/trace/tracer.ts b/packages/opentelemetry-types/src/trace/tracer.ts
@@ -34,7 +34,7 @@ export interface Tracer {
    *
    * @returns Span The currently active Span
    */
-  getCurrentSpan(): Span | null;
+  getCurrentSpan(): Span | undefined;
 
   /**
    * Starts a new {@link Span}.