Skip to content

Commit 3b76a62

Browse files
authored
feat(nextjs): Record transaction name source when creating transactions (#5391)
This adds information about the source of the transaction name to all transactions created by the nextjs SDK.
1 parent e97a639 commit 3b76a62

File tree

8 files changed

+71
-4
lines changed

8 files changed

+71
-4
lines changed

packages/nextjs/src/performance/client.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,16 @@ export function nextRouterInstrumentation(
3535
// route name. Setting the transaction name after the transaction is started could lead
3636
// to possible race conditions with the router, so this approach was taken.
3737
if (startTransactionOnPageLoad) {
38-
prevTransactionName = Router.route !== null ? stripUrlQueryAndFragment(Router.route) : global.location.pathname;
38+
const pathIsRoute = Router.route !== null;
39+
40+
prevTransactionName = pathIsRoute ? stripUrlQueryAndFragment(Router.route) : global.location.pathname;
3941
activeTransaction = startTransactionCb({
4042
name: prevTransactionName,
4143
op: 'pageload',
4244
tags: DEFAULT_TAGS,
45+
metadata: {
46+
source: pathIsRoute ? 'route' : 'url',
47+
},
4348
});
4449
}
4550

@@ -105,6 +110,7 @@ function changeStateWrapper(originalChangeStateWrapper: RouterChangeState): Wrap
105110
name: prevTransactionName,
106111
op: 'navigation',
107112
tags,
113+
metadata: { source: 'route' },
108114
});
109115
}
110116
return originalChangeStateWrapper.call(this, method, url, as, options, ...args);

packages/nextjs/src/utils/instrumentServer.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,14 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
269269
{
270270
name: `${namePrefix}${reqPath}`,
271271
op: 'http.server',
272-
metadata: { requestPath: reqPath, baggage },
272+
metadata: {
273+
baggage,
274+
requestPath: reqPath,
275+
// TODO: Investigate if there's a way to tell if this is a dynamic route, so that we can make this more
276+
// like `source: isDynamicRoute? 'url' : 'route'`
277+
// TODO: What happens when `withSentry` is used also? Which values of `name` and `source` win?
278+
source: 'url',
279+
},
273280
...traceparentData,
274281
},
275282
// Extra context passed to the `tracesSampler` (Note: We're combining `nextReq` and `req` this way in order
@@ -326,6 +333,7 @@ function makeWrappedMethodForGettingParameterizedPath(
326333
if (transaction && transaction.metadata.requestPath) {
327334
const origPath = transaction.metadata.requestPath;
328335
transaction.name = transaction.name.replace(origPath, parameterizedPath);
336+
transaction.setMetadata({ source: 'route' });
329337
}
330338

331339
return origMethod.call(this, parameterizedPath, ...args);

packages/nextjs/src/utils/withSentry.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler =
6060
// Replace with placeholder
6161
if (req.query) {
6262
// TODO get this from next if possible, to avoid accidentally replacing non-dynamic parts of the path if
63-
// they match dynamic parts
63+
// they happen to match the values of any of the dynamic parts
6464
for (const [key, value] of Object.entries(req.query)) {
6565
reqPath = reqPath.replace(`${value}`, `[${key}]`);
6666
}
@@ -72,7 +72,7 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler =
7272
name: `${reqMethod}${reqPath}`,
7373
op: 'http.server',
7474
...traceparentData,
75-
metadata: { baggage },
75+
metadata: { baggage, source: 'route' },
7676
},
7777
// extra context passed to the `tracesSampler`
7878
{ request: req },

packages/nextjs/test/integration/test/server/tracing200.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ module.exports = async ({ url: urlBase, argv }) => {
1616
},
1717
},
1818
transaction: 'GET /api/users',
19+
transaction_info: {
20+
source: 'route',
21+
},
1922
type: 'transaction',
2023
request: {
2124
url,

packages/nextjs/test/integration/test/server/tracing500.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ module.exports = async ({ url: urlBase, argv }) => {
1515
},
1616
},
1717
transaction: 'GET /api/broken',
18+
transaction_info: {
19+
source: 'route',
20+
},
1821
type: 'transaction',
1922
request: {
2023
url,

packages/nextjs/test/integration/test/server/tracingHttp.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ module.exports = async ({ url: urlBase, argv }) => {
2929
},
3030
],
3131
transaction: 'GET /api/http',
32+
transaction_info: {
33+
source: 'route',
34+
},
3235
type: 'transaction',
3336
request: {
3437
url,

packages/nextjs/test/performance/client.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ describe('client', () => {
4545
tags: {
4646
'routing.instrumentation': 'next-router',
4747
},
48+
metadata: {
49+
source: 'route',
50+
},
4851
});
4952
});
5053

