chore: remove explicit parent option (#1612)

open-telemetry · Nov 2, 2020 · 18b49af · 18b49af
1 parent c4cdc4c
commit 18b49af
Show file tree

Hide file tree

Showing 8 changed files with 45 additions and 100 deletions.
diff --git a/packages/opentelemetry-api/src/api/global-utils.ts b/packages/opentelemetry-api/src/api/global-utils.ts
@@ -65,4 +65,4 @@ export function makeGetter<T>(
  * version. If the global API is not compatible with the API package
  * attempting to get it, a NOOP API implementation will be returned.
  */
-export const API_BACKWARDS_COMPATIBILITY_VERSION = 1;
+export const API_BACKWARDS_COMPATIBILITY_VERSION = 2;
diff --git a/packages/opentelemetry-api/src/trace/NoopTracer.ts b/packages/opentelemetry-api/src/trace/NoopTracer.ts
@@ -30,11 +30,14 @@ export class NoopTracer implements Tracer {
 
   // startSpan starts a noop span.
   startSpan(name: string, options?: SpanOptions, context?: Context): Span {
-    const parent = options?.parent;
+    const root = Boolean(options?.root);
+    if (root) {
+      return NOOP_SPAN;
+    }
+
     const parentFromContext = context && getActiveSpan(context)?.context();
-    if (isSpanContext(parent) && isSpanContextValid(parent)) {
-      return new NoopSpan(parent);
-    } else if (
+
+    if (
       isSpanContext(parentFromContext) &&
       isSpanContextValid(parentFromContext)
     ) {

diff --git a/packages/opentelemetry-api/src/trace/SpanOptions.ts b/packages/opentelemetry-api/src/trace/SpanOptions.ts
@@ -17,8 +17,6 @@
 import { Attributes } from './attributes';
 import { Link } from './link';
 import { SpanKind } from './span_kind';
-import { Span } from './span';
-import { SpanContext } from './span_context';
 
 /**
  * Options needed for span creation
@@ -36,20 +34,9 @@ export interface SpanOptions {
   /** {@link Link}s span to other spans */
   links?: Link[];
 
-  /**
-   * This option is NOT RECOMMENDED for normal use and should ONLY be used
-   * if your application manages context manually without the global context
-   * manager, or you are trying to override the parent extracted from context.
-   *
-   * A parent `SpanContext` (or `Span`, for convenience) that the newly-started
-   * span will be the child of. This overrides the parent span extracted from
-   * the currently active context.
-   *
-   * A null value here should prevent the SDK from extracting a parent from
-   * the current context, forcing the new span to be a root span.
-   */
-  parent?: Span | SpanContext | null;
-
   /** A manually specified start time for the created `Span` object. */
   startTime?: number;
+
+  /** The new span should be a root span. (Ignore parent from context). */
+  root?: boolean;
 }
diff --git a/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts b/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts
@@ -60,19 +60,6 @@ describe('NoopTracer', () => {
     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 = {

diff --git a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts
@@ -14,7 +14,12 @@
  * limitations under the License.
  */
 
-import { context, TraceFlags, setActiveSpan } from '@opentelemetry/api';
+import {
+  context,
+  TraceFlags,
+  setActiveSpan,
+  setExtractedSpanContext,
+} from '@opentelemetry/api';
 import {
   AlwaysOnSampler,
   AlwaysOffSampler,
@@ -26,7 +31,7 @@ import { Span } from '@opentelemetry/tracing';
 import { Resource, TELEMETRY_SDK_RESOURCE } from '@opentelemetry/resources';
 import * as assert from 'assert';
 import * as path from 'path';
-import { ContextManager } from '@opentelemetry/context-base';
+import { ContextManager, ROOT_CONTEXT } from '@opentelemetry/context-base';
 import { NodeTracerProvider } from '../src/NodeTracerProvider';
 
 const sleep = (time: number) =>
@@ -160,25 +165,29 @@ describe('NodeTracerProvider', () => {
         logger: new NoopLogger(),
       });
 
-      const sampledParent = provider
-        .getTracer('default')
-        .startSpan('not-sampled-span', {
-          parent: {
-            traceId: 'd4cda95b652f4a1592b449d5929fda1b',
-            spanId: '6e0c63257de34c92',
-            traceFlags: TraceFlags.NONE,
-          },
-        });
+      const sampledParent = provider.getTracer('default').startSpan(
+        'not-sampled-span',
+        {},
+        setExtractedSpanContext(ROOT_CONTEXT, {
+          traceId: 'd4cda95b652f4a1592b449d5929fda1b',
+          spanId: '6e0c63257de34c92',
+          traceFlags: TraceFlags.NONE,
+        })
+      );
       assert.ok(sampledParent instanceof Span);
       assert.strictEqual(
         sampledParent.context().traceFlags,
         TraceFlags.SAMPLED
       );
       assert.strictEqual(sampledParent.isRecording(), true);
 
-      const span = provider.getTracer('default').startSpan('child-span', {
-        parent: sampledParent,
-      });
+      const span = provider
+        .getTracer('default')
+        .startSpan(
+          'child-span',
+          {},
+          setActiveSpan(ROOT_CONTEXT, sampledParent)
+        );
       assert.ok(span instanceof Span);
       assert.strictEqual(span.context().traceFlags, TraceFlags.SAMPLED);
       assert.strictEqual(span.isRecording(), true);

diff --git a/packages/opentelemetry-plugin-fetch/src/fetch.ts b/packages/opentelemetry-plugin-fetch/src/fetch.ts
@@ -21,6 +21,7 @@ import * as web from '@opentelemetry/web';
 import { AttributeNames } from './enums/AttributeNames';
 import { FetchError, FetchResponse, SpanData } from './types';
 import { VERSION } from './version';
+import { setActiveSpan } from '@opentelemetry/api';
 
 // how long to wait for observer to collect information about resources
 // this is needed as event "load" is called before observer
@@ -63,10 +64,13 @@ export class FetchPlugin extends core.BasePlugin<Promise<Response>> {
     span: api.Span,
     corsPreFlightRequest: PerformanceResourceTiming
   ): void {
-    const childSpan = this._tracer.startSpan('CORS Preflight', {
-      parent: span,
-      startTime: corsPreFlightRequest[web.PerformanceTimingNames.FETCH_START],
-    });
+    const childSpan = this._tracer.startSpan(
+      'CORS Preflight',
+      {
+        startTime: corsPreFlightRequest[web.PerformanceTimingNames.FETCH_START],
+      },
+      setActiveSpan(api.context.active(), span)
+    );
     web.addSpanNetworkEvents(childSpan, corsPreFlightRequest);
     childSpan.end(
       corsPreFlightRequest[web.PerformanceTimingNames.RESPONSE_END]

diff --git a/packages/opentelemetry-tracing/src/Tracer.ts b/packages/opentelemetry-tracing/src/Tracer.ts
@@ -177,15 +177,6 @@ function getParent(
   options: api.SpanOptions,
   context: api.Context
 ): api.SpanContext | undefined {
-  if (options.parent === null) return undefined;
-  if (options.parent) return getContext(options.parent);
+  if (options.root) return undefined;
   return api.getParentSpanContext(context);
 }
-
-function getContext(span: api.Span | api.SpanContext): api.SpanContext {
-  return isSpan(span) ? span.context() : span;
-}
-
-function isSpan(span: api.Span | api.SpanContext): span is api.Span {
-  return typeof (span as api.Span).context === 'function';
-}
diff --git a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts
@@ -210,49 +210,13 @@ describe('BasicTracerProvider', () => {
       childSpan.end();
     });
 
-    it('should override context parent with option parent', () => {
-      const tracer = new BasicTracerProvider().getTracer('default');
-      const span = tracer.startSpan('my-span');
-      const overrideParent = tracer.startSpan('my-parent-override-span');
-      const childSpan = tracer.startSpan(
-        'child-span',
-        {
-          parent: overrideParent,
-        },
-        setActiveSpan(ROOT_CONTEXT, span)
-      );
-      const context = childSpan.context();
-      assert.strictEqual(context.traceId, overrideParent.context().traceId);
-      assert.strictEqual(context.traceFlags, TraceFlags.SAMPLED);
-      span.end();
-      childSpan.end();
-    });
-
-    it('should override context parent with option parent context', () => {
-      const tracer = new BasicTracerProvider().getTracer('default');
-      const span = tracer.startSpan('my-span');
-      const overrideParent = tracer.startSpan('my-parent-override-span');
-      const childSpan = tracer.startSpan(
-        'child-span',
-        {
-          parent: overrideParent.context(),
-        },
-        setActiveSpan(ROOT_CONTEXT, span)
-      );
-      const context = childSpan.context();
-      assert.strictEqual(context.traceId, overrideParent.context().traceId);
-      assert.strictEqual(context.traceFlags, TraceFlags.SAMPLED);
-      span.end();
-      childSpan.end();
-    });
-
-    it('should create a root span when parent is null', () => {
+    it('should create a root span when root is true', () => {
       const tracer = new BasicTracerProvider().getTracer('default');
       const span = tracer.startSpan('my-span');
       const overrideParent = tracer.startSpan('my-parent-override-span');
       const rootSpan = tracer.startSpan(
         'root-span',
-        { parent: null },
+        { root: true },
         setActiveSpan(ROOT_CONTEXT, span)
       );
       const context = rootSpan.context();