From 8c8a6015558c169fa23259d03945fd145837362e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 2 Mar 2023 13:52:42 +0000 Subject: [PATCH 1/6] feat(nextjs): Connect traces for server components --- packages/nextjs/src/common/types.ts | 2 ++ .../src/config/loaders/wrappingLoader.ts | 2 +- .../serverComponentWrapperTemplate.ts | 30 +++++++++++++++++-- .../server/wrapServerComponentWithSentry.ts | 9 ++++++ 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/packages/nextjs/src/common/types.ts b/packages/nextjs/src/common/types.ts index d21f3fa92880..cfac0c460a84 100644 --- a/packages/nextjs/src/common/types.ts +++ b/packages/nextjs/src/common/types.ts @@ -1,4 +1,6 @@ export type ServerComponentContext = { componentRoute: string; componentType: string; + sentryTraceHeader?: string; + baggageHeader?: string; }; diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index 879855cafa08..fb3e76be72f0 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -119,7 +119,7 @@ export default function wrappingLoader( // https://github.com/vercel/next.js/blob/295f9da393f7d5a49b0c2e15a2f46448dbdc3895/packages/next/build/analysis/get-page-static-info.ts#L37 // https://github.com/vercel/next.js/blob/a1c15d84d906a8adf1667332a3f0732be615afa0/packages/next-swc/crates/core/src/react_server_components.rs#L247 // We do not want to wrap client components - if (userCode.includes('/* __next_internal_client_entry_do_not_use__ */')) { + if (userCode.includes('__next_internal_client_entry_do_not_use__')) { this.callback(null, userCode, userModuleSourceMap); return; } diff --git a/packages/nextjs/src/config/templates/serverComponentWrapperTemplate.ts b/packages/nextjs/src/config/templates/serverComponentWrapperTemplate.ts index 74e8e1a5b1c3..61eba1aa2353 100644 --- a/packages/nextjs/src/config/templates/serverComponentWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/serverComponentWrapperTemplate.ts @@ -11,6 +11,9 @@ import * as wrapee from '__SENTRY_WRAPPING_TARGET_FILE__'; // eslint-disable-next-line import/no-extraneous-dependencies import * as Sentry from '@sentry/nextjs'; +// @ts-ignore TODO +// eslint-disable-next-line import/no-unresolved +import { headers } from 'next/headers'; type ServerComponentModule = { default: unknown; @@ -22,9 +25,30 @@ const serverComponent = serverComponentModule.default; let wrappedServerComponent; if (typeof serverComponent === 'function') { - wrappedServerComponent = Sentry.wrapServerComponentWithSentry(serverComponent, { - componentRoute: '__ROUTE__', - componentType: '__COMPONENT_TYPE__', + // TODO: explain why this code needs to be in this file + wrappedServerComponent = new Proxy(serverComponent, { + apply: (originalFunction, thisArg, args) => { + let sentryTraceHeader: string | undefined = undefined; + let baggageHeader: string | undefined = undefined; + + if (process.env.NEXT_PHASE !== 'phase-production-build') { + // @ts-ignore TODO + // eslint-disable-next-line import/no-unresolved + // const { headers } = await import('next/headers'); + const headersList = headers(); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + sentryTraceHeader = headersList.get('sentry-trace'); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + baggageHeader = headersList.get('baggage'); + } + + return Sentry.wrapServerComponentWithSentry(originalFunction, { + componentRoute: '__ROUTE__', + componentType: '__COMPONENT_TYPE__', + sentryTraceHeader, + baggageHeader, + }).apply(thisArg, args); + }, }); } else { wrappedServerComponent = serverComponent; diff --git a/packages/nextjs/src/server/wrapServerComponentWithSentry.ts b/packages/nextjs/src/server/wrapServerComponentWithSentry.ts index 05c17c6bede3..eac6a9405ac6 100644 --- a/packages/nextjs/src/server/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/server/wrapServerComponentWithSentry.ts @@ -1,4 +1,5 @@ import { captureException, getCurrentHub, startTransaction } from '@sentry/core'; +import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils'; import * as domain from 'domain'; import type { ServerComponentContext } from '../common/types'; @@ -20,12 +21,20 @@ export function wrapServerComponentWithSentry any> return domain.create().bind(() => { let maybePromiseResult; + const traceparentData = context.sentryTraceHeader + ? extractTraceparentData(context.sentryTraceHeader) + : undefined; + + const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(context.baggageHeader); + const transaction = startTransaction({ op: 'function.nextjs', name: `${componentType} Server Component (${componentRoute})`, status: 'ok', + ...traceparentData, metadata: { source: 'component', + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, }, }); From 0bd417404663b7090ea006bd3a338f68b42d5cdd Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 6 Mar 2023 12:14:42 +0000 Subject: [PATCH 2/6] refine --- .../config/templates/serverComponentWrapperTemplate.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/nextjs/src/config/templates/serverComponentWrapperTemplate.ts b/packages/nextjs/src/config/templates/serverComponentWrapperTemplate.ts index 61eba1aa2353..533d6bc68089 100644 --- a/packages/nextjs/src/config/templates/serverComponentWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/serverComponentWrapperTemplate.ts @@ -11,10 +11,12 @@ import * as wrapee from '__SENTRY_WRAPPING_TARGET_FILE__'; // eslint-disable-next-line import/no-extraneous-dependencies import * as Sentry from '@sentry/nextjs'; -// @ts-ignore TODO +// @ts-ignore This template is only used with the app directory so we know that this dependency exists. // eslint-disable-next-line import/no-unresolved import { headers } from 'next/headers'; +declare function headers(): { get: (header: string) => string | undefined }; + type ServerComponentModule = { default: unknown; }; @@ -32,13 +34,8 @@ if (typeof serverComponent === 'function') { let baggageHeader: string | undefined = undefined; if (process.env.NEXT_PHASE !== 'phase-production-build') { - // @ts-ignore TODO - // eslint-disable-next-line import/no-unresolved - // const { headers } = await import('next/headers'); const headersList = headers(); - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access sentryTraceHeader = headersList.get('sentry-trace'); - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access baggageHeader = headersList.get('baggage'); } From ca8a250dbee0358140548a1a0bc0afb00e62be28 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 6 Mar 2023 12:44:00 +0000 Subject: [PATCH 3/6] explain --- .../src/config/templates/serverComponentWrapperTemplate.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/src/config/templates/serverComponentWrapperTemplate.ts b/packages/nextjs/src/config/templates/serverComponentWrapperTemplate.ts index 533d6bc68089..1c937a5f355c 100644 --- a/packages/nextjs/src/config/templates/serverComponentWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/serverComponentWrapperTemplate.ts @@ -27,12 +27,16 @@ const serverComponent = serverComponentModule.default; let wrappedServerComponent; if (typeof serverComponent === 'function') { - // TODO: explain why this code needs to be in this file + // For some odd Next.js magic reason, `headers()` will not work if used inside `wrapServerComponentsWithSentry`. + // Current assumption is that Next.js applies some loader magic to userfiles, but not files in node_modules. This file + // is technically a userfile so it gets the loader magic applied. wrappedServerComponent = new Proxy(serverComponent, { apply: (originalFunction, thisArg, args) => { let sentryTraceHeader: string | undefined = undefined; let baggageHeader: string | undefined = undefined; + // If we call the headers function inside the build phase, Next.js will automatically mark the server component as + // dynamic(SSR) which we do not want in case the users have a static component. if (process.env.NEXT_PHASE !== 'phase-production-build') { const headersList = headers(); sentryTraceHeader = headersList.get('sentry-trace'); From f1e54bcc145f83a66547e460eb30b3b4ad17ced7 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 6 Mar 2023 15:59:46 +0000 Subject: [PATCH 4/6] Add e2e tests --- .../components/client-error-debug-tools.tsx | 38 +++++++++++++++---- .../components/transaction-context.tsx | 37 ++++++++++-------- .../nextjs-app-dir/tests/trace.test.ts | 31 +++++++++++++++ .../test-utils/event-proxy-server.ts | 38 +++++++++++++------ 4 files changed, 110 insertions(+), 34 deletions(-) create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/components/client-error-debug-tools.tsx b/packages/e2e-tests/test-applications/nextjs-app-dir/components/client-error-debug-tools.tsx index 4e7572a3fb3f..9eeaa227996f 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/components/client-error-debug-tools.tsx +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/components/client-error-debug-tools.tsx @@ -5,7 +5,8 @@ import { TransactionContext } from './transaction-context'; import { captureException } from '@sentry/nextjs'; export function ClientErrorDebugTools() { - const { transactionActive, toggle } = useContext(TransactionContext); + const transactionContextValue = useContext(TransactionContext); + const [transactionName, setTransactionName] = useState(''); const [isFetchingAPIRoute, setIsFetchingAPIRoute] = useState(); const [isFetchingEdgeAPIRoute, setIsFetchingEdgeAPIRoute] = useState(); @@ -18,13 +19,34 @@ export function ClientErrorDebugTools() { return (
- + {transactionContextValue.transactionActive ? ( + + ) : ( + <> + { + setTransactionName(e.target.value); + }} + /> + + + )}