fix: pass SpanContext to span instead of destructuring

open-telemetry · Aug 2, 2019 · 36fe796 · 36fe796
1 parent 4f362fd
commit 36fe796
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 37 deletions.
diff --git a/packages/opentelemetry-basic-tracer/src/BasicTracer.ts b/packages/opentelemetry-basic-tracer/src/BasicTracer.ts
@@ -54,34 +54,24 @@ export class BasicTracer implements types.Tracer {
    * decision.
    */
   startSpan(name: string, options: types.SpanOptions = {}): types.Span {
-    const parentSpanContext = this._getParentSpanContext(options.parent);
+    let parentSpanContext = this._getParentSpanContext(options.parent);
     // make sampling decision
     if (!this._sampler.shouldSample(parentSpanContext)) {
       // TODO: propagate SpanContext, for more information see
       // https://github.com/open-telemetry/opentelemetry-js/pull/99#issuecomment-513325536
       return NOOP_SPAN;
     }
 
-    let traceId;
-    let parentSpanId;
-    let traceState;
     if (!parentSpanContext || !isValid(parentSpanContext)) {
       // New root span.
-      traceId = randomTraceId();
-    } else {
-      // New child span.
-      traceId = parentSpanContext.traceId;
-      parentSpanId = parentSpanContext.spanId;
-      traceState = parentSpanContext.traceState;
+      parentSpanContext = { traceId: randomTraceId(), spanId: '' };
     }
 
     const span = new Span(
       this,
       name,
-      traceId,
+      parentSpanContext,
       options.kind || types.SpanKind.INTERNAL,
-      parentSpanId,
-      traceState,
       options.startTime
     );
     // Set default attributes

diff --git a/packages/opentelemetry-basic-tracer/src/Span.ts b/packages/opentelemetry-basic-tracer/src/Span.ts
@@ -17,7 +17,7 @@
 import * as types from '@opentelemetry/types';
 import { randomSpanId } from '@opentelemetry/core';
 import { performance } from 'perf_hooks';
-import { TraceOptions, SpanKind, TraceState } from '@opentelemetry/types';
+import { TraceOptions, SpanKind, SpanContext } from '@opentelemetry/types';
 
 /**
  * This class represents a span.
@@ -42,21 +42,19 @@ export class Span implements types.Span {
   constructor(
     parentTracer: types.Tracer,
     spanName: string,
-    traceId: string,
+    parentSpanContext: SpanContext,
     kind: SpanKind,
-    parentSpanId?: string,
-    traceState?: TraceState,
     startTime?: number
   ) {
     this._tracer = parentTracer;
     this._name = spanName;
     this._spanContext = {
-      traceId,
+      traceId: parentSpanContext.traceId,
       spanId: randomSpanId(),
       traceOptions: TraceOptions.SAMPLED,
-      traceState,
+      traceState: parentSpanContext.traceState,
     };
-    this._parentId = parentSpanId;
+    this._parentId = parentSpanContext.spanId;
     this._kind = kind;
     this._startTime = startTime || performance.now();
   }

diff --git a/packages/opentelemetry-basic-tracer/test/Span.test.ts b/packages/opentelemetry-basic-tracer/test/Span.test.ts
@@ -27,32 +27,35 @@ import { NoopTracer } from '@opentelemetry/core';
 describe('Span', () => {
   const tracer = new NoopTracer();
   const name = 'span1';
-  const traceId = 'd4cda95b652f4a1592b449d5929fda1b';
-  const parentSpanId = '5d0c63257de34c92';
+  const parentSpanContext: SpanContext = {
+    traceId: 'd4cda95b652f4a1592b449d5929fda1b',
+    spanId: '6e0c63257de34c92',
+    traceOptions: TraceOptions.SAMPLED,
+  };
 
   it('should create a Span instance', () => {
-    const span = new Span(tracer, name, traceId, SpanKind.SERVER);
+    const span = new Span(tracer, name, parentSpanContext, 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, traceId, SpanKind.CLIENT, parentSpanId);
+    const span = new Span(tracer, name, parentSpanContext, SpanKind.CLIENT);
     const context = span.context();
-    assert.strictEqual(context.traceId, traceId);
+    assert.strictEqual(context.traceId, parentSpanContext.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, traceId, SpanKind.CLIENT);
+    const span = new Span(tracer, name, parentSpanContext, SpanKind.CLIENT);
     assert.ok(span.isRecordingEvents());
   });
 
   it('should set an attribute', () => {
-    const span = new Span(tracer, name, traceId, SpanKind.CLIENT);
+    const span = new Span(tracer, name, parentSpanContext, SpanKind.CLIENT);
 
     ['String', 'Number', 'Boolean'].map(attType => {
       span.setAttribute('testKey' + attType, 'testValue' + attType);
@@ -61,24 +64,24 @@ describe('Span', () => {
   });
 
   it('should set an event', () => {
-    const span = new Span(tracer, name, traceId, SpanKind.CLIENT);
+    const span = new Span(tracer, name, parentSpanContext, SpanKind.CLIENT);
     span.addEvent('sent');
     span.addEvent('rev', { attr1: 'value', attr2: 123, attr3: true });
   });
 
   it('should set a link', () => {
     const spanContext: SpanContext = {
-      traceId: 'd4cda95b652f4a1592b449d5929fda1b',
-      spanId: '6e0c63257de34c92',
+      traceId: 'a3cda95b652f4a1592b449d5929fda1b',
+      spanId: '5e0c63257de34c92',
       traceOptions: TraceOptions.SAMPLED,
     };
-    const span = new Span(tracer, name, traceId, SpanKind.CLIENT);
+    const span = new Span(tracer, name, parentSpanContext, 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, traceId, SpanKind.CLIENT);
+    const span = new Span(tracer, name, parentSpanContext, SpanKind.CLIENT);
     span.setStatus({
       code: CanonicalCode.PERMISSION_DENIED,
       message: 'This is an error',
@@ -89,17 +92,14 @@ describe('Span', () => {
     const span = new Span(
       tracer,
       name,
-      traceId,
+      parentSpanContext,
       SpanKind.SERVER,
-      undefined,
-      undefined,
       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":"${parentSpanContext.spanId}","name":"${name}","kind":1,"status":{"code":0},"startTime":100,"endTime":0}`
     );
   });
 });