ref(tracing): Ignore third party baggage entries from incoming requests

packages/nextjs/src/utils/withSentry.ts

@@ -72,7 +72,7 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler =
               name: `${reqMethod}${reqPath}`,
               op: 'http.server',
               ...traceparentData,
-              metadata: { baggage: baggage },
+              metadata: { baggage },
             },
             // extra context passed to the `tracesSampler`
             { request: req },

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

@@ -7,30 +7,31 @@ test('Should not overwrite baggage if the incoming request already has Sentry ba
   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-release=2.0.0,sentry-environment=myEnv',
   })) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({
     test_data: {
       host: 'somewhere.not.sentry',
-      baggage: 'sentry-version=2.0.0,sentry-environment=myEnv',
+      baggage: 'sentry-release=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 sentry 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,dogs=great',
+    'sentry-trace': '',
+    baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great',
   })) 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: 'sentry-release=2.0.0,sentry-environment=myEnv',
     },
   });
 });
@@ -51,7 +52,7 @@ test('Should propagate empty baggage if sentry-trace header is present in incomi
   });
 });
 
-test('Should propagate empty sentry and original 3rd party baggage if sentry-trace header is present', async () => {
+test('Should propagate empty sentry and ignore original 3rd party baggage entries 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`), {
@@ -63,7 +64,7 @@ test('Should propagate empty sentry and original 3rd party baggage if sentry-tra
   expect(response).toMatchObject({
     test_data: {
       host: 'somewhere.not.sentry',
-      baggage: 'foo=bar',
+      baggage: '',
     },
   });
 });
@@ -85,7 +86,7 @@ test('Should populate and propagate sentry baggage if sentry-trace header does n
   });
 });
 
-test('Should populate Sentry and propagate 3rd party content if sentry-trace header does not exist', async () => {
+test('Should populate Sentry and ignore 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`), {
@@ -98,7 +99,7 @@ test('Should populate Sentry and propagate 3rd party content if sentry-trace hea
       host: 'somewhere.not.sentry',
       // 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-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,' +
           'sentry-public_key=public,sentry-trace_id=',
       ),
     },

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

@@ -0,0 +1,42 @@
+import * as Sentry from '@sentry/node';
+import * as Tracing from '@sentry/tracing';
+import cors from 'cors';
+import express from 'express';
+import http from 'http';
+
+const app = express();
+
+export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } };
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  environment: 'prod',
+  integrations: [new Sentry.Integrations.Http({ tracing: true }), new Tracing.Integrations.Express({ app })],
+  tracesSampleRate: 1.0,
+});
+
+app.use(Sentry.Handlers.requestHandler());
+app.use(Sentry.Handlers.tracingHandler());
+
+app.use(cors());
+
+app.get('/test/express', (_req, res) => {
+  // simulate setting a "third party" baggage header which the Sentry SDK should merge with Sentry DSC entries
+  const headers = http
+    .get({
+      hostname: 'somewhere.not.sentry',
+      headers: {
+        baggage:
+          'other=vendor,foo=bar,third=party,sentry-release=9.9.9,sentry-environment=staging,sentry-sample_rate=0.54,last=item',
+      },
+    })
+    .getHeaders();
+
+  // Responding with the headers outgoing request headers back to the assertions.
+  res.send({ test_data: headers });
+});
+
+app.use(Sentry.Handlers.errorHandler());
+
+export default app;

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

