fix: don't use spanId from invalid parent (#2105)

Co-authored-by: Valentin Marchaud <contact@vmarchaud.fr>
open-telemetry · Apr 14, 2021 · c0b021b · c0b021b
1 parent 181f11e
commit c0b021b
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 4 deletions.
diff --git a/packages/opentelemetry-instrumentation-http/test/utils/DummyPropagation.ts b/packages/opentelemetry-instrumentation-http/test/utils/DummyPropagation.ts
@@ -28,7 +28,7 @@ export class DummyPropagation implements TextMapPropagator {
   extract(context: Context, carrier: http.OutgoingHttpHeaders) {
     const extractedSpanContext = {
       traceId: carrier[DummyPropagation.TRACE_CONTEXT_KEY] as string,
-      spanId: DummyPropagation.SPAN_CONTEXT_KEY,
+      spanId: carrier[DummyPropagation.SPAN_CONTEXT_KEY] as string,
       traceFlags: TraceFlags.SAMPLED,
       isRemote: true,
     };

diff --git a/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts b/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts
@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { SpanKind, SpanStatus } from '@opentelemetry/api';
+import { isValidSpanId, SpanKind, SpanStatus } from '@opentelemetry/api';
 import { hrTimeToNanoseconds } from '@opentelemetry/core';
 import { ReadableSpan } from '@opentelemetry/tracing';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
@@ -161,7 +161,8 @@ export const assertSpan = (
         'must have HOST_IP'
       );
     }
-    assert.strictEqual(span.parentSpanId, DummyPropagation.SPAN_CONTEXT_KEY);
+    assert.ok(typeof span.parentSpanId === 'string');
+    assert.ok(isValidSpanId(span.parentSpanId));
   } else if (validations.reqHeaders) {
     assert.ok(validations.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY]);
     assert.ok(validations.reqHeaders[DummyPropagation.SPAN_CONTEXT_KEY]);

diff --git a/packages/opentelemetry-tracing/src/Tracer.ts b/packages/opentelemetry-tracing/src/Tracer.ts
@@ -71,13 +71,15 @@ export class Tracer implements api.Tracer {
     const spanId = this._idGenerator.generateSpanId();
     let traceId;
     let traceState;
+    let parentSpanId;
     if (!parentContext || !api.trace.isSpanContextValid(parentContext)) {
       // New root span.
       traceId = this._idGenerator.generateTraceId();
     } else {
       // New child span.
       traceId = parentContext.traceId;
       traceState = parentContext.traceState;
+      parentSpanId = parentContext.spanId;
     }
 
     const spanKind = options.kind ?? api.SpanKind.INTERNAL;
@@ -113,7 +115,7 @@ export class Tracer implements api.Tracer {
       name,
       spanContext,
       spanKind,
-      parentContext ? parentContext.spanId : undefined,
+      parentSpanId,
       links,
       options.startTime
     );

diff --git a/packages/opentelemetry-tracing/test/Tracer.test.ts b/packages/opentelemetry-tracing/test/Tracer.test.ts
@@ -21,6 +21,9 @@ import {
   TraceFlags,
   ROOT_CONTEXT,
   suppressInstrumentation,
+  SpanContext,
+  INVALID_TRACEID,
+  setSpanContext,
 } from '@opentelemetry/api';
 import { BasicTracerProvider, Tracer, Span } from '../src';
 import {
@@ -132,6 +135,45 @@ describe('Tracer', () => {
     });
   });
 
+  it('should use traceId and spanId from parent', () => {
+    const parent: SpanContext = {
+      traceId: '00112233445566778899001122334455',
+      spanId: '0011223344556677',
+      traceFlags: TraceFlags.SAMPLED,
+    };
+    const tracer = new Tracer(
+      { name: 'default', version: '0.0.1' },
+      {},
+      tracerProvider
+    );
+    const span = tracer.startSpan(
+      'aSpan',
+      undefined,
+      setSpanContext(ROOT_CONTEXT, parent)
+    );
+    assert.strictEqual((span as Span).parentSpanId, parent.spanId);
+    assert.strictEqual(span.context().traceId, parent.traceId);
+  });
+
+  it('should not use spanId from invalid parent', () => {
+    const parent: SpanContext = {
+      traceId: INVALID_TRACEID,
+      spanId: '0011223344556677',
+      traceFlags: TraceFlags.SAMPLED,
+    };
+    const tracer = new Tracer(
+      { name: 'default', version: '0.0.1' },
+      {},
+      tracerProvider
+    );
+    const span = tracer.startSpan(
+      'aSpan',
+      undefined,
+      setSpanContext(ROOT_CONTEXT, parent)
+    );
+    assert.strictEqual((span as Span).parentSpanId, undefined);
+  });
+
   if (typeof process !== 'undefined' && process.release.name === 'node') {
     it('should sample a trace when OTEL_SAMPLING_PROBABILITY is invalid', () => {
       process.env.OTEL_SAMPLING_PROBABILITY = 'invalid value';