Skip to content

Commit d48711f

Browse files
Lms24Luca Forstner
Lms24
and
Luca Forstner
authored
feat(tracing): Add empty baggage header propagation to outgoing requests (#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 <luca.forstner@sentry.io>
1 parent 9759e27 commit d48711f

File tree

26 files changed

+507
-59
lines changed

26 files changed

+507
-59
lines changed

‎MIGRATION.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ Below we will outline all the breaking changes you should consider when upgradin
1010
- 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.
1111
- `whitelistUrls` and `blacklistUrls` have been renamed to `allowUrls` and `denyUrls` in the `Sentry.init()` options.
1212
- The `UserAgent` integration is now called `HttpContext`.
13+
- If you are using Performance Monitoring and with tracing enabled, you might have to [make adjustments to
14+
your server's CORS settings](#-propagation-of-baggage-header)
1315

1416
## Dropping Support for Node.js v6
1517

@@ -319,6 +321,13 @@ session.update({ environment: 'prod' });
319321
session.close('ok');
320322
```
321323

324+
## Propagation of Baggage Header
325+
326+
We introduced a new way of propagating tracing and transaction-related information between services. This
327+
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
328+
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)
329+
for more in-depth instructions what to change.
330+
322331
## General API Changes
323332

324333
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:

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { DsnComponents, Event } from '@sentry/types';
2-
import { EventTraceContext } from '@sentry/types/build/types/envelope';
1+
import { DsnComponents, Event, EventTraceContext } from '@sentry/types';
32

43
import { createEventEnvelope } from '../../src/envelope';
54

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,6 @@
22
<head>
33
<meta charset="utf-8" />
44
<meta name="sentry-trace" content="12312012123120121231201212312012-1121201211212012-1" />
5+
<meta name="baggage" content="sentry-version=2.1.12" />
56
</head>
67
</html>

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { expect } from '@playwright/test';
2-
import { Event } from '@sentry/types';
2+
import { Event, EventEnvelopeHeaders } from '@sentry/types';
33

44
import { sentryTest } from '../../../../utils/fixtures';
5-
import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
5+
import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
66

77
sentryTest(
88
'should create a pageload transaction based on `sentry-trace` <meta>',
@@ -21,6 +21,20 @@ sentryTest(
2121
},
2222
);
2323

24+
// TODO this we can't really test until we actually propagate sentry- entries in baggage
25+
// skipping for now but this must be adjusted later on
26+
sentryTest.skip(
27+
'should pick up `baggage` <meta> tag and propagate the content in transaction',
28+
async ({ getLocalTestPath, page }) => {
29+
const url = await getLocalTestPath({ testDir: __dirname });
30+
31+
const envHeader = await getFirstSentryEnvelopeRequest<EventEnvelopeHeaders>(page, url, envelopeHeaderRequestParser);
32+
33+
expect(envHeader.trace).toBeDefined();
34+
expect(envHeader.trace).toEqual('{version:2.1.12}');
35+
},
36+
);
37+
2438
sentryTest(
2539
"should create a navigation that's not influenced by `sentry-trace` <meta>",
2640
async ({ getLocalTestPath, page }) => {

‎packages/nextjs/src/utils/instrumentServer.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,14 @@ import {
88
startTransaction,
99
} from '@sentry/node';
1010
import { extractTraceparentData, getActiveTransaction, hasTracingEnabled } from '@sentry/tracing';
11-
import { addExceptionMechanism, fill, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils';
11+
import {
12+
addExceptionMechanism,
13+
fill,
14+
isString,
15+
logger,
16+
parseBaggageString,
17+
stripUrlQueryAndFragment,
18+
} from '@sentry/utils';
1219
import * as domain from 'domain';
1320
import * as http from 'http';
1421
import { default as createNextServer } from 'next';
@@ -252,6 +259,9 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
252259
IS_DEBUG_BUILD && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`);
253260
}
254261

262+
const baggage =
263+
nextReq.headers && isString(nextReq.headers.baggage) && parseBaggageString(nextReq.headers.baggage);
264+
255265
// pull off query string, if any
256266
const reqPath = stripUrlQueryAndFragment(nextReq.url);
257267

@@ -265,6 +275,7 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
265275
op: 'http.server',
266276
metadata: { requestPath: reqPath },
267277
...traceparentData,
278+
...(baggage && { metadata: { baggage: baggage } }),
268279
},
269280
// Extra context passed to the `tracesSampler` (Note: We're combining `nextReq` and `req` this way in order
270281
// to not break people's `tracesSampler` functions, even though the format of `nextReq` has changed (see

‎packages/nextjs/src/utils/withSentry.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node';
22
import { extractTraceparentData, hasTracingEnabled } from '@sentry/tracing';
33
import { Transaction } from '@sentry/types';
4-
import { addExceptionMechanism, isString, logger, objectify, stripUrlQueryAndFragment } from '@sentry/utils';
4+
import {
5+
addExceptionMechanism,
6+
isString,
7+
logger,
8+
objectify,
9+
parseBaggageString,
10+
stripUrlQueryAndFragment,
11+
} from '@sentry/utils';
512
import * as domain from 'domain';
613
import { NextApiHandler, NextApiRequest, NextApiResponse } from 'next';
714

