feat(opentelemetry): Ignore propagation context when not continuing trace

Author: mydea

dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-root-spans/scenario.ts

@@ -14,17 +14,5 @@ Sentry.getCurrentScope().setPropagationContext({
   traceId: '12345678901234567890123456789012',
 });
 
-const spanIdTraceId = Sentry.startSpan(
-  {
-    name: 'test_span_1',
-  },
-  span1 => span1.spanContext().traceId,
-);
-
-Sentry.startSpan(
-  {
-    name: 'test_span_2',
-    attributes: { spanIdTraceId },
-  },
-  () => undefined,
-);
+Sentry.startSpan({ name: 'test_span_1' }, () => undefined);
+Sentry.startSpan({ name: 'test_span_2' }, () => undefined);

dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-root-spans/test.ts

@@ -5,24 +5,29 @@ afterAll(() => {
 });
 
 test('should send manually started parallel root spans in root context', done => {
-  expect.assertions(7);
-
   createRunner(__dirname, 'scenario.ts')
-    .expect({ transaction: { transaction: 'test_span_1' } })
     .expect({
-      transaction: transaction => {
-        expect(transaction).toBeDefined();
-        const traceId = transaction.contexts?.trace?.trace_id;
-        expect(traceId).toBeDefined();
-
-        // It ignores propagation context of the root context
-        expect(traceId).not.toBe('12345678901234567890123456789012');
-        expect(transaction.contexts?.trace?.parent_span_id).toBeUndefined();
-
-        // Different trace ID than the first span
-        const trace1Id = transaction.contexts?.trace?.data?.spanIdTraceId;
-        expect(trace1Id).toBeDefined();
-        expect(trace1Id).not.toBe(traceId);
+      transaction: {
+        transaction: 'test_span_1',
+        contexts: {
+          trace: {
+            span_id: expect.any(String),
+            parent_span_id: '1234567890123456',
+            trace_id: '12345678901234567890123456789012',
+          },
+        },
+      },
+    })
+    .expect({
+      transaction: {
+        transaction: 'test_span_2',
+        contexts: {
+          trace: {
+            span_id: expect.any(String),
+            parent_span_id: '1234567890123456',
+            trace_id: '12345678901234567890123456789012',
+          },
+        },
       },
     })
     .start(done);

dev-packages/node-integration-tests/suites/public-api/startSpan/parallel-spans-in-scope/test.ts

@@ -19,8 +19,7 @@ test('should send manually started parallel root spans outside of root context',
         const trace1Id = transaction.contexts?.trace?.data?.spanIdTraceId;
         expect(trace1Id).toBeDefined();
 
-        // Same trace ID as the first span
-        expect(trace1Id).toBe(traceId);
+        expect(trace1Id).not.toBe(traceId);
       },
     })
     .start(done);

packages/opentelemetry/src/trace.ts

@@ -176,18 +176,18 @@ function ensureTimestampInMilliseconds(timestamp: number): number {
 
 function getContext(scope: Scope | undefined, forceTransaction: boolean | undefined): Context {
   const ctx = getContextForScope(scope);
-  // Note: If the context is the ROOT_CONTEXT, no scope is attached
-  // Thus we will not use the propagation context in this case, which is desired
-  const actualScope = getScopesFromContext(ctx)?.scope;
   const parentSpan = trace.getSpan(ctx);
 
   // In the case that we have no parent span, we need to "simulate" one to ensure the propagation context is correct
   if (!parentSpan) {
-    const client = getClient();
-
-    if (actualScope && client) {
-      const propagationContext = actualScope.getPropagationContext();
-
+    // If we cannot pick a scope from the context (e.g. if it is the root context), we just take the current scope
+    const actualScope = getScopesFromContext(ctx)?.scope || getCurrentScope();
+    const propagationContext = actualScope.getPropagationContext();
+
+    // If we are continuing an incoming trace, we use it
+    // This is signified by the presence of `parentSpanId` -
+    // if this is not set, then we are not continuing an incoming trace
+    if (propagationContext.parentSpanId) {
       // We store the DSC as OTEL trace state on the span context
       const traceState = makeTraceState({
         parentSpanId: propagationContext.parentSpanId,
@@ -198,7 +198,7 @@ function getContext(scope: Scope | undefined, forceTransaction: boolean | undefi
 
       const spanOptions: SpanContext = {
         traceId: propagationContext.traceId,
-        spanId: propagationContext.parentSpanId || propagationContext.spanId,
+        spanId: propagationContext.parentSpanId,
         isRemote: true,
         traceFlags: propagationContext.sampled ? TraceFlags.SAMPLED : TraceFlags.NONE,
         traceState,
@@ -208,7 +208,8 @@ function getContext(scope: Scope | undefined, forceTransaction: boolean | undefi
       return trace.setSpanContext(ctx, spanOptions);
     }
 
-    // if we have no scope or client, we just return the context as-is
+    // if we have no scope, or the propagationContext is not continuing an incoming trace,
+    // we just let OTEL start a new trace
     return ctx;
   }
 

packages/opentelemetry/test/trace.test.ts

@@ -329,9 +329,7 @@ describe('trace', () => {
     it('allows to pass parentSpan=null', () => {
       startSpan({ name: 'GET users/[id' }, () => {
         startSpan({ name: 'child', parentSpan: null }, span => {
-          // Due to the way we propagate the scope in OTEL,
-          // the parent_span_id is not actually undefined here, but comes from the propagation context
-          expect(spanToJSON(span).parent_span_id).toBe(getCurrentScope().getPropagationContext().spanId);
+          expect(spanToJSON(span).parent_span_id).toBe(undefined);
         });
       });
     });
@@ -591,10 +589,7 @@ describe('trace', () => {
     it('allows to pass parentSpan=null', () => {
       startSpan({ name: 'outer' }, () => {
         const span = startInactiveSpan({ name: 'test span', parentSpan: null });
-
-        // Due to the way we propagate the scope in OTEL,
-        // the parent_span_id is not actually undefined here, but comes from the propagation context
-        expect(spanToJSON(span).parent_span_id).toBe(getCurrentScope().getPropagationContext().spanId);
+        expect(spanToJSON(span).parent_span_id).toBe(undefined);
         span.end();
       });
     });
@@ -881,9 +876,7 @@ describe('trace', () => {
     it('allows to pass parentSpan=null', () => {
       startSpan({ name: 'outer' }, () => {
         startSpanManual({ name: 'GET users/[id]', parentSpan: null }, span => {
-          // Due to the way we propagate the scope in OTEL,
-          // the parent_span_id is not actually undefined here, but comes from the propagation context
-          expect(spanToJSON(span).parent_span_id).toBe(getCurrentScope().getPropagationContext().spanId);
+          expect(spanToJSON(span).parent_span_id).toBe(undefined);
           span.end();
         });
       });
@@ -1016,20 +1009,23 @@ describe('trace', () => {
   });
 
   describe('propagation', () => {
-    it('picks up the trace context from the scope, if there is no parent', () => {
+    it('ignores the trace context from the scope, if there is no parent & no propagationContext.parentSpanId', () => {
       withScope(scope => {
         const propagationContext = scope.getPropagationContext();
         const span = startInactiveSpan({ name: 'test span' });
 
         expect(span).toBeDefined();
-        expect(spanToJSON(span).trace_id).toEqual(propagationContext.traceId);
-        expect(spanToJSON(span).parent_span_id).toEqual(propagationContext.spanId);
+        expect(spanToJSON(span).trace_id).toMatch(/^[0-9a-f]{32}$/);
+        expect(spanToJSON(span).trace_id).not.toEqual(propagationContext.traceId);
+        expect(spanToJSON(span).parent_span_id).toEqual(undefined);
 
         expect(getDynamicSamplingContextFromSpan(span)).toEqual({
-          ...getDynamicSamplingContextFromClient(propagationContext.traceId, getClient()!),
+          environment: 'production',
+          public_key: 'username',
           sample_rate: '1',
           sampled: 'true',
           transaction: 'test span',
+          trace_id: spanToJSON(span).trace_id,
         });
       });
     });