diff --git a/MIGRATION.md b/MIGRATION.md index d2d1695719b4..d6aad039ecaf 100644 --- 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 index 72060cb81364..13dda0e276d4 100644 --- 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 index 0afff8864522..60c6c062c7f3 100644 --- a/packages/integration-tests/suites/tracing/browsertracing/meta/template.html +++ b/packages/integration-tests/suites/tracing/browsertracing/meta/template.html @@ -2,5 +2,6 @@ + diff --git a/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts b/packages/integration-tests/suites/tracing/browsertracing/meta/test.ts index 263fd9257186..d2d44ae74050 100644 --- 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` ', @@ -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` tag and propagate the content in transaction', + async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const envHeader = await getFirstSentryEnvelopeRequest(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` ", async ({ getLocalTestPath, page }) => { diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts index cba8e0ab9423..bf4adc9ec8b3 100644 --- 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 index 00e76ec40b69..742d9f4c9280 100644 --- 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 new file mode 100644 index 000000000000..81dddb7c1f97 --- /dev/null +++ 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 new file mode 100644 index 000000000000..70b6cc19edee --- /dev/null +++ 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 index e93b50f18cd6..632d9b8338fb 100644 --- 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://public@dsn.ingest.sentry.io/1337', diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index 451ef14e9223..27dbeb3fc9cf 100644 --- 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 index 19a4e6ba1423..074c81ffe842 100644 --- 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 index 57f6528139f9..8cee24f678fd 100644 --- 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 index ff0af8b65acf..50d26e92452a 100644 --- 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 index 5e00e3419153..a2825b1e1e2e 100644 --- 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( 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(); diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index a1eec1e48736..26c32b4a21cb 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/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, stripUrlQueryAndFragment } from '@sentry/utils'; +import { isString, logger, parseBaggageString, stripUrlQueryAndFragment } from '@sentry/utils'; import { IS_DEBUG_BUILD } from '../flags'; import { domainify, getActiveDomain, proxyFunction } from './../utils'; @@ -56,10 +56,17 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial { await wrappedHandler(fakeEvent, fakeContext, fakeCallback); }); + test('incoming trace headers are correctly parsed and used', async () => { + expect.assertions(1); + + fakeEvent.headers = { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', + baggage: 'sentry-release=2.12.1,maisey=silly,charlie=goofy', + }; + + const handler: Handler = (_event, _context, callback) => { + expect(Sentry.startTransaction).toBeCalledWith( + expect.objectContaining({ + parentSpanId: '1121201211212012', + parentSampled: false, + op: 'awslambda.handler', + name: 'functionName', + traceId: '12312012123120121231201212312012', + metadata: { + baggage: [ + { + release: '2.12.1', + }, + 'maisey=silly,charlie=goofy', + ], + }, + }), + ); + + callback(undefined, { its: 'fine' }); + }; + + const wrappedHandler = wrapHandler(handler); + await wrappedHandler(fakeEvent, fakeContext, fakeCallback); + }); + test('capture error', async () => { expect.assertions(10); diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts index 5072de8b17b7..89b6aa084b9b 100644 --- a/packages/serverless/test/gcpfunction.test.ts +++ b/packages/serverless/test/gcpfunction.test.ts @@ -120,6 +120,39 @@ describe('GCPFunction', () => { expect(Sentry.flush).toBeCalledWith(2000); }); + test('incoming trace headers are correctly parsed and used', async () => { + expect.assertions(1); + + const handler: HttpFunction = (_req, res) => { + res.statusCode = 200; + res.end(); + }; + const wrappedHandler = wrapHttpFunction(handler); + const traceHeaders = { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', + baggage: 'sentry-release=2.12.1,maisey=silly,charlie=goofy', + }; + await handleHttp(wrappedHandler, traceHeaders); + + expect(Sentry.startTransaction).toBeCalledWith( + expect.objectContaining({ + name: 'POST /path', + op: 'gcp.function.http', + traceId: '12312012123120121231201212312012', + parentSpanId: '1121201211212012', + parentSampled: false, + metadata: { + baggage: [ + { + release: '2.12.1', + }, + 'maisey=silly,charlie=goofy', + ], + }, + }), + ); + }); + test('capture error', async () => { expect.assertions(5); diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 780edf9052d4..0d8e24de8c92 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -1,6 +1,6 @@ import { Hub } from '@sentry/hub'; import { EventProcessor, Integration, Transaction, TransactionContext } from '@sentry/types'; -import { getGlobalObject, logger } from '@sentry/utils'; +import { getGlobalObject, logger, parseBaggageString } from '@sentry/utils'; import { IS_DEBUG_BUILD } from '../flags'; import { startIdleTransaction } from '../hubextensions'; @@ -204,7 +204,7 @@ export class BrowserTracing implements Integration { // eslint-disable-next-line @typescript-eslint/unbound-method const { beforeNavigate, idleTimeout, finalTimeout } = this.options; - const parentContextFromHeader = context.op === 'pageload' ? getHeaderContext() : undefined; + const parentContextFromHeader = context.op === 'pageload' ? extractTraceDataFromMetaTags() : undefined; const expandedContext = { ...context, @@ -243,14 +243,22 @@ export class BrowserTracing implements Integration { } /** - * Gets transaction context from a sentry-trace meta. - * - * @returns Transaction context data from the header or undefined if there's no header or the header is malformed + * Gets transaction context data from `sentry-trace` and `baggage` tags. + * @returns Transaction context data or undefined neither tag exists or has valid data */ -export function getHeaderContext(): Partial | undefined { - const header = getMetaContent('sentry-trace'); - if (header) { - return extractTraceparentData(header); +export function extractTraceDataFromMetaTags(): Partial | undefined { + const sentrytraceValue = getMetaContent('sentry-trace'); + const baggageValue = getMetaContent('baggage'); + + const sentrytraceData = sentrytraceValue ? extractTraceparentData(sentrytraceValue) : undefined; + const baggage = baggageValue ? parseBaggageString(baggageValue) : undefined; + + // TODO more extensive checks for baggage validity/emptyness? + if (sentrytraceData || baggage) { + return { + ...(sentrytraceData && sentrytraceData), + ...(baggage && { metadata: { baggage } }), + }; } return undefined; diff --git a/packages/tracing/src/browser/request.ts b/packages/tracing/src/browser/request.ts index 9a71a7387bed..b45775edd05f 100644 --- a/packages/tracing/src/browser/request.ts +++ b/packages/tracing/src/browser/request.ts @@ -1,4 +1,11 @@ -import { addInstrumentationHandler, isInstanceOf, isMatchingPattern } from '@sentry/utils'; +/* eslint-disable max-lines */ +import { + addInstrumentationHandler, + BAGGAGE_HEADER_NAME, + isInstanceOf, + isMatchingPattern, + mergeAndSerializeBaggage, +} from '@sentry/utils'; import { Span } from '../span'; import { getActiveTransaction, hasTracingEnabled } from '../utils'; @@ -70,12 +77,24 @@ export interface XHRData { }; __sentry_xhr_span_id__?: string; setRequestHeader?: (key: string, val: string) => void; + getRequestHeader?: (key: string) => string; __sentry_own_request__?: boolean; }; startTimestamp: number; endTimestamp?: number; } +type PolymorphicRequestHeaders = + | Record + | Array<[string, string]> + // the below is not preicsely the Header type used in Request, but it'll pass duck-typing + | { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [key: string]: any; + append: (key: string, value: string) => void; + get: (key: string) => string; + }; + export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions = { traceFetch: true, traceXHR: true, @@ -179,25 +198,48 @@ export function fetchCallback( const request = (handlerData.args[0] = handlerData.args[0] as string | Request); // eslint-disable-next-line @typescript-eslint/no-explicit-any const options = (handlerData.args[1] = (handlerData.args[1] as { [key: string]: any }) || {}); - let headers = options.headers; - if (isInstanceOf(request, Request)) { - headers = (request as Request).headers; - } - if (headers) { + options.headers = addTracingHeaders(request, span, options); + } +} + +function addTracingHeaders( + request: string | Request, + span: Span, + options: { [key: string]: any }, +): PolymorphicRequestHeaders { + let headers = options.headers; + + if (isInstanceOf(request, Request)) { + headers = (request as Request).headers; + } + const incomingBaggage = span.getBaggage(); + + if (headers) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + if (typeof headers.append === 'function') { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (typeof headers.append === 'function') { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - headers.append('sentry-trace', span.toTraceparent()); - } else if (Array.isArray(headers)) { - headers = [...headers, ['sentry-trace', span.toTraceparent()]]; - } else { - headers = { ...headers, 'sentry-trace': span.toTraceparent() }; - } + headers.append('sentry-trace', span.toTraceparent()); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + headers.append(BAGGAGE_HEADER_NAME, mergeAndSerializeBaggage(incomingBaggage, headers.get(BAGGAGE_HEADER_NAME))); + } else if (Array.isArray(headers)) { + const [, headerBaggageString] = headers.find(([key, _]) => key === BAGGAGE_HEADER_NAME); + headers = [ + ...headers, + ['sentry-trace', span.toTraceparent()], + [BAGGAGE_HEADER_NAME, mergeAndSerializeBaggage(incomingBaggage, headerBaggageString)], + ]; } else { - headers = { 'sentry-trace': span.toTraceparent() }; + headers = { + ...headers, + 'sentry-trace': span.toTraceparent(), + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + baggage: mergeAndSerializeBaggage(incomingBaggage, headers.baggage), + }; } - options.headers = headers; + } else { + headers = { 'sentry-trace': span.toTraceparent(), baggage: mergeAndSerializeBaggage(incomingBaggage) }; } + return headers; } /** @@ -254,6 +296,14 @@ export function xhrCallback( if (handlerData.xhr.setRequestHeader) { try { handlerData.xhr.setRequestHeader('sentry-trace', span.toTraceparent()); + + const headerBaggageString = + handlerData.xhr.getRequestHeader && handlerData.xhr.getRequestHeader(BAGGAGE_HEADER_NAME); + + handlerData.xhr.setRequestHeader( + BAGGAGE_HEADER_NAME, + mergeAndSerializeBaggage(span.getBaggage(), headerBaggageString), + ); } catch (_) { // Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED. } diff --git a/packages/tracing/src/span.ts b/packages/tracing/src/span.ts index baece680f52c..68cba79eb7cc 100644 --- a/packages/tracing/src/span.ts +++ b/packages/tracing/src/span.ts @@ -1,5 +1,5 @@ /* eslint-disable max-lines */ -import { Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types'; +import { Baggage, Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types'; import { dropUndefinedKeys, timestampWithMs, uuid4 } from '@sentry/utils'; /** @@ -298,6 +298,13 @@ export class Span implements SpanInterface { }); } + /** + * @inheritdoc + */ + public getBaggage(): Baggage | undefined { + return this.transaction && this.transaction.metadata.baggage; + } + /** * @inheritDoc */ diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index 4bfc093fbe2b..0d5265522406 100644 --- a/packages/tracing/test/browser/browsertracing.test.ts +++ b/packages/tracing/test/browser/browsertracing.test.ts @@ -1,12 +1,13 @@ import { BrowserClient } from '@sentry/browser'; import { Hub, makeMain } from '@sentry/hub'; +import { BaggageObj } from '@sentry/types'; import { getGlobalObject, InstrumentHandlerCallback, InstrumentHandlerType } from '@sentry/utils'; import { JSDOM } from 'jsdom'; import { BrowserTracing, BrowserTracingOptions, - getHeaderContext, + extractTraceDataFromMetaTags, getMetaContent, } from '../../src/browser/browsertracing'; import { MetricsInstrumentation } from '../../src/browser/metrics'; @@ -215,7 +216,8 @@ describe('BrowserTracing', () => { it('sets transaction context from sentry-trace header', () => { const name = 'sentry-trace'; const content = '126de09502ae4e0fb26c6967190756a4-b6e54397b12a2a0f-1'; - document.head.innerHTML = ``; + document.head.innerHTML = + `` + ''; const startIdleTransaction = jest.spyOn(hubExtensions, 'startIdleTransaction'); createBrowserTracing(true, { routingInstrumentation: customInstrumentRouting }); @@ -226,6 +228,9 @@ describe('BrowserTracing', () => { traceId: '126de09502ae4e0fb26c6967190756a4', parentSpanId: 'b6e54397b12a2a0f', parentSampled: true, + metadata: { + baggage: [{ release: '2.1.14' }, 'foo=bar'], + }, }), expect.any(Number), expect.any(Number), @@ -322,7 +327,7 @@ describe('BrowserTracing', () => { }); }); - describe('sentry-trace element', () => { + describe('sentry-trace and baggage elements', () => { describe('getMetaContent', () => { it('finds the specified tag and extracts the value', () => { const name = 'sentry-trace'; @@ -352,12 +357,12 @@ describe('BrowserTracing', () => { }); }); - describe('getHeaderContext', () => { + describe('extractTraceDataFromMetaTags()', () => { it('correctly parses a valid sentry-trace meta header', () => { document.head.innerHTML = ''; - const headerContext = getHeaderContext(); + const headerContext = extractTraceDataFromMetaTags(); expect(headerContext).toBeDefined(); expect(headerContext!.traceId).toEqual('12312012123120121231201212312012'); @@ -365,54 +370,93 @@ describe('BrowserTracing', () => { expect(headerContext!.parentSampled).toEqual(false); }); - it('returns undefined if the header is malformed', () => { + it('correctly parses a valid baggage meta header', () => { + document.head.innerHTML = ''; + + const headerContext = extractTraceDataFromMetaTags(); + + expect(headerContext).toBeDefined(); + expect(headerContext?.metadata?.baggage).toBeDefined(); + const baggage = headerContext?.metadata?.baggage; + expect(baggage && baggage[0]).toBeDefined(); + expect(baggage && baggage[0]).toEqual({ + release: '2.1.12', + } as BaggageObj); + expect(baggage && baggage[1]).toBeDefined(); + expect(baggage && baggage[1]).toEqual('foo=bar'); + }); + + it('returns undefined if the sentry-trace header is malformed', () => { document.head.innerHTML = ''; - const headerContext = getHeaderContext(); + const headerContext = extractTraceDataFromMetaTags(); expect(headerContext).toBeUndefined(); }); + it('does not crash if the baggage header is malformed', () => { + document.head.innerHTML = ''; + + const headerContext = extractTraceDataFromMetaTags(); + + // TODO currently this creates invalid baggage. This must be adressed in a follow-up PR + expect(headerContext).toBeDefined(); + expect(headerContext?.metadata?.baggage).toBeDefined(); + const baggage = headerContext?.metadata?.baggage; + expect(baggage && baggage[0]).toBeDefined(); + expect(baggage && baggage[1]).toBeDefined(); + }); + it("returns undefined if the header isn't there", () => { document.head.innerHTML = ''; - const headerContext = getHeaderContext(); + const headerContext = extractTraceDataFromMetaTags(); expect(headerContext).toBeUndefined(); }); }); describe('using the data', () => { - it('uses the data for pageload transactions', () => { + it('uses the tracing data for pageload transactions', () => { // make sampled false here, so we can see that it's being used rather than the tracesSampleRate-dictated one document.head.innerHTML = - ''; + '' + + ''; // pageload transactions are created as part of the BrowserTracing integration's initialization createBrowserTracing(true); const transaction = getActiveTransaction(hub) as IdleTransaction; + const baggage = transaction.getBaggage()!; expect(transaction).toBeDefined(); expect(transaction.op).toBe('pageload'); expect(transaction.traceId).toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toEqual('1121201211212012'); expect(transaction.sampled).toBe(false); + expect(baggage).toBeDefined(); + expect(baggage[0]).toBeDefined(); + expect(baggage[0]).toEqual({ release: '2.1.14' }); + expect(baggage[1]).toBeDefined(); + expect(baggage[1]).toEqual('foo=bar'); }); it('ignores the data for navigation transactions', () => { mockChangeHistory = () => undefined; document.head.innerHTML = - ''; + '' + + ''; createBrowserTracing(true); mockChangeHistory({ to: 'here', from: 'there' }); const transaction = getActiveTransaction(hub) as IdleTransaction; + const baggage = transaction.getBaggage()!; expect(transaction).toBeDefined(); expect(transaction.op).toBe('navigation'); expect(transaction.traceId).not.toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toBeUndefined(); + expect(baggage).toBeUndefined(); }); }); }); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 6f9e3342b779..3032d7d6cdec 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -19,6 +19,7 @@ export type { EventEnvelope, EventEnvelopeHeaders, EventItem, + EventTraceContext, SessionEnvelope, SessionItem, UserFeedbackItem, diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index d26887a93f2a..120d754cc329 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -1,3 +1,4 @@ +import { Baggage } from './baggage'; import { Primitive } from './misc'; import { Transaction } from './transaction'; @@ -174,4 +175,7 @@ export interface Span extends SpanContext { timestamp?: number; trace_id: string; }; + + /** return the baggage for dynamic sampling and trace propagation */ + getBaggage(): Baggage | undefined; } diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts index 95f8b447e19a..b87fc2b151cf 100644 --- a/packages/types/src/transaction.ts +++ b/packages/types/src/transaction.ts @@ -1,6 +1,6 @@ +import { Baggage } from './baggage'; import { ExtractedNodeRequestData, Primitive, WorkerLocation } from './misc'; import { Span, SpanContext } from './span'; - /** * Interface holding Transaction-specific properties */ @@ -131,11 +131,8 @@ export type TransactionSamplingMethod = 'explicitly_set' | 'client_sampler' | 'c export interface TransactionMetadata { transactionSampling?: { rate?: number; method: TransactionSamplingMethod }; - /** The two halves (sentry and third-party) of a transaction's tracestate header, used for dynamic sampling */ - tracestate?: { - sentry?: string; - thirdparty?: string; - }; + /** The baggage object of a transaction's baggage header, used for dynamic sampling */ + baggage?: Baggage; /** For transactions tracing server-side request handling, the path of the request being tracked. */ requestPath?: string; diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts index ddf3b9b24afa..bc2f6568ff1f 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -41,6 +41,14 @@ export function getSentryBaggageItems(baggage: Baggage): BaggageObj { return baggage[0]; } +/** + * Returns 3rd party baggage string of @param baggage + * @param baggage + */ +export function getThirdPartyBaggage(baggage: Baggage): string { + return baggage[1]; +} + /** Serialize a baggage object */ export function serializeBaggage(baggage: Baggage): string { return Object.keys(baggage[0]).reduce((prev, key: keyof BaggageObj) => { @@ -78,3 +86,32 @@ export function parseBaggageString(inputBaggageString: string): Baggage { [{}, ''], ); } + +/** + * Merges the baggage header we saved from the incoming request (or meta tag) with + * a possibly created or modified baggage header by a third party that's been added + * to the outgoing request header. + * + * In case @param headerBaggageString exists, we can safely add the the 3rd party part of @param headerBaggage + * with our @param incomingBaggage. This is possible because if we modified anything beforehand, + * it would only affect parts of the sentry baggage (@see Baggage interface). + * + * @param incomingBaggage the baggage header of the incoming request that might contain sentry entries + * @param headerBaggageString possibly existing baggage header string added from a third party to request headers + * + * @return a merged and serialized baggage string to be propagated with the outgoing request + */ +export function mergeAndSerializeBaggage(incomingBaggage?: Baggage, headerBaggageString?: string): string { + if (!incomingBaggage && !headerBaggageString) { + return ''; + } + + const headerBaggage = (headerBaggageString && parseBaggageString(headerBaggageString)) || undefined; + const thirdPartyHeaderBaggage = headerBaggage && getThirdPartyBaggage(headerBaggage); + + const finalBaggage = createBaggage( + (incomingBaggage && incomingBaggage[0]) || {}, + thirdPartyHeaderBaggage || (incomingBaggage && incomingBaggage[1]) || '', + ); + return serializeBaggage(finalBaggage); +} diff --git a/packages/utils/test/baggage.test.ts b/packages/utils/test/baggage.test.ts index e1c91a87f5ee..07ccdaaedd48 100644 --- a/packages/utils/test/baggage.test.ts +++ b/packages/utils/test/baggage.test.ts @@ -2,6 +2,7 @@ import { createBaggage, getBaggageValue, isBaggageEmpty, + mergeAndSerializeBaggage, parseBaggageString, serializeBaggage, setBaggageValue, @@ -105,4 +106,27 @@ describe('Baggage', () => { expect(isBaggageEmpty(baggage)).toEqual(outcome); }); }); + + describe('mergeAndSerializeBaggage', () => { + it.each([ + [ + 'returns original baggage when there is no additional baggage', + createBaggage({ release: '1.1.1', userid: '1234' }, 'foo=bar'), + undefined, + 'foo=bar,sentry-release=1.1.1,sentry-userid=1234', + ], + [ + 'returns merged baggage when there is a 3rd party header added', + createBaggage({ release: '1.1.1', userid: '1234' }, 'foo=bar'), + 'bar=baz,key=value', + 'bar=baz,key=value,sentry-release=1.1.1,sentry-userid=1234', + ], + ['returns merged baggage original baggage is empty', createBaggage({}), 'bar=baz,key=value', 'bar=baz,key=value'], + ['returns empty string when original and 3rd party baggage are empty', createBaggage({}), '', ''], + ['returns merged baggage original baggage is undefined', undefined, 'bar=baz,key=value', 'bar=baz,key=value'], + ['returns empty string when both params are undefined', undefined, undefined, ''], + ])('%s', (_: string, baggage, headerBaggageString, outcome) => { + expect(mergeAndSerializeBaggage(baggage, headerBaggageString)).toEqual(outcome); + }); + }); });