feat(browser): Update propagationContext on spanEnd to keep trace consistent (#11631)

Lms24 · web-flow · commit cfcd226e9ca7 · 2024-04-17T13:39:19.000+02:00
As spec'd out in #11599 and agreed upon in internal discussions, a trace should to stay consistent over the entire time span of one route. Therefore, when the initial pageload or navigation span ends, we update the scope's propagation context to keep span-specific attributes like the `sampled` decision and the dynamic sampling context on the scope's propagation context, even after the transaction has ended. This ensures that the trace data is consistent for the entire duration of the route. Subsequent navigations will reset the propagation context (see #11377).
diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts
@@ -130,11 +130,11 @@ sentryTest(
     const request = await requestPromise;
     const headers = request.headers();
 
-    // sampling decision is deferred b/c of no active span at the time of request
+    // sampling decision and DSC are continued from navigation span, even after it ended
     const navigationTraceId = navigationTraceContext?.trace_id;
-    expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}$`));
+    expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`));
     expect(headers['baggage']).toEqual(
-      `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId}`,
+      `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true`,
     );
   },
 );
@@ -203,11 +203,11 @@ sentryTest(
     const request = await xhrPromise;
     const headers = request.headers();
 
-    // sampling decision is deferred b/c of no active span at the time of request
+    // sampling decision and DSC are continued from navigation span, even after it ended
     const navigationTraceId = navigationTraceContext?.trace_id;
-    expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}$`));
+    expect(headers['sentry-trace']).toMatch(new RegExp(`^${navigationTraceId}-[0-9a-f]{16}-1$`));
     expect(headers['baggage']).toEqual(
-      `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId}`,
+      `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${navigationTraceId},sentry-sample_rate=1,sentry-sampled=true`,
     );
   },
 );
diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/template.html
@@ -4,7 +4,7 @@
     <meta charset="utf-8" />
     <meta name="sentry-trace" content="12345678901234567890123456789012-1234567890123456-1" />
     <meta name="baggage"
-          content="sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=0.2,sentry-transaction=my-transaction,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod"/>
+          content="sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=0.2,sentry-sampled=true,sentry-transaction=my-transaction,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod"/>
   </head>
   <body>
     <button id="errorBtn">Throw Error</button>
diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts
@@ -10,7 +10,7 @@ import {
 const META_TAG_TRACE_ID = '12345678901234567890123456789012';
 const META_TAG_PARENT_SPAN_ID = '1234567890123456';
 const META_TAG_BAGGAGE =
-  'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=0.2,sentry-transaction=my-transaction,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod';
+  'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=0.2,sentry-sampled=true,sentry-transaction=my-transaction,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=prod';
 
 sentryTest(
   'create a new trace for a navigation after the <meta> tag pageload trace',
diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload/test.ts
@@ -124,11 +124,11 @@ sentryTest(
     const request = await requestPromise;
     const headers = request.headers();
 
-    // sampling decision is deferred b/c of no active span at the time of request
+    // sampling decision and DSC are continued from the pageload span even after it ended
     const pageloadTraceId = pageloadTraceContext?.trace_id;
-    expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}$`));
+    expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`));
     expect(headers['baggage']).toEqual(
-      `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId}`,
+      `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true`,
     );
   },
 );
@@ -191,11 +191,11 @@ sentryTest(
     const request = await requestPromise;
     const headers = request.headers();
 
-    // sampling decision is deferred b/c of no active span at the time of request
+    // sampling decision and DSC are continued from the pageload span even after it ended
     const pageloadTraceId = pageloadTraceContext?.trace_id;
-    expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}$`));
+    expect(headers['sentry-trace']).toMatch(new RegExp(`^${pageloadTraceId}-[0-9a-f]{16}-1$`));
     expect(headers['baggage']).toEqual(
-      `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId}`,
+      `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${pageloadTraceId},sentry-sample_rate=1,sentry-sampled=true`,
     );
   },
 );
diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts
@@ -14,9 +14,11 @@ import {
   getActiveSpan,
   getClient,
   getCurrentScope,
+  getDynamicSamplingContextFromSpan,
   getIsolationScope,
   getRootSpan,
   registerSpanErrorInstrumentation,
+  spanIsSampled,
   spanToJSON,
   startIdleSpan,
   withScope,
@@ -282,6 +284,29 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         });
       });
 
+      // A trace should to stay the consistent over the entire time span of one route.
+      // Therefore, when the initial pageload or navigation transaction ends, we update the
+      // scope's propagation context to keep span-specific attributes like the `sampled` decision and
+      // the dynamic sampling context valid, even after the transaction has ended.
+      // This ensures that the trace data is consistent for the entire duration of the route.
+      client.on('spanEnd', span => {
+        const op = spanToJSON(span).op;
+        if (span !== getRootSpan(span) || (op !== 'navigation' && op !== 'pageload')) {
+          return;
+        }
+
+        const scope = getCurrentScope();
+        const oldPropagationContext = scope.getPropagationContext();
+
+        const newPropagationContext = {
+          ...oldPropagationContext,
+          sampled: oldPropagationContext.sampled !== undefined ? oldPropagationContext.sampled : spanIsSampled(span),
+          dsc: oldPropagationContext.dsc || getDynamicSamplingContextFromSpan(span),
+        };
+
+        scope.setPropagationContext(newPropagationContext);
+      });
+
       if (options.instrumentPageLoad && WINDOW.location) {
         const startSpanOptions: StartSpanOptions = {
           name: WINDOW.location.pathname,
diff --git a/packages/browser/test/unit/tracing/browserTracingIntegration.test.ts b/packages/browser/test/unit/tracing/browserTracingIntegration.test.ts
@@ -638,7 +638,7 @@ describe('browserTracingIntegration', () => {
       expect(getCurrentScope().getScopeData().transactionName).toBe('test navigation span');
     });
 
-    it("resets the scopes' propagationContexts", () => {
+    it("updates the scopes' propagationContexts on a navigation", () => {
       const client = new BrowserClient(
         getDefaultBrowserClientOptions({
           integrations: [browserTracingIntegration()],
@@ -675,6 +675,45 @@ describe('browserTracingIntegration', () => {
       expect(newIsolationScopePropCtx?.traceId).not.toEqual(oldIsolationScopePropCtx?.traceId);
       expect(newCurrentScopePropCtx?.traceId).not.toEqual(oldCurrentScopePropCtx?.traceId);
     });
+
+    it("saves the span's sampling decision and its DSC on the propagationContext when the span finishes", () => {
+      const client = new BrowserClient(
+        getDefaultBrowserClientOptions({
+          tracesSampleRate: 1,
+          integrations: [browserTracingIntegration({ instrumentPageLoad: false })],
+        }),
+      );
+      setCurrentClient(client);
+      client.init();
+
+      const navigationSpan = startBrowserTracingNavigationSpan(client, {
+        name: 'mySpan',
+        attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' },
+      });
+
+      const propCtxBeforeEnd = getCurrentScope().getPropagationContext();
+      expect(propCtxBeforeEnd).toStrictEqual({
+        spanId: expect.stringMatching(/[a-f0-9]{16}/),
+        traceId: expect.stringMatching(/[a-f0-9]{32}/),
+      });
+
+      navigationSpan!.end();
+
+      const propCtxAfterEnd = getCurrentScope().getPropagationContext();
+      expect(propCtxAfterEnd).toStrictEqual({
+        spanId: propCtxBeforeEnd?.spanId,
+        traceId: propCtxBeforeEnd?.traceId,
+        sampled: true,
+        dsc: {
+          environment: 'production',
+          public_key: 'examplePublicKey',
+          sample_rate: '1',
+          sampled: 'true',
+          transaction: 'mySpan',
+          trace_id: propCtxBeforeEnd?.traceId,
+        },
+      });
+    });
   });
 
   describe('using the <meta> tag data', () => {
