From 347fbe4cc82b1e9398f7223227e0ff884662bdd3 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 15 Sep 2022 11:35:22 +0200 Subject: [PATCH] ref: Make dynamic sampling context mutable (#5710) This PR makes Dynamic Sampling Context in Head-SDKs mutable. List of changes: - Remove our `Baggage` abstraction (keeping track of foreign baggage and mutability is not necessary) - Replace `Baggage` abstraction with functions that convert `baggage` headers into DSC and vice-versa - Replace `getBaggage()` method on the `Transaction` class with `getDynamicSamplingContext()` which doesn't freeze DSC upon retrieval - Refactor the adding of Sentry `baggage` items so that we're (almost) always adding an additional independent `baggage` header, in addition to any that were already there. This was done to reduce code complexity and bundle size. An exception here is in the browser where there is a case where I don't know if adding an additional header might break things. - Explicitly handle the case of multiple incoming baggage headers. (if there are multiple items that are named the same, last one always wins) - Freeze DSC in the transaction creation if DSC was provided via `transactionContext.metadata.dynamicSamplingContext` - used in non-head-SDKs - Fix `extractTraceparentData` so that it returns `undefined` (ie. "invalid") when passed an empty string - also added a test for that. --- packages/core/src/envelope.ts | 9 +- packages/core/test/lib/envelope.test.ts | 31 +- .../wrappers/withSentryGetServerSideProps.ts | 6 +- .../withSentryServerSideAppGetInitialProps.ts | 7 +- ...ithSentryServerSideErrorGetInitialProps.ts | 6 +- .../withSentryServerSideGetInitialProps.ts | 6 +- .../src/config/wrappers/wrapperUtils.ts | 8 +- packages/nextjs/src/performance/client.ts | 5 +- packages/nextjs/src/utils/instrumentServer.ts | 8 +- packages/nextjs/src/utils/withSentry.ts | 11 +- .../nextjs/test/performance/client.test.ts | 4 +- packages/nextjs/test/utils/withSentry.test.ts | 1 - .../baggage-header-assign/test.ts | 6 +- .../test.ts | 14 +- .../baggage-other-vendors/test.ts | 2 +- .../node-integration-tests/utils/index.ts | 13 +- packages/node/src/handlers.ts | 12 +- packages/node/src/integrations/http.ts | 59 +++- packages/node/test/handlers.test.ts | 26 +- packages/node/test/integrations/http.test.ts | 26 +- packages/remix/src/utils/instrumentServer.ts | 14 +- packages/serverless/src/awslambda.ts | 12 +- packages/serverless/src/gcpfunction/http.ts | 12 +- packages/serverless/test/awslambda.test.ts | 24 +- packages/serverless/test/gcpfunction.test.ts | 14 +- .../tracing/src/browser/browsertracing.ts | 49 +-- packages/tracing/src/browser/request.ts | 122 +++++--- packages/tracing/src/transaction.ts | 96 +++--- .../test/browser/browsertracing.test.ts | 112 +------ packages/tracing/test/span.test.ts | 28 +- packages/tracing/test/utils.test.ts | 3 + packages/types/src/baggage.ts | 22 -- packages/types/src/index.ts | 1 - packages/types/src/transaction.ts | 13 +- packages/utils/src/baggage.ts | 255 ++++++--------- packages/utils/src/tracing.ts | 30 +- packages/utils/test/baggage.test.ts | 290 +++--------------- 37 files changed, 502 insertions(+), 855 deletions(-) delete mode 100644 packages/types/src/baggage.ts diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index a6f04defc86d..130dd6fafb23 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -1,7 +1,5 @@ import { - Baggage, DsnComponents, - DynamicSamplingContext, Event, EventEnvelope, EventEnvelopeHeaders, @@ -13,7 +11,7 @@ import { SessionEnvelope, SessionItem, } from '@sentry/types'; -import { createEnvelope, dropUndefinedKeys, dsnToString, getSentryBaggageItems } from '@sentry/utils'; +import { createEnvelope, dropUndefinedKeys, dsnToString } from '@sentry/utils'; /** Extract sdk info from from the API metadata */ function getSdkMetadataForEnvelopeHeader(metadata?: SdkMetadata): SdkInfo | undefined { @@ -101,8 +99,7 @@ function createEventEnvelopeHeaders( tunnel: string | undefined, dsn: DsnComponents, ): EventEnvelopeHeaders { - const baggage: Baggage | undefined = event.sdkProcessingMetadata && event.sdkProcessingMetadata.baggage; - const dynamicSamplingContext = baggage && getSentryBaggageItems(baggage); + const dynamicSamplingContext = event.sdkProcessingMetadata && event.sdkProcessingMetadata.dynamicSamplingContext; return { event_id: event.event_id as string, @@ -111,7 +108,7 @@ function createEventEnvelopeHeaders( ...(!!tunnel && { dsn: dsnToString(dsn) }), ...(event.type === 'transaction' && dynamicSamplingContext && { - trace: dropUndefinedKeys({ ...dynamicSamplingContext }) as DynamicSamplingContext, + trace: dropUndefinedKeys({ ...dynamicSamplingContext }), }), }; } diff --git a/packages/core/test/lib/envelope.test.ts b/packages/core/test/lib/envelope.test.ts index 8398b4831f53..ecd409b23041 100644 --- a/packages/core/test/lib/envelope.test.ts +++ b/packages/core/test/lib/envelope.test.ts @@ -20,7 +20,7 @@ describe('createEventEnvelope', () => { { type: 'transaction', sdkProcessingMetadata: { - baggage: [{ trace_id: '1234', public_key: 'pubKey123' }, '', false], + dynamicSamplingContext: { trace_id: '1234', public_key: 'pubKey123' }, }, }, { trace_id: '1234', public_key: 'pubKey123' }, @@ -30,7 +30,12 @@ describe('createEventEnvelope', () => { { type: 'transaction', sdkProcessingMetadata: { - baggage: [{ environment: 'prod', release: '1.0.0', public_key: 'pubKey123', trace_id: '1234' }, '', false], + dynamicSamplingContext: { + environment: 'prod', + release: '1.0.0', + public_key: 'pubKey123', + trace_id: '1234', + }, }, }, { release: '1.0.0', environment: 'prod', trace_id: '1234', public_key: 'pubKey123' }, @@ -40,19 +45,15 @@ describe('createEventEnvelope', () => { { type: 'transaction', sdkProcessingMetadata: { - baggage: [ - { - environment: 'prod', - release: '1.0.0', - transaction: 'TX', - user_segment: 'segmentA', - sample_rate: '0.95', - public_key: 'pubKey123', - trace_id: '1234', - }, - '', - false, - ], + dynamicSamplingContext: { + environment: 'prod', + release: '1.0.0', + transaction: 'TX', + user_segment: 'segmentA', + sample_rate: '0.95', + public_key: 'pubKey123', + trace_id: '1234', + }, }, }, { diff --git a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts index dee5ce918c45..700d1899d56b 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts @@ -1,5 +1,5 @@ import { hasTracingEnabled } from '@sentry/tracing'; -import { serializeBaggage } from '@sentry/utils'; +import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import { GetServerSideProps } from 'next'; import { isBuild } from '../../utils/isBuild'; @@ -45,7 +45,9 @@ export function withSentryGetServerSideProps( const requestTransaction = getTransactionFromRequest(req); if (requestTransaction) { serverSideProps.props._sentryTraceData = requestTransaction.toTraceparent(); - serverSideProps.props._sentryBaggage = serializeBaggage(requestTransaction.getBaggage()); + + const dynamicSamplingContext = requestTransaction.getDynamicSamplingContext(); + serverSideProps.props._sentryBaggage = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); } } diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts index 86cf92fb544e..74e811f8f224 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts @@ -1,5 +1,5 @@ import { hasTracingEnabled } from '@sentry/tracing'; -import { serializeBaggage } from '@sentry/utils'; +import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import App from 'next/app'; import { isBuild } from '../../utils/isBuild'; @@ -51,7 +51,10 @@ export function withSentryServerSideAppGetInitialProps(origAppGetInitialProps: A const requestTransaction = getTransactionFromRequest(req!); if (requestTransaction) { appGetInitialProps.pageProps._sentryTraceData = requestTransaction.toTraceparent(); - appGetInitialProps.pageProps._sentryBaggage = serializeBaggage(requestTransaction.getBaggage()); + + const dynamicSamplingContext = requestTransaction.getDynamicSamplingContext(); + appGetInitialProps.pageProps._sentryBaggage = + dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); } return appGetInitialProps; diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts index e7d4c7c5b46c..8950fcf720a0 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts @@ -1,5 +1,5 @@ import { hasTracingEnabled } from '@sentry/tracing'; -import { serializeBaggage } from '@sentry/utils'; +import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import { NextPageContext } from 'next'; import { ErrorProps } from 'next/error'; @@ -52,7 +52,9 @@ export function withSentryServerSideErrorGetInitialProps( const requestTransaction = getTransactionFromRequest(req!); if (requestTransaction) { errorGetInitialProps._sentryTraceData = requestTransaction.toTraceparent(); - errorGetInitialProps._sentryBaggage = serializeBaggage(requestTransaction.getBaggage()); + + const dynamicSamplingContext = requestTransaction.getDynamicSamplingContext(); + errorGetInitialProps._sentryBaggage = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); } return errorGetInitialProps; diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts index 8837ae121cf5..f591a4048c85 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts @@ -1,5 +1,5 @@ import { hasTracingEnabled } from '@sentry/tracing'; -import { serializeBaggage } from '@sentry/utils'; +import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import { NextPage } from 'next'; import { isBuild } from '../../utils/isBuild'; @@ -42,7 +42,9 @@ export function withSentryServerSideGetInitialProps(origGetInitialProps: GetInit const requestTransaction = getTransactionFromRequest(req!); if (requestTransaction) { initialProps._sentryTraceData = requestTransaction.toTraceparent(); - initialProps._sentryBaggage = serializeBaggage(requestTransaction.getBaggage()); + + const dynamicSamplingContext = requestTransaction.getDynamicSamplingContext(); + initialProps._sentryBaggage = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); } return initialProps; diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts index a7be6784dbf6..0b64d17acf9d 100644 --- a/packages/nextjs/src/config/wrappers/wrapperUtils.ts +++ b/packages/nextjs/src/config/wrappers/wrapperUtils.ts @@ -2,7 +2,7 @@ import { captureException, getCurrentHub, startTransaction } from '@sentry/core' import { addRequestDataToEvent } from '@sentry/node'; import { getActiveTransaction } from '@sentry/tracing'; import { Transaction } from '@sentry/types'; -import { extractTraceparentData, fill, isString, parseBaggageSetMutability } from '@sentry/utils'; +import { baggageHeaderToDynamicSamplingContext, extractTraceparentData, fill } from '@sentry/utils'; import * as domain from 'domain'; import { IncomingMessage, ServerResponse } from 'http'; @@ -88,11 +88,11 @@ export function callTracedServerSideDataFetcher Pr if (requestTransaction === undefined) { const sentryTraceHeader = req.headers['sentry-trace']; - const rawBaggageString = req.headers && isString(req.headers.baggage) && req.headers.baggage; + const rawBaggageString = req.headers && req.headers.baggage; const traceparentData = typeof sentryTraceHeader === 'string' ? extractTraceparentData(sentryTraceHeader) : undefined; - const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData); + const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(rawBaggageString); const newTransaction = startTransaction({ op: 'nextjs.data.server', @@ -100,7 +100,7 @@ export function callTracedServerSideDataFetcher Pr ...traceparentData, metadata: { source: 'route', - baggage, + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, }, }); diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts index 764d11e27fe8..fd7d7dadbf2a 100644 --- a/packages/nextjs/src/performance/client.ts +++ b/packages/nextjs/src/performance/client.ts @@ -1,10 +1,10 @@ import { getCurrentHub } from '@sentry/hub'; import { Primitive, TraceparentData, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; import { + baggageHeaderToDynamicSamplingContext, extractTraceparentData, getGlobalObject, logger, - parseBaggageHeader, stripUrlQueryAndFragment, } from '@sentry/utils'; import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils'; @@ -128,6 +128,7 @@ export function nextRouterInstrumentation( if (startTransactionOnPageLoad) { const source = route ? 'route' : 'url'; + const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggage); activeTransaction = startTransactionCb({ name: prevLocationName, @@ -137,7 +138,7 @@ export function nextRouterInstrumentation( ...traceParentData, metadata: { source, - ...(baggage && { baggage: parseBaggageHeader(baggage) }), + dynamicSamplingContext: traceParentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, }, }); } diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts index 2d5bcba3161b..5fe439024ae8 100644 --- a/packages/nextjs/src/utils/instrumentServer.ts +++ b/packages/nextjs/src/utils/instrumentServer.ts @@ -10,10 +10,10 @@ import { import { extractTraceparentData, getActiveTransaction, hasTracingEnabled } from '@sentry/tracing'; import { addExceptionMechanism, + baggageHeaderToDynamicSamplingContext, fill, isString, logger, - parseBaggageSetMutability, stripUrlQueryAndFragment, } from '@sentry/utils'; import * as domain from 'domain'; @@ -255,8 +255,8 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler { __DEBUG_BUILD__ && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); } - const rawBaggageString = nextReq.headers && isString(nextReq.headers.baggage) && nextReq.headers.baggage; - const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData); + const baggageHeader = nextReq.headers && nextReq.headers.baggage; + const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggageHeader); // pull off query string, if any const reqPath = stripUrlQueryAndFragment(nextReq.url); @@ -270,7 +270,7 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler { name: `${namePrefix}${reqPath}`, op: 'http.server', metadata: { - baggage, + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, requestPath: reqPath, // TODO: Investigate if there's a way to tell if this is a dynamic route, so that we can make this more // like `source: isDynamicRoute? 'url' : 'route'` diff --git a/packages/nextjs/src/utils/withSentry.ts b/packages/nextjs/src/utils/withSentry.ts index 0229d91135cc..bd6ca7c715ab 100644 --- a/packages/nextjs/src/utils/withSentry.ts +++ b/packages/nextjs/src/utils/withSentry.ts @@ -3,10 +3,10 @@ import { extractTraceparentData, hasTracingEnabled } from '@sentry/tracing'; import { Transaction } from '@sentry/types'; import { addExceptionMechanism, + baggageHeaderToDynamicSamplingContext, isString, logger, objectify, - parseBaggageSetMutability, stripUrlQueryAndFragment, } from '@sentry/utils'; import * as domain from 'domain'; @@ -66,8 +66,8 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler = __DEBUG_BUILD__ && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); } - const rawBaggageString = req.headers && isString(req.headers.baggage) && req.headers.baggage; - const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData); + const baggageHeader = req.headers && req.headers.baggage; + const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggageHeader); const url = `${req.url}`; // pull off query string, if any @@ -87,7 +87,10 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler = name: `${reqMethod}${reqPath}`, op: 'http.server', ...traceparentData, - metadata: { baggage, source: 'route' }, + metadata: { + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + source: 'route', + }, }, // extra context passed to the `tracesSampler` { request: req }, diff --git a/packages/nextjs/test/performance/client.test.ts b/packages/nextjs/test/performance/client.test.ts index 01494fd7c519..2e152b6decfd 100644 --- a/packages/nextjs/test/performance/client.test.ts +++ b/packages/nextjs/test/performance/client.test.ts @@ -142,7 +142,7 @@ describe('nextRouterInstrumentation', () => { }, metadata: { source: 'route', - baggage: [{ environment: 'myEnv', release: '2.1.0' }, '', true], + dynamicSamplingContext: { environment: 'myEnv', release: '2.1.0' }, }, traceId: 'c82b8554881b4d28ad977de04a4fb40a', parentSpanId: 'a755953cd3394d5f', @@ -168,7 +168,7 @@ describe('nextRouterInstrumentation', () => { }, metadata: { source: 'route', - baggage: [{ environment: 'myEnv', release: '2.1.0' }, '', true], + dynamicSamplingContext: { environment: 'myEnv', release: '2.1.0' }, }, traceId: 'c82b8554881b4d28ad977de04a4fb40a', parentSpanId: 'a755953cd3394d5f', diff --git a/packages/nextjs/test/utils/withSentry.test.ts b/packages/nextjs/test/utils/withSentry.test.ts index 4d8709e8e735..105d23735e09 100644 --- a/packages/nextjs/test/utils/withSentry.test.ts +++ b/packages/nextjs/test/utils/withSentry.test.ts @@ -117,7 +117,6 @@ describe('withSentry', () => { op: 'http.server', metadata: { - baggage: expect.any(Array), source: 'route', }, }, 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 index 432790cb3de9..2d51f934b7f4 100644 --- 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 @@ -35,7 +35,7 @@ test('Should propagate sentry trace baggage data from an incoming to an outgoing }); }); -test('Should propagate empty baggage if sentry-trace header is present in incoming request but no baggage header', async () => { +test('Should not propagate baggage if sentry-trace header is present in incoming request but no baggage header', async () => { const env = await TestEnv.init(__dirname, `${path.resolve(__dirname, '..')}/server.ts`); const response = (await env.getAPIResponse(`${env.url}/express`, { @@ -46,12 +46,11 @@ test('Should propagate empty baggage if sentry-trace header is present in incomi expect(response).toMatchObject({ test_data: { host: 'somewhere.not.sentry', - baggage: '', }, }); }); -test('Should propagate empty sentry and ignore original 3rd party baggage entries if sentry-trace header is present', async () => { +test('Should not propagate baggage and ignore original 3rd party baggage entries if sentry-trace header is present', async () => { const env = await TestEnv.init(__dirname, `${path.resolve(__dirname, '..')}/server.ts`); const response = (await env.getAPIResponse(`${env.url}/express`, { @@ -63,7 +62,6 @@ test('Should propagate empty sentry and ignore original 3rd party baggage entrie expect(response).toMatchObject({ test_data: { host: 'somewhere.not.sentry', - baggage: '', }, }); }); diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts index 71e60bc7c4f3..3de98d14bf06 100644 --- a/packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts +++ b/packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts @@ -15,7 +15,10 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an 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', + baggage: [ + 'other=vendor,foo=bar,third=party,sentry-release=9.9.9,sentry-environment=staging,sentry-sample_rate=0.54,last=item', + 'sentry-release=2.1.0,sentry-environment=myEnv', + ], }, }); }); @@ -29,9 +32,12 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an 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', - ), + baggage: [ + 'other=vendor,foo=bar,third=party,sentry-release=9.9.9,sentry-environment=staging,sentry-sample_rate=0.54,last=item', + expect.stringMatching( + /sentry-environment=prod,sentry-release=1\.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-public_key=public,sentry-trace_id=[0-9a-f]{32},sentry-sample_rate=1/, + ), + ], }, }); }); diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors/test.ts index 3f50b71bc265..2f88610509c5 100644 --- a/packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors/test.ts +++ b/packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors/test.ts @@ -15,7 +15,7 @@ test('should merge `baggage` header of a third party vendor with the Sentry DSC expect(response).toMatchObject({ test_data: { host: 'somewhere.not.sentry', - baggage: 'other=vendor,foo=bar,third=party,sentry-release=2.0.0,sentry-environment=myEnv', + baggage: ['other=vendor,foo=bar,third=party', 'sentry-release=2.0.0,sentry-environment=myEnv'], }, }); }); diff --git a/packages/node-integration-tests/utils/index.ts b/packages/node-integration-tests/utils/index.ts index 153cbcb794d2..0be096fc65c5 100644 --- a/packages/node-integration-tests/utils/index.ts +++ b/packages/node-integration-tests/utils/index.ts @@ -200,12 +200,13 @@ export class TestEnv { * @return {*} {Promise} */ public async getAPIResponse(url?: string, headers?: Record): Promise { - const { data } = await axios.get(url || this.url, { headers: headers || {} }); - - await Sentry.flush(); - this.server.close(); - - return data; + try { + const { data } = await axios.get(url || this.url, { headers: headers || {} }); + return data; + } finally { + await Sentry.flush(); + this.server.close(); + } } public async setupNock( diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index df67d7fe3755..c29d5d836efe 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -4,11 +4,11 @@ import { Event, Span } from '@sentry/types'; import { AddRequestDataToEventOptions, addRequestDataToTransaction, + baggageHeaderToDynamicSamplingContext, extractPathForTransaction, extractTraceparentData, isString, logger, - parseBaggageSetMutability, } from '@sentry/utils'; import * as domain from 'domain'; import * as http from 'http'; @@ -54,9 +54,8 @@ export function tracingHandler(): ( // If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision) const traceparentData = req.headers && isString(req.headers['sentry-trace']) && extractTraceparentData(req.headers['sentry-trace']); - const rawBaggageString = req.headers && isString(req.headers.baggage) && req.headers.baggage; - - const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData); + const incomingBaggageHeaders = req.headers?.baggage; + const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(incomingBaggageHeaders); const [name, source] = extractPathForTransaction(req, { path: true, method: true }); const transaction = startTransaction( @@ -64,7 +63,10 @@ export function tracingHandler(): ( name, op: 'http.server', ...traceparentData, - metadata: { baggage, source }, + metadata: { + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + source, + }, }, // 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 0bdb91396a52..151fa384b9f9 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -1,6 +1,12 @@ import { getCurrentHub, Hub } from '@sentry/core'; import { EventProcessor, Integration, Span, TracePropagationTargets } from '@sentry/types'; -import { fill, isMatchingPattern, logger, mergeAndSerializeBaggage, parseSemver } from '@sentry/utils'; +import { + dynamicSamplingContextToSentryBaggageHeader, + fill, + isMatchingPattern, + logger, + parseSemver, +} from '@sentry/utils'; import * as http from 'http'; import * as https from 'https'; @@ -137,7 +143,7 @@ function _createWrappedRequestMethodFactory( return originalRequestMethod.apply(httpModule, requestArgs); } - let span: Span | undefined; + let requestSpan: Span | undefined; let parentSpan: Span | undefined; const scope = getCurrentHub().getScope(); @@ -146,26 +152,47 @@ function _createWrappedRequestMethodFactory( parentSpan = scope.getSpan(); if (parentSpan) { - span = parentSpan.startChild({ + requestSpan = parentSpan.startChild({ description: `${requestOptions.method || 'GET'} ${requestUrl}`, op: 'http.client', }); if (shouldAttachTraceData(requestUrl)) { - const sentryTraceHeader = span.toTraceparent(); + const sentryTraceHeader = requestSpan.toTraceparent(); __DEBUG_BUILD__ && logger.log( `[Tracing] Adding sentry-trace header ${sentryTraceHeader} to outgoing request to "${requestUrl}": `, ); - const baggage = parentSpan.transaction && parentSpan.transaction.getBaggage(); - const headerBaggageString = requestOptions.headers && requestOptions.headers.baggage; - requestOptions.headers = { ...requestOptions.headers, 'sentry-trace': sentryTraceHeader, - baggage: mergeAndSerializeBaggage(baggage, headerBaggageString), }; + + if (parentSpan.transaction) { + const dynamicSamplingContext = parentSpan.transaction.getDynamicSamplingContext(); + const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); + + let newBaggageHeaderField; + if (!requestOptions.headers || !requestOptions.headers.baggage) { + newBaggageHeaderField = sentryBaggageHeader; + } else if (!sentryBaggageHeader) { + newBaggageHeaderField = requestOptions.headers.baggage; + } else if (Array.isArray(requestOptions.headers.baggage)) { + newBaggageHeaderField = [...requestOptions.headers.baggage, sentryBaggageHeader]; + } else { + // Type-cast explanation: + // Technically this the following could be of type `(number | string)[]` but for the sake of simplicity + // we say this is undefined behaviour, since it would not be baggage spec conform if the user did this. + newBaggageHeaderField = [requestOptions.headers.baggage, sentryBaggageHeader] as string[]; + } + + requestOptions.headers = { + ...requestOptions.headers, + // Setting a hader to `undefined` will crash in node so we only set the baggage header when it's defined + ...(newBaggageHeaderField && { baggage: newBaggageHeaderField }), + }; + } } else { __DEBUG_BUILD__ && logger.log( @@ -189,12 +216,12 @@ function _createWrappedRequestMethodFactory( if (breadcrumbsEnabled) { addRequestBreadcrumb('response', requestUrl, req, res); } - if (tracingEnabled && span) { + if (tracingEnabled && requestSpan) { if (res.statusCode) { - span.setHttpStatus(res.statusCode); + requestSpan.setHttpStatus(res.statusCode); } - span.description = cleanSpanDescription(span.description, requestOptions, req); - span.finish(); + requestSpan.description = cleanSpanDescription(requestSpan.description, requestOptions, req); + requestSpan.finish(); } }) .once('error', function (this: http.ClientRequest): void { @@ -204,10 +231,10 @@ function _createWrappedRequestMethodFactory( if (breadcrumbsEnabled) { addRequestBreadcrumb('error', requestUrl, req); } - if (tracingEnabled && span) { - span.setHttpStatus(500); - span.description = cleanSpanDescription(span.description, requestOptions, req); - span.finish(); + if (tracingEnabled && requestSpan) { + requestSpan.setHttpStatus(500); + requestSpan.description = cleanSpanDescription(requestSpan.description, requestOptions, req); + requestSpan.finish(); } }); }; diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index 10d2181e3549..9910048cd942 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -2,8 +2,8 @@ import * as sentryCore from '@sentry/core'; import * as sentryHub from '@sentry/hub'; import { Hub } from '@sentry/hub'; import { Transaction } from '@sentry/tracing'; -import { Baggage, Event } from '@sentry/types'; -import { getThirdPartyBaggage, isBaggageMutable, isSentryBaggageEmpty, SentryError } from '@sentry/utils'; +import { Event } from '@sentry/types'; +import { SentryError } from '@sentry/utils'; import * as http from 'http'; import { NodeClient } from '../src/client'; @@ -221,10 +221,7 @@ describe('tracingHandler', () => { expect(transaction.traceId).toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toEqual('1121201211212012'); expect(transaction.sampled).toEqual(false); - expect(transaction.metadata?.baggage).toBeDefined(); - expect(isSentryBaggageEmpty(transaction.metadata?.baggage)).toBe(true); - expect(getThirdPartyBaggage(transaction.metadata?.baggage)).toEqual(''); - expect(isBaggageMutable(transaction.metadata?.baggage)).toBe(false); + expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({}); }); it("pulls parent's data from tracing and baggage headers on the request", () => { @@ -241,15 +238,10 @@ describe('tracingHandler', () => { 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' }, - '', - false, - ] as Baggage); + expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({ version: '1.0', environment: 'production' }); }); - it('ignores 3rd party entries in incoming baggage header', () => { + it("doesn't populate dynamic sampling context with 3rd party baggage", () => { req.headers = { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0', baggage: 'sentry-version=1.0,sentry-environment=production,dogs=great,cats=boring', @@ -258,13 +250,7 @@ describe('tracingHandler', () => { 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' }, - '', - false, - ] as Baggage); + expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({ version: '1.0', environment: 'production' }); }); 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 94d1e7565f68..3f48b648b5bc 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -118,8 +118,6 @@ describe('tracing', () => { const request = http.get('http://dogs.are.great/'); const baggageHeader = request.getHeader('baggage') as string; - expect(baggageHeader).toBeDefined(); - expect(typeof baggageHeader).toEqual('string'); expect(baggageHeader).toEqual( 'sentry-environment=production,sentry-release=1.0.0,' + 'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' + @@ -127,7 +125,7 @@ describe('tracing', () => { ); }); - it('propagates 3rd party baggage header data to outgoing non-sentry requests', async () => { + it('keeps 3rd party baggage header data to outgoing non-sentry requests', async () => { nock('http://dogs.are.great').get('/').reply(200); createTransactionOnScope(); @@ -135,13 +133,10 @@ describe('tracing', () => { const request = http.get({ host: 'http://dogs.are.great/', headers: { baggage: 'dog=great' } }); const baggageHeader = request.getHeader('baggage') as string; - expect(baggageHeader).toBeDefined(); - expect(typeof baggageHeader).toEqual('string'); - expect(baggageHeader).toEqual( - 'dog=great,sentry-environment=production,sentry-release=1.0.0,' + - 'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' + - 'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1', - ); + expect(baggageHeader).toEqual([ + 'dog=great', + 'sentry-environment=production,sentry-release=1.0.0,sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1', + ]); }); it('adds the transaction name to the the baggage header if a valid transaction source is set', async () => { @@ -152,13 +147,10 @@ describe('tracing', () => { const request = http.get({ host: 'http://dogs.are.great/', headers: { baggage: 'dog=great' } }); const baggageHeader = request.getHeader('baggage') as string; - expect(baggageHeader).toBeDefined(); - expect(typeof baggageHeader).toEqual('string'); - expect(baggageHeader).toEqual( - 'dog=great,sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' + - 'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' + - 'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1', - ); + expect(baggageHeader).toEqual([ + 'dog=great', + 'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1', + ]); }); it("doesn't attach the sentry-trace header to outgoing sentry requests", () => { diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 36a62a3749c0..bb37dffea8f3 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -4,14 +4,13 @@ import { getActiveTransaction, hasTracingEnabled } from '@sentry/tracing'; import { Transaction, TransactionSource, WrappedFunction } from '@sentry/types'; import { addExceptionMechanism, + baggageHeaderToDynamicSamplingContext, + dynamicSamplingContextToSentryBaggageHeader, extractTraceparentData, fill, isNodeEnv, - isSentryBaggageEmpty, loadModule, logger, - parseBaggageSetMutability, - serializeBaggage, } from '@sentry/utils'; import * as domain from 'domain'; @@ -194,9 +193,11 @@ function getTraceAndBaggage(): { sentryTrace?: string; sentryBaggage?: string } const span = currentScope.getSpan(); if (span && transaction) { + const dynamicSamplingContext = transaction.getDynamicSamplingContext(); + return { sentryTrace: span.toTraceparent(), - sentryBaggage: serializeBaggage(transaction.getBaggage()), + sentryBaggage: dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext), }; } } @@ -312,7 +313,7 @@ export function startRequestHandlerTransaction( ): Transaction { // If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision) const traceparentData = extractTraceparentData(request.headers['sentry-trace']); - const baggage = parseBaggageSetMutability(request.headers.baggage, traceparentData); + const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(request.headers.baggage); const transaction = hub.startTransaction({ name, @@ -323,8 +324,7 @@ export function startRequestHandlerTransaction( ...traceparentData, metadata: { source, - // Only attach baggage if it's defined - ...(!isSentryBaggageEmpty(baggage) && { baggage }), + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, }, }); diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 6cd916270cb5..ca76cf12382a 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -3,7 +3,7 @@ import * as Sentry from '@sentry/node'; import { captureException, captureMessage, flush, getCurrentHub, Scope, withScope } from '@sentry/node'; import { extractTraceparentData } from '@sentry/tracing'; import { Integration } from '@sentry/types'; -import { dsnFromString, dsnToString, isString, logger, parseBaggageSetMutability } from '@sentry/utils'; +import { baggageHeaderToDynamicSamplingContext, dsnFromString, dsnToString, isString, logger } 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'; @@ -308,9 +308,8 @@ export function wrapHandler( traceparentData = extractTraceparentData(eventWithHeaders.headers['sentry-trace']); } - const rawBaggageString = - eventWithHeaders.headers && isString(eventWithHeaders.headers.baggage) && eventWithHeaders.headers.baggage; - const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData); + const baggageHeader = eventWithHeaders.headers && eventWithHeaders.headers.baggage; + const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggageHeader); const hub = getCurrentHub(); @@ -318,7 +317,10 @@ export function wrapHandler( name: context.functionName, op: 'awslambda.handler', ...traceparentData, - metadata: { baggage, source: 'component' }, + metadata: { + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + source: 'component', + }, }); const scope = hub.pushScope(); diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index 99cec2811fae..70703be9728a 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -6,7 +6,7 @@ import { getCurrentHub, } from '@sentry/node'; import { extractTraceparentData } from '@sentry/tracing'; -import { isString, logger, parseBaggageSetMutability, stripUrlQueryAndFragment } from '@sentry/utils'; +import { baggageHeaderToDynamicSamplingContext, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils'; import { domainify, getActiveDomain, proxyFunction } from './../utils'; import { HttpFunction, WrapperOptions } from './general'; @@ -77,10 +77,9 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial { const fakeTransactionContext = { name: 'functionName', op: 'awslambda.handler', - metadata: { baggage: [{}, '', true], source: 'component' }, + metadata: { source: 'component' }, }; expect(rv).toStrictEqual(42); @@ -219,7 +219,7 @@ describe('AWSLambda', () => { const fakeTransactionContext = { name: 'functionName', op: 'awslambda.handler', - metadata: { baggage: [{}, '', true], source: 'component' }, + metadata: { source: 'component' }, }; // @ts-ignore see "Why @ts-ignore" note @@ -262,13 +262,9 @@ describe('AWSLambda', () => { name: 'functionName', traceId: '12312012123120121231201212312012', metadata: { - baggage: [ - { - release: '2.12.1', - }, - '', - false, - ], + dynamicSamplingContext: { + release: '2.12.1', + }, source: 'component', }, }), @@ -300,7 +296,7 @@ describe('AWSLambda', () => { traceId: '12312012123120121231201212312012', parentSpanId: '1121201211212012', parentSampled: false, - metadata: { baggage: [{}, '', false], source: 'component' }, + metadata: { dynamicSamplingContext: {}, source: 'component' }, }; // @ts-ignore see "Why @ts-ignore" note @@ -327,7 +323,7 @@ describe('AWSLambda', () => { const fakeTransactionContext = { name: 'functionName', op: 'awslambda.handler', - metadata: { baggage: [{}, '', true], source: 'component' }, + metadata: { source: 'component' }, }; expect(rv).toStrictEqual(42); @@ -365,7 +361,7 @@ describe('AWSLambda', () => { const fakeTransactionContext = { name: 'functionName', op: 'awslambda.handler', - metadata: { baggage: [{}, '', true], source: 'component' }, + metadata: { source: 'component' }, }; // @ts-ignore see "Why @ts-ignore" note @@ -407,7 +403,7 @@ describe('AWSLambda', () => { const fakeTransactionContext = { name: 'functionName', op: 'awslambda.handler', - metadata: { baggage: [{}, '', true], source: 'component' }, + metadata: { source: 'component' }, }; expect(rv).toStrictEqual(42); @@ -445,7 +441,7 @@ describe('AWSLambda', () => { const fakeTransactionContext = { name: 'functionName', op: 'awslambda.handler', - metadata: { baggage: [{}, '', true], source: 'component' }, + metadata: { source: 'component' }, }; // @ts-ignore see "Why @ts-ignore" note diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts index 9f1498714d0e..1c587df44634 100644 --- a/packages/serverless/test/gcpfunction.test.ts +++ b/packages/serverless/test/gcpfunction.test.ts @@ -114,7 +114,7 @@ describe('GCPFunction', () => { const fakeTransactionContext = { name: 'POST /path', op: 'gcp.function.http', - metadata: { baggage: [{}, '', true], source: 'route' }, + metadata: { source: 'route' }, }; // @ts-ignore see "Why @ts-ignore" note const fakeTransaction = { ...Sentry.fakeTransaction, ...fakeTransactionContext }; @@ -151,13 +151,9 @@ describe('GCPFunction', () => { parentSpanId: '1121201211212012', parentSampled: false, metadata: { - baggage: [ - { - release: '2.12.1', - }, - '', - false, - ], + dynamicSamplingContext: { + release: '2.12.1', + }, source: 'route', }, }; @@ -190,7 +186,7 @@ describe('GCPFunction', () => { traceId: '12312012123120121231201212312012', parentSpanId: '1121201211212012', parentSampled: false, - metadata: { baggage: [{}, '', false], source: 'route' }, + metadata: { dynamicSamplingContext: {}, source: 'route' }, }; // @ts-ignore see "Why @ts-ignore" note const fakeTransaction = { ...Sentry.fakeTransaction, ...fakeTransactionContext }; diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 63ff481a417e..8a06f029d53f 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -1,7 +1,7 @@ /* eslint-disable max-lines */ import { Hub } from '@sentry/hub'; import { EventProcessor, Integration, Transaction, TransactionContext } from '@sentry/types'; -import { getDomElement, getGlobalObject, logger, parseBaggageSetMutability } from '@sentry/utils'; +import { baggageHeaderToDynamicSamplingContext, getDomElement, getGlobalObject, logger } from '@sentry/utils'; import { startIdleTransaction } from '../hubextensions'; import { DEFAULT_FINAL_TIMEOUT, DEFAULT_IDLE_TIMEOUT } from '../idletransaction'; @@ -215,19 +215,26 @@ export class BrowserTracing implements Integration { // eslint-disable-next-line @typescript-eslint/unbound-method const { beforeNavigate, idleTimeout, finalTimeout } = this.options; - const parentContextFromHeader = context.op === 'pageload' ? extractTraceDataFromMetaTags() : undefined; + const isPageloadTransaction = context.op === 'pageload'; - const expandedContext = { + const sentryTraceMetaTagValue = isPageloadTransaction ? getMetaContent('sentry-trace') : null; + const baggageMetaTagValue = isPageloadTransaction ? getMetaContent('baggage') : null; + + const traceParentData = sentryTraceMetaTagValue ? extractTraceparentData(sentryTraceMetaTagValue) : undefined; + const dynamicSamplingContext = baggageMetaTagValue + ? baggageHeaderToDynamicSamplingContext(baggageMetaTagValue) + : undefined; + + const expandedContext: TransactionContext = { ...context, - ...parentContextFromHeader, - ...(parentContextFromHeader && { - metadata: { - ...context.metadata, - ...parentContextFromHeader.metadata, - }, - }), + ...traceParentData, + metadata: { + ...context.metadata, + dynamicSamplingContext: traceParentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + }, trimEnd: true, }; + const modifiedContext = typeof beforeNavigate === 'function' ? beforeNavigate(expandedContext) : expandedContext; // For backwards compatibility reasons, beforeNavigate can return undefined to "drop" the transaction (prevent it @@ -270,28 +277,6 @@ export class BrowserTracing implements Integration { } } -/** - * 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 extractTraceDataFromMetaTags(): Partial | undefined { - const sentrytraceValue = getMetaContent('sentry-trace'); - const baggageValue = getMetaContent('baggage'); - - const sentrytraceData = sentrytraceValue ? extractTraceparentData(sentrytraceValue) : undefined; - const baggage = parseBaggageSetMutability(baggageValue, sentrytraceValue); - - // TODO more extensive checks for baggage validity/emptyness? - if (sentrytraceData || baggage) { - return { - ...(sentrytraceData && sentrytraceData), - ...(baggage && { metadata: { baggage } }), - }; - } - - return undefined; -} - /** Returns the value of a meta tag */ export function getMetaContent(metaName: string): string | null { // Can't specify generic to `getDomElement` because tracing can be used diff --git a/packages/tracing/src/browser/request.ts b/packages/tracing/src/browser/request.ts index 31322c9c1daf..4f25c787e158 100644 --- a/packages/tracing/src/browser/request.ts +++ b/packages/tracing/src/browser/request.ts @@ -1,11 +1,11 @@ /* eslint-disable max-lines */ -import type { Baggage, Span } from '@sentry/types'; +import type { DynamicSamplingContext, Span } from '@sentry/types'; import { addInstrumentationHandler, BAGGAGE_HEADER_NAME, + dynamicSamplingContextToSentryBaggageHeader, isInstanceOf, isMatchingPattern, - mergeAndSerializeBaggage, } from '@sentry/utils'; import { getActiveTransaction, hasTracingEnabled } from '../utils'; @@ -85,7 +85,7 @@ export interface XHRData { } type PolymorphicRequestHeaders = - | Record + | Record | Array<[string, string]> // the below is not preicsely the Header type used in Request, but it'll pass duck-typing | { @@ -195,52 +195,87 @@ export function fetchCallback( handlerData.fetchData.__span = span.spanId; spans[span.spanId] = span; - const request = (handlerData.args[0] = handlerData.args[0] as string | Request); + const request: string | Request = handlerData.args[0]; + + // In case the user hasn't set the second argument of a fetch call we default it to `{}`. + handlerData.args[1] = handlerData.args[1] || {}; + // eslint-disable-next-line @typescript-eslint/no-explicit-any - const options = (handlerData.args[1] = (handlerData.args[1] as { [key: string]: any }) || {}); - options.headers = addTracingHeaders(request, activeTransaction.getBaggage(), span, options); + const options: { [key: string]: any } = handlerData.args[1]; + + options.headers = addTracingHeadersToFetchRequest( + request, + activeTransaction.getDynamicSamplingContext(), + span, + options, + ); + activeTransaction.metadata.propagations += 1; } } -function addTracingHeaders( +function addTracingHeadersToFetchRequest( request: string | Request, - incomingBaggage: Baggage | undefined, + dynamicSamplingContext: Partial, span: Span, - options: { [key: string]: any }, + options: { + headers?: + | { + [key: string]: string[] | string | undefined; + } + | Request['headers']; + }, ): PolymorphicRequestHeaders { - let headers = options.headers; + const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); + const sentryTraceHeader = span.toTraceparent(); - if (isInstanceOf(request, Request)) { - headers = (request as Request).headers; - } + const headers = + typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : options.headers; - 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 - 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 = { - ...headers, - 'sentry-trace': span.toTraceparent(), - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - baggage: mergeAndSerializeBaggage(incomingBaggage, headers.baggage), - }; + if (!headers) { + return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader }; + } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) { + const newHeaders = new Headers(headers as Headers); + + newHeaders.append('sentry-trace', sentryTraceHeader); + + if (sentryBaggageHeader) { + // If the same header is appended miultiple times the browser will merge the values into a single request header. + // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header. + newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader); } + + return newHeaders as PolymorphicRequestHeaders; + } else if (Array.isArray(headers)) { + const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]]; + + if (sentryBaggageHeader) { + // If there are multiple entries with the same key, the browser will merge the values into a single request header. + // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header. + newHeaders.push([BAGGAGE_HEADER_NAME, sentryBaggageHeader]); + } + + return newHeaders; } else { - headers = { 'sentry-trace': span.toTraceparent(), baggage: mergeAndSerializeBaggage(incomingBaggage) }; + const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined; + const newBaggageHeaders: string[] = []; + + if (Array.isArray(existingBaggageHeader)) { + newBaggageHeaders.push(...existingBaggageHeader); + } else if (existingBaggageHeader) { + newBaggageHeaders.push(existingBaggageHeader); + } + + if (sentryBaggageHeader) { + newBaggageHeaders.push(sentryBaggageHeader); + } + + return { + ...(headers as Exclude), + 'sentry-trace': sentryTraceHeader, + baggage: newBaggageHeaders.length > 0 ? newBaggageHeaders.join(',') : undefined, + }; } - return headers; } /** @@ -298,13 +333,16 @@ export function xhrCallback( try { handlerData.xhr.setRequestHeader('sentry-trace', span.toTraceparent()); - const headerBaggageString = - handlerData.xhr.getRequestHeader && handlerData.xhr.getRequestHeader(BAGGAGE_HEADER_NAME); + const dynamicSamplingContext = activeTransaction.getDynamicSamplingContext(); + const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); + + if (sentryBaggageHeader) { + // From MDN: "If this method is called several times with the same header, the values are merged into one single request header." + // We can therefore simply set a baggage header without checking what was there before + // https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/setRequestHeader + handlerData.xhr.setRequestHeader(BAGGAGE_HEADER_NAME, sentryBaggageHeader); + } - handlerData.xhr.setRequestHeader( - BAGGAGE_HEADER_NAME, - mergeAndSerializeBaggage(activeTransaction.getBaggage(), headerBaggageString), - ); activeTransaction.metadata.propagations += 1; } catch (_) { // Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED. diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts index cc796d1949ff..7f65a935f3d9 100644 --- a/packages/tracing/src/transaction.ts +++ b/packages/tracing/src/transaction.ts @@ -1,7 +1,6 @@ import { getCurrentHub, Hub } from '@sentry/hub'; import { - Baggage, - BaggageObj, + DynamicSamplingContext, Event, Measurements, MeasurementUnit, @@ -9,14 +8,7 @@ import { TransactionContext, TransactionMetadata, } from '@sentry/types'; -import { - createBaggage, - dropUndefinedKeys, - getSentryBaggageItems, - isBaggageMutable, - logger, - timestampInSeconds, -} from '@sentry/utils'; +import { dropUndefinedKeys, logger, timestampInSeconds } from '@sentry/utils'; import { Span as SpanClass, SpanRecorder } from './span'; @@ -35,6 +27,8 @@ export class Transaction extends SpanClass implements TransactionInterface { private _trimEnd?: boolean; + private _frozenDynamicSamplingContext: Readonly> | undefined = undefined; + /** * This constructor should never be called manually. Those instrumenting tracing should use * `Sentry.startTransaction()`, and internal methods should use `hub.startTransaction()`. @@ -60,6 +54,14 @@ export class Transaction extends SpanClass implements TransactionInterface { // this is because transactions are also spans, and spans have a transaction pointer this.transaction = this; + + // If Dynamic Sampling Context is provided during the creation of the transaction, we freeze it as it usually means + // there is incoming Dynamic Sampling Context. (Either through an incoming request, a baggage meta-tag, or other means) + const incomingDynamicSamplingContext = this.metadata.dynamicSamplingContext; + if (incomingDynamicSamplingContext) { + // We shallow copy this in case anything writes to the original reference of the passed in `dynamicSamplingContext` + this._frozenDynamicSamplingContext = { ...incomingDynamicSamplingContext }; + } } /** Getter for `name` property */ @@ -169,7 +171,7 @@ export class Transaction extends SpanClass implements TransactionInterface { type: 'transaction', sdkProcessingMetadata: { ...metadata, - baggage: this.getBaggage(), + dynamicSamplingContext: this.getDynamicSamplingContext(), }, ...(metadata.source && { transaction_info: { @@ -227,69 +229,43 @@ export class Transaction extends SpanClass implements TransactionInterface { * * @experimental */ - public getBaggage(): Baggage { - const existingBaggage = this.metadata.baggage; - - // Only add Sentry baggage items to baggage, if baggage does not exist yet or it is still - // empty and mutable - const finalBaggage = - !existingBaggage || isBaggageMutable(existingBaggage) - ? this._populateBaggageWithSentryValues(existingBaggage) - : existingBaggage; - - // Update the baggage stored on the transaction. - this.metadata.baggage = finalBaggage; - - return finalBaggage; - } + public getDynamicSamplingContext(): Readonly> { + if (this._frozenDynamicSamplingContext) { + return this._frozenDynamicSamplingContext; + } - /** - * Collects and adds data to the passed baggage object. - * - * Note: This function does not explicitly check if the passed baggage object is allowed - * to be modified. Implicitly, `setBaggageValue` will not make modification to the object - * if it was already set immutable. - * - * After adding the data, the baggage object is set immutable to prevent further modifications. - * - * @param baggage - * - * @returns modified and immutable baggage object - */ - private _populateBaggageWithSentryValues(baggage: Baggage = createBaggage({})): Baggage { const hub: Hub = this._hub || getCurrentHub(); const client = hub && hub.getClient(); - if (!client) return baggage; + if (!client) return {}; const { environment, release } = client.getOptions() || {}; const { publicKey: public_key } = client.getDsn() || {}; - const sample_rate = - this.metadata && - this.metadata.transactionSampling && - this.metadata.transactionSampling.rate && - this.metadata.transactionSampling.rate.toString(); + const maybeSampleRate = (this.metadata.transactionSampling || {}).rate; + const sample_rate = maybeSampleRate !== undefined ? maybeSampleRate.toString() : undefined; const scope = hub.getScope(); const { segment: user_segment } = (scope && scope.getUser()) || {}; const source = this.metadata.source; + + // We don't want to have a transaction name in the DSC if the source is "url" because URLs might contain PII const transaction = source && source !== 'url' ? this.name : undefined; - return createBaggage( - dropUndefinedKeys({ - environment, - release, - transaction, - user_segment, - public_key, - trace_id: this.traceId, - sample_rate, - ...getSentryBaggageItems(baggage), // keep user-added values - } as BaggageObj), - '', - false, // set baggage immutable - ); + const dsc = dropUndefinedKeys({ + environment, + release, + transaction, + user_segment, + public_key, + trace_id: this.traceId, + sample_rate, + }); + + // Uncomment if we want to make DSC immutable + // this._frozenDynamicSamplingContext = dsc; + + return dsc; } } diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index 86c437d59c90..57c7f2eaf112 100644 --- a/packages/tracing/test/browser/browsertracing.test.ts +++ b/packages/tracing/test/browser/browsertracing.test.ts @@ -1,21 +1,10 @@ import { BrowserClient } from '@sentry/browser'; import { Hub, makeMain } from '@sentry/hub'; -import type { Baggage, BaggageObj, BaseTransportOptions, ClientOptions, DsnComponents } from '@sentry/types'; -import { - getGlobalObject, - getThirdPartyBaggage, - InstrumentHandlerCallback, - InstrumentHandlerType, - isSentryBaggageEmpty, -} from '@sentry/utils'; +import type { BaseTransportOptions, ClientOptions, DsnComponents } from '@sentry/types'; +import { getGlobalObject, InstrumentHandlerCallback, InstrumentHandlerType } from '@sentry/utils'; import { JSDOM } from 'jsdom'; -import { - BrowserTracing, - BrowserTracingOptions, - extractTraceDataFromMetaTags, - getMetaContent, -} from '../../src/browser/browsertracing'; +import { BrowserTracing, BrowserTracingOptions, getMetaContent } from '../../src/browser/browsertracing'; import { defaultRequestInstrumentationOptions } from '../../src/browser/request'; import { instrumentRoutingWithDefaults } from '../../src/browser/router'; import * as hubExtensions from '../../src/hubextensions'; @@ -272,7 +261,7 @@ describe('BrowserTracing', () => { parentSpanId: 'b6e54397b12a2a0f', parentSampled: true, metadata: { - baggage: [{ release: '2.1.14' }, '', false], + dynamicSamplingContext: { release: '2.1.14' }, }, }), expect.any(Number), @@ -400,71 +389,6 @@ describe('BrowserTracing', () => { }); }); - describe('extractTraceDataFromMetaTags()', () => { - it('correctly parses a valid sentry-trace meta header', () => { - document.head.innerHTML = - ''; - - const headerContext = extractTraceDataFromMetaTags(); - - expect(headerContext).toBeDefined(); - expect(headerContext!.traceId).toEqual('12312012123120121231201212312012'); - expect(headerContext!.parentSpanId).toEqual('1121201211212012'); - expect(headerContext!.parentSampled).toEqual(false); - }); - - it('correctly parses a valid baggage meta header and ignored 3rd party entries', () => { - 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(''); - }); - - it('returns undefined if the sentry-trace header is malformed', () => { - document.head.innerHTML = ''; - - const headerContext = extractTraceDataFromMetaTags(); - - expect(headerContext).toBeDefined(); - expect(headerContext?.metadata?.baggage).toBeDefined(); - expect(isSentryBaggageEmpty(headerContext?.metadata?.baggage as Baggage)).toBe(true); - expect(getThirdPartyBaggage(headerContext?.metadata?.baggage as Baggage)).toEqual(''); - }); - - 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 default object if the header isn't there", () => { - document.head.innerHTML = ''; - - const headerContext = extractTraceDataFromMetaTags(); - - expect(headerContext).toBeDefined(); - expect(headerContext?.metadata?.baggage).toBeDefined(); - expect(isSentryBaggageEmpty(headerContext?.metadata?.baggage as Baggage)).toBe(true); - expect(getThirdPartyBaggage(headerContext?.metadata?.baggage as Baggage)).toEqual(''); - }); - }); - describe('using the tag data', () => { beforeEach(() => { hub.getClient()!.getOptions = () => { @@ -490,21 +414,18 @@ describe('BrowserTracing', () => { // pageload transactions are created as part of the BrowserTracing integration's initialization createBrowserTracing(true); const transaction = getActiveTransaction(hub) as IdleTransaction; - const baggage = transaction.getBaggage()!; + const dynamicSamplingContext = transaction.getDynamicSamplingContext()!; 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(''); + expect(dynamicSamplingContext).toBeDefined(); + expect(dynamicSamplingContext).toStrictEqual({ release: '2.1.14' }); }); - it('does not add Sentry baggage data to pageload transactions if sentry-trace data is present but passes on 3rd party baggage', () => { + it('puts frozen Dynamic Sampling Context on pageload transactions if sentry-trace data and only 3rd party baggage is present', () => { // make sampled false here, so we can see that it's being used rather than the tracesSampleRate-dictated one document.head.innerHTML = '' + @@ -513,20 +434,17 @@ describe('BrowserTracing', () => { // pageload transactions are created as part of the BrowserTracing integration's initialization createBrowserTracing(true); const transaction = getActiveTransaction(hub) as IdleTransaction; - const baggage = transaction.getBaggage()!; + const dynamicSamplingContext = transaction.getDynamicSamplingContext()!; 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(isSentryBaggageEmpty(baggage)).toBe(true); - expect(baggage[1]).toBeDefined(); - expect(baggage[1]).toEqual(''); + expect(dynamicSamplingContext).toStrictEqual({}); }); - it('ignores the data for navigation transactions', () => { + it('ignores the meta tag data for navigation transactions', () => { mockChangeHistory = () => undefined; document.head.innerHTML = '' + @@ -536,22 +454,18 @@ describe('BrowserTracing', () => { mockChangeHistory({ to: 'here', from: 'there' }); const transaction = getActiveTransaction(hub) as IdleTransaction; - const baggage = transaction.getBaggage()!; + const dynamicSamplingContext = transaction.getDynamicSamplingContext()!; expect(transaction).toBeDefined(); expect(transaction.op).toBe('navigation'); expect(transaction.traceId).not.toEqual('12312012123120121231201212312012'); expect(transaction.parentSpanId).toBeUndefined(); - expect(baggage).toBeDefined(); - expect(baggage[0]).toBeDefined(); - expect(baggage[0]).toEqual({ + expect(dynamicSamplingContext).toStrictEqual({ release: '1.0.0', environment: 'production', public_key: 'pubKey', trace_id: expect.not.stringMatching('12312012123120121231201212312012'), }); - expect(baggage[1]).toBeDefined(); - expect(baggage[1]).toEqual(''); }); }); }); diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index a59c295edbe8..9c7655d910b9 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -1,7 +1,6 @@ import { BrowserClient } from '@sentry/browser'; import { Hub, makeMain, Scope } from '@sentry/hub'; import { BaseTransportOptions, ClientOptions, TransactionSource } from '@sentry/types'; -import { createBaggage, getSentryBaggageItems, getThirdPartyBaggage, isSentryBaggageEmpty } from '@sentry/utils'; import { Span, Transaction } from '../src'; import { TRACEPARENT_REGEXP } from '../src/utils'; @@ -393,7 +392,7 @@ describe('Span', () => { }); }); - describe('getBaggage and _populateBaggageWithSentryValues', () => { + describe('getDynamicSamplingContext', () => { beforeEach(() => { hub.getClient()!.getOptions = () => { return { @@ -403,31 +402,28 @@ describe('Span', () => { }; }); - test('leave baggage content untouched and just return baggage if it is immutable', () => { + test('should return DSC that was provided during transaction creation, if it was provided', () => { const transaction = new Transaction( { name: 'tx', - metadata: { baggage: createBaggage({ environment: 'myEnv' }, '', false) }, + metadata: { dynamicSamplingContext: { environment: 'myEnv' } }, }, hub, ); const hubSpy = jest.spyOn(hub.getClient()!, 'getOptions'); - const baggage = transaction.getBaggage(); + const dynamicSamplingContext = transaction.getDynamicSamplingContext(); - expect(hubSpy).toHaveBeenCalledTimes(0); - expect(baggage && isSentryBaggageEmpty(baggage)).toBe(false); - expect(baggage && getSentryBaggageItems(baggage)).toStrictEqual({ environment: 'myEnv' }); - expect(baggage && getThirdPartyBaggage(baggage)).toStrictEqual(''); + expect(hubSpy).not.toHaveBeenCalled(); + expect(dynamicSamplingContext).toStrictEqual({ environment: 'myEnv' }); }); - test('add Sentry baggage data to baggage if Sentry content is empty and baggage is mutable', () => { + test('should return new DSC, if no DSC was provided during transaction creation', () => { const transaction = new Transaction( { name: 'tx', metadata: { - baggage: createBaggage({}, '', true), transactionSampling: { rate: 0.56, method: 'client_rate' }, }, }, @@ -436,17 +432,15 @@ describe('Span', () => { const getOptionsSpy = jest.spyOn(hub.getClient()!, 'getOptions'); - const baggage = transaction.getBaggage(); + const dynamicSamplingContext = transaction.getDynamicSamplingContext(); expect(getOptionsSpy).toHaveBeenCalledTimes(1); - expect(baggage && isSentryBaggageEmpty(baggage)).toBe(false); - expect(baggage && getSentryBaggageItems(baggage)).toStrictEqual({ + expect(dynamicSamplingContext).toStrictEqual({ release: '1.0.1', environment: 'production', sample_rate: '0.56', trace_id: expect.any(String), }); - expect(baggage && getThirdPartyBaggage(baggage)).toStrictEqual(''); }); describe('Including transaction name in DSC', () => { @@ -464,7 +458,7 @@ describe('Span', () => { hub, ); - const dsc = getSentryBaggageItems(transaction.getBaggage()); + const dsc = transaction.getDynamicSamplingContext()!; expect(dsc.transaction).toBeUndefined(); }); @@ -483,7 +477,7 @@ describe('Span', () => { hub, ); - const dsc = getSentryBaggageItems(transaction.getBaggage()); + const dsc = transaction.getDynamicSamplingContext()!; expect(dsc.transaction).toEqual('tx'); }); diff --git a/packages/tracing/test/utils.test.ts b/packages/tracing/test/utils.test.ts index eae5213758ea..3bd3e9f5f35f 100644 --- a/packages/tracing/test/utils.test.ts +++ b/packages/tracing/test/utils.test.ts @@ -60,6 +60,9 @@ describe('extractTraceparentData', () => { }); test('invalid', () => { + // empty string + expect(extractTraceparentData('')).toBeUndefined(); + // trace id wrong length expect(extractTraceparentData('a-bbbbbbbbbbbbbbbb-1')).toBeUndefined(); diff --git a/packages/types/src/baggage.ts b/packages/types/src/baggage.ts deleted file mode 100644 index d347b2aa9036..000000000000 --- a/packages/types/src/baggage.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { DynamicSamplingContext } from './envelope'; - -export type AllowedBaggageKeys = keyof DynamicSamplingContext; - -export type BaggageObj = Partial & Record>; - -/** - * The baggage data structure represents key,value pairs based on the baggage - * spec: https://www.w3.org/TR/baggage - * - * It is expected that users interact with baggage using the helpers methods: - * `createBaggage`, `getBaggageValue`, and `setBaggageValue`. - * - * Internally, the baggage data structure is a tuple of length 3, separating baggage values - * based on if they are related to Sentry or not. If the baggage values are - * set/used by sentry, they will be stored in an object to be easily accessed. - * If they are not, they are kept as a string to be only accessed when serialized - * at baggage propagation time. - * The third tuple member controls the mutability of the baggage. If it is `false`, - * the baggage can no longer longer be modified (i.e. is immutable). - */ -export type Baggage = [BaggageObj, string, boolean]; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 20718b53830f..99ba88469a20 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -1,5 +1,4 @@ export type { Attachment } from './attachment'; -export type { AllowedBaggageKeys, Baggage, BaggageObj } from './baggage'; export type { Breadcrumb, BreadcrumbHint } from './breadcrumb'; export type { Client } from './client'; export type { ClientReport, Outcome, EventDropReason } from './clientreport'; diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts index 2c083611b03a..035a6063fbf5 100644 --- a/packages/types/src/transaction.ts +++ b/packages/types/src/transaction.ts @@ -1,4 +1,4 @@ -import { Baggage } from './baggage'; +import { DynamicSamplingContext } from './envelope'; import { MeasurementUnit } from './measurement'; import { ExtractedNodeRequestData, Primitive, WorkerLocation } from './misc'; import { Span, SpanContext } from './span'; @@ -95,8 +95,8 @@ export interface Transaction extends TransactionContext, Span { */ setMetadata(newMetadata: Partial): void; - /** return the baggage for dynamic sampling and trace propagation */ - getBaggage(): Baggage; + /** Return the current Dynamic Sampling Context of this transaction */ + getDynamicSamplingContext(): Partial; } /** @@ -139,8 +139,11 @@ export type TransactionSamplingMethod = 'explicitly_set' | 'client_sampler' | 'c export interface TransactionMetadata { transactionSampling?: { rate?: number; method: TransactionSamplingMethod }; - /** The baggage object of a transaction's baggage header, used for dynamic sampling */ - baggage?: Baggage; + /** + * The Dynamic Sampling Context of a transaction. If provided during transaction creation, its Dynamic Sampling + * Context Will be frozen + */ + dynamicSamplingContext?: Partial; /** 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 9ca8d34df1ae..1a2fa58d52a6 100644 --- a/packages/utils/src/baggage.ts +++ b/packages/utils/src/baggage.ts @@ -1,4 +1,4 @@ -import { Baggage, BaggageObj, HttpHeaderValue, TraceparentData } from '@sentry/types'; +import { DynamicSamplingContext } from '@sentry/types'; import { isString } from './is'; import { logger } from './logger'; @@ -16,180 +16,129 @@ export const SENTRY_BAGGAGE_KEY_PREFIX_REGEX = /^sentry-/; */ export const MAX_BAGGAGE_STRING_LENGTH = 8192; -/** Create an instance of Baggage */ -export function createBaggage(initItems: BaggageObj, baggageString: string = '', mutable: boolean = true): Baggage { - return [{ ...initItems }, baggageString, mutable]; -} - -/** Get a value from baggage */ -export function getBaggageValue(baggage: Baggage, key: keyof BaggageObj): BaggageObj[keyof BaggageObj] { - return baggage[0][key]; -} - -/** Add a value to baggage */ -export function setBaggageValue(baggage: Baggage, key: keyof BaggageObj, value: BaggageObj[keyof BaggageObj]): void { - if (isBaggageMutable(baggage)) { - baggage[0][key] = value; - } -} - -/** Check if the Sentry part of the passed baggage (i.e. the first element in the tuple) is empty */ -export function isSentryBaggageEmpty(baggage: Baggage): boolean { - return Object.keys(baggage[0]).length === 0; -} - -/** Returns Sentry specific baggage values */ -export function getSentryBaggageItems(baggage: Baggage): BaggageObj { - return baggage[0]; -} - /** - * Returns 3rd party baggage string of @param baggage - * @param baggage + * Takes a baggage header and turns it into Dynamic Sampling Context, by extracting all the "sentry-" prefixed values + * from it. + * + * @param baggageHeader A very bread definition of a baggage header as it might appear in various frameworks. + * @returns The Dynamic Sampling Context that was found on `baggageHeader`, if there was any, `undefined` otherwise. */ -export function getThirdPartyBaggage(baggage: Baggage): string { - return baggage[1]; -} +export function baggageHeaderToDynamicSamplingContext( + // Very liberal definition of what any incoming header might look like + baggageHeader: string | string[] | number | null | undefined | boolean, +): Partial | undefined { + if (!isString(baggageHeader) && !Array.isArray(baggageHeader)) { + return undefined; + } -/** - * Checks if baggage is mutable - * @param baggage - * @returns true if baggage is mutable, else false - */ -export function isBaggageMutable(baggage: Baggage): boolean { - return baggage[2]; -} + // Intermediary object to store baggage key value pairs of incoming baggage headers on. + // It is later used to read Sentry-DSC-values from. + let baggageObject: Readonly> = {}; + + if (Array.isArray(baggageHeader)) { + // Combine all baggage headers into one object containing the baggage values so we can later read the Sentry-DSC-values from it + baggageObject = baggageHeader.reduce>((acc, curr) => { + const currBaggageObject = baggageHeaderToObject(curr); + return { + ...acc, + ...currBaggageObject, + }; + }, {}); + } else { + // Return undefined if baggage header is an empty string (technically an empty baggage header is not spec conform but + // this is how we choose to handle it) + if (!baggageHeader) { + return undefined; + } -/** - * Sets the passed baggage immutable - * @param baggage - */ -export function setBaggageImmutable(baggage: Baggage): void { - baggage[2] = false; -} + baggageObject = baggageHeaderToObject(baggageHeader); + } -/** Serialize a baggage object */ -export function serializeBaggage(baggage: Baggage): string { - return Object.keys(baggage[0]).reduce((prev, key: keyof BaggageObj) => { - const val = baggage[0][key] as string; - const baggageEntry = `${SENTRY_BAGGAGE_KEY_PREFIX}${encodeURIComponent(key)}=${encodeURIComponent(val)}`; - const newVal = prev === '' ? baggageEntry : `${prev},${baggageEntry}`; - if (newVal.length > MAX_BAGGAGE_STRING_LENGTH) { - __DEBUG_BUILD__ && - logger.warn(`Not adding key: ${key} with val: ${val} to baggage due to exceeding baggage size limits.`); - return prev; - } else { - return newVal; + // Read all "sentry-" prefixed values out of the baggage object and put it onto a dynamic sampling context object. + const dynamicSamplingContext = Object.entries(baggageObject).reduce>((acc, [key, value]) => { + if (key.match(SENTRY_BAGGAGE_KEY_PREFIX_REGEX)) { + const nonPrefixedKey = key.slice(SENTRY_BAGGAGE_KEY_PREFIX.length); + acc[nonPrefixedKey] = value; } - }, baggage[1]); + return acc; + }, {}); + + // Only return a dynamic sampling context object if there are keys in it. + // A keyless object means there were no sentry values on the header, which means that there is no DSC. + if (Object.keys(dynamicSamplingContext).length > 0) { + return dynamicSamplingContext as Partial; + } else { + return undefined; + } } /** - * Parse a baggage header from a string or a string array and return a Baggage object + * Turns a Dynamic Sampling Object into a baggage header by prefixing all the keys on the object with "sentry-". * - * If @param includeThirdPartyEntries is set to true, third party baggage entries are added to the Baggage object - * (This is necessary for merging potentially pre-existing baggage headers in outgoing requests with - * our `sentry-` values) + * @param dynamicSamplingContext The Dynamic Sampling Context to turn into a header. For convenience and compatibility + * with the `getDynamicSamplingContext` method on the Transaction class ,this argument can also be `undefined`. If it is + * `undefined` the function will return `undefined`. + * @returns a baggage header, created from `dynamicSamplingContext`, or `undefined` either if `dynamicSamplingContext` + * was `undefined`, or if `dynamicSamplingContext` didn't contain any values. */ -export function parseBaggageHeader( - inputBaggageValue: HttpHeaderValue, - includeThirdPartyEntries: boolean = false, -): Baggage { - // Adding this check here because we got reports of this function failing due to the input value - // not being a string. This debug log might help us determine what's going on here. - if ((!Array.isArray(inputBaggageValue) && !isString(inputBaggageValue)) || typeof inputBaggageValue === 'number') { - __DEBUG_BUILD__ && - logger.warn( - '[parseBaggageHeader] Received input value of incompatible type: ', - typeof inputBaggageValue, - inputBaggageValue, - ); - - // Gonna early-return an empty baggage object so that we don't fail later on - return createBaggage({}, ''); - } - - const baggageEntries = (isString(inputBaggageValue) ? inputBaggageValue : inputBaggageValue.join(',')) - .split(',') - .map(entry => entry.trim()) - .filter(entry => entry !== '' && (includeThirdPartyEntries || SENTRY_BAGGAGE_KEY_PREFIX_REGEX.test(entry))); - - return baggageEntries.reduce( - ([baggageObj, baggageString], curr) => { - const [key, val] = curr.split('='); - if (SENTRY_BAGGAGE_KEY_PREFIX_REGEX.test(key)) { - const baggageKey = decodeURIComponent(key.split('-')[1]); - return [ - { - ...baggageObj, - [baggageKey]: decodeURIComponent(val), - }, - baggageString, - true, - ]; - } else { - return [baggageObj, baggageString === '' ? curr : `${baggageString},${curr}`, true]; +export function dynamicSamplingContextToSentryBaggageHeader( + // this also takes undefined for convenience and bundle size in other places + dynamicSamplingContext: Partial, +): string | undefined { + // Prefix all DSC keys with "sentry-" and put them into a new object + const sentryPrefixedDSC = Object.entries(dynamicSamplingContext).reduce>( + (acc, [dscKey, dscValue]) => { + if (dscValue) { + acc[`${SENTRY_BAGGAGE_KEY_PREFIX}${dscKey}`] = dscValue; } + return acc; }, - [{}, '', true], + {}, ); + + return objectToBaggageHeader(sentryPrefixedDSC); } /** - * 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 thirdPartyBaggageHeader possibly existing baggage header string or string[] added from a third - * party to the request headers + * Will parse a baggage header, which is a simple key-value map, into a flat object. * - * @return a merged and serialized baggage string to be propagated with the outgoing request + * @param baggageHeader The baggage header to parse. + * @returns a flat object containing all the key-value pairs from `baggageHeader`. */ -export function mergeAndSerializeBaggage(incomingBaggage?: Baggage, thirdPartyBaggageHeader?: HttpHeaderValue): string { - if (!incomingBaggage && !thirdPartyBaggageHeader) { - return ''; - } - - const headerBaggage = (thirdPartyBaggageHeader && parseBaggageHeader(thirdPartyBaggageHeader, true)) || undefined; - const thirdPartyHeaderBaggage = headerBaggage && getThirdPartyBaggage(headerBaggage); - - const finalBaggage = createBaggage((incomingBaggage && incomingBaggage[0]) || {}, thirdPartyHeaderBaggage || ''); - return serializeBaggage(finalBaggage); +function baggageHeaderToObject(baggageHeader: string): Record { + return baggageHeader + .split(',') + .map(baggageEntry => baggageEntry.split('=').map(keyOrValue => decodeURIComponent(keyOrValue.trim()))) + .reduce>((acc, [key, value]) => { + acc[key] = value; + return acc; + }, {}); } /** - * Helper function that takes a raw baggage value (if available) and the processed sentry-trace header - * data (if available), parses the baggage value and creates a Baggage object. If there is no baggage - * value, it will create an empty Baggage object. - * - * In a second step, this functions determines if the created Baggage object should be set immutable - * to prevent mutation of the Sentry data. It does this by looking at the processed sentry-trace header. + * Turns a flat object (key-value pairs) into a baggage header, which is also just key-value pairs. * - * @param rawBaggageValue baggage value from header - * @param sentryTraceHeader processed Sentry trace header returned from `extractTraceparentData` + * @param object The object to turn into a baggage header. + * @returns a baggage header string, or `undefined` if the object didn't have any values, since an empty baggage header + * is not spec compliant. */ -export function parseBaggageSetMutability( - rawBaggageValue: HttpHeaderValue | false | undefined, - sentryTraceHeader: TraceparentData | string | false | undefined | null, -): Baggage { - const baggage = parseBaggageHeader(rawBaggageValue || ''); - - // Because we are always creating a Baggage object by calling `parseBaggageHeader` above - // (either a filled one or an empty one, even if we didn't get a `baggage` header), - // we only need to check if we have a sentry-trace header or not. As soon as we have it, - // we set baggage immutable. In case we don't get a sentry-trace header, we can assume that - // this SDK is the head of the trace and thus we still permit mutation at this time. - // There is one exception though, which is that we get a baggage-header with `sentry-` - // items but NO sentry-trace header. In this case we also set the baggage immutable for now - // but if something like this would ever happen, we should revisit this and determine - // what this would actually mean for the trace (i.e. is this SDK the head?, what happened - // before that we don't have a sentry-trace header?, etc) - (sentryTraceHeader || !isSentryBaggageEmpty(baggage)) && setBaggageImmutable(baggage); - - return baggage; +function objectToBaggageHeader(object: Record): string | undefined { + if (Object.keys(object).length === 0) { + // An empty baggage header is not spec compliant: We return undefined. + return undefined; + } + + return Object.entries(object).reduce((baggageHeader, [objectKey, objectValue], currentIndex) => { + const baggageEntry = `${encodeURIComponent(objectKey)}=${encodeURIComponent(objectValue)}`; + const newBaggageHeader = currentIndex === 0 ? baggageEntry : `${baggageHeader},${baggageEntry}`; + if (newBaggageHeader.length > MAX_BAGGAGE_STRING_LENGTH) { + __DEBUG_BUILD__ && + logger.warn( + `Not adding key: ${objectKey} with val: ${objectValue} to baggage header due to exceeding baggage size limits.`, + ); + return baggageHeader; + } else { + return newBaggageHeader; + } + }, ''); } diff --git a/packages/utils/src/tracing.ts b/packages/utils/src/tracing.ts index aef92cf5f0ca..cef6e334f7e7 100644 --- a/packages/utils/src/tracing.ts +++ b/packages/utils/src/tracing.ts @@ -17,18 +17,22 @@ export const TRACEPARENT_REGEXP = new RegExp( */ export function extractTraceparentData(traceparent: string): TraceparentData | undefined { const matches = traceparent.match(TRACEPARENT_REGEXP); - if (matches) { - let parentSampled: boolean | undefined; - if (matches[3] === '1') { - parentSampled = true; - } else if (matches[3] === '0') { - parentSampled = false; - } - return { - traceId: matches[1], - parentSampled, - parentSpanId: matches[2], - }; + + if (!traceparent || !matches) { + // empty string or no matches is invalid traceparent data + return undefined; + } + + let parentSampled: boolean | undefined; + if (matches[3] === '1') { + parentSampled = true; + } else if (matches[3] === '0') { + parentSampled = false; } - return undefined; + + return { + traceId: matches[1], + parentSampled, + parentSpanId: matches[2], + }; } diff --git a/packages/utils/test/baggage.test.ts b/packages/utils/test/baggage.test.ts index cfd8ebd5f239..8f848badf9de 100644 --- a/packages/utils/test/baggage.test.ts +++ b/packages/utils/test/baggage.test.ts @@ -1,254 +1,40 @@ -import { Baggage } from '@sentry/types'; - -import { - createBaggage, - getBaggageValue, - isBaggageMutable, - isSentryBaggageEmpty, - mergeAndSerializeBaggage, - parseBaggageHeader, - parseBaggageSetMutability, - serializeBaggage, - setBaggageImmutable, - setBaggageValue, -} from '../src/baggage'; - -describe('Baggage', () => { - describe('createBaggage', () => { - it.each([ - ['creates an empty baggage instance', {}, [{}, '', true]], - [ - 'creates a baggage instance with initial values', - { environment: 'production', anyKey: 'anyValue' }, - [{ environment: 'production', anyKey: 'anyValue' }, '', true], - ], - ])('%s', (_: string, input, output) => { - expect(createBaggage(input)).toEqual(output); - }); - - it('creates a baggage instance and marks it immutable if explicitly specified', () => { - expect(createBaggage({ environment: 'production', anyKey: 'anyValue' }, '', false)).toEqual([ - { environment: 'production', anyKey: 'anyValue' }, - '', - false, - ]); - }); - }); - - describe('getBaggageValue', () => { - it.each([ - [ - 'gets a baggage item', - createBaggage({ environment: 'production', anyKey: 'anyValue' }), - 'environment', - 'production', - ], - ['finds undefined items', createBaggage({}), 'environment', undefined], - ])('%s', (_: string, baggage, key, value) => { - expect(getBaggageValue(baggage, key)).toEqual(value); - }); - }); - - describe('setBaggageValue', () => { - it.each([ - ['sets a baggage item', createBaggage({}), 'environment', 'production'], - ['overwrites a baggage item', createBaggage({ environment: 'development' }), 'environment', 'production'], - [ - 'does not set a value if the passed baggage item is immutable', - createBaggage({ environment: 'development' }, '', false), - 'environment', - 'development', - ], - ])('%s', (_: string, baggage, key, value) => { - setBaggageValue(baggage, key, value); - expect(getBaggageValue(baggage, key)).toEqual(value); - }); - }); - - describe('serializeBaggage', () => { - it.each([ - ['serializes empty baggage', createBaggage({}), ''], - [ - 'serializes baggage with a single value', - createBaggage({ environment: 'production' }), - 'sentry-environment=production', - ], - [ - 'serializes baggage with multiple values', - createBaggage({ environment: 'production', release: '10.0.2' }), - 'sentry-environment=production,sentry-release=10.0.2', - ], - [ - 'keeps non-sentry prefixed baggage items', - createBaggage( - { environment: 'production', release: '10.0.2' }, - 'userId=alice,serverNode=DF%2028,isProduction=false', - ), - 'userId=alice,serverNode=DF%2028,isProduction=false,sentry-environment=production,sentry-release=10.0.2', - ], - [ - 'can only use non-sentry prefixed baggage items', - createBaggage({}, 'userId=alice,serverNode=DF%2028,isProduction=false'), - 'userId=alice,serverNode=DF%2028,isProduction=false', - ], - ])('%s', (_: string, baggage, serializedBaggage) => { - expect(serializeBaggage(baggage)).toEqual(serializedBaggage); - }); - }); - - describe('parseBaggageHeader', () => { - it.each([ - ['parses an empty string', '', undefined, createBaggage({})], - ['parses a blank string', ' ', undefined, createBaggage({})], - [ - 'parses sentry values into baggage', - 'sentry-environment=production,sentry-release=10.0.2', - undefined, - createBaggage({ environment: 'production', release: '10.0.2' }), - ], - [ - 'ignores 3rd party entries by default', - 'userId=alice,serverNode=DF%2028,isProduction=false,sentry-environment=production,sentry-release=10.0.2', - undefined, - createBaggage({ environment: 'production', release: '10.0.2' }, ''), - ], - [ - 'parses sentry- and arbitrary 3rd party values if the 3rd party entries flag is set to true', - 'userId=alice,serverNode=DF%2028,isProduction=false,sentry-environment=production,sentry-release=10.0.2', - true, - createBaggage( - { environment: 'production', release: '10.0.2' }, - 'userId=alice,serverNode=DF%2028,isProduction=false', - ), - ], - [ - 'parses arbitrary baggage entries from string with empty and blank entries', - 'userId=alice, serverNode=DF%2028 , isProduction=false, ,,sentry-environment=production,,sentry-release=10.0.2', - true, - createBaggage( - { environment: 'production', release: '10.0.2' }, - 'userId=alice,serverNode=DF%2028,isProduction=false', - ), - ], - [ - 'parses a string array', - ['userId=alice', 'sentry-environment=production', 'foo=bar'], - true, - createBaggage({ environment: 'production' }, 'userId=alice,foo=bar'), - ], - [ - 'parses a string array with items containing multiple entries', - ['userId=alice, userName=bob', 'sentry-environment=production,sentry-release=1.0.1', 'foo=bar'], - true, - createBaggage({ environment: 'production', release: '1.0.1' }, 'userId=alice,userName=bob,foo=bar'), - ], - [ - 'parses a string array with empty/blank entries', - ['', 'sentry-environment=production,sentry-release=1.0.1', ' ', 'foo=bar'], - true, - createBaggage({ environment: 'production', release: '1.0.1' }, 'foo=bar'), - ], - ['ignorese other input types than string and string[]', 42, undefined, createBaggage({}, '')], - ])('%s', (_: string, baggageValue, includeThirPartyEntries, expectedBaggage) => { - expect(parseBaggageHeader(baggageValue, includeThirPartyEntries)).toEqual(expectedBaggage); - }); - }); - - describe('isSentryBaggageEmpty', () => { - it.each([ - ['returns true if the Sentry part of baggage is empty', createBaggage({}), true], - ['returns false if the Sentry part of baggage is not empty', createBaggage({ release: '10.0.2' }), false], - ])('%s', (_: string, baggage, outcome) => { - expect(isSentryBaggageEmpty(baggage)).toEqual(outcome); - }); - }); - - describe('mergeAndSerializeBaggage', () => { - it.each([ - [ - 'returns original baggage when there is no additional baggage header', - createBaggage({ release: '1.1.1', user_id: '1234' }), - undefined, - 'sentry-release=1.1.1,sentry-user_id=1234', - ], - [ - 'returns merged baggage when there is a 3rd party header added', - createBaggage({ release: '1.1.1', user_id: '1234' }, 'foo=bar'), - 'bar=baz,key=value', - 'bar=baz,key=value,sentry-release=1.1.1,sentry-user_id=1234', - ], - ['returns merged baggage original baggage is empty', createBaggage({}), 'bar=baz,key=value', 'bar=baz,key=value'], - [ - 'ignores sentry- items in 3rd party baggage header', - createBaggage({}), - 'bar=baz,sentry-user_id=abc,key=value,sentry-sample_rate=0.76', - '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); - }); - }); - - describe('parseBaggageSetMutability', () => { - it.each([ - [ - 'returns an empty, mutable baggage object if both params are undefined', - undefined, - undefined, - [{}, '', true] as Baggage, - ], - [ - 'returns an empty, immutable baggage object if sentry-trace header data is defined', - undefined, - {}, - [{}, '', false] as Baggage, - ], - [ - 'returns an empty, immutable baggage object if sentry-trace header data is a string', - undefined, - '123', - [{}, '', false] as Baggage, - ], - [ - 'returns a non-empty, mutable baggage object if sentry-trace is not defined and ignores 3rd party baggage items', - 'foo=bar', - undefined, - [{}, '', true] as Baggage, - ], - [ - 'returns a non-empty, immutable baggage object if sentry-trace is defined', - 'foo=bar,sentry-environment=production,sentry-sample_rate=0.96', - {}, - [{ environment: 'production', sample_rate: '0.96' }, '', false] as Baggage, - ], - ])( - '%s', - (_: string, baggageString: string | undefined, sentryTraceData: any | string | undefined, result: Baggage) => { - expect(parseBaggageSetMutability(baggageString, sentryTraceData)).toEqual(result); - }, - ); - }); - - describe('isBaggageMutable', () => { - it.each([ - ['returns false if baggage is set immutable', false], - ['returns true if baggage is set mutable', true], - ])('%s', (_: string, outcome) => { - const baggage: Baggage = [{}, '', outcome]; - expect(isBaggageMutable(baggage)).toEqual(outcome); - }); - }); +import { baggageHeaderToDynamicSamplingContext, dynamicSamplingContextToSentryBaggageHeader } from '../src/baggage'; + +test.each([ + ['', undefined], + [' ', undefined], + ['sentry-environment=production,sentry-release=10.0.2', { environment: 'production', release: '10.0.2' }], + [ + 'userId=alice,serverNode=DF%2028,isProduction=false,sentry-environment=production,sentry-release=10.0.2', + { environment: 'production', release: '10.0.2' }, + ], + ['userId=alice,serverNode=DF%2028,isProduction=false', undefined], + [ + 'userId=alice, serverNode=DF%2028 , isProduction=false, ,,sentry-environment=production,,sentry-release=10.0.2', + { environment: 'production', release: '10.0.2' }, + ], + [['userId=alice', 'sentry-environment=production', 'foo=bar'], { environment: 'production' }], + [ + ['userId=alice, userName=bob', 'sentry-environment=production,sentry-release=1.0.1', 'foo=bar'], + { environment: 'production', release: '1.0.1' }, + ], + [ + ['', 'sentry-environment=production,sentry-release=1.0.1', ' ', 'foo=bar'], + { environment: 'production', release: '1.0.1' }, + ], + [42, undefined], +])('baggageHeaderToDynamicSamplingContext(%p) should return %p', (input, expectedOutput) => { + expect(baggageHeaderToDynamicSamplingContext(input)).toStrictEqual(expectedOutput); +}); - describe('setBaggageImmutable', () => { - it.each([ - ['sets baggage immutable', [{}, '', true] as Baggage], - ['does not do anything when baggage is already immutable', [{}, '', false] as Baggage], - ])('%s', (_: string, baggage: Baggage) => { - setBaggageImmutable(baggage); - expect(baggage[2]).toEqual(false); - }); - }); +test.each([ + [{}, undefined], + [{ release: 'abcdf' }, 'sentry-release=abcdf'], + [{ release: 'abcdf', environment: '1234' }, 'sentry-release=abcdf,sentry-environment=1234'], + [ + { release: 'abcdf', environment: '1234', someRandomKey: 'foo' }, + 'sentry-release=abcdf,sentry-environment=1234,sentry-someRandomKey=foo', + ], +])('dynamicSamplingContextToSentryBaggageHeader(%p) should return %p', (input, expectedOutput) => { + expect(dynamicSamplingContextToSentryBaggageHeader(input)).toStrictEqual(expectedOutput); });