@@ -68,6 +71,9 @@ describe('client', () => {
6871
method: 'pushState',
6972
'routing.instrumentation': 'next-router',
7073
},
74+
metadata: {
75+
source: 'route',
76+
},
7177
},
7278
],
7379
[
@@ -81,6 +87,9 @@ describe('client', () => {
8187
method: 'replaceState',
8288
'routing.instrumentation': 'next-router',
8389
},
90+
metadata: {
91+
source: 'route',
92+
},
8493
},
8594
],
8695
[
@@ -94,6 +103,9 @@ describe('client', () => {
94103
method: 'pushState',
95104
'routing.instrumentation': 'next-router',
96105
},
106+
metadata: {
107+
source: 'route',
108+
},
97109
},
98110
],
99111
];
@@ -120,6 +132,9 @@ describe('client', () => {
120132
name: '/login',
121133
op: 'navigation',
122134
tags: { from: '/[user]/posts/[id]', method: 'pushState', 'routing.instrumentation': 'next-router' },
135+
metadata: {
136+
source: 'route',
137+
},
123138
});
124139

125140
Router.router?.changeState('pushState', '/login', '/login', {});
@@ -131,6 +146,9 @@ describe('client', () => {
131146
name: '/posts/[id]',
132147
op: 'navigation',
133148
tags: { from: '/login', method: 'pushState', 'routing.instrumentation': 'next-router' },
149+
metadata: {
150+
source: 'route',
151+
},
134152
});
135153
});
136154
});

packages/nextjs/test/utils/withSentry.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
import * as hub from '@sentry/hub';
12
import * as Sentry from '@sentry/node';
3+
import { Client, ClientOptions } from '@sentry/types';
24
import * as utils from '@sentry/utils';
35
import { NextApiHandler, NextApiRequest, NextApiResponse } from 'next';
46

@@ -43,6 +45,7 @@ const flushSpy = jest.spyOn(Sentry, 'flush').mockImplementation(async () => {
4345
await sleep(FLUSH_DURATION);
4446
return true;
4547
});
48+
const startTransactionSpy = jest.spyOn(Sentry, 'startTransaction');
4649

4750
describe('withSentry', () => {
4851
let req: NextApiRequest, res: NextApiResponse;
@@ -99,4 +102,27 @@ describe('withSentry', () => {
99102
expect(loggerSpy).toHaveBeenCalledWith('Done flushing events');
100103
});
101104
});
105+
106+
describe('tracing', () => {
107+
it('starts a transaction when tracing is enabled', async () => {
108+
jest
109+
.spyOn(hub.Hub.prototype, 'getClient')
110+
.mockReturnValueOnce({ getOptions: () => ({ tracesSampleRate: 1 } as ClientOptions) } as Client);
111+
112+
await callWrappedHandler(wrappedHandlerNoError, req, res);
113+
114+
expect(startTransactionSpy).toHaveBeenCalledWith(
115+
{
116+
name: 'GET http://dogs.are.great',
117+
op: 'http.server',
118+
119+
metadata: {
120+
baggage: expect.any(Array),
121+
source: 'route',
122+
},
123+
},
124+
{ request: expect.objectContaining({ url: 'http://dogs.are.great' }) },
125+
);
126+
});
127+
});
102128
});

0 commit comments

Comments
 (0)