feat(tracing): Add empty baggage header propagation to outgoing reque…

…sts (#5133) Adds the propagation of an "empty" baggage header. The word "empty" is however kind of misleading as the header is not necessarily empty. In order to comply with the baggage spec, as of this patch, we propagate incoming (3rd party) baggage to outgoing requests. The important part is that we actually add the `baggage` HTTP header to outgoing requests which is a breaking change in terms of CORS rules having to be adjusted. We don't yet add `sentry-` baggage entries to the propagated baggage. This will come in a follow up PR which does not necessarily have to be part of the initial v7 release as it is no longer a breaking change. Overall, this is heavily inspired from #3945 (thanks @lobsterkatie for doing the hard work) More specifically, this PR does the following things: 1. Extract incoming baggage headers and store them in the created transaction's metadata. Incoming baggage data is intercepted at: * Node SDK: TracingHandler * Serverless SDK: AWS wrapHandler * Serverless SDK: GCP wrapHttpFunction * Next.js: SDK makeWrappedReqHandler * Next.js: SDK withSentry * BrowserTracing Integration: by parsing the `<meta>` tags (analogously to the `sentry-trace` header) 2. Add the extracted baggage data to outgoing requests we instrument at: * Node SDK: HTTP integration * Tracing: instrumented Fetch and XHR callbacks Co-authored-by: Luca Forstner <[email protected]>
getsentry · AbhiPrasad · May 30, 2022 · Apr 7, 2022 · Apr 7, 2022 · Apr 7, 2022
commit d48711f09d5bebe9233e8f8e0a200837b933fe85
diff --git a/MIGRATION.md b/MIGRATION.md
@@ -10,6 +10,8 @@ Below we will outline all the breaking changes you should consider when upgradin
 - We bumped the TypeScript version we generate our types with to 3.8.3. Please check if your TypeScript projects using TypeScript version 3.7 or lower still compile. Otherwise, upgrade your TypeScript version.
 - `whitelistUrls` and `blacklistUrls` have been renamed to `allowUrls` and `denyUrls` in the `Sentry.init()` options.
 - The `UserAgent` integration is now called `HttpContext`.
+- If you are using Performance Monitoring and with tracing enabled, you might have to [make adjustments to
+your server's CORS settings](#-propagation-of-baggage-header)
 
 ## Dropping Support for Node.js v6
 
@@ -319,6 +321,13 @@ session.update({ environment: 'prod' });
 session.close('ok');
 ```
 
+## Propagation of Baggage Header
+
+We introduced a new way of propagating tracing and transaction-related information between services. This
+change adds the [`baggage` HTTP header](https://www.w3.org/TR/baggage/) to outgoing requests if the instrumentation of requests is enabled. Since this adds a header to your HTTP requests, you might need
+to adjust your Server's CORS settings to allow this additional header. Take a look at the [Sentry docs](https://docs.sentry.io/platforms/javascript/performance/connect-services/#navigation-and-other-xhr-requests)
+for more in-depth instructions what to change.
+
 ## General API Changes
 
 For our efforts to reduce bundle size of the SDK we had to remove and refactor parts of the package which introduced a few changes to the API:

diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts
@@ -1,5 +1,4 @@
-import { DsnComponents, Event } from '@sentry/types';
-import { EventTraceContext } from '@sentry/types/build/types/envelope';
+import { DsnComponents, Event, EventTraceContext } from '@sentry/types';
 
 import { createEventEnvelope } from '../../src/envelope';
 

diff --git a/packages/integration-tests/suites/tracing/browsertracing/meta/template.html b/packages/integration-tests/suites/tracing/browsertracing/meta/template.html
@@ -2,5 +2,6 @@
   <head>
     <meta charset="utf-8" />
     <meta name="sentry-trace" content="12312012123120121231201212312012-1121201211212012-1" />
+    <meta name="baggage" content="sentry-version=2.1.12" />
   </head>
 </html>
diff --git a/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts b/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts
@@ -1,8 +1,8 @@
 import { expect } from '@playwright/test';
-import { Event } from '@sentry/types';
+import { Event, EventEnvelopeHeaders } from '@sentry/types';
 
 import { sentryTest } from '../../../../utils/fixtures';
-import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
+import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
 
 sentryTest(
   'should create a pageload transaction based on `sentry-trace` <meta>',
@@ -21,6 +21,20 @@ 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',
+  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}');
+  },
+);
+
 sentryTest(
   "should create a navigation that's not influenced by `sentry-trace` <meta>",
   async ({ getLocalTestPath, page }) => {

diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts
@@ -8,7 +8,14 @@ import {
   startTransaction,
 } from '@sentry/node';
 import { extractTraceparentData, getActiveTransaction, hasTracingEnabled } from '@sentry/tracing';
-import { addExceptionMechanism, fill, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils';
+import {
+  addExceptionMechanism,
+  fill,
+  isString,
+  logger,
+  parseBaggageString,
+  stripUrlQueryAndFragment,
+} from '@sentry/utils';
 import * as domain from 'domain';
 import * as http from 'http';
 import { default as createNextServer } from 'next';
@@ -252,6 +259,9 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
             IS_DEBUG_BUILD && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`);
           }
 
