Skip to content

Commit 8c13337

Browse files
committed
overwrite transaction value after running addRequestDataToEvent
1 parent 17e4cb8 commit 8c13337

File tree

1 file changed

+51
-4
lines changed

1 file changed

+51
-4
lines changed

packages/node/src/integrations/requestdata.ts

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
// TODO (v8 or v9): Whenever this becomes a default integration for `@sentry/browser`, move this to `@sentry/core`. For
22
// now, we leave it in `@sentry/integrations` so that it doesn't contribute bytes to our CDN bundles.
33

4-
import { EventProcessor, Hub, Integration } from '@sentry/types';
4+
import { EventProcessor, Hub, Integration, Transaction } from '@sentry/types';
5+
import { extractPathForTransaction } from '@sentry/utils';
56

67
import {
78
addRequestDataToEvent,
@@ -86,10 +87,15 @@ export class RequestData implements Integration {
8687
* @inheritDoc
8788
*/
8889
public setupOnce(addGlobalEventProcessor: (eventProcessor: EventProcessor) => void, getCurrentHub: () => Hub): void {
89-
const { include, addRequestData } = this._options;
90+
// Note: In the long run, most of the logic here should probably move into the request data utility functions. For
91+
// the moment it lives here, though, until https://github.com/getsentry/sentry-javascript/issues/5718 is addressed.
92+
// (TL;DR: Those functions touch many parts of the repo in many different ways, and need to be clened up. Once
93+
// that's happened, it will be easier to add this logic in without worrying about unexpected side effects.)
94+
const { include, addRequestData, transactionNamingScheme } = this._options;
9095

9196
addGlobalEventProcessor(event => {
92-
const self = getCurrentHub().getIntegration(RequestData);
97+
const hub = getCurrentHub();
98+
const self = hub.getIntegration(RequestData);
9399
const req = event.sdkProcessingMetadata && event.sdkProcessingMetadata.request;
94100

95101
// If the globally installed instance of this integration isn't associated with the current hub, `self` will be
@@ -98,7 +104,36 @@ export class RequestData implements Integration {
98104
return event;
99105
}
100106

101-
return addRequestData(event, req, { include: formatIncludeOption(include) });
107+
const processedEvent = addRequestData(event, req, { include: formatIncludeOption(include) });
108+
109+
// Transaction events already have the right `transaction` value
110+
if (event.type === 'transaction' || transactionNamingScheme === 'handler') {
111+
return processedEvent;
112+
}
113+
114+
// In all other cases, use the request's associated transaction (if any) to overwrite the event's `transaction`
115+
// value with a high-quality one
116+
const reqWithTransaction = req as { _sentryTransaction?: Transaction };
117+
const transaction = reqWithTransaction._sentryTransaction;
118+
if (transaction) {
119+
// TODO (v8): Remove the nextjs check and just base it on `transactionNamingScheme` for all SDKs. (We have to
120+
// keep it the way it is for the moment, because changing the names of transactions in Sentry has the potential
121+
// to break things like alert rules.)
122+
const shouldIncludeMethodInTransactionName =
123+
getSDKName(hub) === 'sentry.javascript.nextjs'
124+
? transaction.name.startsWith('/api')
125+
: transactionNamingScheme !== 'path';
126+
127+
const [transactionValue] = extractPathForTransaction(req, {
128+
path: true,
129+
method: shouldIncludeMethodInTransactionName,
130+
customRoute: transaction.name,
131+
});
132+
133+
processedEvent.transaction = transactionValue;
134+
}
135+
136+
return processedEvent;
102137
});
103138
}
104139
}
@@ -123,3 +158,15 @@ function formatIncludeOption(
123158
request: requestIncludeKeys.length !== 0 ? requestIncludeKeys : undefined,
124159
};
125160
}
161+
162+
function getSDKName(hub: Hub): string | undefined {
163+
try {
164+
// For a long chain like this, it's fewer bytes to combine a try-catch with assuming everything is there than to
165+
// write out a long chain of `a && a.b && a.b.c && ...`
166+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
167+
return hub.getClient()!.getOptions()!._metadata!.sdk!.name;
168+
} catch (err) {
169+
// In theory we should never get here
170+
return undefined;
171+
}
172+
}

0 commit comments

Comments
 (0)