ref(tracing): Check and set mutability of baggage

packages/nextjs/src/utils/instrumentServer.ts

@@ -13,7 +13,7 @@ import {
   fill,
   isString,
   logger,
-  parseBaggageString,
+  parseBaggageSetMutability,
   stripUrlQueryAndFragment,
 } from '@sentry/utils';
 import * as domain from 'domain';
@@ -257,8 +257,8 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
             __DEBUG_BUILD__ && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`);
           }
 
-          const baggage =
-            nextReq.headers && isString(nextReq.headers.baggage) && parseBaggageString(nextReq.headers.baggage);
+          const rawBaggageString = nextReq.headers && isString(nextReq.headers.baggage) && nextReq.headers.baggage;
+          const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);
 
           // pull off query string, if any
           const reqPath = stripUrlQueryAndFragment(nextReq.url);
@@ -271,9 +271,8 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
             {
               name: `${namePrefix}${reqPath}`,
               op: 'http.server',
-              metadata: { requestPath: reqPath },
+              metadata: { requestPath: reqPath, baggage },
               ...traceparentData,
-              ...(baggage && { metadata: { baggage: baggage } }),
             },
             // Extra context passed to the `tracesSampler` (Note: We're combining `nextReq` and `req` this way in order
             // to not break people's `tracesSampler` functions, even though the format of `nextReq` has changed (see

packages/nextjs/src/utils/withSentry.ts

@@ -6,7 +6,7 @@ import {
   isString,
   logger,
   objectify,
-  parseBaggageString,
+  parseBaggageSetMutability,
   stripUrlQueryAndFragment,
 } from '@sentry/utils';
 import * as domain from 'domain';
@@ -53,7 +53,8 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler =
             __DEBUG_BUILD__ && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`);
           }
 
-          const baggage = req.headers && isString(req.headers.baggage) && parseBaggageString(req.headers.baggage);
+          const rawBaggageString = req.headers && isString(req.headers.baggage) && req.headers.baggage;
+          const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);
 
           const url = `${req.url}`;
           // pull off query string, if any
@@ -73,7 +74,7 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler =
               name: `${reqMethod}${reqPath}`,
               op: 'http.server',
               ...traceparentData,
-              ...(baggage && { metadata: { baggage: baggage } }),
+              metadata: { baggage: baggage },
             },
             // extra context passed to the `tracesSampler`
             { request: req },

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

@@ -3,50 +3,97 @@ import * as path from 'path';
 import { getAPIResponse, runServer } from '../../../../utils/index';
 import { TestAPIResponse } from '../server';
 
