fix: pass actual spanContext to Span

packages/opentelemetry-basic-tracer/src/BasicTracer.ts

@@ -22,8 +22,13 @@ import {
   NOOP_SPAN,
   randomTraceId,
   isValid,
+  randomSpanId,
 } from '@opentelemetry/core';
-import { BinaryFormat, HttpTextFormat } from '@opentelemetry/types';
+import {
+  BinaryFormat,
+  HttpTextFormat,
+  TraceOptions,
+} from '@opentelemetry/types';
 import { BasicTracerConfig } from '../src/types';
 import { ScopeManager } from '@opentelemetry/scope-base';
 import { Span } from './Span';
@@ -54,24 +59,37 @@ export class BasicTracer implements types.Tracer {
    * decision.
    */
   startSpan(name: string, options: types.SpanOptions = {}): types.Span {
-    let parentSpanContext = this._getParentSpanContext(options.parent);
+    const parentContext = this._getParentSpanContext(options.parent);
     // make sampling decision
-    if (!this._sampler.shouldSample(parentSpanContext)) {
+    const samplingDecision = this._sampler.shouldSample(parentContext);
+    const spanId = randomSpanId();
+    let traceId;
+    let traceState;
+    if (!parentContext || !isValid(parentContext)) {
+      // New root span.
+      traceId = randomTraceId();
+    } else {
+      // New child span.
+      traceId = parentContext.traceId;
+      traceState = parentContext.traceState;
+    }
+    const traceOptions = samplingDecision
+      ? TraceOptions.SAMPLED
+      : TraceOptions.UNSAMPLED;
+    const spanContext = { traceId, spanId, traceOptions, traceState };
+
+    if (!samplingDecision) {
       // TODO: propagate SpanContext, for more information see
       // https://github.com/open-telemetry/opentelemetry-js/pull/99#issuecomment-513325536
       return NOOP_SPAN;
     }
 
-    if (!parentSpanContext || !isValid(parentSpanContext)) {
-      // New root span.
-      parentSpanContext = { traceId: randomTraceId(), spanId: '' };
-    }
-
     const span = new Span(
       this,
       name,
-      parentSpanContext,
+      spanContext,
       options.kind || types.SpanKind.INTERNAL,
+      parentContext ? parentContext.spanId : undefined,
       options.startTime
     );
     // Set default attributes

packages/opentelemetry-basic-tracer/src/Span.ts

@@ -15,9 +15,8 @@
  */
 
 import * as types from '@opentelemetry/types';
-import { randomSpanId } from '@opentelemetry/core';
 import { performance } from 'perf_hooks';
-import { TraceOptions, SpanKind, SpanContext } from '@opentelemetry/types';
+import { SpanKind, SpanContext } from '@opentelemetry/types';
 
 /**
  * This class represents a span.
@@ -42,19 +41,15 @@ export class Span implements types.Span {
   constructor(
     parentTracer: types.Tracer,
     spanName: string,
-    parentSpanContext: SpanContext,
+    spanContext: SpanContext,
     kind: SpanKind,
+    parentSpanId?: string,
     startTime?: number
   ) {
     this._tracer = parentTracer;
     this._name = spanName;
-    this._spanContext = {
-      traceId: parentSpanContext.traceId,
-      spanId: randomSpanId(),
-      traceOptions: TraceOptions.SAMPLED,
-      traceState: parentSpanContext.traceState,
-    };
-    this._parentId = parentSpanContext.spanId;
+    this._spanContext = spanContext;
+    this._parentId = parentSpanId;
     this._kind = kind;
     this._startTime = startTime || performance.now();
   }

packages/opentelemetry-basic-tracer/test/BasicTracer.test.ts

@@ -125,6 +125,19 @@ describe('BasicTracer', () => {
       assert.deepStrictEqual(context.traceState, state);
     });
 
+    it('should start a span with name and parent span', () => {
+      const tracer = new BasicTracer({
+        scopeManager: new NoopScopeManager(),
+      });
+      const span = tracer.startSpan('my-span');
+      const childSpan = tracer.startSpan('child-span', {
+        parent: span,
+      });
+      const context = childSpan.context();
+      assert.strictEqual(context.traceId, span.context().traceId);
+      assert.strictEqual(context.traceOptions, TraceOptions.SAMPLED);
+    });
+
     it('should start a span with name and with invalid spancontext', () => {
       const tracer = new BasicTracer({
         scopeManager: new NoopScopeManager(),

packages/opentelemetry-basic-tracer/test/Span.test.ts

@@ -27,35 +27,35 @@ import { NoopTracer } from '@opentelemetry/core';
 describe('Span', () => {
   const tracer = new NoopTracer();
   const name = 'span1';
-  const parentSpanContext: SpanContext = {
+  const spanContext: SpanContext = {
     traceId: 'd4cda95b652f4a1592b449d5929fda1b',
     spanId: '6e0c63257de34c92',
     traceOptions: TraceOptions.SAMPLED,
   };
 
   it('should create a Span instance', () => {
-    const span = new Span(tracer, name, parentSpanContext, SpanKind.SERVER);
+    const span = new Span(tracer, name, spanContext, SpanKind.SERVER);
     assert.ok(span instanceof Span);
     assert.strictEqual(span.tracer(), tracer);
   });
 
   it('should get the span context of span', () => {
-    const span = new Span(tracer, name, parentSpanContext, SpanKind.CLIENT);
+    const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
     const context = span.context();
-    assert.strictEqual(context.traceId, parentSpanContext.traceId);
+    assert.strictEqual(context.traceId, spanContext.traceId);
     assert.strictEqual(context.traceOptions, TraceOptions.SAMPLED);
     assert.strictEqual(context.traceState, undefined);
     assert.ok(context.spanId.match(/[a-f0-9]{16}/));
     assert.ok(span.isRecordingEvents());
   });
 
   it('should return true when isRecordingEvents:true', () => {
-    const span = new Span(tracer, name, parentSpanContext, SpanKind.CLIENT);
+    const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
     assert.ok(span.isRecordingEvents());
   });
 
   it('should set an attribute', () => {
-    const span = new Span(tracer, name, parentSpanContext, SpanKind.CLIENT);
+    const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
 
     ['String', 'Number', 'Boolean'].map(attType => {
       span.setAttribute('testKey' + attType, 'testValue' + attType);
@@ -64,7 +64,7 @@ describe('Span', () => {
   });
 
   it('should set an event', () => {
-    const span = new Span(tracer, name, parentSpanContext, SpanKind.CLIENT);
+    const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
     span.addEvent('sent');
     span.addEvent('rev', { attr1: 'value', attr2: 123, attr3: true });
   });
@@ -75,31 +75,33 @@ describe('Span', () => {
       spanId: '5e0c63257de34c92',
       traceOptions: TraceOptions.SAMPLED,
     };
-    const span = new Span(tracer, name, parentSpanContext, SpanKind.CLIENT);
+    const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
     span.addLink(spanContext);
     span.addLink(spanContext, { attr1: 'value', attr2: 123, attr3: true });
   });
 
   it('should set an error status', () => {
-    const span = new Span(tracer, name, parentSpanContext, SpanKind.CLIENT);
+    const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
     span.setStatus({
       code: CanonicalCode.PERMISSION_DENIED,
       message: 'This is an error',
     });
   });
 
   it('should return toString', () => {
+    const parentId = '5c1c63257de34c67';
     const span = new Span(
       tracer,
       name,
-      parentSpanContext,
+      spanContext,
       SpanKind.SERVER,
+      parentId,
       100
     );
     const context = span.context();
     assert.strictEqual(
       span.toString(),
-      `Span{"traceId":"${context.traceId}","spanId":"${context.spanId}","parentId":"${parentSpanContext.spanId}","name":"${name}","kind":1,"status":{"code":0},"startTime":100,"endTime":0}`
+      `Span{"traceId":"${context.traceId}","spanId":"${context.spanId}","parentId":"${parentId}","name":"${name}","kind":1,"status":{"code":0},"startTime":100,"endTime":0}`
     );
   });
 });