feat(api): propagate spanContext only using API #1456

vmarchaud · vmarchaud · commit fd217237c402 · 2020-09-23T19:09:13.000+02:00
diff --git a/benchmark/propagator.js b/benchmark/propagator.js
@@ -31,7 +31,7 @@ for (const setup of setups) {
   const suite = benchmark(100)
     .add('#Inject', function () {
       propagator.inject(
-        opentelemetry.setExtractedSpanContext(Context.ROOT_CONTEXT, {
+        api.setExtractedSpanContext(Context.ROOT_CONTEXT, {
           traceId: 'd4cda95b652f4a1592b449d5929fda1b',
           spanId: '6e0c63257de34c92'
         }), setup.injectCarrier, api.defaultSetter);
diff --git a/packages/opentelemetry-api/src/context/context.ts b/packages/opentelemetry-api/src/context/context.ts
@@ -14,17 +14,13 @@
  * limitations under the License.
  */
 
-import {
-  Span,
-  SpanContext,
-  createContextKey,
-  Context,
-} from '@opentelemetry/api';
+import { Span, SpanContext } from '../';
+import { Context, createContextKey } from '@opentelemetry/context-base';
 
 /**
  * Active span key
  */
-export const ACTIVE_SPAN_KEY = createContextKey(
+const ACTIVE_SPAN_KEY = createContextKey(
   'OpenTelemetry Context Key ACTIVE_SPAN'
 );
 const EXTRACTED_SPAN_CONTEXT_KEY = createContextKey(
@@ -34,7 +30,7 @@ const EXTRACTED_SPAN_CONTEXT_KEY = createContextKey(
  * Shared key for indicating if instrumentation should be suppressed beyond
  * this current scope.
  */
-export const SUPPRESS_INSTRUMENTATION_KEY = createContextKey(
+const SUPPRESS_INSTRUMENTATION_KEY = createContextKey(
   'OpenTelemetry Context Key SUPPRESS_INSTRUMENTATION'
 );
 
diff --git a/packages/opentelemetry-api/src/index.ts b/packages/opentelemetry-api/src/index.ts
@@ -17,6 +17,7 @@
 export * from './common/Exception';
 export * from './common/Logger';
 export * from './common/Time';
+export * from './context/context';
 export * from './context/propagation/getter';
 export * from './context/propagation/TextMapPropagator';
 export * from './context/propagation/NoopTextMapPropagator';
diff --git a/packages/opentelemetry-api/src/trace/NoopTracer.ts b/packages/opentelemetry-api/src/trace/NoopTracer.ts
@@ -14,8 +14,11 @@
  * limitations under the License.
  */
 
-import { Span, SpanOptions, Tracer } from '..';
-import { NOOP_SPAN } from './NoopSpan';
+import { Span, SpanOptions, Tracer, SpanContext } from '..';
+import { Context } from '@opentelemetry/context-base';
+import { NoopSpan, NOOP_SPAN } from './NoopSpan';
+import { isSpanContextValid } from './spancontext-utils';
+import { getExtractedSpanContext } from '../context/context';
 
 /**
  * No-op implementations of {@link Tracer}.
@@ -26,8 +29,19 @@ export class NoopTracer implements Tracer {
   }
 
   // startSpan starts a noop span.
-  startSpan(name: string, options?: SpanOptions): Span {
-    return NOOP_SPAN;
+  startSpan(name: string, options?: SpanOptions, context?: Context): Span {
+    const parent = options?.parent;
+    const parentFromContext = context && getExtractedSpanContext(context);
+    if (isSpanContext(parent) && isSpanContextValid(parent)) {
+      return new NoopSpan(parent);
+    } else if (
+      isSpanContext(parentFromContext) &&
+      isSpanContextValid(parentFromContext)
+    ) {
+      return new NoopSpan(parentFromContext);
+    } else {
+      return NOOP_SPAN;
+    }
   }
 
   withSpan<T extends (...args: unknown[]) => ReturnType<T>>(
@@ -42,4 +56,13 @@ export class NoopTracer implements Tracer {
   }
 }
 
+function isSpanContext(spanContext: any): spanContext is SpanContext {
+  return (
+    typeof spanContext === 'object' &&
+    typeof spanContext['spanId'] === 'string' &&
+    typeof spanContext['traceId'] === 'string' &&
+    typeof spanContext['traceFlags'] === 'number'
+  );
+}
+
 export const NOOP_TRACER = new NoopTracer();
diff --git a/packages/opentelemetry-api/test/context/context.test.ts b/packages/opentelemetry-api/test/context/context.test.ts
@@ -15,37 +15,29 @@
  */
 
 import * as assert from 'assert';
-
+import { ROOT_CONTEXT, createContextKey } from '@opentelemetry/context-base'
 import {
-  SUPPRESS_INSTRUMENTATION_KEY,
   suppressInstrumentation,
   unsuppressInstrumentation,
   isInstrumentationSuppressed,
 } from '../../src/context/context';
-import { ROOT_CONTEXT } from '@opentelemetry/api';
+
+const SUPPRESS_INSTRUMENTATION_KEY = createContextKey(
+  'OpenTelemetry Context Key SUPPRESS_INSTRUMENTATION'
+);
 
 describe('Context Helpers', () => {
   describe('suppressInstrumentation', () => {
     it('should set suppress to true', () => {
-      const expectedValue = true;
       const context = suppressInstrumentation(ROOT_CONTEXT);
-
-      const value = context.getValue(SUPPRESS_INSTRUMENTATION_KEY);
-      const boolValue = value as boolean;
-
-      assert.equal(boolValue, expectedValue);
+      assert.deepStrictEqual(isInstrumentationSuppressed(context), true);
     });
   });
 
   describe('unsuppressInstrumentation', () => {
     it('should set suppress to false', () => {
-      const expectedValue = false;
       const context = unsuppressInstrumentation(ROOT_CONTEXT);
-
-      const value = context.getValue(SUPPRESS_INSTRUMENTATION_KEY);
-      const boolValue = value as boolean;
-
-      assert.equal(boolValue, expectedValue);
+      assert.deepStrictEqual(isInstrumentationSuppressed(context), false);
     });
   });
 
diff --git a/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts b/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts
@@ -15,7 +15,15 @@
  */
 
 import * as assert from 'assert';
-import { NoopTracer, NOOP_SPAN, SpanKind } from '../../src';
+import {
+  NoopTracer,
+  NOOP_SPAN,
+  SpanContext,
+  SpanKind,
+  TraceFlags,
+  context,
+  setExtractedSpanContext,
+} from '../../src';
 
 describe('NoopTracer', () => {
   it('should not crash', () => {
@@ -51,4 +59,34 @@ describe('NoopTracer', () => {
     const patchedFn = tracer.bind(fn, NOOP_SPAN);
     return patchedFn();
   });
+
+  it('should propagate valid spanContext on the span (from parent)', () => {
+    const tracer = new NoopTracer();
+    const parent: SpanContext = {
+      traceId: 'd4cda95b652f4a1592b449d5929fda1b',
+      spanId: '6e0c63257de34c92',
+      traceFlags: TraceFlags.NONE,
+    };
+    const span = tracer.startSpan('test-1', { parent });
+    assert(span.context().traceId === parent.traceId);
+    assert(span.context().spanId === parent.spanId);
+    assert(span.context().traceFlags === parent.traceFlags);
+  });
+
+  it('should propagate valid spanContext on the span (from context)', () => {
+    const tracer = new NoopTracer();
+    const parent: SpanContext = {
+      traceId: 'd4cda95b652f4a1592b449dd92ffda3b',
+      spanId: '6e0c63ffe4e34c42',
+      traceFlags: TraceFlags.NONE,
+    };
+    const span = tracer.startSpan(
+      'test-1',
+      {},
+      setExtractedSpanContext(context.active(), parent)
+    );
+    assert(span.context().traceId === parent.traceId);
+    assert(span.context().spanId === parent.spanId);
+    assert(span.context().traceFlags === parent.traceFlags);
+  });
 });
diff --git a/packages/opentelemetry-core/src/context/propagation/B3Propagator.ts b/packages/opentelemetry-core/src/context/propagation/B3Propagator.ts
@@ -20,9 +20,11 @@ import {
   TextMapPropagator,
   SetterFunction,
   TraceFlags,
-  createContextKey,
+  getParentSpanContext,
+  setExtractedSpanContext,
 } from '@opentelemetry/api';
-import { getParentSpanContext, setExtractedSpanContext } from '../context';
+
+import { createContextKey } from '@opentelemetry/context-base'
 
 export const X_B3_TRACE_ID = 'x-b3-traceid';
 export const X_B3_SPAN_ID = 'x-b3-spanid';
diff --git a/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts b/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts
@@ -21,9 +21,10 @@ import {
   SetterFunction,
   SpanContext,
   TraceFlags,
+  getParentSpanContext,
+  setExtractedSpanContext,
 } from '@opentelemetry/api';
 import { TraceState } from '../../trace/TraceState';
-import { getParentSpanContext, setExtractedSpanContext } from '../context';
 
 export const TRACE_PARENT_HEADER = 'traceparent';
 export const TRACE_STATE_HEADER = 'tracestate';
diff --git a/packages/opentelemetry-core/src/index.ts b/packages/opentelemetry-core/src/index.ts
@@ -21,7 +21,6 @@ export * from './common/time';
 export * from './common/types';
 export * from './ExportResult';
 export * from './version';
-export * from './context/context';
 export * from './context/propagation/B3Propagator';
 export * from './context/propagation/composite';
 export * from './context/propagation/HttpTraceContext';
diff --git a/packages/opentelemetry-core/test/context/B3Propagator.test.ts b/packages/opentelemetry-core/test/context/B3Propagator.test.ts
@@ -19,13 +19,11 @@ import {
   defaultSetter,
   SpanContext,
   TraceFlags,
+  getExtractedSpanContext,
+  setExtractedSpanContext,
 } from '@opentelemetry/api';
 import { ROOT_CONTEXT } from '@opentelemetry/context-base';
 import * as assert from 'assert';
-import {
-  getExtractedSpanContext,
-  setExtractedSpanContext,
-} from '../../src/context/context';
 import {
   B3Propagator,
   X_B3_FLAGS,
diff --git a/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts b/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts
@@ -19,13 +19,11 @@ import {
   defaultSetter,
   SpanContext,
   TraceFlags,
+  getExtractedSpanContext,
+  setExtractedSpanContext,
 } from '@opentelemetry/api';
 import { ROOT_CONTEXT } from '@opentelemetry/context-base';
 import * as assert from 'assert';
-import {
-  getExtractedSpanContext,
-  setExtractedSpanContext,
-} from '../../src/context/context';
 import {
   HttpTraceContext,
   TRACE_PARENT_HEADER,
diff --git a/packages/opentelemetry-core/test/context/composite.test.ts b/packages/opentelemetry-core/test/context/composite.test.ts
@@ -19,6 +19,8 @@ import {
   defaultSetter,
   TextMapPropagator,
   SpanContext,
+  getExtractedSpanContext,
+  setExtractedSpanContext,
 } from '@opentelemetry/api';
 import { Context, ROOT_CONTEXT } from '@opentelemetry/context-base';
 import * as assert from 'assert';
@@ -27,10 +29,6 @@ import {
   HttpTraceContext,
   RandomIdGenerator,
 } from '../../src';
-import {
-  getExtractedSpanContext,
-  setExtractedSpanContext,
-} from '../../src/context/context';
 import {
   B3Propagator,
   X_B3_SAMPLED,
diff --git a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts
@@ -14,13 +14,12 @@
  * limitations under the License.
  */
 
-import { context, TraceFlags } from '@opentelemetry/api';
+import { context, TraceFlags, setActiveSpan } from '@opentelemetry/api';
 import {
   AlwaysOnSampler,
   AlwaysOffSampler,
   NoopLogger,
   NoRecordingSpan,
-  setActiveSpan,
 } from '@opentelemetry/core';
 import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
 import { Span } from '@opentelemetry/tracing';
diff --git a/packages/opentelemetry-plugin-http/src/http.ts b/packages/opentelemetry-plugin-http/src/http.ts
@@ -23,11 +23,11 @@ import {
   Status,
   SpanContext,
   TraceFlags,
+  getExtractedSpanContext,
 } from '@opentelemetry/api';
 import {
   BasePlugin,
   NoRecordingSpan,
-  getExtractedSpanContext,
 } from '@opentelemetry/core';
 import type {
   ClientRequest,
diff --git a/packages/opentelemetry-plugin-http/test/utils/DummyPropagation.ts b/packages/opentelemetry-plugin-http/test/utils/DummyPropagation.ts
@@ -13,11 +13,13 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { Context, TextMapPropagator, TraceFlags } from '@opentelemetry/api';
 import {
+  Context,
+  TextMapPropagator,
+  TraceFlags,
   getParentSpanContext,
   setExtractedSpanContext,
-} from '@opentelemetry/core';
+} from '@opentelemetry/api';
 import * as http from 'http';
 
 export class DummyPropagation implements TextMapPropagator {
diff --git a/packages/opentelemetry-plugin-https/test/utils/DummyPropagation.ts b/packages/opentelemetry-plugin-https/test/utils/DummyPropagation.ts
@@ -13,11 +13,13 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { Context, TextMapPropagator, TraceFlags } from '@opentelemetry/api';
 import {
+  Context,
+  TextMapPropagator,
+  TraceFlags,
   setExtractedSpanContext,
   getParentSpanContext,
-} from '@opentelemetry/core';
+} from '@opentelemetry/api';
 import * as http from 'http';
 
 export class DummyPropagation implements TextMapPropagator {
diff --git a/packages/opentelemetry-shim-opentracing/src/shim.ts b/packages/opentelemetry-shim-opentracing/src/shim.ts
@@ -16,11 +16,8 @@
 
 import * as api from '@opentelemetry/api';
 import {
-  getExtractedSpanContext,
   NoopLogger,
-  setExtractedSpanContext,
   setCorrelationContext,
-  setActiveSpan,
   getCorrelationContext,
 } from '@opentelemetry/core';
 import * as opentracing from 'opentracing';
@@ -57,9 +54,9 @@ function translateSpanOptions(
 function getContextWithParent(options: opentracing.SpanOptions) {
   if (options.childOf) {
     if (options.childOf instanceof SpanShim) {
-      return setActiveSpan(api.context.active(), options.childOf.getSpan());
+      return api.setActiveSpan(api.context.active(), options.childOf.getSpan());
     } else if (options.childOf instanceof SpanContextShim) {
-      return setExtractedSpanContext(
+      return api.setExtractedSpanContext(
         api.context.active(),
         options.childOf.getSpanContext()
       );
@@ -181,7 +178,7 @@ export class TracerShim extends opentracing.Tracer {
           carrier,
           api.defaultSetter,
           setCorrelationContext(
-            setExtractedSpanContext(api.ROOT_CONTEXT, oTelSpanContext),
+            api.setExtractedSpanContext(api.ROOT_CONTEXT, oTelSpanContext),
             oTelSpanCorrelationContext
           )
         );
@@ -203,7 +200,7 @@ export class TracerShim extends opentracing.Tracer {
       case opentracing.FORMAT_HTTP_HEADERS:
       case opentracing.FORMAT_TEXT_MAP: {
         const context: api.Context = api.propagation.extract(carrier);
-        const spanContext = getExtractedSpanContext(context);
+        const spanContext = api.getExtractedSpanContext(context);
         const correlationContext = getCorrelationContext(context);
 
         if (!spanContext) {
diff --git a/packages/opentelemetry-tracing/src/Tracer.ts b/packages/opentelemetry-tracing/src/Tracer.ts
diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts
diff --git a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts
diff --git a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts
diff --git a/packages/opentelemetry-tracing/test/Tracer.test.ts b/packages/opentelemetry-tracing/test/Tracer.test.ts
diff --git a/packages/opentelemetry-tracing/test/export/InMemorySpanExporter.test.ts b/packages/opentelemetry-tracing/test/export/InMemorySpanExporter.test.ts