@@ -48,6 +55,8 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler =
4855
IS_DEBUG_BUILD && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`);
4956
}
5057

58+
const baggage = req.headers && isString(req.headers.baggage) && parseBaggageString(req.headers.baggage);
59+
5160
const url = `${req.url}`;
5261
// pull off query string, if any
5362
let reqPath = stripUrlQueryAndFragment(url);
@@ -66,6 +75,7 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler =
6675
name: `${reqMethod}${reqPath}`,
6776
op: 'http.server',
6877
...traceparentData,
78+
...(baggage && { metadata: { baggage: baggage } }),
6979
},
7080
// extra context passed to the `tracesSampler`
7181
{ request: req },

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import * as path from 'path';
2+
3+
import { getAPIResponse, runServer } from '../../../../utils/index';
4+
import { TestAPIResponse } from '../server';
5+
6+
test('Should assign `baggage` header which contains 3rd party trace baggage data of an outgoing request.', async () => {
7+
const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
8+
9+
const response = (await getAPIResponse(new URL(`${url}/express`), {
10+
baggage: 'foo=bar,bar=baz',
11+
})) as TestAPIResponse;
12+
13+
expect(response).toBeDefined();
14+
expect(response).toMatchObject({
15+
test_data: {
16+
host: 'somewhere.not.sentry',
17+
baggage: expect.stringContaining('foo=bar,bar=baz'),
18+
},
19+
});
20+
});
21+
22+
test('Should assign `baggage` header which contains sentry trace baggage data of an outgoing request.', async () => {
23+
const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
24+
25+
const response = (await getAPIResponse(new URL(`${url}/express`), {
26+
baggage: 'sentry-version=1.0.0,sentry-environment=production',
27+
})) as TestAPIResponse;
28+
29+
expect(response).toBeDefined();
30+
expect(response).toMatchObject({
31+
test_data: {
32+
host: 'somewhere.not.sentry',
33+
baggage: expect.stringContaining('sentry-version=1.0.0,sentry-environment=production'),
34+
},
35+
});
36+
});
37+
38+
test('Should assign `baggage` header which contains sentry and 3rd party trace baggage data of an outgoing request.', async () => {
39+
const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
40+
41+
const response = (await getAPIResponse(new URL(`${url}/express`), {
42+
baggage: 'sentry-version=1.0.0,sentry-environment=production,dogs=great',
43+
})) as TestAPIResponse;
44+
45+
expect(response).toBeDefined();
46+
expect(response).toMatchObject({
47+
test_data: {
48+
host: 'somewhere.not.sentry',
49+
baggage: expect.stringContaining('dogs=great,sentry-version=1.0.0,sentry-environment=production'),
50+
},
51+
});
52+
});

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import * as path from 'path';
2+
3+
import { getAPIResponse, runServer } from '../../../../utils/index';
4+
import { TestAPIResponse } from '../server';
5+
6+
test('should attach a `baggage` header to an outgoing request.', async () => {
7+
const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
8+
9+
const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse;
10+
11+
expect(response).toBeDefined();
12+
expect(response).toMatchObject({
13+
test_data: {
14+
host: 'somewhere.not.sentry',
15+
// TODO this is currently still empty but eventually it should contain sentry data
16+
baggage: expect.stringMatching(''),
17+
},
18+
});
19+
});

‎packages/node-integration-tests/suites/express/sentry-trace/server.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import http from 'http';
66

77
const app = express();
88

9-
export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string } };
9+
export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } };
1010

1111
Sentry.init({
1212
dsn: 'https://public@dsn.ingest.sentry.io/1337',

‎packages/node/src/handlers.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
isString,
99
logger,
1010
normalize,
11+
parseBaggageString,
1112
stripUrlQueryAndFragment,
1213
} from '@sentry/utils';
1314
import * as cookie from 'cookie';
@@ -61,16 +62,16 @@ export function tracingHandler(): (
6162
next: (error?: any) => void,
6263
): void {
6364
// If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision)
64-
let traceparentData;
65-
if (req.headers && isString(req.headers['sentry-trace'])) {
66-
traceparentData = extractTraceparentData(req.headers['sentry-trace']);
67-
}
65+
const traceparentData =
66+
req.headers && isString(req.headers['sentry-trace']) && extractTraceparentData(req.headers['sentry-trace']);
67+
const baggage = req.headers && isString(req.headers.baggage) && parseBaggageString(req.headers.baggage);
6868

6969
const transaction = startTransaction(
7070
{
7171
name: extractExpressTransactionName(req, { path: true, method: true }),
7272
op: 'http.server',
7373
...traceparentData,
74+
...(baggage && { metadata: { baggage: baggage } }),
7475
},
7576
// extra context passed to the tracesSampler
7677
{ request: extractRequestData(req) },

0 commit comments

Comments
 (0)