From c7cf887b1da3f6ce37f2255e82cfd4442e1f6af3 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Tue, 30 Apr 2024 10:21:17 +0200 Subject: [PATCH 1/7] only add transaction name when page or layout --- packages/nextjs/src/common/types.ts | 2 +- .../nextjs/src/common/wrapGenerationFunctionWithSentry.ts | 4 +++- packages/nextjs/src/common/wrapServerComponentWithSentry.ts | 5 ++++- packages/nextjs/src/config/loaders/wrappingLoader.ts | 4 ++-- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/nextjs/src/common/types.ts b/packages/nextjs/src/common/types.ts index 9e74983b078e..c182e80c2d20 100644 --- a/packages/nextjs/src/common/types.ts +++ b/packages/nextjs/src/common/types.ts @@ -5,7 +5,7 @@ import type { RequestAsyncStorage } from '../config/templates/requestAsyncStorag export type ServerComponentContext = { componentRoute: string; - componentType: string; + componentType: 'Page' | 'Layout' | 'Head' | 'Not-found' | 'Loading' | 'Unknown'; headers?: WebFetchHeaders; }; diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index 5a320ac4556a..bee058ea6f8c 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -59,7 +59,9 @@ export function wrapGenerationFunctionWithSentry a const propagationContext = commonObjectToPropagationContext(headers, incomingPropagationContext); return withIsolationScope(isolationScope, () => { - isolationScope.setTransactionName(`${componentType}.${generationFunctionIdentifier} (${componentRoute})`); + if(componentType === 'Page' || componentType === 'Layout') { + isolationScope.setTransactionName(`${componentType}.${generationFunctionIdentifier} (${componentRoute})`); + } isolationScope.setSDKProcessingMetadata({ request: { headers: headers ? winterCGHeadersToDict(headers) : undefined, diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts index 7785cb0df2fa..b408d836891c 100644 --- a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts @@ -55,7 +55,10 @@ export function wrapServerComponentWithSentry any> const propagationContext = commonObjectToPropagationContext(context.headers, incomingPropagationContext); return withIsolationScope(isolationScope, () => { - isolationScope.setTransactionName(`${componentType} Server Component (${componentRoute})`); + if(componentType === 'Page' || componentType === 'Layout'){ + isolationScope.setTransactionName(`${componentType} Server Component (${componentRoute})`); + } + getCurrentScope().setPropagationContext(propagationContext); return startSpanManual( { diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index 8689082de95b..a0d953d8315b 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -6,7 +6,7 @@ import * as chalk from 'chalk'; import type { RollupBuild, RollupError } from 'rollup'; import { rollup } from 'rollup'; -import type { VercelCronsConfig } from '../../common/types'; +import type { ServerComponentContext, VercelCronsConfig } from '../../common/types'; import type { LoaderThis } from './types'; // Just a simple placeholder to make referencing module consistent @@ -185,7 +185,7 @@ export default function wrappingLoader( .match(/\/?([^/]+)\.(?:js|ts|jsx|tsx)$/); if (componentTypeMatch && componentTypeMatch[1]) { - let componentType; + let componentType: ServerComponentContext['componentType']; switch (componentTypeMatch[1]) { case 'page': componentType = 'Page'; From 76299f81765862b533bda014246c68960b4f6cc5 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Tue, 30 Apr 2024 14:00:05 +0200 Subject: [PATCH 2/7] only set name if not available --- packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts | 3 ++- packages/nextjs/src/common/wrapServerComponentWithSentry.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index bee058ea6f8c..12563f5311a8 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -59,7 +59,8 @@ export function wrapGenerationFunctionWithSentry a const propagationContext = commonObjectToPropagationContext(headers, incomingPropagationContext); return withIsolationScope(isolationScope, () => { - if(componentType === 'Page' || componentType === 'Layout') { + if (componentType && !isolationScope.getScopeData().transactionName) { + // only set name if not already set, otherwise it gets overwritten by subsequent calls isolationScope.setTransactionName(`${componentType}.${generationFunctionIdentifier} (${componentRoute})`); } isolationScope.setSDKProcessingMetadata({ diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts index b408d836891c..82f3601ea84d 100644 --- a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts @@ -55,7 +55,8 @@ export function wrapServerComponentWithSentry any> const propagationContext = commonObjectToPropagationContext(context.headers, incomingPropagationContext); return withIsolationScope(isolationScope, () => { - if(componentType === 'Page' || componentType === 'Layout'){ + if (componentType && !isolationScope.getScopeData().transactionName) { + // only set name if not already set, otherwise it gets overwritten by subsequent calls isolationScope.setTransactionName(`${componentType} Server Component (${componentRoute})`); } From 90b9500fb9f5c89a821b696b0be2d9a489963a2c Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Tue, 30 Apr 2024 14:46:17 +0200 Subject: [PATCH 3/7] overwrite name if not from generation function --- .../nextjs/src/common/wrapGenerationFunctionWithSentry.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index 12563f5311a8..1173765e26aa 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -59,8 +59,12 @@ export function wrapGenerationFunctionWithSentry a const propagationContext = commonObjectToPropagationContext(headers, incomingPropagationContext); return withIsolationScope(isolationScope, () => { - if (componentType && !isolationScope.getScopeData().transactionName) { - // only set name if not already set, otherwise it gets overwritten by subsequent calls + if ( + componentType && + (!isolationScope.getScopeData().transactionName || + !isolationScope.getScopeData().transactionName?.includes('generate')) + ) { + // only set name if not already set (or overwrite if not "generate..."), otherwise it gets overwritten by subsequent calls isolationScope.setTransactionName(`${componentType}.${generationFunctionIdentifier} (${componentRoute})`); } isolationScope.setSDKProcessingMetadata({ From 525d705d1c12643c5765a8b18f1580c0af57d839 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Tue, 30 Apr 2024 15:45:20 +0200 Subject: [PATCH 4/7] revert conditional for generationFunction --- .../src/common/wrapGenerationFunctionWithSentry.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index 1173765e26aa..5a320ac4556a 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -59,14 +59,7 @@ export function wrapGenerationFunctionWithSentry a const propagationContext = commonObjectToPropagationContext(headers, incomingPropagationContext); return withIsolationScope(isolationScope, () => { - if ( - componentType && - (!isolationScope.getScopeData().transactionName || - !isolationScope.getScopeData().transactionName?.includes('generate')) - ) { - // only set name if not already set (or overwrite if not "generate..."), otherwise it gets overwritten by subsequent calls - isolationScope.setTransactionName(`${componentType}.${generationFunctionIdentifier} (${componentRoute})`); - } + isolationScope.setTransactionName(`${componentType}.${generationFunctionIdentifier} (${componentRoute})`); isolationScope.setSDKProcessingMetadata({ request: { headers: headers ? winterCGHeadersToDict(headers) : undefined, From 97eeed6dad7f802c60fb12ffc952fdcebd70eb3f Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 2 May 2024 11:05:01 +0200 Subject: [PATCH 5/7] set transactionName on scope --- .../common/wrapServerComponentWithSentry.ts | 84 +++++++++---------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts index 82f3601ea84d..60719c83bf47 100644 --- a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts @@ -7,6 +7,7 @@ import { handleCallbackErrors, startSpanManual, withIsolationScope, + withScope, } from '@sentry/core'; import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils'; @@ -55,51 +56,50 @@ export function wrapServerComponentWithSentry any> const propagationContext = commonObjectToPropagationContext(context.headers, incomingPropagationContext); return withIsolationScope(isolationScope, () => { - if (componentType && !isolationScope.getScopeData().transactionName) { - // only set name if not already set, otherwise it gets overwritten by subsequent calls - isolationScope.setTransactionName(`${componentType} Server Component (${componentRoute})`); - } + withScope(scope => { + scope.setTransactionName(`${componentType} Server Component (${componentRoute})`); - getCurrentScope().setPropagationContext(propagationContext); - return startSpanManual( - { - op: 'function.nextjs', - name: `${componentType} Server Component (${componentRoute})`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', - }, - }, - span => { - return handleCallbackErrors( - () => originalFunction.apply(thisArg, args), - error => { - if (isNotFoundNavigationError(error)) { - // We don't want to report "not-found"s - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); - } else if (isRedirectNavigationError(error)) { - // We don't want to report redirects - span.setStatus({ code: SPAN_STATUS_OK }); - } else { - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(error, { - mechanism: { - handled: false, - }, - }); - } + getCurrentScope().setPropagationContext(propagationContext); + return startSpanManual( + { + op: 'function.nextjs', + name: `${componentType} Server Component (${componentRoute})`, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', }, - () => { - span.end(); + }, + span => { + return handleCallbackErrors( + () => originalFunction.apply(thisArg, args), + error => { + if (isNotFoundNavigationError(error)) { + // We don't want to report "not-found"s + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + } else if (isRedirectNavigationError(error)) { + // We don't want to report redirects + span.setStatus({ code: SPAN_STATUS_OK }); + } else { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureException(error, { + mechanism: { + handled: false, + }, + }); + } + }, + () => { + span.end(); - // flushQueue should not throw - // eslint-disable-next-line @typescript-eslint/no-floating-promises - flushQueue(); - }, - ); - }, - ); + // flushQueue should not throw + // eslint-disable-next-line @typescript-eslint/no-floating-promises + flushQueue(); + }, + ); + }, + ); + }); }); }); }, From 271c7d1e0888d1d2ff467ef95565790b6561c829 Mon Sep 17 00:00:00 2001 From: Sigrid Huemer <32902192+s1gr1d@users.noreply.github.com> Date: Thu, 2 May 2024 11:16:55 +0200 Subject: [PATCH 6/7] Update packages/nextjs/src/common/wrapServerComponentWithSentry.ts Co-authored-by: Francesco Novy --- packages/nextjs/src/common/wrapServerComponentWithSentry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts index 60719c83bf47..41500f5b9979 100644 --- a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts @@ -59,7 +59,7 @@ export function wrapServerComponentWithSentry any> withScope(scope => { scope.setTransactionName(`${componentType} Server Component (${componentRoute})`); - getCurrentScope().setPropagationContext(propagationContext); + scope.setPropagationContext(propagationContext); return startSpanManual( { op: 'function.nextjs', From 043780c6e785eab55c9f25a108d5f30b4e8c407c Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 2 May 2024 11:34:46 +0200 Subject: [PATCH 7/7] add missing return --- packages/nextjs/src/common/wrapServerComponentWithSentry.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts index 41500f5b9979..1234ea448a3d 100644 --- a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts @@ -3,7 +3,6 @@ import { SPAN_STATUS_ERROR, SPAN_STATUS_OK, captureException, - getCurrentScope, handleCallbackErrors, startSpanManual, withIsolationScope, @@ -56,7 +55,7 @@ export function wrapServerComponentWithSentry any> const propagationContext = commonObjectToPropagationContext(context.headers, incomingPropagationContext); return withIsolationScope(isolationScope, () => { - withScope(scope => { + return withScope(scope => { scope.setTransactionName(`${componentType} Server Component (${componentRoute})`); scope.setPropagationContext(propagationContext);