From ffa00448668af47bf16ba48e080824f8b38119d4 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Mon, 25 Nov 2019 13:12:02 -0500 Subject: [PATCH 1/5] chore: allow parent span to be null --- packages/opentelemetry-plugin-dns/src/dns.ts | 2 +- .../opentelemetry-plugin-document-load/src/documentLoad.ts | 4 +--- packages/opentelemetry-plugin-grpc/src/grpc.ts | 6 ++---- .../opentelemetry-plugin-pg/src/utils.ts | 2 +- packages/opentelemetry-plugin-redis/src/utils.ts | 4 +--- packages/opentelemetry-tracing/src/BasicTracer.ts | 2 +- packages/opentelemetry-types/src/trace/SpanOptions.ts | 2 +- 7 files changed, 8 insertions(+), 14 deletions(-) diff --git a/packages/opentelemetry-plugin-dns/src/dns.ts b/packages/opentelemetry-plugin-dns/src/dns.ts index 2046ecb320..11eac18399 100644 --- 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 { 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 index 8e2e3da7d8..78900de3e4 100644 --- a/packages/opentelemetry-plugin-document-load/src/documentLoad.ts +++ b/packages/opentelemetry-plugin-document-load/src/documentLoad.ts @@ -121,8 +121,6 @@ export class DocumentLoad extends BasePlugin { const metaElement = [...document.getElementsByTagName('meta')].find( e => e.getAttribute('name') === TRACE_PARENT_HEADER ); - const serverContext = - parseTraceParent((metaElement && metaElement.content) || '') || undefined; const entries = this._getEntries(); @@ -130,7 +128,7 @@ export class DocumentLoad extends BasePlugin { AttributeNames.DOCUMENT_LOAD, PTN.FETCH_START, entries, - { parent: serverContext } + { parent: parseTraceParent(metaElement?.content ?? '') } ); if (!rootSpan) { return; diff --git a/packages/opentelemetry-plugin-grpc/src/grpc.ts b/packages/opentelemetry-plugin-grpc/src/grpc.ts index 9c44c65abd..b6e5fd5b77 100644 --- 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 { 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 { 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 index 5e74204b3b..2d42ab8f48 100644 --- 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-redis/src/utils.ts b/packages/opentelemetry-plugin-redis/src/utils.ts index 0be83cf644..a9f20d3dd6 100644 --- 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 index 2e4b9b072d..601850f360 100644 --- a/packages/opentelemetry-tracing/src/BasicTracer.ts +++ b/packages/opentelemetry-tracing/src/BasicTracer.ts @@ -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-types/src/trace/SpanOptions.ts b/packages/opentelemetry-types/src/trace/SpanOptions.ts index cf0fce2594..a546876a5f 100644 --- 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; From 5534c82375c2c6e1ee5cb0ffc309b8191ba2d8cc Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Mon, 25 Nov 2019 13:29:28 -0500 Subject: [PATCH 2/5] chore: revert nullish operator --- packages/opentelemetry-plugin-document-load/src/documentLoad.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-plugin-document-load/src/documentLoad.ts b/packages/opentelemetry-plugin-document-load/src/documentLoad.ts index 78900de3e4..4ca569a54d 100644 --- a/packages/opentelemetry-plugin-document-load/src/documentLoad.ts +++ b/packages/opentelemetry-plugin-document-load/src/documentLoad.ts @@ -128,7 +128,7 @@ export class DocumentLoad extends BasePlugin { AttributeNames.DOCUMENT_LOAD, PTN.FETCH_START, entries, - { parent: parseTraceParent(metaElement?.content ?? '') } + { parent: parseTraceParent((metaElement && metaElement.content) || '') } ); if (!rootSpan) { return; From 4da472804c77fca7532ba187922c4e507b56e92e Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 27 Nov 2019 09:54:30 -0500 Subject: [PATCH 3/5] chore: return undefined for missing span --- packages/opentelemetry-core/src/trace/TracerDelegate.ts | 2 +- .../opentelemetry-plugin-pg/test/pg.test.ts | 4 ++-- packages/opentelemetry-tracing/src/BasicTracer.ts | 4 ++-- packages/opentelemetry-types/src/trace/tracer.ts | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/opentelemetry-core/src/trace/TracerDelegate.ts b/packages/opentelemetry-core/src/trace/TracerDelegate.ts index cb90722d72..529862f98f 100644 --- 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-plugin-postgres/opentelemetry-plugin-pg/test/pg.test.ts b/packages/opentelemetry-plugin-postgres/opentelemetry-plugin-pg/test/pg.test.ts index d30ad3a325..b260131a1b 100644 --- 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('pg@7.x', () => { 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('pg@7.x', () => { 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-tracing/src/BasicTracer.ts b/packages/opentelemetry-tracing/src/BasicTracer.ts index 601850f360..65e1fdbc7f 100644 --- 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; } diff --git a/packages/opentelemetry-types/src/trace/tracer.ts b/packages/opentelemetry-types/src/trace/tracer.ts index be8c43d26e..6d17cbc5a1 100644 --- 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}. From 9ee1491f61cbd28ba97af30e7ce6197f7eb08a16 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 27 Nov 2019 11:53:32 -0500 Subject: [PATCH 4/5] test: undefined spans --- packages/opentelemetry-tracing/test/BasicTracer.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry-tracing/test/BasicTracer.test.ts b/packages/opentelemetry-tracing/test/BasicTracer.test.ts index 44af7ba31e..92f9bfa1e7 100644 --- 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); From 9bdf0264ec09fb79f78720c50848652633fc31b7 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 27 Nov 2019 13:23:25 -0500 Subject: [PATCH 5/5] test: undefined spans --- packages/opentelemetry-node/test/NodeTracer.test.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/opentelemetry-node/test/NodeTracer.test.ts b/packages/opentelemetry-node/test/NodeTracer.test.ts index 0cffd80c0d..6c0df61de7 100644 --- 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); }); });