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
open-telemetry · Dec 14, 2022 · 2fb80eb · 2fb80eb
1 parent 263ee61
commit 2fb80eb
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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',