refactor: move traceId generation in Tracer instead of Span (#154)

* refactor: move traceId generation in Tracer instead of Span

* fix: pass SpanContext to span instead of destructuring

* fix: pass actual spanContext to Span

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

@@ -20,8 +20,15 @@ import {
   BinaryTraceContext,
   HttpTraceContext,
   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';
@@ -52,19 +59,39 @@ export class BasicTracer implements types.Tracer {
    * decision.
    */
   startSpan(name: string, options: types.SpanOptions = {}): types.Span {
-    const 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;
     }
 
-    const spanOptions = Object.assign({}, options, {
-      parent: parentSpanContext,
-    });
-    const span = new Span(this, name, spanOptions);
-
+    const span = new Span(
+      this,
+      name,
+      spanContext,
+      options.kind || types.SpanKind.INTERNAL,
+      parentContext ? parentContext.spanId : undefined,
+      options.startTime
+    );
     // Set default attributes
     span.setAttributes(this._defaultAttributes);
     return span;
@@ -92,6 +119,7 @@ export class BasicTracer implements types.Tracer {
   /**
    * Records a SpanData.
    */
+  /* c8 ignore next 3 */
   recordSpanData(span: types.Span): void {
     // TODO: notify exporter
   }

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

@@ -15,20 +15,14 @@
  */
 
 import * as types from '@opentelemetry/types';
-import {
-  randomSpanId,
-  randomTraceId,
-  INVALID_SPAN_CONTEXT,
-  isValid,
-} from '@opentelemetry/core';
 import { performance } from 'perf_hooks';
-import { TraceOptions } from '@opentelemetry/types';
+import { SpanKind, SpanContext } from '@opentelemetry/types';
 
 /**
  * This class represents a span.
  */
 export class Span implements types.Span {
-  private readonly _spanContext: types.SpanContext = INVALID_SPAN_CONTEXT;
+  private readonly _spanContext: types.SpanContext;
   private readonly _tracer: types.Tracer;
   private readonly _parentId?: string;
   private readonly _kind: types.SpanKind;
@@ -47,24 +41,17 @@ export class Span implements types.Span {
   constructor(
     parentTracer: types.Tracer,
     spanName: string,
-    options: types.SpanOptions
+    spanContext: SpanContext,
+    kind: SpanKind,
+    parentSpanId?: string,
+    startTime?: number
   ) {
     this._tracer = parentTracer;
     this._name = spanName;
-    this._spanContext.spanId = randomSpanId();
-    this._spanContext.traceOptions = TraceOptions.SAMPLED;
-    const parentSpanContext = this._getParentSpanContext(options.parent);
-    if (parentSpanContext && isValid(parentSpanContext)) {
-      // New child span.
-      this._spanContext.traceId = parentSpanContext.traceId;
-      this._spanContext.traceState = parentSpanContext.traceState;
-      this._parentId = parentSpanContext.spanId;
-    } else {
-      // This is a root span so no remote or local parent.
-      this._spanContext.traceId = randomTraceId();
-    }
-    this._kind = options.kind || types.SpanKind.INTERNAL;
-    this._startTime = options.startTime || performance.now();
+    this._spanContext = spanContext;
+    this._parentId = parentSpanId;
+    this._kind = kind;
+    this._startTime = startTime || performance.now();
   }
 
   tracer(): types.Tracer {
@@ -137,18 +124,6 @@ export class Span implements types.Span {
     return `Span${json}`;
   }
 
-  private _getParentSpanContext(
-    parent: types.Span | types.SpanContext | undefined
-  ): types.SpanContext | undefined {
-    if (!parent) return undefined;
-
-    // parent is a SpanContext
-    if ((parent as types.SpanContext).traceId) {
-      return parent as types.SpanContext;
-    }
-    return (parent as Span).context();
-  }
-
   private _isSpanEnded(): boolean {
     if (this._ended) {
       // @todo: log a warning

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

@@ -22,10 +22,12 @@ import {
   NEVER_SAMPLER,
   NOOP_SPAN,
   NoopLogger,
+  TraceState,
 } from '@opentelemetry/core';
 import { BasicTracer } from '../src/BasicTracer';
 import { NoopScopeManager } from '@opentelemetry/scope-base';
 import { Span } from '../src/Span';
+import { TraceOptions } from '@opentelemetry/types';
 
 describe('BasicTracer', () => {
   describe('constructor', () => {
@@ -97,6 +99,58 @@ describe('BasicTracer', () => {
       const span = tracer.startSpan('my-span', {});
       assert.ok(span);
       assert.ok(span instanceof Span);
+      const context = span.context();
+      assert.ok(context.traceId.match(/[a-f0-9]{32}/));
+      assert.ok(context.spanId.match(/[a-f0-9]{16}/));
+      assert.strictEqual(context.traceOptions, TraceOptions.SAMPLED);
+      assert.deepStrictEqual(context.traceState, undefined);
+    });
+
+    it('should start a span with name and parent spancontext', () => {
+      const tracer = new BasicTracer({
+        scopeManager: new NoopScopeManager(),
+      });
+      const state = new TraceState('a=1,b=2');
+      const span = tracer.startSpan('my-span', {
+        parent: {
+          traceId: 'd4cda95b652f4a1592b449d5929fda1b',
+          spanId: '6e0c63257de34c92',
+          traceState: state,
+        },
+      });
+      assert.ok(span instanceof Span);
+      const context = span.context();
+      assert.strictEqual(context.traceId, 'd4cda95b652f4a1592b449d5929fda1b');
+      assert.strictEqual(context.traceOptions, TraceOptions.SAMPLED);
+      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(),
+      });
+      const span = tracer.startSpan('my-span', {
+        parent: { traceId: '0', spanId: '0' },
+      });
+      assert.ok(span instanceof Span);
+      const context = span.context();
+      assert.ok(context.traceId.match(/[a-f0-9]{32}/));
+      assert.ok(context.spanId.match(/[a-f0-9]{16}/));
+      assert.strictEqual(context.traceOptions, TraceOptions.SAMPLED);
+      assert.deepStrictEqual(context.traceState, undefined);
     });
 
     it('should return a default span with no sampling', () => {

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

@@ -18,47 +18,44 @@ import * as assert from 'assert';
 import { Span } from '../src/Span';
 import {
   SpanKind,
-  SpanContext,
-  TraceOptions,
   CanonicalCode,
+  TraceOptions,
+  SpanContext,
 } from '@opentelemetry/types';
 import { NoopTracer } from '@opentelemetry/core';
 
 describe('Span', () => {
   const tracer = new NoopTracer();
+  const name = 'span1';
   const spanContext: SpanContext = {
     traceId: 'd4cda95b652f4a1592b449d5929fda1b',
     spanId: '6e0c63257de34c92',
     traceOptions: TraceOptions.SAMPLED,
   };
-  const name = 'span1';
 
   it('should create a Span instance', () => {
-    const span = new Span(tracer, name, { kind: 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, { parent: spanContext });
+    const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
     const context = span.context();
     assert.strictEqual(context.traceId, spanContext.traceId);
-    assert.notStrictEqual(context.spanId, spanContext.spanId);
-    assert.strictEqual(context.traceOptions, spanContext.traceOptions);
+    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, { isRecordingEvents: true });
+    const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
     assert.ok(span.isRecordingEvents());
   });
 
   it('should set an attribute', () => {
-    const span = new Span(tracer, name, {
-      attributes: { service: 'service-1' },
-    });
+    const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
 
     ['String', 'Number', 'Boolean'].map(attType => {
       span.setAttribute('testKey' + attType, 'testValue' + attType);
@@ -67,35 +64,44 @@ describe('Span', () => {
   });
 
   it('should set an event', () => {
-    const span = new Span(tracer, name, {});
+    const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
     span.addEvent('sent');
     span.addEvent('rev', { attr1: 'value', attr2: 123, attr3: true });
   });
 
   it('should set a link', () => {
-    const span = new Span(tracer, name, {});
+    const spanContext: SpanContext = {
+      traceId: 'a3cda95b652f4a1592b449d5929fda1b',
+      spanId: '5e0c63257de34c92',
+      traceOptions: TraceOptions.SAMPLED,
+    };
+    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, {});
+    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 span = new Span(tracer, name, {
-      kind: SpanKind.SERVER,
-      startTime: 100,
-    });
+    const parentId = '5c1c63257de34c67';
+    const span = new Span(
+      tracer,
+      name,
+      spanContext,
+      SpanKind.SERVER,
+      parentId,
+      100
+    );
     const context = span.context();
-
     assert.strictEqual(
       span.toString(),
-      `Span{"traceId":"${context.traceId}","spanId":"${context.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}`
     );
   });
 });

packages/opentelemetry-core/src/index.ts

@@ -26,3 +26,4 @@ export * from './trace/NoopTracer';
 export * from './trace/sampler/ProbabilitySampler';
 export * from './trace/spancontext-utils';
 export * from './trace/TracerDelegate';
+export * from './trace/TraceState';