Skip to content

Commit 0dbbac6

Browse files
lforstlforst
authored
feat(vue): Add transaction source to VueRouter instrumentation (#5381)
1 parent 4984870 commit 0dbbac6

File tree

3 files changed

+205
-16
lines changed

3 files changed

+205
-16
lines changed

‎packages/vue/src/router.ts

+41-15
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,30 @@
1-
/* eslint-disable @typescript-eslint/no-explicit-any */
21
import { captureException } from '@sentry/browser';
3-
import { Transaction, TransactionContext } from '@sentry/types';
2+
import { Transaction, TransactionContext, TransactionSource } from '@sentry/types';
43

54
export type VueRouterInstrumentation = <T extends Transaction>(
65
startTransaction: (context: TransactionContext) => T | undefined,
76
startTransactionOnPageLoad?: boolean,
87
startTransactionOnLocationChange?: boolean,
98
) => void;
109

11-
// This is not great, but kinda necessary to make it work with VueRouter@3 and VueRouter@4 at the same time.
12-
type Route = {
13-
params: any;
14-
query: any;
15-
name?: any;
16-
path: any;
17-
matched: any[];
10+
// The following type is an intersection of the Route type from VueRouter v2, v3, and v4.
11+
// This is not great, but kinda necessary to make it work with all versions at the same time.
12+
export type Route = {
13+
/** Unparameterized URL */
14+
path: string;
15+
/**
16+
* Query params (keys map to null when there is no value associated, e.g. "?foo" and to an array when there are
17+
* multiple query params that have the same key, e.g. "?foo&foo=bar")
18+
*/
19+
query: Record<string, string | null | (string | null)[]>;
20+
/** Route name (VueRouter provides a way to give routes individual names) */
21+
name?: string | symbol | null | undefined;
22+
/** Evaluated parameters */
23+
params: Record<string, string | string[]>;
24+
/** All the matched route objects as defined in VueRouter constructor */
25+
matched: { path: string }[];
1826
};
27+
1928
interface VueRouter {
2029
onError: (fn: (err: Error) => void) => void;
2130
beforeEach: (fn: (to: Route, from: Route, next: () => void) => void) => void;
@@ -39,8 +48,10 @@ export function vueRouterInstrumentation(router: VueRouter): VueRouterInstrument
3948
// https://router.vuejs.org/api/#router-start-location
4049
// https://next.router.vuejs.org/api/#start-location
4150

42-
// Vue2 - null
43-
// Vue3 - undefined
51+
// from.name:
52+
// - Vue 2: null
53+
// - Vue 3: undefined
54+
// hence only '==' instead of '===', because `undefined == null` evaluates to `true`
4455
const isPageLoadNavigation = from.name == null && from.matched.length === 0;
4556

4657
const tags = {
@@ -51,23 +62,38 @@ export function vueRouterInstrumentation(router: VueRouter): VueRouterInstrument
5162
query: to.query,
5263
};
5364

65+
// Determine a name for the routing transaction and where that name came from
66+
let transactionName: string = to.path;
67+
let transactionSource: TransactionSource = 'url';
68+
if (to.name) {
69+
transactionName = to.name.toString();
70+
transactionSource = 'custom';
71+
} else if (to.matched[0] && to.matched[0].path) {
72+
transactionName = to.matched[0].path;
73+
transactionSource = 'route';
74+
}
75+
5476
if (startTransactionOnPageLoad && isPageLoadNavigation) {
5577
startTransaction({
56-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
57-
name: (to.name && to.name.toString()) || to.path,
78+
name: transactionName,
5879
op: 'pageload',
5980
tags,
6081
data,
82+
metadata: {
83+
source: transactionSource,
84+
},
6185
});
6286
}
6387

6488
if (startTransactionOnLocationChange && !isPageLoadNavigation) {
6589
startTransaction({
66-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
67-
name: (to.name && to.name.toString()) || (to.matched[0] && to.matched[0].path) || to.path,
90+
name: transactionName,
6891
op: 'navigation',
6992
tags,
7093
data,
94+
metadata: {
95+
source: transactionSource,
96+
},
7197
});
7298
}
7399

‎packages/vue/src/tracing.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export const createTracingMixins = (options: TracingOptions): Mixins => {
8383
// Skip components that we don't want to track to minimize the noise and give a more granular control to the user
8484
const name = formatComponentName(this, false);
8585
const shouldTrack = Array.isArray(options.trackComponents)
86-
? options.trackComponents.includes(name)
86+
? options.trackComponents.indexOf(name) > -1
8787
: options.trackComponents;
8888

8989
// We always want to track root component

‎packages/vue/test/router.test.ts

+163
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
import * as SentryBrowser from '@sentry/browser';
2+
3+
import { vueRouterInstrumentation } from '../src';
4+
import { Route } from '../src/router';
5+
6+
const captureExceptionSpy = jest.spyOn(SentryBrowser, 'captureException');
7+
8+
const mockVueRouter = {
9+
onError: jest.fn<void, [(error: Error) => void]>(),
10+
beforeEach: jest.fn<void, [(from: Route, to: Route, next: () => void) => void]>(),
11+
};
12+
13+
const mockStartTransaction = jest.fn();
14+
const mockNext = jest.fn();
15+
16+
const testRoutes: Record<string, Route> = {
17+
initialPageloadRoute: { matched: [], params: {}, path: '', query: {} },
18+
normalRoute1: {
19+
matched: [{ path: '/books/:bookId/chapter/:chapterId' }],
20+
params: {
21+
bookId: '12',
22+
chapterId: '3',
23+
},
24+
path: '/books/12/chapter/3',
25+
query: {
26+
utm_source: 'google',
27+
},
28+
},
29+
normalRoute2: {
30+
matched: [{ path: '/accounts/:accountId' }],
31+
params: {
32+
accountId: '4',
33+
},
34+
path: '/accounts/4',
35+
query: {},
36+
},
37+
namedRoute: {
38+
matched: [{ path: '/login' }],
39+
name: 'login-screen',
40+
params: {},
41+
path: '/login',
42+
query: {},
43+
},
44+
unmatchedRoute: {
45+
matched: [],
46+
params: {},
47+
path: '/e8733846-20ac-488c-9871-a5cbcb647294',
48+
query: {},
49+
},
50+
};
51+
52+
describe('vueRouterInstrumentation()', () => {
53+
afterEach(() => {
54+
jest.clearAllMocks();
55+
});
56+
57+
it('should return instrumentation that instruments VueRouter.onError', () => {
58+
// create instrumentation
59+
const instrument = vueRouterInstrumentation(mockVueRouter);
60+
61+
// instrument
62+
instrument(mockStartTransaction);
63+
64+
// check
65+
expect(mockVueRouter.onError).toHaveBeenCalledTimes(1);
66+
67+
const onErrorCallback = mockVueRouter.onError.mock.calls[0][0];
68+
69+
const testError = new Error();
70+
onErrorCallback(testError);
71+
72+
expect(captureExceptionSpy).toHaveBeenCalledTimes(1);
73+
expect(captureExceptionSpy).toHaveBeenCalledWith(testError);
74+
});
75+
76+
it.each([
77+
['initialPageloadRoute', 'normalRoute1', 'pageload', '/books/:bookId/chapter/:chapterId', 'route'],
78+
['normalRoute1', 'normalRoute2', 'navigation', '/accounts/:accountId', 'route'],
79+
['normalRoute2', 'namedRoute', 'navigation', 'login-screen', 'custom'],
80+
['normalRoute2', 'unmatchedRoute', 'navigation', '/e8733846-20ac-488c-9871-a5cbcb647294', 'url'],
81+
])(
82+
'should return instrumentation that instruments VueRouter.beforeEach(%s, %s)',
83+
(fromKey, toKey, op, transactionName, transactionSource) => {
84+
// create instrumentation
85+
const instrument = vueRouterInstrumentation(mockVueRouter);
86+
87+
// instrument
88+
instrument(mockStartTransaction, true, true);
89+
90+
// check
91+
expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1);
92+
const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0];
93+
94+
const from = testRoutes[fromKey];
95+
const to = testRoutes[toKey];
96+
beforeEachCallback(to, from, mockNext);
97+
98+
expect(mockStartTransaction).toHaveBeenCalledTimes(1);
99+
expect(mockStartTransaction).toHaveBeenCalledWith({
100+
name: transactionName,
101+
metadata: {
102+
source: transactionSource,
103+
},
104+
data: {
105+
params: to.params,
106+
query: to.query,
107+
},
108+
op: op,
109+
tags: {
110+
'routing.instrumentation': 'vue-router',
111+
},
112+
});
113+
114+
expect(mockNext).toHaveBeenCalledTimes(1);
115+
},
116+
);
117+
118+
test.each([
119+
[undefined, 1],
120+
[false, 0],
121+
[true, 1],
122+
])(
123+
'should return instrumentation that considers the startTransactionOnPageLoad option = %p',
124+
(startTransactionOnPageLoad, expectedCallsAmount) => {
125+
// create instrumentation
126+
const instrument = vueRouterInstrumentation(mockVueRouter);
127+
128+
// instrument
129+
instrument(mockStartTransaction, startTransactionOnPageLoad, true);
130+
131+
// check
132+
expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1);
133+
134+
const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0];
135+
beforeEachCallback(testRoutes['normalRoute1'], testRoutes['initialPageloadRoute'], mockNext);
136+
137+
expect(mockStartTransaction).toHaveBeenCalledTimes(expectedCallsAmount);
138+
},
139+
);
140+
141+
test.each([
142+
[undefined, 1],
143+
[false, 0],
144+
[true, 1],
145+
])(
146+
'should return instrumentation that considers the startTransactionOnLocationChange option = %p',
147+
(startTransactionOnLocationChange, expectedCallsAmount) => {
148+
// create instrumentation
149+
const instrument = vueRouterInstrumentation(mockVueRouter);
150+
151+
// instrument
152+
instrument(mockStartTransaction, true, startTransactionOnLocationChange);
153+
154+
// check
155+
expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1);
156+
157+
const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0];
158+
beforeEachCallback(testRoutes['normalRoute2'], testRoutes['normalRoute1'], mockNext);
159+
160+
expect(mockStartTransaction).toHaveBeenCalledTimes(expectedCallsAmount);
161+
},
162+
);
163+
});

0 commit comments

Comments
 (0)