ref: Read propagation context off of scope and isolation scope when propagating and applying trace context (#10297)

Author: Luca Forstner

dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts

@@ -5,15 +5,15 @@ afterAll(() => {
   cleanupChildProcesses();
 });
 
-test('Should not overwrite baggage if the incoming request already has Sentry baggage data.', async () => {
+test('Should overwrite baggage if the incoming request already has Sentry baggage data but no sentry-trace', async () => {
   const runner = createRunner(__dirname, '..', 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
     baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
   });
 
   expect(response).toBeDefined();
-  expect(response).toMatchObject({
+  expect(response).not.toMatchObject({
     test_data: {
       host: 'somewhere.not.sentry',
       baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
@@ -25,7 +25,7 @@ test('Should propagate sentry trace baggage data from an incoming to an outgoing
   const runner = createRunner(__dirname, '..', 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    'sentry-trace': '',
+    'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
     baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great',
   });
 
@@ -38,11 +38,28 @@ test('Should propagate sentry trace baggage data from an incoming to an outgoing
   });
 });
 
-test('Should not propagate baggage if sentry-trace header is present in incoming request but no baggage header', async () => {
+test('Should not propagate baggage data from an incoming to an outgoing request if sentry-trace is faulty.', async () => {
   const runner = createRunner(__dirname, '..', 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
     'sentry-trace': '',
+    baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great',
+  });
+
+  expect(response).toBeDefined();
+  expect(response).not.toMatchObject({
+    test_data: {
+      host: 'somewhere.not.sentry',
+      baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
+    },
+  });
+});
+
+test('Should not propagate baggage if sentry-trace header is present in incoming request but no baggage header', async () => {
+  const runner = createRunner(__dirname, '..', 'server.ts').start();
+
+  const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
+    'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
   });
 
   expect(response).toBeDefined();
@@ -57,7 +74,7 @@ test('Should not propagate baggage and ignore original 3rd party baggage entries
   const runner = createRunner(__dirname, '..', 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    'sentry-trace': '',
+    'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
     baggage: 'foo=bar',
   });
 

dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts

@@ -9,7 +9,7 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an
   const runner = createRunner(__dirname, 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    'sentry-trace': '',
+    'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
     baggage: 'sentry-release=2.1.0,sentry-environment=myEnv',
   });
 

dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors/test.ts

@@ -9,7 +9,7 @@ test('should merge `baggage` header of a third party vendor with the Sentry DSC
   const runner = createRunner(__dirname, 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    'sentry-trace': '',
+    'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
     baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
   });
 

packages/core/src/baseclient.ts

@@ -20,7 +20,6 @@ import type {
   MetricsAggregator,
   Outcome,
   ParameterizedString,
-  PropagationContext,
   SdkMetadata,
   Session,
   SessionAggregates,
@@ -662,13 +661,14 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
         return evt;
       }
 
-      // If a trace context is not set on the event, we use the propagationContext set on the event to
-      // generate a trace context. If the propagationContext does not have a dynamic sampling context, we
-      // also generate one for it.
-      const { propagationContext } = evt.sdkProcessingMetadata || {};
+      const propagationContext = {
+        ...isolationScope.getPropagationContext(),
+        ...(scope ? scope.getPropagationContext() : undefined),
+      };
+
       const trace = evt.contexts && evt.contexts.trace;
       if (!trace && propagationContext) {
-        const { traceId: trace_id, spanId, parentSpanId, dsc } = propagationContext as PropagationContext;
+        const { traceId: trace_id, spanId, parentSpanId, dsc } = propagationContext;
         evt.contexts = {
           trace: {
             trace_id,

packages/core/src/tracing/trace.ts

@@ -237,7 +237,7 @@ export function continueTrace<V>(
   const transactionContext: Partial<TransactionContext> = {
     ...traceparentData,
     metadata: dropUndefinedKeys({
-      dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
+      dynamicSamplingContext,
     }),
   };
 

packages/core/src/utils/applyScopeDataToEvent.ts

@@ -1,4 +1,4 @@
-import type { Breadcrumb, Event, PropagationContext, ScopeData, Span } from '@sentry/types';
+import type { Breadcrumb, Event, ScopeData, Span } from '@sentry/types';
 import { arrayify, dropUndefinedKeys } from '@sentry/utils';
 import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext';
 import { getRootSpan } from './getRootSpan';
@@ -8,7 +8,7 @@ import { spanToJSON, spanToTraceContext } from './spanUtils';
  * Applies data from the scope to the event and runs all event processors on it.
  */
 export function applyScopeDataToEvent(event: Event, data: ScopeData): void {
-  const { fingerprint, span, breadcrumbs, sdkProcessingMetadata, propagationContext } = data;
+  const { fingerprint, span, breadcrumbs, sdkProcessingMetadata } = data;
 
   // Apply general data
   applyDataToEvent(event, data);
@@ -22,7 +22,7 @@ export function applyScopeDataToEvent(event: Event, data: ScopeData): void {
 
   applyFingerprintToEvent(event, fingerprint);
   applyBreadcrumbsToEvent(event, breadcrumbs);
-  applySdkMetadataToEvent(event, sdkProcessingMetadata, propagationContext);
+  applySdkMetadataToEvent(event, sdkProcessingMetadata);
 }
 
 /** Merge data of two scopes together. */
@@ -163,15 +163,10 @@ function applyBreadcrumbsToEvent(event: Event, breadcrumbs: Breadcrumb[]): void
   event.breadcrumbs = mergedBreadcrumbs.length ? mergedBreadcrumbs : undefined;
 }
 
-function applySdkMetadataToEvent(
-  event: Event,
-  sdkProcessingMetadata: ScopeData['sdkProcessingMetadata'],
-  propagationContext: PropagationContext,
-): void {
+function applySdkMetadataToEvent(event: Event, sdkProcessingMetadata: ScopeData['sdkProcessingMetadata']): void {
   event.sdkProcessingMetadata = {
     ...event.sdkProcessingMetadata,
     ...sdkProcessingMetadata,
-    propagationContext: propagationContext,
   };
 }
 

packages/core/test/lib/prepareEvent.test.ts

@@ -228,12 +228,7 @@ describe('prepareEvent', () => {
       event_id: expect.any(String),
       environment: 'production',
       message: 'foo',
-      sdkProcessingMetadata: {
-        propagationContext: {
-          spanId: expect.any(String),
-          traceId: expect.any(String),
-        },
-      },
+      sdkProcessingMetadata: {},
     });
   });
 
@@ -309,16 +304,15 @@ describe('prepareEvent', () => {
       user: { id: '1', email: 'test@example.com' },
       tags: { tag1: 'aa', tag2: 'aa' },
       extra: { extra1: 'aa', extra2: 'aa' },
-      contexts: { os: { name: 'os1' }, culture: { display_name: 'name1' } },
+      contexts: {
+        os: { name: 'os1' },
+        culture: { display_name: 'name1' },
+      },
       fingerprint: ['dd', 'aa'],
       breadcrumbs: [breadcrumb4, breadcrumb2, breadcrumb3, breadcrumb1],
       sdkProcessingMetadata: {
         aa: 'aa',
         bb: 'bb',
-        propagationContext: {
-          spanId: '1',
-          traceId: '1',
-        },
       },
     });
   });
@@ -382,7 +376,6 @@ describe('prepareEvent', () => {
       sdkProcessingMetadata: {
         aa: 'aa',
         bb: 'bb',
-        propagationContext: isolationScope.getPropagationContext(),
       },
     });
   });

packages/core/test/lib/scope.test.ts

@@ -132,12 +132,7 @@ describe('Scope', () => {
 
       expect(event).toEqual({
         message: 'foo',
-        sdkProcessingMetadata: {
-          propagationContext: {
-            spanId: expect.any(String),
-            traceId: expect.any(String),
-          },
-        },
+        sdkProcessingMetadata: {},
       });
     });
 
@@ -166,15 +161,14 @@ describe('Scope', () => {
         user: { id: '1', email: 'test@example.com' },
         tags: { tag1: 'aa', tag2: 'aa' },
         extra: { extra1: 'aa', extra2: 'aa' },
-        contexts: { os: { name: 'os1' }, culture: { display_name: 'name1' } },
+        contexts: {
+          os: { name: 'os1' },
+          culture: { display_name: 'name1' },
+        },
         fingerprint: ['dd', 'aa'],
         breadcrumbs: [breadcrumb2, breadcrumb1],
         sdkProcessingMetadata: {
           aa: 'aa',
-          propagationContext: {
-            spanId: '1',
-            traceId: '1',
-          },
         },
       });
     });

packages/core/test/lib/tracing/trace.test.ts

@@ -489,6 +489,7 @@ describe('continueTrace', () => {
     const scope = getCurrentScope();
 
     expect(scope.getPropagationContext()).toEqual({
+      dsc: {}, // DSC should be an empty object (frozen), because there was an incoming trace
       sampled: false,
       parentSpanId: '1121201211212012',
       spanId: expect.any(String),

packages/hub/test/scope.test.ts

@@ -261,8 +261,6 @@ describe('Scope', () => {
       expect(processedEvent!.contexts).toEqual({ os: { id: '1' } });
       expect(processedEvent!.sdkProcessingMetadata).toEqual({
         dogs: 'are great!',
-        // @ts-expect-error accessing private property for test
-        propagationContext: scope._propagationContext,
       });
     });