-test('Should assign `baggage` header which contains 3rd party trace baggage data to an outgoing request.', async () => {
+test('Should not overwrite baggage if the incoming request already has Sentry baggage data.', async () => {
   const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
 
   const response = (await getAPIResponse(new URL(`${url}/express`), {
-    baggage: 'foo=bar,bar=baz',
+    baggage: 'sentry-version=2.0.0,sentry-environment=myEnv',
   })) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({
     test_data: {
       host: 'somewhere.not.sentry',
-      baggage: 'foo=bar,bar=baz,sentry-environment=prod,sentry-release=1.0',
+      baggage: 'sentry-version=2.0.0,sentry-environment=myEnv',
     },
   });
 });
 
-test('Should not overwrite baggage if the incoming request already has Sentry baggage data.', async () => {
+test('Should pass along sentry and 3rd party trace baggage data from an incoming to an outgoing request.', async () => {
   const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
 
   const response = (await getAPIResponse(new URL(`${url}/express`), {
-    baggage: 'sentry-version=2.0.0,sentry-environment=myEnv',
+    baggage: 'sentry-version=2.0.0,sentry-environment=myEnv,dogs=great',
   })) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({
     test_data: {
       host: 'somewhere.not.sentry',
-      baggage: 'sentry-version=2.0.0,sentry-environment=myEnv',
+      baggage: expect.stringContaining('dogs=great,sentry-version=2.0.0,sentry-environment=myEnv'),
     },
   });
 });
 
-test('Should pass along sentry and 3rd party trace baggage data from an incoming to an outgoing request.', async () => {
+test('Should propagate empty baggage if sentry-trace header is present in incoming request but no baggage header', async () => {
   const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
 
   const response = (await getAPIResponse(new URL(`${url}/express`), {
-    baggage: 'sentry-version=2.0.0,sentry-environment=myEnv,dogs=great',
+    'sentry-trace': '',
   })) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({
     test_data: {
       host: 'somewhere.not.sentry',
-      baggage: expect.stringContaining('dogs=great,sentry-version=2.0.0,sentry-environment=myEnv'),
+      baggage: '',
+    },
+  });
+});
+
+test('Should propagate empty sentry and original 3rd party baggage if sentry-trace header is present', async () => {
+  const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
+
+  const response = (await getAPIResponse(new URL(`${url}/express`), {
+    'sentry-trace': '',
+    baggage: 'foo=bar',
+  })) as TestAPIResponse;
+
+  expect(response).toBeDefined();
+  expect(response).toMatchObject({
+    test_data: {
+      host: 'somewhere.not.sentry',
+      baggage: 'foo=bar',
+    },
+  });
+});
+
+test('Should populate and propagate sentry baggage if sentry-trace header does not exist', async () => {
+  const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
+
+  const response = (await getAPIResponse(new URL(`${url}/express`), {})) as TestAPIResponse;
+
+  expect(response).toBeDefined();
+  expect(response).toMatchObject({
+    test_data: {
+      host: 'somewhere.not.sentry',
+      baggage: 'sentry-environment=prod,sentry-release=1.0',
+    },
+  });
+});
+
+test('Should populate Sentry and propagate 3rd party content if sentry-trace header does not exist', async () => {
+  const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
+
+  const response = (await getAPIResponse(new URL(`${url}/express`), {
+    baggage: 'foo=bar,bar=baz',
+  })) as TestAPIResponse;
+
+  expect(response).toBeDefined();
+  expect(response).toMatchObject({
+    test_data: {
+      host: 'somewhere.not.sentry',
+      baggage: 'foo=bar,bar=baz,sentry-environment=prod,sentry-release=1.0',
     },
   });
 });

packages/node/src/handlers.ts

@@ -8,7 +8,7 @@ import {
   isString,
   logger,
   normalize,
-  parseBaggageString,
+  parseBaggageSetMutability,
   stripUrlQueryAndFragment,
 } from '@sentry/utils';
 import * as cookie from 'cookie';
@@ -63,14 +63,16 @@ export function tracingHandler(): (
     // If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision)
     const traceparentData =
       req.headers && isString(req.headers['sentry-trace']) && extractTraceparentData(req.headers['sentry-trace']);
-    const baggage = req.headers && isString(req.headers.baggage) && parseBaggageString(req.headers.baggage);
+    const rawBaggageString = req.headers && isString(req.headers.baggage) && req.headers.baggage;
+
+    const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);
 
     const transaction = startTransaction(
       {
         name: extractExpressTransactionName(req, { path: true, method: true }),
         op: 'http.server',
         ...traceparentData,
-        ...(baggage && { metadata: { baggage: baggage } }),
+        metadata: { baggage: baggage },
       },
       // extra context passed to the tracesSampler
       { request: extractRequestData(req) },

packages/node/test/handlers.test.ts

@@ -3,7 +3,7 @@ import * as sentryHub from '@sentry/hub';
 import { Hub } from '@sentry/hub';
 import { Transaction } from '@sentry/tracing';
 import { Baggage, Runtime } from '@sentry/types';
-import { SentryError } from '@sentry/utils';
+import { isBaggageEmpty, isBaggageMutable, SentryError } from '@sentry/utils';
 import * as http from 'http';
 import * as net from 'net';
 
@@ -368,7 +368,9 @@ describe('tracingHandler', () => {
     expect(transaction.traceId).toEqual('12312012123120121231201212312012');
     expect(transaction.parentSpanId).toEqual('1121201211212012');
     expect(transaction.sampled).toEqual(false);
-    expect(transaction.metadata?.baggage).toBeUndefined();
+    expect(transaction.metadata?.baggage).toBeDefined();
+    expect(isBaggageEmpty(transaction.metadata?.baggage)).toBe(true);
+    expect(isBaggageMutable(transaction.metadata?.baggage)).toBe(false);
   });
 
   it("pulls parent's data from tracing and baggage headers on the request", () => {
@@ -386,7 +388,11 @@ describe('tracingHandler', () => {
     expect(transaction.parentSpanId).toEqual('1121201211212012');
     expect(transaction.sampled).toEqual(false);
     expect(transaction.metadata?.baggage).toBeDefined();
-    expect(transaction.metadata?.baggage).toEqual([{ version: '1.0', environment: 'production' }, ''] as Baggage);
+    expect(transaction.metadata?.baggage).toEqual([
+      { version: '1.0', environment: 'production' },
+      '',
+      false,
+    ] as Baggage);
   });
 
   it("pulls parent's baggage (sentry + third party entries) headers on the request", () => {
@@ -402,6 +408,7 @@ describe('tracingHandler', () => {
     expect(transaction.metadata?.baggage).toEqual([
       { version: '1.0', environment: 'production' },
       'dogs=great,cats=boring',
+      false,
     ] as Baggage);
   });
 

packages/serverless/src/awslambda.ts

@@ -11,7 +11,7 @@ import {
 } from '@sentry/node';
 import { extractTraceparentData } from '@sentry/tracing';
 import { Integration } from '@sentry/types';
-import { isString, logger, parseBaggageString } from '@sentry/utils';
+import { isString, logger, parseBaggageSetMutability } from '@sentry/utils';
 // NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil
 // eslint-disable-next-line import/no-unresolved
 import { Context, Handler } from 'aws-lambda';
@@ -286,16 +286,15 @@ export function wrapHandler<TEvent, TResult>(
       traceparentData = extractTraceparentData(eventWithHeaders.headers['sentry-trace']);
     }
 
-    const baggage =
-      eventWithHeaders.headers &&
-      isString(eventWithHeaders.headers.baggage) &&
-      parseBaggageString(eventWithHeaders.headers.baggage);
+    const rawBaggageString =
+      eventWithHeaders.headers && isString(eventWithHeaders.headers.baggage) && eventWithHeaders.headers.baggage;
+    const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);
 
     const transaction = startTransaction({
       name: context.functionName,
       op: 'awslambda.handler',
       ...traceparentData,
-      ...(baggage && { metadata: { baggage: baggage } }),
+      metadata: { baggage: baggage },
     });
 
     const hub = getCurrentHub();

packages/serverless/src/gcpfunction/http.ts

@@ -1,6 +1,6 @@
 import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node';
 import { extractTraceparentData } from '@sentry/tracing';
-import { isString, logger, parseBaggageString, stripUrlQueryAndFragment } from '@sentry/utils';
+import { isString, logger, parseBaggageSetMutability, stripUrlQueryAndFragment } from '@sentry/utils';
 
 import { domainify, getActiveDomain, proxyFunction } from './../utils';
 import { HttpFunction, WrapperOptions } from './general';
@@ -56,16 +56,16 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
       traceparentData = extractTraceparentData(reqWithHeaders.headers['sentry-trace']);
     }
 
-    const baggage =
-      reqWithHeaders.headers &&
-      isString(reqWithHeaders.headers.baggage) &&
-      parseBaggageString(reqWithHeaders.headers.baggage);
+    const rawBaggageString =
+      reqWithHeaders.headers && isString(reqWithHeaders.headers.baggage) && reqWithHeaders.headers.baggage;
+
+    const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);
 
     const transaction = startTransaction({
       name: `${reqMethod} ${reqUrl}`,
       op: 'gcp.function.http',
       ...traceparentData,
-      ...(baggage && { metadata: { baggage: baggage } }),
+      metadata: { baggage: baggage },
     });
 
     // getCurrentHub() is expected to use current active domain as a carrier

packages/serverless/test/awslambda.test.ts

@@ -189,7 +189,11 @@ describe('AWSLambda', () => {
       const wrappedHandler = wrapHandler(handler);
       const rv = await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
       expect(rv).toStrictEqual(42);
-      expect(Sentry.startTransaction).toBeCalledWith({ name: 'functionName', op: 'awslambda.handler' });
+      expect(Sentry.startTransaction).toBeCalledWith({
+        name: 'functionName',
+        op: 'awslambda.handler',
+        metadata: { baggage: [{}, '', true] },
+      });
       expectScopeSettings();
       // @ts-ignore see "Why @ts-ignore" note
       expect(Sentry.fakeTransaction.finish).toBeCalled();
@@ -208,7 +212,11 @@ describe('AWSLambda', () => {
       try {
         await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
       } catch (e) {
-        expect(Sentry.startTransaction).toBeCalledWith({ name: 'functionName', op: 'awslambda.handler' });
+        expect(Sentry.startTransaction).toBeCalledWith({
+          name: 'functionName',
+          op: 'awslambda.handler',
+          metadata: { baggage: [{}, '', true] },
+        });
         expectScopeSettings();
         expect(Sentry.captureException).toBeCalledWith(error);
         // @ts-ignore see "Why @ts-ignore" note
@@ -251,6 +259,7 @@ describe('AWSLambda', () => {
                   release: '2.12.1',
                 },
                 'maisey=silly,charlie=goofy',
+                false,
               ],
             },
           }),
@@ -282,6 +291,7 @@ describe('AWSLambda', () => {
           traceId: '12312012123120121231201212312012',
           parentSpanId: '1121201211212012',
           parentSampled: false,
+          metadata: { baggage: [{}, '', false] },
         });
         expectScopeSettings();
         expect(Sentry.captureException).toBeCalledWith(e);
@@ -302,7 +312,11 @@ describe('AWSLambda', () => {
       const wrappedHandler = wrapHandler(handler);
       const rv = await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
       expect(rv).toStrictEqual(42);
-      expect(Sentry.startTransaction).toBeCalledWith({ name: 'functionName', op: 'awslambda.handler' });
+      expect(Sentry.startTransaction).toBeCalledWith({
+        name: 'functionName',
+        op: 'awslambda.handler',
+        metadata: { baggage: [{}, '', true] },
+      });
       expectScopeSettings();
       // @ts-ignore see "Why @ts-ignore" note
       expect(Sentry.fakeTransaction.finish).toBeCalled();
@@ -332,7 +346,11 @@ describe('AWSLambda', () => {
       try {
         await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
       } catch (e) {
-        expect(Sentry.startTransaction).toBeCalledWith({ name: 'functionName', op: 'awslambda.handler' });
+        expect(Sentry.startTransaction).toBeCalledWith({
+          name: 'functionName',
+          op: 'awslambda.handler',
+          metadata: { baggage: [{}, '', true] },
+        });
         expectScopeSettings();
         expect(Sentry.captureException).toBeCalledWith(error);
         // @ts-ignore see "Why @ts-ignore" note
@@ -367,7 +385,11 @@ describe('AWSLambda', () => {
       const wrappedHandler = wrapHandler(handler);
       const rv = await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
       expect(rv).toStrictEqual(42);
-      expect(Sentry.startTransaction).toBeCalledWith({ name: 'functionName', op: 'awslambda.handler' });
+      expect(Sentry.startTransaction).toBeCalledWith({
+        name: 'functionName',
+        op: 'awslambda.handler',
+        metadata: { baggage: [{}, '', true] },
+      });
       expectScopeSettings();
       // @ts-ignore see "Why @ts-ignore" note
       expect(Sentry.fakeTransaction.finish).toBeCalled();
@@ -397,7 +419,11 @@ describe('AWSLambda', () => {
       try {
         await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
       } catch (e) {
-        expect(Sentry.startTransaction).toBeCalledWith({ name: 'functionName', op: 'awslambda.handler' });
+        expect(Sentry.startTransaction).toBeCalledWith({
+          name: 'functionName',
+          op: 'awslambda.handler',
+          metadata: { baggage: [{}, '', true] },
+        });
         expectScopeSettings();
         expect(Sentry.captureException).toBeCalledWith(error);
         // @ts-ignore see "Why @ts-ignore" note