ref(tracing): Sync baggage data in Http and envelope headers (#5218)

Sync the content of the trace envelope header with the baggage Http header we propagate with outgoing requests. This means that both headers will from now on have the same baggage contents (the envelope header has additional fields that are not part of Baggage)

Author: Lms24

packages/core/src/baseclient.ts

@@ -496,8 +496,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
     // For now the decision is to skip normalization of Transactions and Spans,
     // so this block overwrites the normalized event to add back the original
     // Transaction information prior to normalization.
-    if (event.contexts && event.contexts.trace) {
-      normalized.contexts = {};
+    if (event.contexts && event.contexts.trace && normalized.contexts) {
       normalized.contexts.trace = event.contexts.trace;
 
       // event.contexts.trace.data may contain circular/dangerous data so we need to normalize it

packages/core/src/envelope.ts

@@ -1,9 +1,11 @@
 import {
+  BaggageObj,
   DsnComponents,
   Event,
   EventEnvelope,
   EventEnvelopeHeaders,
   EventItem,
+  EventTraceContext,
   SdkInfo,
   SdkMetadata,
   Session,
@@ -99,28 +101,34 @@ function createEventEnvelopeHeaders(
   tunnel: string | undefined,
   dsn: DsnComponents,
 ): EventEnvelopeHeaders {
+  const baggage = event.contexts && (event.contexts.baggage as BaggageObj);
+  const { environment, release, transaction, userid, usersegment } = 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 && {
-        // TODO: Grab this from 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.trace_id as string,
-          environment: event.environment,
-          release: event.release,
-          transaction: event.transaction,
-          user: event.user && {
-            id: event.user.id,
-            segment: event.user.segment,
-          },
+          trace_id: (event.contexts!.trace as Record<string, unknown>).trace_id as string,
           public_key: dsn.publicKey,
-        }),
+          environment: environment,
+          release: release,
+          transaction: transaction,
+          ...((userid || usersegment) && {
+            user: {
+              id: userid,
+              segment: usersegment,
+            },
+          }),
+        }) as EventTraceContext,
       }),
   };
 }

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

