feat(tracing): Add additional Dynamic Sampling Context items to baggage and envelope headers (#5292)

Add several additional Dynamic Sampling Context items to be collected, populated and propagated via baggage or sent via envelope headers. The propagated/sent data now conforms with the specs proposed in getsentry/develop#613. 

With this PR, the following fields were added to DSC:
* transaction name
* user id
* user segment
* sample rate
* public key
* trace id

Note that trace id and public key were already previously sent with envelope headers but the SDK would send its own values instead of the trace's values in case it is not the head-of-trace SDK. This PR also fixes that, in the sense that all `trace` envelope header data items are directly taken from the DSC we either received from an incoming baggage header or from the one we populate instead.

In addition to these fields, this patch makes the following changes:
* store final baggage in `event.sdkProcessingMetadata` (was previously stored in `event.contexts.baggage` which was wrong, caused a bug and caused the baggage to even be sent to Sentry (which we do not want in this form).
* fix a bug where populated DSC (which was set immutable) would not be stored on the span. This bug would have affected data consistency across multiple outgoing requests in one transaction, where the second/third/etc outgoing requests would potentially still find a mutable baggage object and populate it again
* rename `_getBaggageDataWithSentryValues` to `_populateBaggageDataWithSentryValues` because it better describes the purpose of the function

Author: Lms24

packages/core/src/envelope.ts

@@ -1,5 +1,4 @@
 import {
-  BaggageObj,
   DsnComponents,
   Event,
   EventEnvelope,
@@ -13,7 +12,7 @@ import {
   SessionEnvelope,
   SessionItem,
 } from '@sentry/types';
-import { createEnvelope, dropUndefinedKeys, dsnToString } from '@sentry/utils';
+import { createEnvelope, dropUndefinedKeys, dsnToString, getSentryBaggageItems } from '@sentry/utils';
 
 /** Extract sdk info from from the API metadata */
 function getSdkMetadataForEnvelopeHeader(metadata?: SdkMetadata): SdkInfo | undefined {
@@ -77,14 +76,14 @@ export function createEventEnvelope(
 
   enhanceEventWithSdkInfo(event, metadata && metadata.sdk);
 
+  const envelopeHeaders = createEventEnvelopeHeaders(event, sdkInfo, tunnel, dsn);
+
   // Prevent this data (which, if it exists, was used in earlier steps in the processing pipeline) from being sent to
   // sentry. (Note: Our use of this property comes and goes with whatever we might be debugging, whatever hacks we may
   // have temporarily added, etc. Even if we don't happen to be using it at some point in the future, let's not get rid
   // of this `delete`, lest we miss putting it back in the next time the property is in use.)
   delete event.sdkProcessingMetadata;
 
-  const envelopeHeaders = createEventEnvelopeHeaders(event, sdkInfo, tunnel, dsn);
-
   const eventItem: EventItem = [
     {
       type: eventType,
@@ -101,27 +100,24 @@ function createEventEnvelopeHeaders(
   tunnel: string | undefined,
   dsn: DsnComponents,
 ): EventEnvelopeHeaders {
-  const baggage = event.contexts && (event.contexts.baggage as BaggageObj);
-  const { environment, release, transaction, userid, usersegment } = baggage || {};
+  const baggage = event.sdkProcessingMetadata && event.sdkProcessingMetadata.baggage;
+  const { environment, release, transaction, userid, usersegment, samplerate, publickey, traceid } =
+    (baggage && getSentryBaggageItems(baggage)) || {};
 
   return {
     event_id: event.event_id as string,
     sent_at: new Date().toISOString(),
     ...(sdkInfo && { sdk: sdkInfo }),
     ...(!!tunnel && { dsn: dsnToString(dsn) }),
     ...(event.type === 'transaction' &&
-      // If we don't already have a trace context in the event, we can't get the trace id, which makes adding any other
-      // trace data pointless
-      event.contexts &&
-      event.contexts.trace && {
+      baggage && {
         trace: dropUndefinedKeys({
-          // Trace context must be defined for transactions
-          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-          trace_id: (event.contexts!.trace as Record<string, unknown>).trace_id as string,
-          public_key: dsn.publicKey,
-          environment: environment,
-          release: release,
-          transaction: transaction,
+          trace_id: traceid,
+          public_key: publickey,
+          sample_rate: samplerate,
+          environment,
+          release,
+          transaction,
           ...((userid || usersegment) && {
             user: {
               id: userid,

packages/core/test/lib/envelope.test.ts

@@ -14,49 +14,23 @@ describe('createEventEnvelope', () => {
       expect(envelopeHeaders.trace).toBeUndefined();
     });
 
-    it('adds minimal trace data if event is a transaction and no other baggage-related data is available', () => {
-      const event: Event = {
-        type: 'transaction',
-        contexts: {
-          trace: {
-            trace_id: '1234',
-          },
-        },
-      };
-      const envelopeHeaders = createEventEnvelope(event, testDsn)[0];
-
-      expect(envelopeHeaders).toBeDefined();
-      expect(envelopeHeaders.trace).toEqual({ trace_id: '1234', public_key: 'pubKey123' });
-    });
-
     const testTable: Array<[string, Event, EventTraceContext]> = [
       [
-        'adds one baggage item',
+        'adds minimal baggage items',
         {
           type: 'transaction',
-          contexts: {
-            trace: {
-              trace_id: '1234',
-            },
-            baggage: {
-              release: '1.0.0',
-            },
+          sdkProcessingMetadata: {
+            baggage: [{ traceid: '1234', publickey: 'pubKey123' }, '', false],
           },
         },
-        { release: '1.0.0', trace_id: '1234', public_key: 'pubKey123' },
+        { trace_id: '1234', public_key: 'pubKey123' },
       ],
       [
-        'adds two baggage items',
+        'adds multiple baggage items',
         {
           type: 'transaction',
-          contexts: {
-            trace: {
-              trace_id: '1234',
-            },
-            baggage: {
-              environment: 'prod',
-              release: '1.0.0',
-            },
+          sdkProcessingMetadata: {
+            baggage: [{ environment: 'prod', release: '1.0.0', publickey: 'pubKey123', traceid: '1234' }, '', false],
           },
         },
         { release: '1.0.0', environment: 'prod', trace_id: '1234', public_key: 'pubKey123' },
@@ -65,17 +39,21 @@ describe('createEventEnvelope', () => {
         'adds all baggage items',
         {
           type: 'transaction',
-          contexts: {
-            trace: {
-              trace_id: '1234',
-            },
-            baggage: {
-              environment: 'prod',
-              release: '1.0.0',
-              userid: 'bob',
-              usersegment: 'segmentA',
-              transaction: 'TX',
-            },
+          sdkProcessingMetadata: {
+            baggage: [
+              {
+                environment: 'prod',
+                release: '1.0.0',
+                userid: 'bob',
+                usersegment: 'segmentA',
+                transaction: 'TX',
+                samplerate: '0.95',
+                publickey: 'pubKey123',
+                traceid: '1234',
+              },
+              '',
+              false,
+            ],
           },
         },
         {
@@ -85,6 +63,7 @@ describe('createEventEnvelope', () => {
           transaction: 'TX',
           trace_id: '1234',
           public_key: 'pubKey123',
+          sample_rate: '0.95',
         },
       ],
     ];

packages/hub/src/scope.ts

@@ -454,6 +454,7 @@ export class Scope implements ScopeInterface {
     if (this._transactionName) {
       event.transaction = this._transactionName;
     }
+
     // We want to set the trace context for normal events only if there isn't already
     // a trace context on the event. There is a product feature in place where we link
     // errors with transaction and it relies on that.
@@ -470,7 +471,7 @@ export class Scope implements ScopeInterface {
     event.breadcrumbs = [...(event.breadcrumbs || []), ...this._breadcrumbs];
     event.breadcrumbs = event.breadcrumbs.length > 0 ? event.breadcrumbs : undefined;
 
-    event.sdkProcessingMetadata = this._sdkProcessingMetadata;
+    event.sdkProcessingMetadata = { ...event.sdkProcessingMetadata, ...this._sdkProcessingMetadata };
 
     return this._notifyEventProcessors([...getGlobalEventProcessors(), ...this._eventProcessors], event, hint);
   }

packages/integration-tests/suites/tracing/baggage/test.ts

@@ -2,6 +2,9 @@
   <head>
     <meta charset="utf-8" />
     <meta name="sentry-trace" content="12312012123120121231201212312012-1121201211212012-1" />
-    <meta name="baggage" content="sentry-release=2.1.12" />
+    <meta
+      name="baggage"
+      content="sentry-release=2.1.12,sentry-publickey=public,sentry-traceid=123,sentry-samplerate=0.3232"
+    />
   </head>
 </html>

packages/integration-tests/suites/tracing/browsertracing/meta/template.html

@@ -30,9 +30,10 @@ sentryTest(
 
     expect(envHeader.trace).toBeDefined();
     expect(envHeader.trace).toEqual({
-      public_key: 'public',
-      trace_id: expect.any(String),
       release: '2.1.12',
+      sample_rate: '0.3232',
+      trace_id: '123',
+      public_key: 'public',
     });
   },
 );

packages/integration-tests/suites/tracing/browsertracing/meta/test.ts

@@ -8,9 +8,10 @@ Sentry.init({
   integrations: [new Integrations.BrowserTracing({ tracingOrigins: [/.*/] })],
   environment: 'production',
   tracesSampleRate: 1,
+  debug: true,
 });
 
 Sentry.configureScope(scope => {
   scope.setUser({ id: 'user123', segment: 'segmentB' });
-  scope.setTransactionName('testTransactionBaggage');
+  scope.setTransactionName('testTransactionDSC');
 });

packages/integration-tests/suites/tracing/baggage/init.js renamed to packages/integration-tests/suites/tracing/envelope-header/init.js

@@ -0,0 +1,27 @@
+import { expect } from '@playwright/test';
+import { EventEnvelopeHeaders } from '@sentry/types';
+
+import { sentryTest } from '../../../utils/fixtures';
+import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../utils/helpers';
+
+sentryTest(
+  'should send dynamic sampling context data in transaction envelope header',
+  async ({ getLocalTestPath, page }) => {
+    const url = await getLocalTestPath({ testDir: __dirname });
+
+    const envHeader = await getFirstSentryEnvelopeRequest<EventEnvelopeHeaders>(page, url, envelopeHeaderRequestParser);
+
+    expect(envHeader.trace).toBeDefined();
+    expect(envHeader.trace).toEqual({
+      environment: 'production',
+      transaction: expect.stringContaining('index.html'),
+      user: {
+        id: 'user123',
+        segment: 'segmentB',
+      },
+      sample_rate: '1',
+      trace_id: expect.any(String),
+      public_key: 'public',
+    });
+  },
+);

packages/integration-tests/suites/tracing/envelope-header/test.ts

@@ -77,7 +77,10 @@ test('Should populate and propagate sentry baggage if sentry-trace header does n
   expect(response).toMatchObject({
     test_data: {
       host: 'somewhere.not.sentry',
-      baggage: 'sentry-environment=prod,sentry-release=1.0',
+      baggage: expect.stringContaining(
+        'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-samplerate=1,' +
+          'sentry-publickey=public,sentry-traceid=',
+      ),
     },
   });
 });
@@ -93,7 +96,11 @@ test('Should populate Sentry and propagate 3rd party content if sentry-trace hea
   expect(response).toMatchObject({
     test_data: {
       host: 'somewhere.not.sentry',
-      baggage: 'foo=bar,bar=baz,sentry-environment=prod,sentry-release=1.0',
+      // TraceId changes, hence we only expect that the string contains the traceid key
+      baggage: expect.stringContaining(
+        'foo=bar,bar=baz,sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,' +
+          'sentry-samplerate=1,sentry-publickey=public,sentry-traceid=',
+      ),
     },
   });
 });

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

@@ -27,12 +27,22 @@ describe('tracing', () => {
     const hub = new Hub(new NodeClient(options));
     addExtensionMethods();
 
+    hub.configureScope(scope =>
+      scope.setUser({
+        id: 'uid123',
+        segment: 'segmentA',
+      }),
+    );
+
     // we need to mock both of these because the tracing handler relies on `@sentry/core` while the sampler relies on
     // `@sentry/hub`, and mocking breaks the link between the two
     jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
     jest.spyOn(hubModule, 'getCurrentHub').mockReturnValue(hub);
 
-    const transaction = hub.startTransaction({ name: 'dogpark' });
+    const transaction = hub.startTransaction({
+      name: 'dogpark',
+      traceId: '12312012123120121231201212312012',
+    });
     hub.getScope()?.setSpan(transaction);
 
     return transaction;
@@ -98,7 +108,11 @@ describe('tracing', () => {
     const baggageHeader = request.getHeader('baggage') as string;
 
     expect(baggageHeader).toBeDefined();
-    expect(baggageHeader).toEqual('sentry-environment=production,sentry-release=1.0.0');
+    expect(baggageHeader).toEqual(
+      'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,sentry-userid=uid123,' +
+        'sentry-usersegment=segmentA,sentry-samplerate=1,sentry-publickey=dogsarebadatkeepingsecrets,' +
+        'sentry-traceid=12312012123120121231201212312012',
+    );
   });
 
   it('propagates 3rd party baggage header data to outgoing non-sentry requests', async () => {
@@ -110,7 +124,11 @@ describe('tracing', () => {
     const baggageHeader = request.getHeader('baggage') as string;
 
     expect(baggageHeader).toBeDefined();
-    expect(baggageHeader).toEqual('dog=great,sentry-environment=production,sentry-release=1.0.0');
+    expect(baggageHeader).toEqual(
+      'dog=great,sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' +
+        'sentry-userid=uid123,sentry-usersegment=segmentA,sentry-samplerate=1,' +
+        'sentry-publickey=dogsarebadatkeepingsecrets,sentry-traceid=12312012123120121231201212312012',
+    );
   });
 
   it("doesn't attach the sentry-trace header to outgoing sentry requests", () => {