From e661b2ad41bb88a88e517c105675a9d6bdb7068f Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 28 Jun 2023 15:40:40 -0400 Subject: [PATCH 1/5] update tests --- .../suites/replay/captureReplay/test.ts | 4 ++++ .../suites/replay/captureReplayFromReplayPackage/test.ts | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/packages/browser-integration-tests/suites/replay/captureReplay/test.ts b/packages/browser-integration-tests/suites/replay/captureReplay/test.ts index 421e725bb3e4..f1aff48ac9df 100644 --- a/packages/browser-integration-tests/suites/replay/captureReplay/test.ts +++ b/packages/browser-integration-tests/suites/replay/captureReplay/test.ts @@ -56,6 +56,10 @@ sentryTest('should capture replays (@sentry/browser export)', async ({ getLocalT version: SDK_VERSION, name: 'sentry.javascript.browser', }, +<<<<<<< HEAD +======= + sdkProcessingMetadata: expect.any(Object), +>>>>>>> 929289c22 (update tests) request: { url: expect.stringContaining('/dist/index.html'), headers: { diff --git a/packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts b/packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts index c42bfc692018..2509bb764020 100644 --- a/packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts +++ b/packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts @@ -56,6 +56,10 @@ sentryTest('should capture replays (@sentry/replay export)', async ({ getLocalTe version: SDK_VERSION, name: 'sentry.javascript.browser', }, +<<<<<<< HEAD +======= + sdkProcessingMetadata: expect.any(Object), +>>>>>>> 929289c22 (update tests) request: { url: expect.stringContaining('/dist/index.html'), headers: { From 9c09a95cfd5673a317ace43e7d6fe2299298651b Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 28 Jun 2023 16:01:56 -0400 Subject: [PATCH 2/5] remove sdkProcessingMetadata from replay fixtures --- .../suites/replay/captureReplay/test.ts | 4 ---- .../suites/replay/captureReplayFromReplayPackage/test.ts | 4 ---- 2 files changed, 8 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/captureReplay/test.ts b/packages/browser-integration-tests/suites/replay/captureReplay/test.ts index f1aff48ac9df..421e725bb3e4 100644 --- a/packages/browser-integration-tests/suites/replay/captureReplay/test.ts +++ b/packages/browser-integration-tests/suites/replay/captureReplay/test.ts @@ -56,10 +56,6 @@ sentryTest('should capture replays (@sentry/browser export)', async ({ getLocalT version: SDK_VERSION, name: 'sentry.javascript.browser', }, -<<<<<<< HEAD -======= - sdkProcessingMetadata: expect.any(Object), ->>>>>>> 929289c22 (update tests) request: { url: expect.stringContaining('/dist/index.html'), headers: { diff --git a/packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts b/packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts index 2509bb764020..c42bfc692018 100644 --- a/packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts +++ b/packages/browser-integration-tests/suites/replay/captureReplayFromReplayPackage/test.ts @@ -56,10 +56,6 @@ sentryTest('should capture replays (@sentry/replay export)', async ({ getLocalTe version: SDK_VERSION, name: 'sentry.javascript.browser', }, -<<<<<<< HEAD -======= - sdkProcessingMetadata: expect.any(Object), ->>>>>>> 929289c22 (update tests) request: { url: expect.stringContaining('/dist/index.html'), headers: { From 733bca83ff1b545b3a22f1d8a4c8fc56473581f2 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 29 Jun 2023 12:26:47 -0400 Subject: [PATCH 3/5] ref(nextjs): Set propagation context for tracing --- packages/nextjs/src/client/performance.ts | 40 ++++++++++++------- .../nextjs/src/edge/utils/edgeWrapperUtils.ts | 26 +++++------- .../nextjs/src/server/utils/wrapperUtils.ts | 18 +++++---- .../src/server/wrapApiHandlerWithSentry.ts | 22 +++++----- .../server/wrapServerComponentWithSentry.ts | 15 +++---- 5 files changed, 66 insertions(+), 55 deletions(-) diff --git a/packages/nextjs/src/client/performance.ts b/packages/nextjs/src/client/performance.ts index 0dd0b109408f..f6dc91c6f158 100644 --- a/packages/nextjs/src/client/performance.ts +++ b/packages/nextjs/src/client/performance.ts @@ -1,12 +1,14 @@ import { getCurrentHub } from '@sentry/core'; import { WINDOW } from '@sentry/react'; -import type { Primitive, TraceparentData, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; -import { - baggageHeaderToDynamicSamplingContext, - extractTraceparentData, - logger, - stripUrlQueryAndFragment, -} from '@sentry/utils'; +import type { + DynamicSamplingContext, + Primitive, + TraceparentData, + Transaction, + TransactionContext, + TransactionSource, +} from '@sentry/types'; +import { logger, stripUrlQueryAndFragment, tracingContextFromHeaders } from '@sentry/utils'; import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils'; import { default as Router } from 'next/router'; import type { ParsedUrlQuery } from 'querystring'; @@ -38,7 +40,7 @@ interface SentryEnhancedNextData extends NextData { interface NextDataTagInfo { route?: string; traceParentData?: TraceparentData; - baggage?: string; + dynamicSamplingContext?: Partial; params?: ParsedUrlQuery; } @@ -83,13 +85,23 @@ function extractNextDataTagInformation(): NextDataTagInfo { nextDataTagInfo.params = query; if (props && props.pageProps) { - if (props.pageProps._sentryBaggage) { - nextDataTagInfo.baggage = props.pageProps._sentryBaggage; + const sentryTrace = props.pageProps._sentryTraceData; + const baggage = props.pageProps._sentryBaggage; + + const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders( + sentryTrace, + baggage, + ); + + if (dynamicSamplingContext) { + nextDataTagInfo.dynamicSamplingContext = dynamicSamplingContext; } - if (props.pageProps._sentryTraceData) { - nextDataTagInfo.traceParentData = extractTraceparentData(props.pageProps._sentryTraceData); + if (traceparentData) { + nextDataTagInfo.traceParentData = traceparentData; } + + getCurrentHub().getScope().setPropagationContext(propagationContext); } return nextDataTagInfo; @@ -121,13 +133,11 @@ export function nextRouterInstrumentation( startTransactionOnPageLoad: boolean = true, startTransactionOnLocationChange: boolean = true, ): void { - const { route, traceParentData, baggage, params } = extractNextDataTagInformation(); + const { route, traceParentData, dynamicSamplingContext, params } = extractNextDataTagInformation(); prevLocationName = route || globalObject.location.pathname; if (startTransactionOnPageLoad) { const source = route ? 'route' : 'url'; - const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggage); - activeTransaction = startTransactionCb({ name: prevLocationName, op: 'pageload', diff --git a/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts b/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts index 384074e317bd..256499c97185 100644 --- a/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts +++ b/packages/nextjs/src/edge/utils/edgeWrapperUtils.ts @@ -1,12 +1,6 @@ import { captureException, getCurrentHub, hasTracingEnabled, startTransaction } from '@sentry/core'; import type { Span } from '@sentry/types'; -import { - addExceptionMechanism, - baggageHeaderToDynamicSamplingContext, - extractTraceparentData, - logger, - objectify, -} from '@sentry/utils'; +import { addExceptionMechanism, logger, objectify, tracingContextFromHeaders } from '@sentry/utils'; import type { EdgeRouteHandler } from '../types'; import { flush } from './flush'; @@ -32,17 +26,17 @@ export function withEdgeWrapping( op: options.spanOp, }); } else if (req instanceof Request) { - // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) - let traceparentData; - - const sentryTraceHeader = req.headers.get('sentry-trace'); - if (sentryTraceHeader) { - traceparentData = extractTraceparentData(sentryTraceHeader); - __DEBUG_BUILD__ && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); + const sentryTrace = req.headers.get('sentry-trace') || ''; + const baggage = req.headers.get('baggage'); + const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders( + sentryTrace, + baggage, + ); + currentScope.setPropagationContext(propagationContext); + if (traceparentData) { + __DEBUG_BUILD__ && logger.log(`[Tracing] Continuing trace ${traceparentData.traceId}.`); } - const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(req.headers.get('baggage')); - span = startTransaction({ name: options.spanDescription, op: options.spanOp, diff --git a/packages/nextjs/src/server/utils/wrapperUtils.ts b/packages/nextjs/src/server/utils/wrapperUtils.ts index c0fb4037c58b..a3cf13736900 100644 --- a/packages/nextjs/src/server/utils/wrapperUtils.ts +++ b/packages/nextjs/src/server/utils/wrapperUtils.ts @@ -6,7 +6,7 @@ import { startTransaction, } from '@sentry/core'; import type { Span, Transaction } from '@sentry/types'; -import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils'; +import { isString, tracingContextFromHeaders } from '@sentry/utils'; import type { IncomingMessage, ServerResponse } from 'http'; import { platformSupportsStreaming } from './platformSupportsStreaming'; @@ -38,6 +38,7 @@ function setTransactionOnRequest(transaction: Transaction, req: IncomingMessage) * * Note: This function turns the wrapped function into an asynchronous one. */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any export function withErrorInstrumentation any>( origFunction: F, ): (...params: Parameters) => Promise> { @@ -66,6 +67,7 @@ export function withErrorInstrumentation any>( * @param options Options providing details for the created transaction and span. * @returns what the data fetching method call returned. */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any export function withTracedServerSideDataFetcher Promise | any>( origDataFetcher: F, req: IncomingMessage, @@ -86,12 +88,14 @@ export function withTracedServerSideDataFetcher Pr const previousSpan: Span | undefined = getTransactionFromRequest(req) ?? scope.getSpan(); let dataFetcherSpan; - const sentryTraceHeader = req.headers['sentry-trace']; - const rawBaggageString = req.headers && req.headers.baggage; - const traceparentData = - typeof sentryTraceHeader === 'string' ? extractTraceparentData(sentryTraceHeader) : undefined; - - const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(rawBaggageString); + const sentryTrace = + req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined; + const baggage = req.headers?.baggage; + const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders( + sentryTrace, + baggage, + ); + scope.setPropagationContext(propagationContext); if (platformSupportsStreaming()) { let spanToContinue: Span; diff --git a/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts index 9aecbd9a6c6b..524e29fa0ca0 100644 --- a/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts @@ -3,12 +3,11 @@ import { captureException, startTransaction } from '@sentry/node'; import type { Transaction } from '@sentry/types'; import { addExceptionMechanism, - baggageHeaderToDynamicSamplingContext, - extractTraceparentData, isString, logger, objectify, stripUrlQueryAndFragment, + tracingContextFromHeaders, } from '@sentry/utils'; import type { AugmentedNextApiRequest, AugmentedNextApiResponse, NextApiHandler } from './types'; @@ -73,15 +72,18 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri currentScope.setSDKProcessingMetadata({ request: req }); if (hasTracingEnabled(options) && options?.instrumenter === 'sentry') { - // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) - let traceparentData; - if (req.headers && isString(req.headers['sentry-trace'])) { - traceparentData = extractTraceparentData(req.headers['sentry-trace']); - __DEBUG_BUILD__ && logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); - } + const sentryTrace = + req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined; + const baggage = req.headers?.baggage; + const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders( + sentryTrace, + baggage, + ); + hub.getScope().setPropagationContext(propagationContext); - const baggageHeader = req.headers && req.headers.baggage; - const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggageHeader); + if (__DEBUG_BUILD__ && traceparentData) { + logger.log(`[Tracing] Continuing trace ${traceparentData.traceId}.`); + } // prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler) let reqPath = parameterizedRoute; diff --git a/packages/nextjs/src/server/wrapServerComponentWithSentry.ts b/packages/nextjs/src/server/wrapServerComponentWithSentry.ts index 8b0f1d8f2d79..81f030fa824c 100644 --- a/packages/nextjs/src/server/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/server/wrapServerComponentWithSentry.ts @@ -5,7 +5,7 @@ import { runWithAsyncContext, startTransaction, } from '@sentry/core'; -import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils'; +import { tracingContextFromHeaders } from '@sentry/utils'; import { isNotFoundNavigationError, isRedirectNavigationError } from '../common/nextNavigationErrorUtils'; import type { ServerComponentContext } from '../common/types'; @@ -13,6 +13,7 @@ import type { ServerComponentContext } from '../common/types'; /** * Wraps an `app` directory server component with Sentry error instrumentation. */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any export function wrapServerComponentWithSentry any>( appDirComponent: F, context: ServerComponentContext, @@ -28,14 +29,15 @@ export function wrapServerComponentWithSentry any> apply: (originalFunction, thisArg, args) => { return runWithAsyncContext(() => { const hub = getCurrentHub(); + const currentScope = hub.getScope(); let maybePromiseResult; - const traceparentData = context.sentryTraceHeader - ? extractTraceparentData(context.sentryTraceHeader) - : undefined; - - const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(context.baggageHeader); + const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders( + context.sentryTraceHeader, + context.baggageHeader, + ); + currentScope.setPropagationContext(propagationContext); const transaction = startTransaction({ op: 'function.nextjs', @@ -48,7 +50,6 @@ export function wrapServerComponentWithSentry any> }, }); - const currentScope = hub.getScope(); if (currentScope) { currentScope.setSpan(transaction); } From 25255e28b2b1a4b4dac2e0f55b9b12335a80cbaa Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 29 Jun 2023 15:35:46 -0400 Subject: [PATCH 4/5] make tracingContextFromHeaders more liberal with input --- packages/utils/src/tracing.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/utils/src/tracing.ts b/packages/utils/src/tracing.ts index 445c271a0ee5..053a05cd1535 100644 --- a/packages/utils/src/tracing.ts +++ b/packages/utils/src/tracing.ts @@ -18,11 +18,13 @@ export const TRACEPARENT_REGEXP = new RegExp( * * @returns Object containing data from the header, or undefined if traceparent string is malformed */ -export function extractTraceparentData(traceparent: string): TraceparentData | undefined { - const matches = traceparent.match(TRACEPARENT_REGEXP); +export function extractTraceparentData(traceparent?: string): TraceparentData | undefined { + if (!traceparent) { + return undefined; + } - if (!traceparent || !matches) { - // empty string or no matches is invalid traceparent data + const matches = traceparent.match(TRACEPARENT_REGEXP); + if (!matches) { return undefined; } From 240f0f23305b0a2974f3d287f7c21fa153d7c612 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 30 Jun 2023 10:40:16 -0400 Subject: [PATCH 5/5] pr review feedback --- packages/nextjs/src/client/performance.ts | 44 ++++++++--------------- 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/packages/nextjs/src/client/performance.ts b/packages/nextjs/src/client/performance.ts index f6dc91c6f158..fd5f979a08b0 100644 --- a/packages/nextjs/src/client/performance.ts +++ b/packages/nextjs/src/client/performance.ts @@ -1,13 +1,6 @@ import { getCurrentHub } from '@sentry/core'; import { WINDOW } from '@sentry/react'; -import type { - DynamicSamplingContext, - Primitive, - TraceparentData, - Transaction, - TransactionContext, - TransactionSource, -} from '@sentry/types'; +import type { Primitive, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; import { logger, stripUrlQueryAndFragment, tracingContextFromHeaders } from '@sentry/utils'; import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils'; import { default as Router } from 'next/router'; @@ -39,9 +32,9 @@ interface SentryEnhancedNextData extends NextData { interface NextDataTagInfo { route?: string; - traceParentData?: TraceparentData; - dynamicSamplingContext?: Partial; params?: ParsedUrlQuery; + sentryTrace?: string; + baggage?: string; } /** @@ -85,23 +78,8 @@ function extractNextDataTagInformation(): NextDataTagInfo { nextDataTagInfo.params = query; if (props && props.pageProps) { - const sentryTrace = props.pageProps._sentryTraceData; - const baggage = props.pageProps._sentryBaggage; - - const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders( - sentryTrace, - baggage, - ); - - if (dynamicSamplingContext) { - nextDataTagInfo.dynamicSamplingContext = dynamicSamplingContext; - } - - if (traceparentData) { - nextDataTagInfo.traceParentData = traceparentData; - } - - getCurrentHub().getScope().setPropagationContext(propagationContext); + nextDataTagInfo.sentryTrace = props.pageProps._sentryTraceData; + nextDataTagInfo.baggage = props.pageProps._sentryBaggage; } return nextDataTagInfo; @@ -133,7 +111,13 @@ export function nextRouterInstrumentation( startTransactionOnPageLoad: boolean = true, startTransactionOnLocationChange: boolean = true, ): void { - const { route, traceParentData, dynamicSamplingContext, params } = extractNextDataTagInformation(); + const { route, params, sentryTrace, baggage } = extractNextDataTagInformation(); + const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders( + sentryTrace, + baggage, + ); + + getCurrentHub().getScope().setPropagationContext(propagationContext); prevLocationName = route || globalObject.location.pathname; if (startTransactionOnPageLoad) { @@ -143,10 +127,10 @@ export function nextRouterInstrumentation( op: 'pageload', tags: DEFAULT_TAGS, ...(params && client && client.getOptions().sendDefaultPii && { data: params }), - ...traceParentData, + ...traceparentData, metadata: { source, - dynamicSamplingContext: traceParentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, }, }); }