diff --git a/MIGRATION.md b/MIGRATION.md index 7f4c04044bf2..98d92427c03f 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -14,6 +14,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar * `span.toContext()`: Access the fields directly instead. * `span.updateWithContext(newSpanContext)`: Update the fields directly instead. +* `span.setName(newName)`: Use `span.updateName(newName)` instead. ## Deprecate `pushScope` & `popScope` in favor of `withScope` diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index 3138f1be1c53..e65ecd84d8df 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -118,7 +118,8 @@ export class TraceService implements OnDestroy { const transaction = getActiveTransaction(); // TODO (v8 / #5416): revisit the source condition. Do we want to make the parameterized route the default? if (transaction && transaction.metadata.source === 'url') { - transaction.setName(route, 'route'); + transaction.updateName(route); + transaction.setMetadata({ source: 'route' }); } }), ); diff --git a/packages/angular/test/tracing.test.ts b/packages/angular/test/tracing.test.ts index 37ef01348699..695e3d7af564 100644 --- a/packages/angular/test/tracing.test.ts +++ b/packages/angular/test/tracing.test.ts @@ -10,7 +10,8 @@ let transaction: any; const defaultStartTransaction = (ctx: any) => { transaction = { ...ctx, - setName: jest.fn(name => (transaction.name = name)), + updateName: jest.fn(name => (transaction.name = name)), + setMetadata: jest.fn(), }; return transaction; @@ -110,7 +111,7 @@ describe('Angular Tracing', () => { ...ctx.metadata, source: 'custom', }, - setName: jest.fn(name => (transaction.name = name)), + updateName: jest.fn(name => (transaction.name = name)), }; return transaction; @@ -137,7 +138,7 @@ describe('Angular Tracing', () => { metadata: { source: 'url' }, }); - expect(transaction.setName).toHaveBeenCalledTimes(0); + expect(transaction.updateName).toHaveBeenCalledTimes(0); expect(transaction.name).toEqual(url); expect(transaction.metadata.source).toBe('custom'); @@ -327,7 +328,8 @@ describe('Angular Tracing', () => { origin: 'auto.navigation.angular', metadata: { source: 'url' }, }); - expect(transaction.setName).toHaveBeenCalledWith(result, 'route'); + expect(transaction.updateName).toHaveBeenCalledWith(result); + expect(transaction.setMetadata).toHaveBeenCalledWith({ source: 'route' }); env.destroy(); }); diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index 6e484ec92ddc..29584f65858d 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -176,9 +176,11 @@ export class Span implements SpanInterface { public get name(): string { return this.description || ''; } - /** Update the name of the span. */ + /** + * Update the name of the span. + */ public set name(name: string) { - this.setName(name); + this.updateName(name); } /** @@ -267,11 +269,17 @@ export class Span implements SpanInterface { return this; } + /** @inheritdoc */ + public setName(name: string): void { + this.updateName(name); + } + /** * @inheritDoc */ - public setName(name: string): void { + public updateName(name: string): this { this.description = name; + return this; } /** diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index d16a15d7db41..339d8d51e740 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/packages/core/src/tracing/transaction.ts @@ -82,19 +82,30 @@ export class Transaction extends SpanClass implements TransactionInterface { return this._name; } - /** Setter for `name` property, which also sets `source` as custom */ + /** + * Setter for `name` property, which also sets `source` as custom. + */ public set name(newName: string) { + // eslint-disable-next-line deprecation/deprecation this.setName(newName); } /** - * JSDoc + * Setter for `name` property, which also sets `source` on the metadata. + * + * @deprecated Use `updateName()` and `setMetadata()` instead. */ public setName(name: string, source: TransactionMetadata['source'] = 'custom'): void { this._name = name; this.metadata.source = source; } + /** @inheritdoc */ + public updateName(name: string): this { + this._name = name; + return this; + } + /** * Attaches SpanRecorder to the span itself * @param maxlen maximum number of spans that can be recorded diff --git a/packages/core/test/lib/tracing/span.test.ts b/packages/core/test/lib/tracing/span.test.ts index 512141358dd4..c0b13df647f6 100644 --- a/packages/core/test/lib/tracing/span.test.ts +++ b/packages/core/test/lib/tracing/span.test.ts @@ -19,7 +19,7 @@ describe('span', () => { expect(span.description).toEqual(undefined); }); - it('allows to update the name', () => { + it('allows to update the name via setter', () => { const span = new Span({ name: 'span name' }); expect(span.name).toEqual('span name'); expect(span.description).toEqual('span name'); @@ -30,6 +30,29 @@ describe('span', () => { expect(span.description).toEqual('new name'); }); + it('allows to update the name via setName', () => { + const span = new Span({ name: 'span name' }); + expect(span.name).toEqual('span name'); + expect(span.description).toEqual('span name'); + + // eslint-disable-next-line deprecation/deprecation + span.setName('new name'); + + expect(span.name).toEqual('new name'); + expect(span.description).toEqual('new name'); + }); + + it('allows to update the name via updateName', () => { + const span = new Span({ name: 'span name' }); + expect(span.name).toEqual('span name'); + expect(span.description).toEqual('span name'); + + span.updateName('new name'); + + expect(span.name).toEqual('new name'); + expect(span.description).toEqual('new name'); + }); + describe('setAttribute', () => { it('allows to set attributes', () => { const span = new Span(); diff --git a/packages/core/test/lib/tracing/transaction.test.ts b/packages/core/test/lib/tracing/transaction.test.ts index b9218eae77cb..3be3d7dccfcc 100644 --- a/packages/core/test/lib/tracing/transaction.test.ts +++ b/packages/core/test/lib/tracing/transaction.test.ts @@ -6,12 +6,39 @@ describe('transaction', () => { expect(transaction.name).toEqual('span name'); }); - it('allows to update the name', () => { + it('allows to update the name via setter', () => { const transaction = new Transaction({ name: 'span name' }); + transaction.setMetadata({ source: 'route' }); expect(transaction.name).toEqual('span name'); transaction.name = 'new name'; expect(transaction.name).toEqual('new name'); + expect(transaction.metadata.source).toEqual('custom'); + }); + + it('allows to update the name via setName', () => { + const transaction = new Transaction({ name: 'span name' }); + transaction.setMetadata({ source: 'route' }); + expect(transaction.name).toEqual('span name'); + + transaction.setMetadata({ source: 'route' }); + + // eslint-disable-next-line deprecation/deprecation + transaction.setName('new name'); + + expect(transaction.name).toEqual('new name'); + expect(transaction.metadata.source).toEqual('custom'); + }); + + it('allows to update the name via updateName', () => { + const transaction = new Transaction({ name: 'span name' }); + transaction.setMetadata({ source: 'route' }); + expect(transaction.name).toEqual('span name'); + + transaction.updateName('new name'); + + expect(transaction.name).toEqual('new name'); + expect(transaction.metadata.source).toEqual('route'); }); }); diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index 857e6c4892d5..9a4bb08bfb4b 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -329,7 +329,8 @@ export function trpcMiddleware(options: SentryTrpcMiddlewareOptions = {}) { const sentryTransaction = getCurrentScope().getTransaction(); if (sentryTransaction) { - sentryTransaction.setName(`trpc/${path}`, 'route'); + sentryTransaction.updateName(`trpc/${path}`); + sentryTransaction.setMetadata({ source: 'route' }); sentryTransaction.op = 'rpc.server'; const trpcContext: Record = { diff --git a/packages/opentelemetry-node/src/spanprocessor.ts b/packages/opentelemetry-node/src/spanprocessor.ts index f220d5893e93..bb2372a3c2b4 100644 --- a/packages/opentelemetry-node/src/spanprocessor.ts +++ b/packages/opentelemetry-node/src/spanprocessor.ts @@ -216,7 +216,8 @@ function updateTransactionWithOtelData(transaction: Transaction, otelSpan: OtelS transaction.setStatus(mapOtelStatus(otelSpan)); transaction.op = op; - transaction.setName(description, source); + transaction.updateName(description); + transaction.setMetadata({ source }); } function convertOtelTimeToSeconds([seconds, nano]: [number, number]): number { diff --git a/packages/react/src/reactrouter.tsx b/packages/react/src/reactrouter.tsx index 3234cdc7871d..8a42c5ff96f1 100644 --- a/packages/react/src/reactrouter.tsx +++ b/packages/react/src/reactrouter.tsx @@ -165,7 +165,8 @@ export function withSentryRouting

