Skip to content

feat(tracing): Add empty baggage header propagation to outgoing requests #5133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
May 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions packages/core/test/lib/envelope.test.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Original file line number Diff line number Diff line change
@@ -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>',
Expand All @@ -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 }) => {
Expand Down
13 changes: 12 additions & 1 deletion packages/nextjs/src/utils/instrumentServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand All @@ -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
Expand Down
12 changes: 11 additions & 1 deletion packages/nextjs/src/utils/withSentry.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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);
Expand All @@ -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 },
Expand Down
Original file line number Diff line number Diff line change
@@ -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'),
},
});
});
Original file line number Diff line number Diff line change
@@ -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(''),
},
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
9 changes: 5 additions & 4 deletions packages/node/src/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
isString,
logger,
normalize,
parseBaggageString,
stripUrlQueryAndFragment,
} from '@sentry/utils';
import * as cookie from 'cookie';
Expand Down Expand Up @@ -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) },
Expand Down
11 changes: 9 additions & 2 deletions packages/node/src/integrations/http.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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),
};
}
}

Expand Down
37 changes: 36 additions & 1 deletion packages/node/test/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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', () => {
Expand Down
37 changes: 37 additions & 0 deletions packages/node/test/integrations/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
9 changes: 8 additions & 1 deletion packages/serverless/src/awslambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
Expand Down
Loading