+          const baggage =
+            nextReq.headers && isString(nextReq.headers.baggage) && parseBaggageString(nextReq.headers.baggage);
+
           // pull off query string, if any
           const reqPath = stripUrlQueryAndFragment(nextReq.url);
 
@@ -265,6 +275,7 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
               op: 'http.server',
               metadata: { requestPath: reqPath },
               ...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

diff --git a/packages/nextjs/src/utils/withSentry.ts b/packages/nextjs/src/utils/withSentry.ts
@@ -1,7 +1,14 @@
 import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node';
 import { extractTraceparentData, hasTracingEnabled } from '@sentry/tracing';
 import { Transaction } from '@sentry/types';
-import { addExceptionMechanism, isString, logger, objectify, stripUrlQueryAndFragment } from '@sentry/utils';
+import {
+  addExceptionMechanism,
+  isString,
+  logger,
+  objectify,
+  parseBaggageString,
+  stripUrlQueryAndFragment,
+} from '@sentry/utils';
 import * as domain from 'domain';
 import { NextApiHandler, NextApiRequest, NextApiResponse } from 'next';
 
@@ -48,6 +55,8 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler =
             IS_DEBUG_BUILD && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`);
           }
 
+          const baggage = req.headers && isString(req.headers.baggage) && parseBaggageString(req.headers.baggage);
+
           const url = `${req.url}`;
           // pull off query string, if any
           let reqPath = stripUrlQueryAndFragment(url);
@@ -66,6 +75,7 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler =
               name: `${reqMethod}${reqPath}`,
               op: 'http.server',
               ...traceparentData,
+              ...(baggage && { metadata: { baggage: baggage } }),
             },
             // extra context passed to the `tracesSampler`
             { request: req },

diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts
@@ -0,0 +1,52 @@
+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 of an outgoing request.', 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: expect.stringContaining('foo=bar,bar=baz'),
+    },
+  });
+});
+
+test('Should assign `baggage` header which contains sentry trace baggage data of 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=1.0.0,sentry-environment=production',
+  })) as TestAPIResponse;
+
+  expect(response).toBeDefined();
+  expect(response).toMatchObject({
+    test_data: {
+      host: 'somewhere.not.sentry',
+      baggage: expect.stringContaining('sentry-version=1.0.0,sentry-environment=production'),
+    },
+  });
+});
+
+test('Should assign `baggage` header which contains sentry and 3rd party trace baggage data of 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=1.0.0,sentry-environment=production,dogs=great',
+  })) as TestAPIResponse;
+
+  expect(response).toBeDefined();
+  expect(response).toMatchObject({
+    test_data: {
+      host: 'somewhere.not.sentry',
+      baggage: expect.stringContaining('dogs=great,sentry-version=1.0.0,sentry-environment=production'),
+    },
+  });
+});
diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts
@@ -0,0 +1,19 @@
+import * as path from 'path';
+
+import { getAPIResponse, runServer } from '../../../../utils/index';
+import { TestAPIResponse } from '../server';
+
+test('should attach a `baggage` header to an outgoing request.', 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',
+      // TODO this is currently still empty but eventually it should contain sentry data
+      baggage: expect.stringMatching(''),
+    },
+  });
+});
diff --git a/packages/node-integration-tests/suites/express/sentry-trace/server.ts b/packages/node-integration-tests/suites/express/sentry-trace/server.ts
@@ -6,7 +6,7 @@ import http from 'http';
 
 const app = express();
 
-export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string } };
+export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } };
 
 Sentry.init({
   dsn: 'https://[email protected]/1337',

diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts
@@ -8,6 +8,7 @@ import {
   isString,
   logger,
   normalize,
+  parseBaggageString,
   stripUrlQueryAndFragment,
 } from '@sentry/utils';
 import * as cookie from 'cookie';
@@ -61,16 +62,16 @@ export function tracingHandler(): (
     next: (error?: any) => void,
   ): void {
     // If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision)
-    let traceparentData;
-    if (req.headers && isString(req.headers['sentry-trace'])) {
-      traceparentData = extractTraceparentData(req.headers['sentry-trace']);
-    }
+    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 transaction = startTransaction(
       {
         name: extractExpressTransactionName(req, { path: true, method: true }),
         op: 'http.server',
         ...traceparentData,
+        ...(baggage && { metadata: { baggage: baggage } }),
       },
       // extra context passed to the tracesSampler
       { request: extractRequestData(req) },

diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts
@@ -1,6 +1,6 @@
 import { getCurrentHub } from '@sentry/core';
 import { Integration, Span } from '@sentry/types';
-import { fill, logger, parseSemver } from '@sentry/utils';
+import { fill, logger, mergeAndSerializeBaggage, parseSemver } from '@sentry/utils';
 import * as http from 'http';
 import * as https from 'https';
 
@@ -123,7 +123,14 @@ function _createWrappedRequestMethodFactory(
             logger.log(
               `[Tracing] Adding sentry-trace header ${sentryTraceHeader} to outgoing request to ${requestUrl}: `,
             );
-          requestOptions.headers = { ...requestOptions.headers, 'sentry-trace': sentryTraceHeader };
+
+          const headerBaggageString = requestOptions.headers && (requestOptions.headers.baggage as string);
+
+          requestOptions.headers = {
+            ...requestOptions.headers,
+            'sentry-trace': sentryTraceHeader,
+            baggage: mergeAndSerializeBaggage(span.getBaggage(), headerBaggageString),
+          };
         }
       }
 

diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts
@@ -2,7 +2,7 @@ import * as sentryCore from '@sentry/core';
 import * as sentryHub from '@sentry/hub';
 import { Hub } from '@sentry/hub';
 import { Transaction } from '@sentry/tracing';
-import { Runtime } from '@sentry/types';
+import { Baggage, Runtime } from '@sentry/types';
 import { SentryError } from '@sentry/utils';
 import * as http from 'http';
 import * as net from 'net';
@@ -368,6 +368,41 @@ describe('tracingHandler', () => {
     expect(transaction.traceId).toEqual('12312012123120121231201212312012');
     expect(transaction.parentSpanId).toEqual('1121201211212012');
     expect(transaction.sampled).toEqual(false);
+    expect(transaction.metadata?.baggage).toBeUndefined();
+  });
+
+  it("pulls parent's data from tracing and baggage headers on the request", () => {
+    req.headers = {
+      'sentry-trace': '12312012123120121231201212312012-1121201211212012-0',
+      baggage: 'sentry-version=1.0,sentry-environment=production',
+    };
+
+    sentryTracingMiddleware(req, res, next);
+
+    const transaction = (res as any).__sentry_transaction;
+
+    // since we have no tracesSampler defined, the default behavior (inherit if possible) applies
+    expect(transaction.traceId).toEqual('12312012123120121231201212312012');
+    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);
+  });
+
+  it("pulls parent's baggage (sentry + third party entries) headers on the request", () => {
+    req.headers = {
+      baggage: 'sentry-version=1.0,sentry-environment=production,dogs=great,cats=boring',
+    };
+
+    sentryTracingMiddleware(req, res, next);
+
+    const transaction = (res as any).__sentry_transaction;
+
+    expect(transaction.metadata?.baggage).toBeDefined();
+    expect(transaction.metadata?.baggage).toEqual([
+      { version: '1.0', environment: 'production' },
+      'dogs=great,cats=boring',
+    ] as Baggage);
   });
 
   it('extracts request data for sampling context', () => {

diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts
@@ -86,6 +86,43 @@ describe('tracing', () => {
 
     expect(sentryTraceHeader).not.toBeDefined();
   });
+
+  it('attaches the baggage header to outgoing non-sentry requests', async () => {
+    nock('http://dogs.are.great').get('/').reply(200);
+
+    createTransactionOnScope();
+
+    const request = http.get('http://dogs.are.great/');
+    const baggageHeader = request.getHeader('baggage') as string;
+
+    expect(baggageHeader).toBeDefined();
+    // this might change once we actually add our baggage data to the header
+    expect(baggageHeader).toEqual('');
+  });
+
+  it('propagates 3rd party baggage header data to outgoing non-sentry requests', async () => {
+    nock('http://dogs.are.great').get('/').reply(200);
+
+    createTransactionOnScope();
+
+    const request = http.get({ host: 'http://dogs.are.great/', headers: { baggage: 'dog=great' } });
+    const baggageHeader = request.getHeader('baggage') as string;
+
+    expect(baggageHeader).toBeDefined();
+    // this might change once we actually add our baggage data to the header
+    expect(baggageHeader).toEqual('dog=great');
+  });
+
+  it("doesn't attach the sentry-trace header to outgoing sentry requests", () => {
+    nock('http://squirrelchasers.ingest.sentry.io').get('/api/12312012/store/').reply(200);
+
+    createTransactionOnScope();
+
+    const request = http.get('http://squirrelchasers.ingest.sentry.io/api/12312012/store/');
+    const baggage = request.getHeader('baggage');
+
+    expect(baggage).not.toBeDefined();
+  });
 });
 
 describe('default protocols', () => {

diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts
@@ -11,7 +11,7 @@ import {
 } from '@sentry/node';
 import { extractTraceparentData } from '@sentry/tracing';
 import { Integration } from '@sentry/types';
-import { extensionRelayDSN, isString, logger } from '@sentry/utils';
+import { extensionRelayDSN, isString, logger, parseBaggageString } 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';
@@ -288,10 +288,17 @@ export function wrapHandler<TEvent, TResult>(
     if (eventWithHeaders.headers && isString(eventWithHeaders.headers['sentry-trace'])) {
       traceparentData = extractTraceparentData(eventWithHeaders.headers['sentry-trace']);
     }
+
+    const baggage =
+      eventWithHeaders.headers &&
+      isString(eventWithHeaders.headers.baggage) &&
+      parseBaggageString(eventWithHeaders.headers.baggage);
+
     const transaction = startTransaction({
       name: context.functionName,
       op: 'awslambda.handler',
       ...traceparentData,
+      ...(baggage && { metadata: { baggage: baggage } }),
     });
 
     const hub = getCurrentHub();