From 2fb80ebe19c7fb16704fb39e3df1faa4845a9f8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= Date: Wed, 14 Dec 2022 17:33:37 +0100 Subject: [PATCH] fix(api): use active context as default in NoopTracer (#3476) * fix(api): use active context as default in NoopTracer To support the API without SDK use cast to forward a span context it's needed to fetch active context via ContextManager also in NoopTracer. Otherwise only direct passing of a context works but not if context.with() is used. * update PR link * fixup: correct link * remove unneeded assert --- api/CHANGELOG.md | 1 + api/src/trace/NoopTracer.ts | 6 ++++- .../noop-implementations/noop-tracer.test.ts | 23 +++++++++++++++++++ .../proxy-tracer.test.ts | 4 ---- 4 files changed, 29 insertions(+), 5 deletions(-) diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index 3784f8abc8..eb11d5c2f9 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. * fix(api): deprecate MetricAttributes and MetricAttributeValue [#3406](https://github.com/open-telemetry/opentelemetry-js/pull/3406) @blumamir * test(api): disable module concatenation in tree-shaking test [#3409](https://github.com/open-telemetry/opentelemetry-js/pull/3409) @legendecas +* fix(api): use active context as default in NoopTracer [#3476](https://github.com/open-telemetry/opentelemetry-js/pull/3476) @flarna ## [1.3.0](https://www.github.com/open-telemetry/opentelemetry-js-api/compare/v1.2.0...v1.3.0) diff --git a/api/src/trace/NoopTracer.ts b/api/src/trace/NoopTracer.ts index cf386c4f57..0656429b86 100644 --- a/api/src/trace/NoopTracer.ts +++ b/api/src/trace/NoopTracer.ts @@ -31,7 +31,11 @@ const contextApi = ContextAPI.getInstance(); */ export class NoopTracer implements Tracer { // startSpan starts a noop span. - startSpan(name: string, options?: SpanOptions, context?: Context): Span { + startSpan( + name: string, + options?: SpanOptions, + context = contextApi.active() + ): Span { const root = Boolean(options?.root); if (root) { return new NonRecordingSpan(); diff --git a/api/test/common/noop-implementations/noop-tracer.test.ts b/api/test/common/noop-implementations/noop-tracer.test.ts index 429f6d481d..1cb82e71fe 100644 --- a/api/test/common/noop-implementations/noop-tracer.test.ts +++ b/api/test/common/noop-implementations/noop-tracer.test.ts @@ -15,8 +15,10 @@ */ import * as assert from 'assert'; +import * as sinon from 'sinon'; import { context, + ROOT_CONTEXT, Span, SpanContext, SpanKind, @@ -27,6 +29,10 @@ import { NonRecordingSpan } from '../../../src/trace/NonRecordingSpan'; import { NoopTracer } from '../../../src/trace/NoopTracer'; describe('NoopTracer', () => { + afterEach(() => { + sinon.restore(); + }); + it('should not crash', () => { const tracer = new NoopTracer(); @@ -58,6 +64,23 @@ describe('NoopTracer', () => { assert(span.spanContext().traceFlags === parent.traceFlags); }); + it('should propagate valid spanContext on the span (from current context)', () => { + const tracer = new NoopTracer(); + const parent: SpanContext = { + traceId: 'd4cda95b652f4a1592b449dd92ffda3b', + spanId: '6e0c63ffe4e34c42', + traceFlags: TraceFlags.NONE, + }; + + const ctx = trace.setSpanContext(ROOT_CONTEXT, parent); + const activeStub = sinon.stub(context, 'active'); + activeStub.returns(ctx); + const span = tracer.startSpan('test-1'); + assert(span.spanContext().traceId === parent.traceId); + assert(span.spanContext().spanId === parent.spanId); + assert(span.spanContext().traceFlags === parent.traceFlags); + }); + it('should accept 2 to 4 args and start an active span', () => { const tracer = new NoopTracer(); const name = 'span-name'; diff --git a/api/test/common/proxy-implementations/proxy-tracer.test.ts b/api/test/common/proxy-implementations/proxy-tracer.test.ts index 9bf7e94409..55c39b3271 100644 --- a/api/test/common/proxy-implementations/proxy-tracer.test.ts +++ b/api/test/common/proxy-implementations/proxy-tracer.test.ts @@ -174,10 +174,6 @@ describe('ProxyTracer', () => { tracer.startSpan(name, options, ctx); // Assert the proxy tracer has the full API of the NoopTracer - assert.strictEqual( - NoopTracer.prototype.startSpan.length, - ProxyTracer.prototype.startSpan.length - ); assert.deepStrictEqual(Object.getOwnPropertyNames(NoopTracer.prototype), [ 'constructor', 'startSpan',