diff --git a/docs/migration/draft-v9-migration-guide.md b/docs/migration/draft-v9-migration-guide.md new file mode 100644 index 000000000000..5630d82ede90 --- /dev/null +++ b/docs/migration/draft-v9-migration-guide.md @@ -0,0 +1,13 @@ + + +# Deprecations + +## `@sentry/utils` + +- Deprecated `AddRequestDataToEventOptions.transaction`. This option effectively doesn't do anything anymore, and will + be removed in v9. +- Deprecated `TransactionNamingScheme` type. + +## `@sentry/core` + +- Deprecated `transactionNamingScheme` option in `requestDataIntegration`. diff --git a/packages/core/src/integrations/requestdata.ts b/packages/core/src/integrations/requestdata.ts index 9475973d8e2c..15be450cf27e 100644 --- a/packages/core/src/integrations/requestdata.ts +++ b/packages/core/src/integrations/requestdata.ts @@ -24,7 +24,11 @@ export type RequestDataIntegrationOptions = { }; }; - /** Whether to identify transactions by parameterized path, parameterized path with method, or handler name */ + /** + * Whether to identify transactions by parameterized path, parameterized path with method, or handler name. + * @deprecated This option does not do anything anymore, and will be removed in v9. + */ + // eslint-disable-next-line deprecation/deprecation transactionNamingScheme?: TransactionNamingScheme; }; @@ -110,6 +114,7 @@ function convertReqDataIntegrationOptsToAddReqDataOpts( integrationOptions: Required, ): AddRequestDataToEventOptions { const { + // eslint-disable-next-line deprecation/deprecation transactionNamingScheme, include: { ip, user, ...requestOptions }, } = integrationOptions; diff --git a/packages/remix/src/utils/errors.ts b/packages/remix/src/utils/errors.ts index 92958c2c3eb3..100dac496c75 100644 --- a/packages/remix/src/utils/errors.ts +++ b/packages/remix/src/utils/errors.ts @@ -1,12 +1,5 @@ import type { AppData, DataFunctionArgs, EntryContext, HandleDocumentRequestFunction } from '@remix-run/node'; -import { - captureException, - getActiveSpan, - getClient, - getRootSpan, - handleCallbackErrors, - spanToJSON, -} from '@sentry/core'; +import { captureException, getClient, handleCallbackErrors } from '@sentry/core'; import type { Span } from '@sentry/types'; import { addExceptionMechanism, isPrimitive, logger, objectify } from '@sentry/utils'; import { DEBUG_BUILD } from './debug-build'; @@ -61,19 +54,9 @@ export async function captureRemixServerException( const objectifiedErr = objectify(err); captureException(isResponse(objectifiedErr) ? await extractResponseError(objectifiedErr) : objectifiedErr, scope => { - const activeSpan = getActiveSpan(); - const rootSpan = activeSpan && getRootSpan(activeSpan); - const activeRootSpanName = rootSpan ? spanToJSON(rootSpan).description : undefined; - scope.setSDKProcessingMetadata({ request: { ...normalizedRequest, - // When `route` is not defined, `RequestData` integration uses the full URL - route: activeRootSpanName - ? { - path: activeRootSpanName, - } - : undefined, }, }); diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index e83c14dfbbc4..666c332afa04 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -314,9 +314,6 @@ function wrapRequestHandler( isolationScope.setSDKProcessingMetadata({ request: { ...normalizedRequest, - route: { - path: name, - }, }, }); diff --git a/packages/utils/src/requestdata.ts b/packages/utils/src/requestdata.ts index edffc6f67da7..26b12b07c69e 100644 --- a/packages/utils/src/requestdata.ts +++ b/packages/utils/src/requestdata.ts @@ -21,7 +21,6 @@ import { getClientIPAddress, ipHeaderNames } from './vendor/getIpAddress'; const DEFAULT_INCLUDES = { ip: false, request: true, - transaction: true, user: true, }; const DEFAULT_REQUEST_INCLUDES = ['cookies', 'data', 'headers', 'method', 'query_string', 'url']; @@ -35,6 +34,8 @@ export type AddRequestDataToEventOptions = { include?: { ip?: boolean; request?: boolean | Array<(typeof DEFAULT_REQUEST_INCLUDES)[number]>; + /** @deprecated This option will be removed in v9. It does not do anything anymore, the `transcation` is set in other places. */ + // eslint-disable-next-line deprecation/deprecation transaction?: boolean | TransactionNamingScheme; user?: boolean | Array<(typeof DEFAULT_USER_INCLUDES)[number]>; }; @@ -52,6 +53,9 @@ export type AddRequestDataToEventOptions = { }; }; +/** + * @deprecated This type will be removed in v9. It is not in use anymore. + */ export type TransactionNamingScheme = 'path' | 'methodPath' | 'handler'; /** @@ -67,6 +71,7 @@ export type TransactionNamingScheme = 'path' | 'methodPath' | 'handler'; * used instead of the request's route) * * @returns A tuple of the fully constructed transaction name [0] and its source [1] (can be either 'route' or 'url') + * @deprecated This method will be removed in v9. It is not in use anymore. */ export function extractPathForTransaction( req: PolymorphicRequest, @@ -102,23 +107,6 @@ export function extractPathForTransaction( return [name, source]; } -function extractTransaction(req: PolymorphicRequest, type: boolean | TransactionNamingScheme): string { - switch (type) { - case 'path': { - return extractPathForTransaction(req, { path: true })[0]; - } - case 'handler': { - return (req.route && req.route.stack && req.route.stack[0] && req.route.stack[0].name) || ''; - } - case 'methodPath': - default: { - // if exist _reconstructedRoute return that path instead of route.path - const customRoute = req._reconstructedRoute ? req._reconstructedRoute : undefined; - return extractPathForTransaction(req, { path: true, method: true, customRoute })[0]; - } - } -} - function extractUserData( user: { [key: string]: unknown; @@ -379,12 +367,6 @@ export function addRequestDataToEvent( } } - if (include.transaction && !event.transaction && event.type === 'transaction') { - // TODO do we even need this anymore? - // TODO make this work for nextjs - event.transaction = extractTransaction(req, include.transaction); - } - return event; } diff --git a/packages/utils/test/requestdata.test.ts b/packages/utils/test/requestdata.test.ts index 570f80647b6b..90c734f23f2c 100644 --- a/packages/utils/test/requestdata.test.ts +++ b/packages/utils/test/requestdata.test.ts @@ -369,67 +369,6 @@ describe('addRequestDataToEvent', () => { } }); }); - - describe('transaction property', () => { - describe('for transaction events', () => { - beforeEach(() => { - mockEvent.type = 'transaction'; - }); - - test('extracts method and full route path by default`', () => { - const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq); - - expect(parsedRequest.transaction).toEqual('POST /routerMountPath/subpath/:parameterName'); - }); - - test('extracts method and full path by default when mountpoint is `/`', () => { - mockReq.originalUrl = mockReq.originalUrl.replace('/routerMountpath', ''); - mockReq.baseUrl = ''; - - const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq); - - // `subpath/` is the full path here, because there's no router mount path - expect(parsedRequest.transaction).toEqual('POST /subpath/:parameterName'); - }); - - test('fallback to method and `originalUrl` if route is missing', () => { - delete mockReq.route; - - const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq); - - expect(parsedRequest.transaction).toEqual('POST /routerMountPath/subpath/specificValue'); - }); - - test('can extract path only instead if configured', () => { - const optionsWithPathTransaction = { - include: { - transaction: 'path', - }, - } as const; - - const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq, optionsWithPathTransaction); - - expect(parsedRequest.transaction).toEqual('/routerMountPath/subpath/:parameterName'); - }); - - test('can extract handler name instead if configured', () => { - const optionsWithHandlerTransaction = { - include: { - transaction: 'handler', - }, - } as const; - - const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq, optionsWithHandlerTransaction); - - expect(parsedRequest.transaction).toEqual('parameterNameRouteHandler'); - }); - }); - it('transaction is not applied to non-transaction events', () => { - const parsedRequest: Event = addRequestDataToEvent(mockEvent, mockReq); - - expect(parsedRequest.transaction).toBeUndefined(); - }); - }); }); describe('extractRequestData', () => { @@ -763,6 +702,7 @@ describe('extractPathForTransaction', () => { expectedRoute: string, expectedSource: TransactionSource, ) => { + // eslint-disable-next-line deprecation/deprecation const [route, source] = extractPathForTransaction(req, options); expect(route).toEqual(expectedRoute); @@ -778,6 +718,7 @@ describe('extractPathForTransaction', () => { originalUrl: '/api/users/123/details', } as PolymorphicRequest; + // eslint-disable-next-line deprecation/deprecation const [route, source] = extractPathForTransaction(req, { path: true, method: true,