@@ -0,0 +1,38 @@
+import * as path from 'path';
+
+import { getAPIResponse, runServer } from '../../../../utils/index';
+import { TestAPIResponse } from '../server';
+
+test('should ignore sentry-values in `baggage` header of a third party vendor and overwrite them with incoming DSC', async () => {
+  const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
+
+  const response = (await getAPIResponse(new URL(`${url}/express`), {
+    'sentry-trace': '',
+    baggage: 'sentry-release=2.1.0,sentry-environment=myEnv',
+  })) as TestAPIResponse;
+
+  expect(response).toBeDefined();
+  expect(response).toMatchObject({
+    test_data: {
+      host: 'somewhere.not.sentry',
+      baggage: 'other=vendor,foo=bar,third=party,last=item,sentry-release=2.1.0,sentry-environment=myEnv',
+    },
+  });
+});
+
+test('should ignore sentry-values in `baggage` header of a third party vendor and overwrite them with new DSC', 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: expect.stringContaining(
+        'other=vendor,foo=bar,third=party,last=item,sentry-environment=prod,sentry-release=1.0,' +
+          'sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-public_key=public',
+      ),
+    },
+  });
+});

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

@@ -0,0 +1,36 @@
+import * as Sentry from '@sentry/node';
+import * as Tracing from '@sentry/tracing';
+import cors from 'cors';
+import express from 'express';
+import http from 'http';
+
+const app = express();
+
+export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } };
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  environment: 'prod',
+  integrations: [new Sentry.Integrations.Http({ tracing: true }), new Tracing.Integrations.Express({ app })],
+  tracesSampleRate: 1.0,
+});
+
+app.use(Sentry.Handlers.requestHandler());
+app.use(Sentry.Handlers.tracingHandler());
+
+app.use(cors());
+
+app.get('/test/express', (_req, res) => {
+  // simulate setting a "third party" baggage header which the Sentry SDK should merge with Sentry DSC entries
+  const headers = http
+    .get({ hostname: 'somewhere.not.sentry', headers: { baggage: 'other=vendor,foo=bar,third=party' } })
+    .getHeaders();
+
+  // Responding with the headers outgoing request headers back to the assertions.
+  res.send({ test_data: headers });
+});
+
+app.use(Sentry.Handlers.errorHandler());
+
+export default app;

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

@@ -0,0 +1,21 @@
+import * as path from 'path';
+
+import { getAPIResponse, runServer } from '../../../../utils/index';
+import { TestAPIResponse } from '../server';
+
+test('should merge `baggage` header of a third party vendor with the Sentry DSC baggage items', async () => {
+  const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
+
+  const response = (await getAPIResponse(new URL(`${url}/express`), {
+    'sentry-trace': '',
+    baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
+  })) as TestAPIResponse;
+
+  expect(response).toBeDefined();
+  expect(response).toMatchObject({
+    test_data: {
+      host: 'somewhere.not.sentry',
+      baggage: 'other=vendor,foo=bar,third=party,sentry-release=2.0.0,sentry-environment=myEnv',
+    },
+  });
+});

packages/node/src/handlers.ts

@@ -45,7 +45,7 @@ export function tracingHandler(): (
         name: extractPathForTransaction(req, { path: true, method: true }),
         op: 'http.server',
         ...traceparentData,
-        metadata: { baggage: baggage },
+        metadata: { 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, Event } from '@sentry/types';
-import { isBaggageEmpty, isBaggageMutable, SentryError } from '@sentry/utils';
+import { getThirdPartyBaggage, isBaggageMutable, isSentryBaggageEmpty, SentryError } from '@sentry/utils';
 import * as http from 'http';
 
 import { NodeClient } from '../src/client';
@@ -193,7 +193,8 @@ describe('tracingHandler', () => {
     expect(transaction.parentSpanId).toEqual('1121201211212012');
     expect(transaction.sampled).toEqual(false);
     expect(transaction.metadata?.baggage).toBeDefined();
-    expect(isBaggageEmpty(transaction.metadata?.baggage)).toBe(true);
+    expect(isSentryBaggageEmpty(transaction.metadata?.baggage)).toBe(true);
+    expect(getThirdPartyBaggage(transaction.metadata?.baggage)).toEqual('');
     expect(isBaggageMutable(transaction.metadata?.baggage)).toBe(false);
   });
 
@@ -219,8 +220,9 @@ describe('tracingHandler', () => {
     ] as Baggage);
   });
 
