From db6b1b30280ad402dd8832fe991f1ca41ce7bffa Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 27 Oct 2022 09:25:57 +0200 Subject: [PATCH 1/2] fix(vue): Don't overwrite custom transaction names of Pageload transactions --- packages/vue/src/router.ts | 4 ++- packages/vue/test/router.test.ts | 53 ++++++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/packages/vue/src/router.ts b/packages/vue/src/router.ts index 1131943612e1..f782246e8fb2 100644 --- a/packages/vue/src/router.ts +++ b/packages/vue/src/router.ts @@ -92,7 +92,9 @@ export function vueRouterInstrumentation(router: VueRouter): VueRouterInstrument if (startTransactionOnPageLoad && isPageLoadNavigation) { const pageloadTransaction = getActiveTransaction(); if (pageloadTransaction) { - pageloadTransaction.setName(transactionName, transactionSource); + if (pageloadTransaction.metadata.source !== 'custom') { + pageloadTransaction.setName(transactionName, 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 d2e9248452a5..d513fc5b6b6c 100644 --- a/packages/vue/test/router.test.ts +++ b/packages/vue/test/router.test.ts @@ -123,10 +123,11 @@ describe('vueRouterInstrumentation()', () => { ['initialPageloadRoute', 'unmatchedRoute', '/e8733846-20ac-488c-9871-a5cbcb647294', 'url'], ])( 'should return instrumentation that instruments VueRouter.beforeEach(%s, %s) for pageloads', - (fromKey, toKey, _transactionName, _transactionSource) => { + (fromKey, toKey, transactionName, transactionSource) => { const mockedTxn = { setName: jest.fn(), setData: jest.fn(), + metadata: {}, }; const customMockStartTxn = { ...mockStartTransaction }.mockImplementation(_ => { return mockedTxn; @@ -160,7 +161,7 @@ describe('vueRouterInstrumentation()', () => { beforeEachCallback(to, from, mockNext); expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); - expect(mockedTxn.setName).toHaveBeenCalledWith(_transactionName, _transactionSource); + expect(mockedTxn.setName).toHaveBeenCalledWith(transactionName, transactionSource); expect(mockedTxn.setData).toHaveBeenNthCalledWith(1, 'params', to.params); expect(mockedTxn.setData).toHaveBeenNthCalledWith(2, 'query', to.query); @@ -168,6 +169,54 @@ 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(), + setData: jest.fn(), + name: '', + metadata: { + source: 'url', + }, + }; + const customMockStartTxn = { ...mockStartTransaction }.mockImplementation(_ => { + return mockedTxn; + }); + jest.spyOn(vueTracing, 'getActiveTransaction').mockImplementation(() => mockedTxn as unknown as Transaction); + + // create instrumentation + const instrument = vueRouterInstrumentation(mockVueRouter); + + // instrument + instrument(customMockStartTxn, true, true); + + // check for transaction start + expect(customMockStartTxn).toHaveBeenCalledTimes(1); + expect(customMockStartTxn).toHaveBeenCalledWith({ + name: '/', + metadata: { + source: 'url', + }, + op: 'pageload', + tags: { + 'routing.instrumentation': 'vue-router', + }, + }); + + // now we simulate a custom route call similar to what would happen if + // users use the `beforeNavigate` hook + mockedTxn.name = 'customTxnName'; + mockedTxn.metadata.source = 'custom'; + + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + beforeEachCallback(testRoutes['normalRoute1'], testRoutes['initialPageloadRoute'], mockNext); + + expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); + + expect(mockedTxn.setName).not.toHaveBeenCalled(); + expect(mockedTxn.metadata.source).toEqual('custom'); + expect(mockedTxn.name).toEqual('customTxnName'); + }); + test.each([ [undefined, 1], [false, 0], From 9e3beb7a0ffe7c16273c716022f4d5c6504ec1fa Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 27 Oct 2022 09:49:50 +0200 Subject: [PATCH 2/2] adjust test comment that made no sense :D --- packages/vue/test/router.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/vue/test/router.test.ts b/packages/vue/test/router.test.ts index d513fc5b6b6c..c4072616d9af 100644 --- a/packages/vue/test/router.test.ts +++ b/packages/vue/test/router.test.ts @@ -202,8 +202,8 @@ describe('vueRouterInstrumentation()', () => { }, }); - // now we simulate a custom route call similar to what would happen if - // users use the `beforeNavigate` hook + // now we give the transaction a custom name, thereby simulating what would + // happen when users use the `beforeNavigate` hook mockedTxn.name = 'customTxnName'; mockedTxn.metadata.source = 'custom';