ref(core): Remove transaction name extraction from requestDataIntegration (#11513)

Lms24 · Luca Forstner · web-flow · commit 2288caf1c107 · 2024-04-25T09:04:06.000Z
Co-authored-by: Luca Forstner &lt;luca.forstner@sentry.io&gt;
diff --git a/packages/core/src/integrations/requestdata.ts b/packages/core/src/integrations/requestdata.ts
@@ -1,8 +1,7 @@
-import type { Client, IntegrationFn, Span } from '@sentry/types';
+import type { IntegrationFn } from '@sentry/types';
 import type { AddRequestDataToEventOptions, TransactionNamingScheme } from '@sentry/utils';
-import { addRequestDataToEvent, extractPathForTransaction } from '@sentry/utils';
+import { addRequestDataToEvent } from '@sentry/utils';
 import { defineIntegration } from '../integration';
-import { spanToJSON } from '../utils/spanUtils';
 
 export type RequestDataIntegrationOptions = {
   /**
@@ -67,12 +66,11 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) =
 
   return {
     name: INTEGRATION_NAME,
-    processEvent(event, _hint, client) {
+    processEvent(event) {
       // Note: In the long run, most of the logic here should probably move into the request data utility functions. For
       // the moment it lives here, though, until https://github.com/getsentry/sentry-javascript/issues/5718 is addressed.
-      // (TL;DR: Those functions touch many parts of the repo in many different ways, and need to be clened up. Once
+      // (TL;DR: Those functions touch many parts of the repo in many different ways, and need to be cleaned up. Once
       // that's happened, it will be easier to add this logic in without worrying about unexpected side effects.)
-      const { transactionNamingScheme } = _options;
 
       const { sdkProcessingMetadata = {} } = event;
       const req = sdkProcessingMetadata.request;
@@ -83,38 +81,7 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) =
 
       const addRequestDataOptions = convertReqDataIntegrationOptsToAddReqDataOpts(_options);
 
-      const processedEvent = addRequestDataToEvent(event, req, addRequestDataOptions);
-
-      // Transaction events already have the right `transaction` value
-      if (event.type === 'transaction' || transactionNamingScheme === 'handler') {
-        return processedEvent;
-      }
-
-      // In all other cases, use the request's associated transaction (if any) to overwrite the event's `transaction`
-      // value with a high-quality one
-      const reqWithTransaction = req as { _sentryTransaction?: Span };
-      const transaction = reqWithTransaction._sentryTransaction;
-      if (transaction) {
-        const name = spanToJSON(transaction).description || '';
-
-        // TODO (v8): Remove the nextjs check and just base it on `transactionNamingScheme` for all SDKs. (We have to
-        // keep it the way it is for the moment, because changing the names of transactions in Sentry has the potential
-        // to break things like alert rules.)
-        const shouldIncludeMethodInTransactionName =
-          getSDKName(client) === 'sentry.javascript.nextjs'
-            ? name.startsWith('/api')
-            : transactionNamingScheme !== 'path';
-
-        const [transactionValue] = extractPathForTransaction(req, {
-          path: true,
-          method: shouldIncludeMethodInTransactionName,
-          customRoute: name,
-        });
-
-        processedEvent.transaction = transactionValue;
-      }
-
-      return processedEvent;
+      return addRequestDataToEvent(event, req, addRequestDataOptions);
     },
   };
 }) satisfies IntegrationFn;
@@ -166,15 +133,3 @@ function convertReqDataIntegrationOptsToAddReqDataOpts(
     },
   };
 }
-
-function getSDKName(client: Client): string | undefined {
-  try {
-    // For a long chain like this, it's fewer bytes to combine a try-catch with assuming everything is there than to
-    // write out a long chain of `a && a.b && a.b.c && ...`
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    return client.getOptions()._metadata!.sdk!.name;
-  } catch (err) {
-    // In theory we should never get here
-    return undefined;
-  }
-}
diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts
@@ -6,7 +6,7 @@ import {
   startSpanManual,
   withIsolationScope,
 } from '@sentry/core';
-import { consoleSandbox, isString, logger, objectify, stripUrlQueryAndFragment } from '@sentry/utils';
+import { consoleSandbox, isString, logger, objectify } from '@sentry/utils';
 
 import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
 import type { AugmentedNextApiRequest, AugmentedNextApiResponse, NextApiHandler } from './types';
@@ -20,7 +20,7 @@ import { escapeNextjsTracing } from './utils/tracingUtils';
  *
  * @param apiHandler The handler exported from the user's API page route file, which may or may not already be
  * wrapped with `withSentry`
- * @param parameterizedRoute The page's route, passed in via the proxy loader
+ * @param parameterizedRoute The page's parameterized route.
  * @returns The wrapped handler
  */
 export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameterizedRoute: string): NextApiHandler {
@@ -62,30 +62,14 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz
               baggage: req.headers?.baggage,
             },
             () => {
-              // prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler)
-              let reqPath = parameterizedRoute;
-
-              // If not, fake it by just replacing parameter values with their names, hoping that none of them match either
-              // each other or any hard-coded parts of the path
-              if (!reqPath) {
-                const url = `${req.url}`;
-                // pull off query string, if any
-                reqPath = stripUrlQueryAndFragment(url);
-                // Replace with placeholder
-                if (req.query) {
-                  for (const [key, value] of Object.entries(req.query)) {
-                    reqPath = reqPath.replace(`${value}`, `[${key}]`);
-                  }
-                }
-              }
-
               const reqMethod = `${(req.method || 'GET').toUpperCase()} `;
 
               isolationScope.setSDKProcessingMetadata({ request: req });
+              isolationScope.setTransactionName(`${reqMethod}${parameterizedRoute}`);
 
               return startSpanManual(
                 {
-                  name: `${reqMethod}${reqPath}`,
+                  name: `${reqMethod}${parameterizedRoute}`,
                   op: 'http.server',
                   forceTransaction: true,
                   attributes: {
diff --git a/packages/node/test/integration/scope.test.ts b/packages/node/test/integration/scope.test.ts
@@ -86,7 +86,6 @@ describe('Integration | Scope', () => {
             tag3: 'val3',
             tag4: 'val4',
           },
-          ...(enableTracing ? { transaction: 'outer' } : {}),
         }),
         {
           event_id: expect.any(String),
@@ -125,7 +124,6 @@ describe('Integration | Scope', () => {
               tag4: 'val4',
             },
             timestamp: expect.any(Number),
-            transaction: 'outer',
             transaction_info: { source: 'custom' },
             type: 'transaction',
           }),
@@ -209,7 +207,6 @@ describe('Integration | Scope', () => {
             tag3: 'val3a',
             tag4: 'val4a',
           },
-          ...(enableTracing ? { transaction: 'outer' } : {}),
         }),
         {
           event_id: expect.any(String),
@@ -236,7 +233,6 @@ describe('Integration | Scope', () => {
             tag3: 'val3b',
             tag4: 'val4b',
           },
-          ...(enableTracing ? { transaction: 'outer' } : {}),
         }),
         {
           event_id: expect.any(String),
diff --git a/packages/opentelemetry/src/setupEventContextTrace.ts b/packages/opentelemetry/src/setupEventContextTrace.ts
@@ -1,10 +1,10 @@
-import { getRootSpan } from '@sentry/core';
 import type { Client } from '@sentry/types';
 import { dropUndefinedKeys } from '@sentry/utils';
 import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext';
 
+import { getRootSpan } from '@sentry/core';
 import { getActiveSpan } from './utils/getActiveSpan';
-import { spanHasName, spanHasParentId } from './utils/spanTypes';
+import { spanHasParentId } from './utils/spanTypes';
 
 /** Ensure the `trace` context is set on all events. */
 export function setupEventContextTrace(client: Client): void {
@@ -35,9 +35,6 @@ export function setupEventContextTrace(client: Client): void {
       ...event.sdkProcessingMetadata,
     };
 
-    const transactionName = spanHasName(rootSpan) ? rootSpan.name : undefined;
-    if (transactionName && !event.transaction) {
-      event.transaction = transactionName;
-    }
+    return event;
   });
 }
diff --git a/packages/opentelemetry/test/integration/scope.test.ts b/packages/opentelemetry/test/integration/scope.test.ts
@@ -93,7 +93,6 @@ describe('Integration | Scope', () => {
             tag3: 'val3',
             tag4: 'val4',
           },
-          ...(enableTracing ? { transaction: 'outer' } : undefined),
         }),
         {
           event_id: expect.any(String),
@@ -132,7 +131,6 @@ describe('Integration | Scope', () => {
               tag4: 'val4',
             },
             timestamp: expect.any(Number),
-            transaction: 'outer',
             transaction_info: { source: 'custom' },
             type: 'transaction',
           }),
@@ -226,7 +224,6 @@ describe('Integration | Scope', () => {
             tag3: 'val3a',
             tag4: 'val4a',
           },
-          ...(enableTracing ? { transaction: 'outer' } : undefined),
         }),
         {
           event_id: expect.any(String),
@@ -253,7 +250,6 @@ describe('Integration | Scope', () => {
             tag3: 'val3b',
             tag4: 'val4b',
           },
-          ...(enableTracing ? { transaction: 'outer' } : undefined),
         }),
         {
           event_id: expect.any(String),
@@ -358,7 +354,6 @@ describe('Integration | Scope', () => {
             isolationTag1: 'val1',
             isolationTag2: 'val2',
           },
-          ...(enableTracing ? { transaction: 'outer' } : undefined),
         }),
         {
           event_id: expect.any(String),
@@ -385,7 +380,6 @@ describe('Integration | Scope', () => {
             isolationTag1: 'val1',
             isolationTag2: 'val2b',
           },
-          ...(enableTracing ? { transaction: 'outer' } : undefined),
         }),
         {
           event_id: expect.any(String),
