Skip to content

Commit 97896f9

Browse files
Lms24Lms24
committed
add tests
1 parent aaf9f3f commit 97896f9

File tree

4 files changed

+297
-11
lines changed

4 files changed

+297
-11
lines changed

‎.vscode/settings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,5 +38,6 @@
3838
"editor.codeActionsOnSave": {
3939
"source.organizeImports.biome": "explicit",
4040
},
41-
"editor.defaultFormatter": "biomejs.biome"
41+
"editor.defaultFormatter": "biomejs.biome",
42+
"cSpell.words": ["pageloads"]
4243
}

‎packages/sveltekit/src/client/browserTracingIntegration.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,23 @@
11
import { navigating, page } from '$app/stores';
2-
import {
3-
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
4-
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
5-
getActiveSpan,
6-
startInactiveSpan,
7-
} from '@sentry/core';
2+
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
83
import {
94
BrowserTracing as OriginalBrowserTracing,
105
WINDOW,
116
browserTracingIntegration as originalBrowserTracingIntegration,
7+
getActiveSpan,
128
startBrowserTracingNavigationSpan,
139
startBrowserTracingPageLoadSpan,
10+
startInactiveSpan,
1411
} from '@sentry/svelte';
1512
import type { Client, Integration, Span } from '@sentry/types';
1613
import { svelteKitRoutingInstrumentation } from './router';
1714