-  it("pulls parent's baggage (sentry + third party entries) headers on the request", () => {
+  it('ignores 3rd party entries in incoming baggage header', () => {
     req.headers = {
+      'sentry-trace': '12312012123120121231201212312012-1121201211212012-0',
       baggage: 'sentry-version=1.0,sentry-environment=production,dogs=great,cats=boring',
     };
 
@@ -231,7 +233,7 @@ describe('tracingHandler', () => {
     expect(transaction.metadata?.baggage).toBeDefined();
     expect(transaction.metadata?.baggage).toEqual([
       { version: '1.0', environment: 'production' },
-      'dogs=great,cats=boring',
+      '',
       false,
     ] as Baggage);
   });

packages/serverless/src/awslambda.ts

@@ -289,7 +289,7 @@ export function wrapHandler<TEvent, TResult>(
       name: context.functionName,
       op: 'awslambda.handler',
       ...traceparentData,
-      metadata: { baggage: baggage },
+      metadata: { baggage },
     });
 
     const hub = getCurrentHub();

packages/serverless/src/gcpfunction/http.ts

@@ -87,7 +87,7 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
       name: `${reqMethod} ${reqUrl}`,
       op: 'gcp.function.http',
       ...traceparentData,
-      metadata: { baggage: baggage },
+      metadata: { baggage },
     });
 
     // getCurrentHub() is expected to use current active domain as a carrier

packages/serverless/test/awslambda.test.ts

@@ -256,7 +256,7 @@ describe('AWSLambda', () => {
                 {
                   release: '2.12.1',
                 },
-                'maisey=silly,charlie=goofy',
+                '',
                 false,
               ],
             },

packages/serverless/test/gcpfunction.test.ts

@@ -150,7 +150,7 @@ describe('GCPFunction', () => {
               {
                 release: '2.12.1',
               },
-              'maisey=silly,charlie=goofy',
+              '',
               false,
             ],
           },

packages/tracing/src/transaction.ts

@@ -8,15 +8,7 @@ import {
   TransactionContext,
   TransactionMetadata,
 } from '@sentry/types';
-import {
-  createBaggage,
-  dropUndefinedKeys,
-  getSentryBaggageItems,
-  getThirdPartyBaggage,
-  isBaggageMutable,
-  isSentryBaggageEmpty,
-  logger,
-} from '@sentry/utils';
+import { createBaggage, dropUndefinedKeys, getSentryBaggageItems, isBaggageMutable, logger } from '@sentry/utils';
 
 import { Span as SpanClass, SpanRecorder } from './span';
 
@@ -197,17 +189,13 @@ export class Transaction extends SpanClass implements TransactionInterface {
 
     // Only add Sentry baggage items to baggage, if baggage does not exist yet or it is still
     // empty and mutable
-    // TODO: we might want to ditch the isSentryBaggageEmpty condition because it prevents
-    //       custom sentry-values in DSC (added by users in the future)
     const finalBaggage =
-      !existingBaggage || (isBaggageMutable(existingBaggage) && isSentryBaggageEmpty(existingBaggage))
+      !existingBaggage || isBaggageMutable(existingBaggage)
         ? this._populateBaggageWithSentryValues(existingBaggage)
         : existingBaggage;
 
-    // In case, we poulated the DSC, we have update the stored one on the transaction.
-    if (existingBaggage !== finalBaggage) {
-      this.metadata.baggage = finalBaggage;
-    }
+    // Update the baggage stored on the transaction.
+    this.metadata.baggage = finalBaggage;
 
     return finalBaggage;
   }
@@ -255,7 +243,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
         sample_rate,
         ...getSentryBaggageItems(baggage), // keep user-added values
       } as BaggageObj),
-      getThirdPartyBaggage(baggage), // TODO: remove once we ignore 3rd party baggage
+      '',
       false, // set baggage immutable
     );
   }