Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: allow parent span to be null #569

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/opentelemetry-core/src/trace/TracerDelegate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions packages/opentelemetry-node/test/NodeTracer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

Expand All @@ -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 => {
Expand All @@ -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 => {
Expand All @@ -216,7 +217,7 @@ describe('NodeTracer', () => {
}
return done();
});
assert.deepStrictEqual(tracer.getCurrentSpan(), null);
assert.deepStrictEqual(tracer.getCurrentSpan(), undefined);
});
});

Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-plugin-dns/src/dns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 2 additions & 4 deletions packages/opentelemetry-plugin-grpc/src/grpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
4 changes: 1 addition & 3 deletions packages/opentelemetry-plugin-redis/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions packages/opentelemetry-tracing/src/BasicTracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;

Expand Down
8 changes: 4 additions & 4 deletions packages/opentelemetry-tracing/test/BasicTracer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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();
});
});
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-types/src/trace/SpanOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-types/src/trace/tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export interface Tracer {
*
* @returns Span The currently active Span
*/
getCurrentSpan(): Span | null;
getCurrentSpan(): Span | undefined;

/**
* Starts a new {@link Span}.
Expand Down