diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts index e3616b3a279e..1b89d4255dd6 100644 --- a/packages/nextjs/src/performance/client.ts +++ b/packages/nextjs/src/performance/client.ts @@ -35,11 +35,16 @@ export function nextRouterInstrumentation( // route name. Setting the transaction name after the transaction is started could lead // to possible race conditions with the router, so this approach was taken. if (startTransactionOnPageLoad) { - prevTransactionName = Router.route !== null ? stripUrlQueryAndFragment(Router.route) : global.location.pathname; + const pathIsRoute = Router.route !== null; + + prevTransactionName = pathIsRoute ? stripUrlQueryAndFragment(Router.route) : global.location.pathname; activeTransaction = startTransactionCb({ name: prevTransactionName, op: 'pageload', tags: DEFAULT_TAGS, + metadata: { + source: pathIsRoute ? 'route' : 'url', + }, }); } @@ -105,6 +110,7 @@ function changeStateWrapper(originalChangeStateWrapper: RouterChangeState): Wrap name: prevTransactionName, op: 'navigation', tags, + metadata: { source: 'route' }, }); } return originalChangeStateWrapper.call(this, method, url, as, options, ...args); diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts index 0106acfb8123..d506778a1dda 100644 --- a/packages/nextjs/src/utils/instrumentServer.ts +++ b/packages/nextjs/src/utils/instrumentServer.ts @@ -269,7 +269,14 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler { { name: `${namePrefix}${reqPath}`, op: 'http.server', - metadata: { requestPath: reqPath, baggage }, + metadata: { + baggage, + requestPath: reqPath, + // TODO: Investigate if there's a way to tell if this is a dynamic route, so that we can make this more + // like `source: isDynamicRoute? 'url' : 'route'` + // TODO: What happens when `withSentry` is used also? Which values of `name` and `source` win? + source: 'url', + }, ...traceparentData, }, // Extra context passed to the `tracesSampler` (Note: We're combining `nextReq` and `req` this way in order @@ -326,6 +333,7 @@ function makeWrappedMethodForGettingParameterizedPath( if (transaction && transaction.metadata.requestPath) { const origPath = transaction.metadata.requestPath; transaction.name = transaction.name.replace(origPath, parameterizedPath); + transaction.setMetadata({ source: 'route' }); } return origMethod.call(this, parameterizedPath, ...args); diff --git a/packages/nextjs/src/utils/withSentry.ts b/packages/nextjs/src/utils/withSentry.ts index 714a2271ceae..d1b249bf87a3 100644 --- a/packages/nextjs/src/utils/withSentry.ts +++ b/packages/nextjs/src/utils/withSentry.ts @@ -60,7 +60,7 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler = // Replace with placeholder if (req.query) { // TODO get this from next if possible, to avoid accidentally replacing non-dynamic parts of the path if - // they match dynamic parts + // they happen to match the values of any of the dynamic parts for (const [key, value] of Object.entries(req.query)) { reqPath = reqPath.replace(`${value}`, `[${key}]`); } @@ -72,7 +72,7 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler = name: `${reqMethod}${reqPath}`, op: 'http.server', ...traceparentData, - metadata: { baggage }, + metadata: { baggage, source: 'route' }, }, // extra context passed to the `tracesSampler` { request: req }, diff --git a/packages/nextjs/test/integration/test/server/tracing200.js b/packages/nextjs/test/integration/test/server/tracing200.js index 91d767e74967..a9055540c3f3 100644 --- a/packages/nextjs/test/integration/test/server/tracing200.js +++ b/packages/nextjs/test/integration/test/server/tracing200.js @@ -16,6 +16,9 @@ module.exports = async ({ url: urlBase, argv }) => { }, }, transaction: 'GET /api/users', + transaction_info: { + source: 'route', + }, type: 'transaction', request: { url, diff --git a/packages/nextjs/test/integration/test/server/tracing500.js b/packages/nextjs/test/integration/test/server/tracing500.js index 54c565f43f23..8267a55e0cac 100644 --- a/packages/nextjs/test/integration/test/server/tracing500.js +++ b/packages/nextjs/test/integration/test/server/tracing500.js @@ -15,6 +15,9 @@ module.exports = async ({ url: urlBase, argv }) => { }, }, transaction: 'GET /api/broken', + transaction_info: { + source: 'route', + }, type: 'transaction', request: { url, diff --git a/packages/nextjs/test/integration/test/server/tracingHttp.js b/packages/nextjs/test/integration/test/server/tracingHttp.js index 3a00be07899b..5825d4ce4d37 100644 --- a/packages/nextjs/test/integration/test/server/tracingHttp.js +++ b/packages/nextjs/test/integration/test/server/tracingHttp.js @@ -29,6 +29,9 @@ module.exports = async ({ url: urlBase, argv }) => { }, ], transaction: 'GET /api/http', + transaction_info: { + source: 'route', + }, type: 'transaction', request: { url, diff --git a/packages/nextjs/test/performance/client.test.ts b/packages/nextjs/test/performance/client.test.ts index 3d08e36cd472..bf8f118cd8c4 100644 --- a/packages/nextjs/test/performance/client.test.ts +++ b/packages/nextjs/test/performance/client.test.ts @@ -45,6 +45,9 @@ describe('client', () => { tags: { 'routing.instrumentation': 'next-router', }, + metadata: { + source: 'route', + }, }); }); @@ -68,6 +71,9 @@ describe('client', () => { method: 'pushState', 'routing.instrumentation': 'next-router', }, + metadata: { + source: 'route', + }, }, ], [ @@ -81,6 +87,9 @@ describe('client', () => { method: 'replaceState', 'routing.instrumentation': 'next-router', }, + metadata: { + source: 'route', + }, }, ], [ @@ -94,6 +103,9 @@ describe('client', () => { method: 'pushState', 'routing.instrumentation': 'next-router', }, + metadata: { + source: 'route', + }, }, ], ]; @@ -120,6 +132,9 @@ describe('client', () => { name: '/login', op: 'navigation', tags: { from: '/[user]/posts/[id]', method: 'pushState', 'routing.instrumentation': 'next-router' }, + metadata: { + source: 'route', + }, }); Router.router?.changeState('pushState', '/login', '/login', {}); @@ -131,6 +146,9 @@ describe('client', () => { name: '/posts/[id]', op: 'navigation', tags: { from: '/login', method: 'pushState', 'routing.instrumentation': 'next-router' }, + metadata: { + source: 'route', + }, }); }); }); diff --git a/packages/nextjs/test/utils/withSentry.test.ts b/packages/nextjs/test/utils/withSentry.test.ts index e0c23506ae6f..4d8709e8e735 100644 --- a/packages/nextjs/test/utils/withSentry.test.ts +++ b/packages/nextjs/test/utils/withSentry.test.ts @@ -1,4 +1,6 @@ +import * as hub from '@sentry/hub'; import * as Sentry from '@sentry/node'; +import { Client, ClientOptions } from '@sentry/types'; import * as utils from '@sentry/utils'; import { NextApiHandler, NextApiRequest, NextApiResponse } from 'next'; @@ -43,6 +45,7 @@ const flushSpy = jest.spyOn(Sentry, 'flush').mockImplementation(async () => { await sleep(FLUSH_DURATION); return true; }); +const startTransactionSpy = jest.spyOn(Sentry, 'startTransaction'); describe('withSentry', () => { let req: NextApiRequest, res: NextApiResponse; @@ -99,4 +102,27 @@ describe('withSentry', () => { expect(loggerSpy).toHaveBeenCalledWith('Done flushing events'); }); }); + + describe('tracing', () => { + it('starts a transaction when tracing is enabled', async () => { + jest + .spyOn(hub.Hub.prototype, 'getClient') + .mockReturnValueOnce({ getOptions: () => ({ tracesSampleRate: 1 } as ClientOptions) } as Client); + + await callWrappedHandler(wrappedHandlerNoError, req, res); + + expect(startTransactionSpy).toHaveBeenCalledWith( + { + name: 'GET http://dogs.are.great', + op: 'http.server', + + metadata: { + baggage: expect.any(Array), + source: 'route', + }, + }, + { request: expect.objectContaining({ url: 'http://dogs.are.great' }) }, + ); + }); + }); });