Skip to content

Commit b5497c7

Browse files
authored
fix(vue): Don't overwrite custom transaction names of pageload transactions (#6060)
As brought to our attention in #6048, our pageload transaction start change (which makes the transaction start earlier) introduced in #5983 caused custom transaction names to be overwritten when `beforeNavigate` is used to update the transaction name. This patch fixes this problem by checking for the transaction source before (not) updating the current transaction name with the resolved route name once the router's `beforeEach` hook was called. Furthermore, it adds a test to cover this case. See #6048 (comment) for the motivation for creating this PR.
1 parent 0a22a61 commit b5497c7

File tree

2 files changed

+54
-3
lines changed

2 files changed

+54
-3
lines changed

packages/vue/src/router.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ export function vueRouterInstrumentation(router: VueRouter): VueRouterInstrument
9292
if (startTransactionOnPageLoad && isPageLoadNavigation) {
9393
const pageloadTransaction = getActiveTransaction();
9494
if (pageloadTransaction) {
95-
pageloadTransaction.setName(transactionName, transactionSource);
95+
if (pageloadTransaction.metadata.source !== 'custom') {
96+
pageloadTransaction.setName(transactionName, transactionSource);
97+
}
9698
pageloadTransaction.setData('params', data.params);
9799
pageloadTransaction.setData('query', data.query);
98100
}

packages/vue/test/router.test.ts

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,11 @@ describe('vueRouterInstrumentation()', () => {
123123
['initialPageloadRoute', 'unmatchedRoute', '/e8733846-20ac-488c-9871-a5cbcb647294', 'url'],
124124
])(
125125
'should return instrumentation that instruments VueRouter.beforeEach(%s, %s) for pageloads',
126-
(fromKey, toKey, _transactionName, _transactionSource) => {
126+
(fromKey, toKey, transactionName, transactionSource) => {
127127
const mockedTxn = {
128128
setName: jest.fn(),
129129
setData: jest.fn(),
130+
metadata: {},
130131
};
131132
const customMockStartTxn = { ...mockStartTransaction }.mockImplementation(_ => {
132133
return mockedTxn;
@@ -160,14 +161,62 @@ describe('vueRouterInstrumentation()', () => {
160161
beforeEachCallback(to, from, mockNext);
161162
expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1);
162163

163-
expect(mockedTxn.setName).toHaveBeenCalledWith(_transactionName, _transactionSource);
164+
expect(mockedTxn.setName).toHaveBeenCalledWith(transactionName, transactionSource);
164165
expect(mockedTxn.setData).toHaveBeenNthCalledWith(1, 'params', to.params);
165166
expect(mockedTxn.setData).toHaveBeenNthCalledWith(2, 'query', to.query);
166167

167168
expect(mockNext).toHaveBeenCalledTimes(1);
168169
},
169170
);
170171

172+
it("doesn't overwrite a pageload transaction name it was set to custom before the router resolved the route", () => {
173+
const mockedTxn = {
174+
setName: jest.fn(),
175+
setData: jest.fn(),
176+
name: '',
177+
metadata: {
178+
source: 'url',
179+
},
180+
};
181+
const customMockStartTxn = { ...mockStartTransaction }.mockImplementation(_ => {
182+
return mockedTxn;
183+
});
184+
jest.spyOn(vueTracing, 'getActiveTransaction').mockImplementation(() => mockedTxn as unknown as Transaction);
185+
186+
// create instrumentation
187+
const instrument = vueRouterInstrumentation(mockVueRouter);
188+
189+
// instrument
190+
instrument(customMockStartTxn, true, true);
191+
192+
// check for transaction start
193+
expect(customMockStartTxn).toHaveBeenCalledTimes(1);
194+
expect(customMockStartTxn).toHaveBeenCalledWith({
195+
name: '/',
196+
metadata: {
197+
source: 'url',
198+
},
199+
op: 'pageload',
200+
tags: {
201+
'routing.instrumentation': 'vue-router',
202+
},
203+
});
204+
205+
// now we give the transaction a custom name, thereby simulating what would
206+
// happen when users use the `beforeNavigate` hook
207+
mockedTxn.name = 'customTxnName';
208+
mockedTxn.metadata.source = 'custom';
209+
210+
const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0];
211+
beforeEachCallback(testRoutes['normalRoute1'], testRoutes['initialPageloadRoute'], mockNext);
212+
213+
expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1);
214+
215+
expect(mockedTxn.setName).not.toHaveBeenCalled();
216+
expect(mockedTxn.metadata.source).toEqual('custom');
217+
expect(mockedTxn.name).toEqual('customTxnName');
218+
});
219+
171220
test.each([
172221
[undefined, 1],
173222
[false, 0],

0 commit comments

Comments
 (0)