@@ -31,14 +31,16 @@ describe('createEventEnvelope', () => {
 
     const testTable: Array<[string, Event, EventTraceContext]> = [
       [
-        'adds only baggage item',
+        'adds one baggage item',
         {
           type: 'transaction',
-          release: '1.0.0',
           contexts: {
             trace: {
               trace_id: '1234',
             },
+            baggage: {
+              release: '1.0.0',
+            },
           },
         },
         { release: '1.0.0', trace_id: '1234', public_key: 'pubKey123' },
@@ -47,28 +49,33 @@ describe('createEventEnvelope', () => {
         'adds two baggage items',
         {
           type: 'transaction',
-          release: '1.0.0',
-          environment: 'prod',
           contexts: {
             trace: {
               trace_id: '1234',
             },
+            baggage: {
+              environment: 'prod',
+              release: '1.0.0',
+            },
           },
         },
         { release: '1.0.0', environment: 'prod', trace_id: '1234', public_key: 'pubKey123' },
       ],
       [
-        'adds all baggageitems',
+        'adds all baggage items',
         {
           type: 'transaction',
-          release: '1.0.0',
-          environment: 'prod',
-          user: { id: 'bob', segment: 'segmentA' },
-          transaction: 'TX',
           contexts: {
             trace: {
               trace_id: '1234',
             },
+            baggage: {
+              environment: 'prod',
+              release: '1.0.0',
+              userid: 'bob',
+              usersegment: 'segmentA',
+              transaction: 'TX',
+            },
           },
         },
         {

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

@@ -12,11 +12,12 @@ sentryTest('should send trace context data in transaction envelope header', asyn
   expect(envHeader.trace).toBeDefined();
   expect(envHeader.trace).toMatchObject({
     environment: 'production',
-    transaction: 'testTransactionBaggage',
-    user: {
-      id: 'user123',
-      segment: 'segmentB',
-    },
+    // TODO comment back in once we properly support transaction and user data in Baggage
+    // transaction: 'testTransactionBaggage',
+    // user: {
+    //   id: 'user123',
+    //   segment: 'segmentB',
+    // },
     public_key: 'public',
     trace_id: expect.any(String),
   });

packages/integration-tests/suites/tracing/browsertracing/meta/init.js

@@ -0,0 +1,11 @@
+import * as Sentry from '@sentry/browser';
+import { Integrations } from '@sentry/tracing';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  integrations: [new Integrations.BrowserTracing()],
+  tracesSampleRate: 1,
+  environment: 'staging',
+});

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

@@ -2,6 +2,6 @@
   <head>
     <meta charset="utf-8" />
     <meta name="sentry-trace" content="12312012123120121231201212312012-1121201211212012-1" />
-    <meta name="baggage" content="sentry-version=2.1.12" />
+    <meta name="baggage" content="sentry-release=2.1.12" />
   </head>
 </html>

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

@@ -21,17 +21,19 @@ sentryTest(
   },
 );
 
-// TODO this we can't really test until we actually propagate sentry- entries in baggage
-// skipping for now but this must be adjusted later on
-sentryTest.skip(
-  'should pick up `baggage` <meta> tag and propagate the content in transaction',
+sentryTest(
+  'should pick up `baggage` <meta> tag, propagate the content in transaction and not add own data',
   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('{version:2.1.12}');
+    expect(envHeader.trace).toEqual({
+      public_key: 'public',
+      trace_id: expect.any(String),
+      release: '2.1.12',
+    });
   },
 );
 

packages/tracing/src/span.ts

@@ -4,7 +4,6 @@ import { Baggage, Hub, Primitive, Span as SpanInterface, SpanContext, Transactio
 import {
   createBaggage,
   dropUndefinedKeys,
-  isBaggageEmpty,
   isBaggageMutable,
   isSentryBaggageEmpty,
   setBaggageImmutable,
@@ -312,7 +311,7 @@ export class Span implements SpanInterface {
   /**
    * @inheritdoc
    */
-  public getBaggage(): Baggage | undefined {
+  public getBaggage(): Baggage {
     const existingBaggage = this.transaction && this.transaction.metadata.baggage;
 
     // Only add Sentry baggage items to baggage, if baggage does not exist yet or it is still
@@ -322,7 +321,7 @@ export class Span implements SpanInterface {
         ? this._getBaggageWithSentryValues(existingBaggage)
         : existingBaggage;
 
-    return isBaggageEmpty(finalBaggage) ? undefined : finalBaggage;
+    return finalBaggage;
   }
 
   /**
@@ -366,7 +365,7 @@ export class Span implements SpanInterface {
    *
    * @param baggage
    *
-   * @returns modified and immutable maggage object
+   * @returns modified and immutable baggage object
    */
   private _getBaggageWithSentryValues(baggage: Baggage = createBaggage({})): Baggage {
     // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any

packages/tracing/src/transaction.ts

@@ -6,7 +6,7 @@ import {
   TransactionContext,
   TransactionMetadata,
 } from '@sentry/types';
-import { dropUndefinedKeys, logger } from '@sentry/utils';
+import { dropUndefinedKeys, getSentryBaggageItems, logger } from '@sentry/utils';
 
 import { Span as SpanClass, SpanRecorder } from './span';
 
@@ -122,6 +122,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
     const transaction: Event = {
       contexts: {
         trace: this.getTraceContext(),
+        baggage: getSentryBaggageItems(this.getBaggage()),
       },
       spans: finishedSpans,
       start_timestamp: this.startTimestamp,

packages/utils/src/baggage.ts

@@ -151,14 +151,14 @@ export function mergeAndSerializeBaggage(incomingBaggage?: Baggage, headerBaggag
  * Extracted this logic to a function because it's duplicated in a lot of places.
  *
  * @param rawBaggageString
- * @param sentryTraceData
+ * @param sentryTraceHeader
  */
 export function parseBaggageSetMutability(
   rawBaggageString: string | false | undefined | null,
-  sentryTraceData: TraceparentData | string | false | undefined | null,
+  sentryTraceHeader: TraceparentData | string | false | undefined | null,
 ): Baggage {
   const baggage = parseBaggageString(rawBaggageString || '');
-  if (!isSentryBaggageEmpty(baggage) || (sentryTraceData && isSentryBaggageEmpty(baggage))) {
+  if (!isSentryBaggageEmpty(baggage) || (sentryTraceHeader && isSentryBaggageEmpty(baggage))) {
     setBaggageImmutable(baggage);
   }
   return baggage;