1815
/**
1916
* A custom BrowserTracing integration for Sveltekit.
2017
*
21-
* @deprecated use `browserTracingIntegration()` instead.
18+
* @deprecated use `browserTracingIntegration()` instead. The new `browserTracingIntegration()`
19+
* includes SvelteKit-specific routing instrumentation out of the box. Therefore there's no need
20+
* to pass in `svelteKitRoutingInstrumentation` anymore.
2221
*/
2322
export class BrowserTracing extends OriginalBrowserTracing {
2423
public constructor(options?: ConstructorParameters<typeof OriginalBrowserTracing>[0]) {
@@ -68,12 +67,12 @@ function _instrumentPageload(client: Client): void {
6867
startBrowserTracingPageLoadSpan(client, {
6968
name: initialPath,
7069
op: 'pageload',
71-
origin: 'auto.pageload.sveltekit',
7270
description: initialPath,
7371
tags: {
7472
'routing.instrumentation': '@sentry/sveltekit',
7573
},
7674
attributes: {
75+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.sveltekit',
7776
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
7877
},
7978
});
@@ -98,7 +97,7 @@ function _instrumentPageload(client: Client): void {
9897
* Use the `navigating` store to start a transaction on navigations.
9998
*/
10099
function _instrumentNavigations(client: Client): void {
101-
let routingSpan: Span | undefined = undefined;
100+
let routingSpan: Span | undefined;
102101
let activeSpan: Span | undefined;
103102

104103
navigating.subscribe(navigation => {
@@ -136,8 +135,8 @@ function _instrumentNavigations(client: Client): void {
136135
startBrowserTracingNavigationSpan(client, {
137136
name: parameterizedRouteDestination || rawRouteDestination || 'unknown',
138137
op: 'navigation',
139-
origin: 'auto.navigation.sveltekit',
140138
attributes: {
139+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.sveltekit',
141140
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: parameterizedRouteDestination ? 'route' : 'url',
142141
},
143142
tags: {

‎packages/sveltekit/src/client/router.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const DEFAULT_TAGS = {
1919
* @param startTransactionOnLocationChange controls if navigation transactions should be created (defauls to `true`)
2020
*
2121
* @deprecated use `browserTracingIntegration()` instead which includes SvelteKit-specific routing instrumentation out of the box.
22+
* Therefore, this function will be removed in v8.
2223
*/
2324
export function svelteKitRoutingInstrumentation<T extends Transaction>(
2425
startTransactionFn: (context: TransactionContext) => T | undefined,

‎packages/sveltekit/test/client/browserTracingIntegration.test.ts

Lines changed: 285 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,285 @@
1+
/* eslint-disable @typescript-eslint/unbound-method */
2+
import type { Span } from '@sentry/types';
3+
import { writable } from 'svelte/store';
4+
import { vi } from 'vitest';
5+
6+
import { navigating, page } from '$app/stores';
7+
8+
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
9+
import { browserTracingIntegration } from '../../src/client';
10+
11+
import * as SentrySvelte from '@sentry/svelte';
12+
13+
// we have to overwrite the global mock from `vitest.setup.ts` here to reset the
14+
// `navigating` store for each test.
15+
vi.mock('$app/stores', async () => {
16+
return {
17+
get navigating() {
18+
return navigatingStore;
19+
},
20+
page: writable(),
21+
};
22+
});
23+
24+
let navigatingStore = writable();
25+
26+
describe('browserTracingIntegration', () => {
27+
const svelteBrowserTracingIntegrationSpy = vi.spyOn(SentrySvelte, 'browserTracingIntegration');
28+
29+
let createdRootSpan: Partial<Span> | undefined;
30+
31+
// @ts-expect-error - only returning a partial span here, that's fine
32+
vi.spyOn(SentrySvelte, 'getActiveSpan').mockImplementation(() => {
33+
return createdRootSpan;
34+
});
35+
36+
const startBrowserTracingPageLoadSpanSpy = vi
37+
.spyOn(SentrySvelte, 'startBrowserTracingPageLoadSpan')
38+
.mockImplementation((_client, txnCtx) => {
39+
createdRootSpan = {
40+
...txnCtx,
41+
updateName: vi.fn(),
42+
setAttribute: vi.fn(),
43+
startChild: vi.fn().mockImplementation(ctx => {
44+
return { ...mockedRoutingSpan, ...ctx };
45+
}),
46+
setTag: vi.fn(),
47+
};
48+
});
49+
50+
const startBrowserTracingNavigationSpanSpy = vi
51+
.spyOn(SentrySvelte, 'startBrowserTracingNavigationSpan')
52+
.mockImplementation((_client, txnCtx) => {
53+
createdRootSpan = {
54+
...txnCtx,
55+
updateName: vi.fn(),
56+
setAttribute: vi.fn(),
57+
setTag: vi.fn(),
58+
};
59+
});
60+
61+
const fakeClient = { getOptions: () => undefined };
62+
63+
const mockedRoutingSpan = {
64+
end: () => {},
65+
};
66+
67+
const routingSpanEndSpy = vi.spyOn(mockedRoutingSpan, 'end');
68+
69+
// @ts-expect-error - mockedRoutingSpan is not a complete Span, that's fine
70+
const startInactiveSpanSpy = vi.spyOn(SentrySvelte, 'startInactiveSpan').mockImplementation(() => mockedRoutingSpan);
71+
72+
beforeEach(() => {
73+
createdRootSpan = undefined;
74+
navigatingStore = writable();
75+
vi.clearAllMocks();
76+
});
77+
78+
it('implements required hooks', () => {
79+
const integration = browserTracingIntegration();
80+
expect(integration.name).toEqual('BrowserTracing');
81+
expect(integration.setupOnce).toBeDefined();
82+
expect(integration.afterAllSetup).toBeDefined();
83+
});
84+
85+
it('passes on the options to the original integration', () => {
86+
browserTracingIntegration({ enableLongTask: true, idleTimeout: 4242 });
87+
expect(svelteBrowserTracingIntegrationSpy).toHaveBeenCalledTimes(1);
88+
expect(svelteBrowserTracingIntegrationSpy).toHaveBeenCalledWith({
89+
enableLongTask: true,
90+
idleTimeout: 4242,
91+
instrumentNavigation: false,
92+
instrumentPageLoad: false,
93+
});
94+
});
95+
96+
it('always disables `instrumentNavigation` and `instrumentPageLoad` in the original integration', () => {
97+
browserTracingIntegration({ instrumentNavigation: true, instrumentPageLoad: true });
98+
expect(svelteBrowserTracingIntegrationSpy).toHaveBeenCalledTimes(1);
99+
// This is fine and expected because we don't want to start the default instrumentation
100+
// SvelteKit's browserTracingIntegration takes care of instrumenting pageloads and navigations on its own.
101+
expect(svelteBrowserTracingIntegrationSpy).toHaveBeenCalledWith({
102+
instrumentNavigation: false,
103+
instrumentPageLoad: false,
104+
});
105+
});
106+
107+
it("starts a pageload span when it's called with default params", () => {
108+
const integration = browserTracingIntegration();
109+
// @ts-expect-error - the fakeClient doesn't satisfy Client but that's fine
110+
integration.afterAllSetup(fakeClient);
111+
112+
expect(startBrowserTracingPageLoadSpanSpy).toHaveBeenCalledTimes(1);
113+
expect(startBrowserTracingPageLoadSpanSpy).toHaveBeenCalledWith(fakeClient, {
114+
name: '/',
115+
op: 'pageload',
116+
description: '/',
117+
tags: {
118+
'routing.instrumentation': '@sentry/sveltekit',
119+
},
120+
attributes: {
121+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.sveltekit',
122+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
123+
},
124+
});
125+
126+
// We emit an update to the `page` store to simulate the SvelteKit router lifecycle
127+
// @ts-expect-error - page is a writable but the types say it's just readable
128+
page.set({ route: { id: 'testRoute' } });
129+
130+
// This should update the transaction name with the parameterized route:
131+
expect(createdRootSpan?.updateName).toHaveBeenCalledTimes(1);
132+
expect(createdRootSpan?.updateName).toHaveBeenCalledWith('testRoute');
133+
expect(createdRootSpan?.setAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
134+
});
135+
136+
it("doesn't start a pageload span if `instrumentPageLoad` is false", () => {
137+
const integration = browserTracingIntegration({
138+
instrumentPageLoad: false,
139+
});
140+
// @ts-expect-error - the fakeClient doesn't satisfy Client but that's fine
141+
integration.afterAllSetup(fakeClient);
142+
143+
expect(startBrowserTracingPageLoadSpanSpy).toHaveBeenCalledTimes(0);
144+
});
145+
146+
it("doesn't start a navigation span when `instrumentNavigation` is false", () => {
147+
const integration = browserTracingIntegration({
148+
instrumentNavigation: false,
149+
});
150+
// @ts-expect-error - the fakeClient doesn't satisfy Client but that's fine
151+
integration.afterAllSetup(fakeClient);
152+
153+
// We emit an update to the `navigating` store to simulate the SvelteKit navigation lifecycle
154+
// @ts-expect-error - page is a writable but the types say it's just readable
155+
navigating.set({
156+
from: { route: { id: '/users' }, url: { pathname: '/users' } },
157+
to: { route: { id: '/users/[id]' }, url: { pathname: '/users/7762' } },
158+
});
159+
160+
// This should update the transaction name with the parameterized route:
161+
expect(startBrowserTracingNavigationSpanSpy).toHaveBeenCalledTimes(0);
162+
});
163+
164+
it('starts a navigation span when `startTransactionOnLocationChange` is true', () => {
165+
const integration = browserTracingIntegration({
166+
instrumentPageLoad: false,
167+
});
168+
// @ts-expect-error - the fakeClient doesn't satisfy Client but that's fine
169+
integration.afterAllSetup(fakeClient);
170+
171+
// We emit an update to the `navigating` store to simulate the SvelteKit navigation lifecycle
172+
// @ts-expect-error - page is a writable but the types say it's just readable
173+
navigating.set({
174+
from: { route: { id: '/users' }, url: { pathname: '/users' } },
175+
to: { route: { id: '/users/[id]' }, url: { pathname: '/users/7762' } },
176+
});
177+
178+
// This should update the transaction name with the parameterized route:
179+
expect(startBrowserTracingNavigationSpanSpy).toHaveBeenCalledTimes(1);
180+
expect(startBrowserTracingNavigationSpanSpy).toHaveBeenCalledWith(fakeClient, {
181+
name: '/users/[id]',
182+
op: 'navigation',
183+
attributes: {
184+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.sveltekit',
185+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
186+
},
187+
tags: {
188+
'routing.instrumentation': '@sentry/sveltekit',
189+
},
190+
});
191+
192+
// eslint-disable-next-line deprecation/deprecation
193+
expect(startInactiveSpanSpy).toHaveBeenCalledWith({
194+
op: 'ui.sveltekit.routing',
195+
name: 'SvelteKit Route Change',
196+
attributes: {
197+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.sveltekit',
198+
},
199+
});
200+
201+
// eslint-disable-next-line deprecation/deprecation
202+
expect(createdRootSpan?.setAttribute).toHaveBeenCalledWith('sentry.sveltekit.navigation.from', '/users');
203+
204+
// We emit `null` here to simulate the end of the navigation lifecycle
205+
// @ts-expect-error - page is a writable but the types say it's just readable
206+
navigating.set(null);
207+
208+
expect(routingSpanEndSpy).toHaveBeenCalledTimes(1);
209+
});
210+
211+
describe('handling same origin and destination navigations', () => {
212+
it("doesn't start a navigation span if the raw navigation origin and destination are equal", () => {
213+
const integration = browserTracingIntegration({
214+
instrumentPageLoad: false,
215+
});
216+
// @ts-expect-error - the fakeClient doesn't satisfy Client but that's fine
217+
integration.afterAllSetup(fakeClient);
218+
219+
// We emit an update to the `navigating` store to simulate the SvelteKit navigation lifecycle
220+
// @ts-expect-error - page is a writable but the types say it's just readable
221+
navigating.set({
222+
from: { route: { id: '/users/[id]' }, url: { pathname: '/users/7762' } },
223+
to: { route: { id: '/users/[id]' }, url: { pathname: '/users/7762' } },
224+
});
225+
226+
expect(startBrowserTracingNavigationSpanSpy).toHaveBeenCalledTimes(0);
227+
});
228+
229+
it('starts a navigation transaction if the raw navigation origin and destination are not equal', () => {
230+
const integration = browserTracingIntegration({
231+
instrumentPageLoad: false,
232+
});
233+
// @ts-expect-error - the fakeClient doesn't satisfy Client but that's fine
234+
integration.afterAllSetup(fakeClient);
235+
236+
// @ts-expect-error - page is a writable but the types say it's just readable
237+
navigating.set({
238+
from: { route: { id: '/users/[id]' }, url: { pathname: '/users/7762' } },
239+
to: { route: { id: '/users/[id]' }, url: { pathname: '/users/223412' } },
240+
});
241+
242+
expect(startBrowserTracingNavigationSpanSpy).toHaveBeenCalledTimes(1);
243+
expect(startBrowserTracingNavigationSpanSpy).toHaveBeenCalledWith(fakeClient, {
244+
name: '/users/[id]',
245+
op: 'navigation',
246+
attributes: {
247+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
248+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.sveltekit',
249+
},
250+
tags: {
251+
'routing.instrumentation': '@sentry/sveltekit',
252+
},
253+
});
254+
255+
// eslint-disable-next-line deprecation/deprecation
256+
expect(startInactiveSpanSpy).toHaveBeenCalledWith({
257+
op: 'ui.sveltekit.routing',
258+
name: 'SvelteKit Route Change',
259+
attributes: {
260+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.sveltekit',
261+
},
262+
});
263+
264+
// eslint-disable-next-line deprecation/deprecation
265+
expect(createdRootSpan?.setAttribute).toHaveBeenCalledWith('sentry.sveltekit.navigation.from', '/users/[id]');
266+
});
267+
268+
it('falls back to `window.location.pathname` to determine the raw origin', () => {
269+
const integration = browserTracingIntegration({
270+
instrumentPageLoad: false,
271+
});
272+
// @ts-expect-error - the fakeClient doesn't satisfy Client but that's fine
273+
integration.afterAllSetup(fakeClient);
274+
275+
// window.location.pathame is "/" in tests
276+
277+
// @ts-expect-error - page is a writable but the types say it's just readable
278+
navigating.set({
279+
to: { route: {}, url: { pathname: '/' } },
280+
});
281+
282+
expect(startBrowserTracingNavigationSpanSpy).toHaveBeenCalledTimes(0);
283+
});
284+
});
285+
});

0 commit comments

Comments
 (0)