, R extends React const WrappedRoute: React.FC

= (props: P) => { if (activeTransaction && props && props.computedMatch && props.computedMatch.isExact) { - activeTransaction.setName(props.computedMatch.path, 'route'); + activeTransaction.updateName(props.computedMatch.path); + activeTransaction.setMetadata({ source: 'route' }); } // @ts-expect-error Setting more specific React Component typing for `R` generic above diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index 14c975a90105..920a6b4f8a0d 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -134,7 +134,9 @@ function updatePageloadTransaction( : (_matchRoutes(routes, location, basename) as unknown as RouteMatch[]); if (activeTransaction && branches) { - activeTransaction.setName(...getNormalizedName(routes, location, branches, basename)); + const [name, source] = getNormalizedName(routes, location, branches, basename); + activeTransaction.updateName(name); + activeTransaction.setMetadata({ source }); } } diff --git a/packages/react/test/reactrouterv4.test.tsx b/packages/react/test/reactrouterv4.test.tsx index a17c885b5edf..2b06b3a196a5 100644 --- a/packages/react/test/reactrouterv4.test.tsx +++ b/packages/react/test/reactrouterv4.test.tsx @@ -12,7 +12,7 @@ describe('React Router v4', () => { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; routes?: RouteConfig[]; - }): [jest.Mock, any, { mockSetName: jest.Mock; mockFinish: jest.Mock }] { + }): [jest.Mock, any, { mockUpdateName: jest.Mock; mockFinish: jest.Mock; mockSetMetadata: jest.Mock }] { const options = { matchPath: _opts && _opts.routes !== undefined ? matchPath : undefined, routes: undefined, @@ -22,14 +22,17 @@ describe('React Router v4', () => { }; const history = createMemoryHistory(); const mockFinish = jest.fn(); - const mockSetName = jest.fn(); - const mockStartTransaction = jest.fn().mockReturnValue({ setName: mockSetName, end: mockFinish }); + const mockUpdateName = jest.fn(); + const mockSetMetadata = jest.fn(); + const mockStartTransaction = jest + .fn() + .mockReturnValue({ updateName: mockUpdateName, end: mockFinish, setMetadata: mockSetMetadata }); reactRouterV4Instrumentation(history, options.routes, options.matchPath)( mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange, ); - return [mockStartTransaction, history, { mockSetName, mockFinish }]; + return [mockStartTransaction, history, { mockUpdateName, mockFinish, mockSetMetadata }]; } it('starts a pageload transaction when instrumentation is started', () => { @@ -166,7 +169,7 @@ describe('React Router v4', () => { }); it('normalizes transaction name with custom Route', () => { - const [mockStartTransaction, history, { mockSetName }] = createInstrumentation(); + const [mockStartTransaction, history, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); const { getByText } = render( @@ -191,12 +194,13 @@ describe('React Router v4', () => { tags: { 'routing.instrumentation': 'react-router-v4' }, metadata: { source: 'url' }, }); - expect(mockSetName).toHaveBeenCalledTimes(2); - expect(mockSetName).toHaveBeenLastCalledWith('/users/:userid', 'route'); + expect(mockUpdateName).toHaveBeenCalledTimes(2); + expect(mockUpdateName).toHaveBeenLastCalledWith('/users/:userid'); + expect(mockSetMetadata).toHaveBeenCalledWith({ source: 'route' }); }); it('normalizes nested transaction names with custom Route', () => { - const [mockStartTransaction, history, { mockSetName }] = createInstrumentation(); + const [mockStartTransaction, history, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); const { getByText } = render( @@ -221,8 +225,9 @@ describe('React Router v4', () => { tags: { 'routing.instrumentation': 'react-router-v4' }, metadata: { source: 'url' }, }); - expect(mockSetName).toHaveBeenCalledTimes(2); - expect(mockSetName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid', 'route'); + expect(mockUpdateName).toHaveBeenCalledTimes(2); + expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid'); + expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); act(() => { history.push('/organizations/543'); @@ -237,8 +242,9 @@ describe('React Router v4', () => { tags: { 'routing.instrumentation': 'react-router-v4' }, metadata: { source: 'url' }, }); - expect(mockSetName).toHaveBeenCalledTimes(3); - expect(mockSetName).toHaveBeenLastCalledWith('/organizations/:orgid', 'route'); + expect(mockUpdateName).toHaveBeenCalledTimes(3); + expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid'); + expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); }); it('matches with route object', () => { diff --git a/packages/react/test/reactrouterv5.test.tsx b/packages/react/test/reactrouterv5.test.tsx index 104374201722..fba57df9a5f8 100644 --- a/packages/react/test/reactrouterv5.test.tsx +++ b/packages/react/test/reactrouterv5.test.tsx @@ -12,7 +12,7 @@ describe('React Router v5', () => { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; routes?: RouteConfig[]; - }): [jest.Mock, any, { mockSetName: jest.Mock; mockFinish: jest.Mock }] { + }): [jest.Mock, any, { mockUpdateName: jest.Mock; mockFinish: jest.Mock; mockSetMetadata: jest.Mock }] { const options = { matchPath: _opts && _opts.routes !== undefined ? matchPath : undefined, routes: undefined, @@ -22,14 +22,17 @@ describe('React Router v5', () => { }; const history = createMemoryHistory(); const mockFinish = jest.fn(); - const mockSetName = jest.fn(); - const mockStartTransaction = jest.fn().mockReturnValue({ setName: mockSetName, end: mockFinish }); + const mockUpdateName = jest.fn(); + const mockSetMetadata = jest.fn(); + const mockStartTransaction = jest + .fn() + .mockReturnValue({ updateName: mockUpdateName, end: mockFinish, setMetadata: mockSetMetadata }); reactRouterV5Instrumentation(history, options.routes, options.matchPath)( mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange, ); - return [mockStartTransaction, history, { mockSetName, mockFinish }]; + return [mockStartTransaction, history, { mockUpdateName, mockFinish, mockSetMetadata }]; } it('starts a pageload transaction when instrumentation is started', () => { @@ -166,7 +169,7 @@ describe('React Router v5', () => { }); it('normalizes transaction name with custom Route', () => { - const [mockStartTransaction, history, { mockSetName }] = createInstrumentation(); + const [mockStartTransaction, history, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); const { getByText } = render( @@ -191,12 +194,13 @@ describe('React Router v5', () => { tags: { 'routing.instrumentation': 'react-router-v5' }, metadata: { source: 'url' }, }); - expect(mockSetName).toHaveBeenCalledTimes(2); - expect(mockSetName).toHaveBeenLastCalledWith('/users/:userid', 'route'); + expect(mockUpdateName).toHaveBeenCalledTimes(2); + expect(mockUpdateName).toHaveBeenLastCalledWith('/users/:userid'); + expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); }); it('normalizes nested transaction names with custom Route', () => { - const [mockStartTransaction, history, { mockSetName }] = createInstrumentation(); + const [mockStartTransaction, history, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); const { getByText } = render( @@ -222,8 +226,9 @@ describe('React Router v5', () => { tags: { 'routing.instrumentation': 'react-router-v5' }, metadata: { source: 'url' }, }); - expect(mockSetName).toHaveBeenCalledTimes(2); - expect(mockSetName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid', 'route'); + expect(mockUpdateName).toHaveBeenCalledTimes(2); + expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid'); + expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); act(() => { history.push('/organizations/543'); @@ -238,8 +243,8 @@ describe('React Router v5', () => { tags: { 'routing.instrumentation': 'react-router-v5' }, metadata: { source: 'url' }, }); - expect(mockSetName).toHaveBeenCalledTimes(3); - expect(mockSetName).toHaveBeenLastCalledWith('/organizations/:orgid', 'route'); + expect(mockUpdateName).toHaveBeenCalledTimes(3); + expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid'); }); it('matches with route object', () => { diff --git a/packages/react/test/reactrouterv6.4.test.tsx b/packages/react/test/reactrouterv6.4.test.tsx index 5c2c9e7b3d5b..a89bb50e1f82 100644 --- a/packages/react/test/reactrouterv6.4.test.tsx +++ b/packages/react/test/reactrouterv6.4.test.tsx @@ -25,7 +25,7 @@ describe('React Router v6.4', () => { function createInstrumentation(_opts?: { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; - }): [jest.Mock, { mockSetName: jest.Mock; mockFinish: jest.Mock }] { + }): [jest.Mock, { mockUpdateName: jest.Mock; mockFinish: jest.Mock; mockSetMetadata: jest.Mock }] { const options = { matchPath: _opts ? matchPath : undefined, startTransactionOnLocationChange: true, @@ -33,8 +33,11 @@ describe('React Router v6.4', () => { ..._opts, }; const mockFinish = jest.fn(); - const mockSetName = jest.fn(); - const mockStartTransaction = jest.fn().mockReturnValue({ setName: mockSetName, end: mockFinish }); + const mockUpdateName = jest.fn(); + const mockSetMetadata = jest.fn(); + const mockStartTransaction = jest + .fn() + .mockReturnValue({ updateName: mockUpdateName, end: mockFinish, setMetadata: mockSetMetadata }); reactRouterV6Instrumentation( React.useEffect, @@ -43,7 +46,7 @@ describe('React Router v6.4', () => { createRoutesFromChildren, matchRoutes, )(mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange); - return [mockStartTransaction, { mockSetName, mockFinish }]; + return [mockStartTransaction, { mockUpdateName, mockFinish, mockSetMetadata }]; } describe('wrapCreateBrowserRouter', () => { @@ -243,7 +246,7 @@ describe('React Router v6.4', () => { }); it('updates pageload transaction to a parameterized route', () => { - const [mockStartTransaction, { mockSetName }] = createInstrumentation(); + const [mockStartTransaction, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); const sentryCreateBrowserRouter = wrapCreateBrowserRouter(createMemoryRouter as CreateRouterFunction); const router = sentryCreateBrowserRouter( @@ -268,7 +271,8 @@ describe('React Router v6.4', () => { render(); expect(mockStartTransaction).toHaveBeenCalledTimes(1); - expect(mockSetName).toHaveBeenLastCalledWith('/about/:page', 'route'); + expect(mockUpdateName).toHaveBeenLastCalledWith('/about/:page'); + expect(mockSetMetadata).toHaveBeenCalledWith({ source: 'route' }); }); it('works with `basename` option', () => { diff --git a/packages/react/test/reactrouterv6.test.tsx b/packages/react/test/reactrouterv6.test.tsx index fd6ad444125e..965ce134bb74 100644 --- a/packages/react/test/reactrouterv6.test.tsx +++ b/packages/react/test/reactrouterv6.test.tsx @@ -21,7 +21,7 @@ describe('React Router v6', () => { function createInstrumentation(_opts?: { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; - }): [jest.Mock, { mockSetName: jest.Mock; mockFinish: jest.Mock }] { + }): [jest.Mock, { mockUpdateName: jest.Mock; mockFinish: jest.Mock; mockSetMetadata: jest.Mock }] { const options = { matchPath: _opts ? matchPath : undefined, startTransactionOnLocationChange: true, @@ -29,8 +29,11 @@ describe('React Router v6', () => { ..._opts, }; const mockFinish = jest.fn(); - const mockSetName = jest.fn(); - const mockStartTransaction = jest.fn().mockReturnValue({ setName: mockSetName, end: mockFinish }); + const mockUpdateName = jest.fn(); + const mockSetMetadata = jest.fn(); + const mockStartTransaction = jest + .fn() + .mockReturnValue({ updateName: mockUpdateName, end: mockFinish, setMetadata: mockSetMetadata }); reactRouterV6Instrumentation( React.useEffect, @@ -39,7 +42,7 @@ describe('React Router v6', () => { createRoutesFromChildren, matchRoutes, )(mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange); - return [mockStartTransaction, { mockSetName, mockFinish }]; + return [mockStartTransaction, { mockUpdateName, mockFinish, mockSetMetadata }]; } describe('withSentryReactRouterV6Routing', () => { @@ -542,7 +545,7 @@ describe('React Router v6', () => { }); it('does not add double slashes to URLS', () => { - const [mockStartTransaction, { mockSetName }] = createInstrumentation(); + const [mockStartTransaction, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); const wrappedUseRoutes = wrapUseRoutes(useRoutes); const Routes = () => @@ -584,11 +587,12 @@ describe('React Router v6', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(1); // should be /tests not //tests - expect(mockSetName).toHaveBeenLastCalledWith('/tests', 'route'); + expect(mockUpdateName).toHaveBeenLastCalledWith('/tests'); + expect(mockSetMetadata).toHaveBeenCalledWith({ source: 'route' }); }); it('handles wildcard routes properly', () => { - const [mockStartTransaction, { mockSetName }] = createInstrumentation(); + const [mockStartTransaction, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); const wrappedUseRoutes = wrapUseRoutes(useRoutes); const Routes = () => @@ -629,7 +633,8 @@ describe('React Router v6', () => { ); expect(mockStartTransaction).toHaveBeenCalledTimes(1); - expect(mockSetName).toHaveBeenLastCalledWith('/tests/:testId/*', 'route'); + expect(mockUpdateName).toHaveBeenLastCalledWith('/tests/:testId/*'); + expect(mockSetMetadata).toHaveBeenCalledWith({ source: 'route' }); }); }); }); diff --git a/packages/remix/src/client/performance.tsx b/packages/remix/src/client/performance.tsx index af6344165b6a..597f6daae48f 100644 --- a/packages/remix/src/client/performance.tsx +++ b/packages/remix/src/client/performance.tsx @@ -125,7 +125,8 @@ export function withSentry

, R extends React.Co _useEffect(() => { if (activeTransaction && matches && matches.length) { - activeTransaction.setName(matches[matches.length - 1].id, 'route'); + activeTransaction.updateName(matches[matches.length - 1].id); + activeTransaction.setMetadata({ source: 'route' }); } isBaseLocation = true; diff --git a/packages/sveltekit/src/client/router.ts b/packages/sveltekit/src/client/router.ts index ede1ace4ae75..0d327335326b 100644 --- a/packages/sveltekit/src/client/router.ts +++ b/packages/sveltekit/src/client/router.ts @@ -56,7 +56,8 @@ function instrumentPageload(startTransactionFn: (context: TransactionContext) => const routeId = page.route && page.route.id; if (pageloadTransaction && routeId) { - pageloadTransaction.setName(routeId, 'route'); + pageloadTransaction.updateName(routeId); + pageloadTransaction.setMetadata({ source: 'route' }); } }); } diff --git a/packages/sveltekit/test/client/router.test.ts b/packages/sveltekit/test/client/router.test.ts index 10e8a0aa0744..648e5344e6c8 100644 --- a/packages/sveltekit/test/client/router.test.ts +++ b/packages/sveltekit/test/client/router.test.ts @@ -26,7 +26,8 @@ describe('sveltekitRoutingInstrumentation', () => { const mockedStartTransaction = vi.fn().mockImplementation(txnCtx => { returnedTransaction = { ...txnCtx, - setName: vi.fn(), + updateName: vi.fn(), + setMetadata: vi.fn(), startChild: vi.fn().mockImplementation(ctx => { return { ...mockedRoutingSpan, ...ctx }; }), @@ -68,8 +69,9 @@ describe('sveltekitRoutingInstrumentation', () => { page.set({ route: { id: 'testRoute' } }); // This should update the transaction name with the parameterized route: - expect(returnedTransaction?.setName).toHaveBeenCalledTimes(1); - expect(returnedTransaction?.setName).toHaveBeenCalledWith('testRoute', 'route'); + expect(returnedTransaction?.updateName).toHaveBeenCalledTimes(1); + expect(returnedTransaction?.updateName).toHaveBeenCalledWith('testRoute'); + expect(returnedTransaction?.setMetadata).toHaveBeenCalledWith({ source: 'route' }); }); it("doesn't start a pageload transaction if `startTransactionOnPageLoad` is false", () => { diff --git a/packages/tracing-internal/src/node/integrations/express.ts b/packages/tracing-internal/src/node/integrations/express.ts index d1f609bd658d..7754fff3ad5b 100644 --- a/packages/tracing-internal/src/node/integrations/express.ts +++ b/packages/tracing-internal/src/node/integrations/express.ts @@ -376,7 +376,9 @@ function instrumentRouter(appOrRouter: ExpressRouter): void { // Therefore, we fall back to setting the final route to '/' in this case. const finalRoute = req._reconstructedRoute || '/'; - transaction.setName(...extractPathForTransaction(req, { path: true, method: true, customRoute: finalRoute })); + const [name, source] = extractPathForTransaction(req, { path: true, method: true, customRoute: finalRoute }); + transaction.updateName(name); + transaction.setMetadata({ source }); } } diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index 7920420dbfbe..011af5ba10d5 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -86,7 +86,7 @@ describe('Span', () => { test('setName', () => { const span = new Span({}); expect(span.description).toBeUndefined(); - span.setName('foo'); + span.updateName('foo'); expect(span.description).toBe('foo'); }); }); diff --git a/packages/tracing/test/transaction.test.ts b/packages/tracing/test/transaction.test.ts index 64691385f533..12c6c799883a 100644 --- a/packages/tracing/test/transaction.test.ts +++ b/packages/tracing/test/transaction.test.ts @@ -61,6 +61,17 @@ describe('`Transaction` class', () => { expect(transaction.metadata.source).toEqual('route'); }); }); + + describe('`updateName` method', () => { + it('does not change the source', () => { + const transaction = new Transaction({ name: 'dogpark' }); + transaction.setMetadata({ source: 'route' }); + transaction.updateName('ballpit'); + + expect(transaction.name).toEqual('ballpit'); + expect(transaction.metadata.source).toEqual('route'); + }); + }); }); describe('setContext', () => { diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index 6a639b4fcab5..3614961e7182 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -204,9 +204,16 @@ export interface Span extends SpanContext { /** * Set the name of the span. + * + * @deprecated Use `updateName()` instead. */ setName(name: string): void; + /** + * Update the name of the span. + */ + updateName(name: string): this; + /** * Creates a new `Span` while setting the current `Span.id` as `parentSpanId`. * Also the `sampled` decision will be inherited. diff --git a/packages/utils/src/requestdata.ts b/packages/utils/src/requestdata.ts index 5860932994d4..f2ffeaf621a1 100644 --- a/packages/utils/src/requestdata.ts +++ b/packages/utils/src/requestdata.ts @@ -74,7 +74,9 @@ export function addRequestDataToTransaction( if (!transaction) return; if (!transaction.metadata.source || transaction.metadata.source === 'url') { // Attempt to grab a parameterized route off of the request - transaction.setName(...extractPathForTransaction(req, { path: true, method: true })); + const [name, source] = extractPathForTransaction(req, { path: true, method: true }); + transaction.updateName(name); + transaction.setMetadata({ source }); } transaction.setData('url', req.originalUrl || req.url); if (req.baseUrl) { diff --git a/packages/vue/src/router.ts b/packages/vue/src/router.ts index 2f8ab7b30c1a..70117e960fe2 100644 --- a/packages/vue/src/router.ts +++ b/packages/vue/src/router.ts @@ -111,7 +111,8 @@ export function vueRouterInstrumentation( const pageloadTransaction = getActiveTransaction(); if (pageloadTransaction) { if (pageloadTransaction.metadata.source !== 'custom') { - pageloadTransaction.setName(transactionName, transactionSource); + pageloadTransaction.updateName(transactionName); + pageloadTransaction.setMetadata({ source: transactionSource }); } pageloadTransaction.setData('params', data.params); pageloadTransaction.setData('query', data.query); diff --git a/packages/vue/test/router.test.ts b/packages/vue/test/router.test.ts index da1e962c9645..2e937e02f154 100644 --- a/packages/vue/test/router.test.ts +++ b/packages/vue/test/router.test.ts @@ -126,8 +126,9 @@ describe('vueRouterInstrumentation()', () => { 'should return instrumentation that instruments VueRouter.beforeEach(%s, %s) for pageloads', (fromKey, toKey, transactionName, transactionSource) => { const mockedTxn = { - setName: jest.fn(), + updateName: jest.fn(), setData: jest.fn(), + setMetadata: jest.fn(), metadata: {}, }; const customMockStartTxn = { ...mockStartTransaction }.mockImplementation(_ => { @@ -163,7 +164,8 @@ describe('vueRouterInstrumentation()', () => { beforeEachCallback(to, from, mockNext); expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); - expect(mockedTxn.setName).toHaveBeenCalledWith(transactionName, transactionSource); + expect(mockedTxn.updateName).toHaveBeenCalledWith(transactionName); + expect(mockedTxn.setMetadata).toHaveBeenCalledWith({ source: transactionSource }); expect(mockedTxn.setData).toHaveBeenNthCalledWith(1, 'params', to.params); expect(mockedTxn.setData).toHaveBeenNthCalledWith(2, 'query', to.query); @@ -237,7 +239,7 @@ describe('vueRouterInstrumentation()', () => { it("doesn't overwrite a pageload transaction name it was set to custom before the router resolved the route", () => { const mockedTxn = { - setName: jest.fn(), + updateName: jest.fn(), setData: jest.fn(), name: '', metadata: { @@ -279,7 +281,7 @@ describe('vueRouterInstrumentation()', () => { expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); - expect(mockedTxn.setName).not.toHaveBeenCalled(); + expect(mockedTxn.updateName).not.toHaveBeenCalled(); expect(mockedTxn.metadata.source).toEqual('custom'); expect(mockedTxn.name).toEqual('customTxnName'); });