-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(sveltekit): Add SvelteKit routing instrumentation #7565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
14750c0
feat(sveltekit): Add SvelteKit routing instrumentation
Lms24 cb6b864
apply code review feedback
Lms24 25efa9c
add JSDoc
Lms24 a099f2b
add test for identical nav origin and destination
Lms24 846eae6
remove unnecessary comment
Lms24 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,10 @@ | ||
import { makeBaseNPMConfig, makeNPMConfigVariants } from '../../rollup/index.js'; | ||
|
||
export default | ||
makeNPMConfigVariants( | ||
makeBaseNPMConfig({ | ||
entrypoints: [ | ||
'src/index.server.ts', | ||
'src/index.client.ts', | ||
'src/client/index.ts', | ||
'src/server/index.ts', | ||
], | ||
}), | ||
) | ||
; | ||
export default makeNPMConfigVariants( | ||
makeBaseNPMConfig({ | ||
entrypoints: ['src/index.server.ts', 'src/index.client.ts', 'src/client/index.ts', 'src/server/index.ts'], | ||
packageSpecificConfig: { | ||
external: ['$app/stores'], | ||
}, | ||
}), | ||
); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
import { getActiveTransaction } from '@sentry/core'; | ||
import { WINDOW } from '@sentry/svelte'; | ||
import type { Span, Transaction, TransactionContext } from '@sentry/types'; | ||
|
||
import { navigating, page } from '$app/stores'; | ||
|
||
const DEFAULT_TAGS = { | ||
'routing.instrumentation': '@sentry/sveltekit', | ||
}; | ||
|
||
/** | ||
* Automatically creates pageload and navigation transactions for the client-side SvelteKit router. | ||
* | ||
* This instrumentation makes use of SvelteKit's `page` and `navigating` stores which can be accessed | ||
* anywhere on the client side. | ||
* | ||
* @param startTransactionFn the function used to start (idle) transactions | ||
* @param startTransactionOnPageLoad controls if pageload transactions should be created (defaults to `true`) | ||
* @param startTransactionOnLocationChange controls if navigation transactions should be created (defauls to `true`) | ||
*/ | ||
export function svelteKitRoutingInstrumentation<T extends Transaction>( | ||
startTransactionFn: (context: TransactionContext) => T | undefined, | ||
startTransactionOnPageLoad: boolean = true, | ||
startTransactionOnLocationChange: boolean = true, | ||
): void { | ||
if (startTransactionOnPageLoad) { | ||
instrumentPageload(startTransactionFn); | ||
} | ||
|
||
if (startTransactionOnLocationChange) { | ||
instrumentNavigations(startTransactionFn); | ||
} | ||
} | ||
|
||
function instrumentPageload(startTransactionFn: (context: TransactionContext) => Transaction | undefined): void { | ||
const initialPath = WINDOW && WINDOW.location && WINDOW.location.pathname; | ||
|
||
const pageloadTransaction = startTransactionFn({ | ||
name: initialPath, | ||
op: 'pageload', | ||
description: initialPath, | ||
tags: { | ||
...DEFAULT_TAGS, | ||
}, | ||
}); | ||
|
||
page.subscribe(page => { | ||
if (!page) { | ||
return; | ||
} | ||
|
||
const routeId = page.route && page.route.id; | ||
|
||
if (pageloadTransaction && routeId) { | ||
pageloadTransaction.setName(routeId, 'route'); | ||
} | ||
}); | ||
} | ||
|
||
/** | ||
* Use the `navigating` store to start a transaction on navigations. | ||
*/ | ||
function instrumentNavigations(startTransactionFn: (context: TransactionContext) => Transaction | undefined): void { | ||
let routingSpan: Span | undefined = undefined; | ||
let activeTransaction: Transaction | undefined; | ||
|
||
navigating.subscribe(navigation => { | ||
if (!navigation) { | ||
// `navigating` emits a 'null' value when the navigation is completed. | ||
// So in this case, we can finish the routing span. If the transaction was an IdleTransaction, | ||
// it will finish automatically and if it was user-created users also need to finish it. | ||
if (routingSpan) { | ||
routingSpan.finish(); | ||
routingSpan = undefined; | ||
} | ||
return; | ||
} | ||
|
||
const routeDestination = navigation.to && navigation.to.route.id; | ||
const routeOrigin = navigation.from && navigation.from.route.id; | ||
|
||
if (routeOrigin === routeDestination) { | ||
return; | ||
Lms24 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
activeTransaction = getActiveTransaction(); | ||
|
||
if (!activeTransaction) { | ||
activeTransaction = startTransactionFn({ | ||
name: routeDestination || (WINDOW && WINDOW.location && WINDOW.location.pathname), | ||
op: 'navigation', | ||
metadata: { source: 'route' }, | ||
tags: { | ||
...DEFAULT_TAGS, | ||
}, | ||
}); | ||
} | ||
|
||
if (activeTransaction) { | ||
if (routingSpan) { | ||
// If a routing span is still open from a previous navigation, we finish it. | ||
routingSpan.finish(); | ||
AbhiPrasad marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
routingSpan = activeTransaction.startChild({ | ||
op: 'ui.sveltekit.routing', | ||
description: 'SvelteKit Route Change', | ||
}); | ||
activeTransaction.setTag('from', routeOrigin); | ||
} | ||
}); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
/* eslint-disable @typescript-eslint/unbound-method */ | ||
import type { Transaction } from '@sentry/types'; | ||
import { writable } from 'svelte/store'; | ||
import type { SpyInstance } from 'vitest'; | ||
import { vi } from 'vitest'; | ||
|
||
import { navigating, page } from '$app/stores'; | ||
|
||
import { svelteKitRoutingInstrumentation } from '../../src/client/router'; | ||
|
||
// we have to overwrite the global mock from `vitest.setup.ts` here to reset the | ||
// `navigating` store for each test. | ||
vi.mock('$app/stores', async () => { | ||
return { | ||
get navigating() { | ||
return navigatingStore; | ||
}, | ||
page: writable(), | ||
}; | ||
}); | ||
|
||
let navigatingStore = writable(); | ||
|
||
describe('sveltekitRoutingInstrumentation', () => { | ||
let returnedTransaction: (Transaction & { returnedTransaction: SpyInstance }) | undefined; | ||
const mockedStartTransaction = vi.fn().mockImplementation(txnCtx => { | ||
returnedTransaction = { | ||
...txnCtx, | ||
setName: vi.fn(), | ||
startChild: vi.fn().mockImplementation(ctx => { | ||
return { ...mockedRoutingSpan, ...ctx }; | ||
}), | ||
setTag: vi.fn(), | ||
}; | ||
return returnedTransaction; | ||
}); | ||
|
||
const mockedRoutingSpan = { | ||
finish: () => {}, | ||
}; | ||
|
||
const routingSpanFinishSpy = vi.spyOn(mockedRoutingSpan, 'finish'); | ||
|
||
beforeEach(() => { | ||
navigatingStore = writable(); | ||
vi.clearAllMocks(); | ||
}); | ||
|
||
it("starts a pageload transaction when it's called with default params", () => { | ||
svelteKitRoutingInstrumentation(mockedStartTransaction); | ||
|
||
expect(mockedStartTransaction).toHaveBeenCalledTimes(1); | ||
expect(mockedStartTransaction).toHaveBeenCalledWith({ | ||
name: '/', | ||
op: 'pageload', | ||
description: '/', | ||
tags: { | ||
'routing.instrumentation': '@sentry/sveltekit', | ||
}, | ||
}); | ||
|
||
// We emit an update to the `page` store to simulate the SvelteKit router lifecycle | ||
// @ts-ignore This is fine because we testUtils/stores.ts defines `page` as a writable store | ||
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'); | ||
}); | ||
|
||
it("doesn't start a pageload transaction if `startTransactionOnPageLoad` is false", () => { | ||
svelteKitRoutingInstrumentation(mockedStartTransaction, false); | ||
expect(mockedStartTransaction).toHaveBeenCalledTimes(0); | ||
}); | ||
|
||
it("doesn't starts a navigation transaction when `startTransactionOnLocationChange` is false", () => { | ||
svelteKitRoutingInstrumentation(mockedStartTransaction, false, false); | ||
|
||
// We emit an update to the `navigating` store to simulate the SvelteKit navigation lifecycle | ||
// @ts-ignore This is fine because we testUtils/stores.ts defines `navigating` as a writable store | ||
navigating.set( | ||
{ from: { route: { id: 'testNavigationOrigin' } } }, | ||
{ to: { route: { id: 'testNavigationDestination' } } }, | ||
); | ||
|
||
// This should update the transaction name with the parameterized route: | ||
expect(mockedStartTransaction).toHaveBeenCalledTimes(0); | ||
}); | ||
|
||
it('starts a navigation transaction when `startTransactionOnLocationChange` is true', () => { | ||
svelteKitRoutingInstrumentation(mockedStartTransaction, false, true); | ||
|
||
// We emit an update to the `navigating` store to simulate the SvelteKit navigation lifecycle | ||
// @ts-ignore This is fine because we testUtils/stores.ts defines `navigating` as a writable store | ||
navigating.set({ | ||
from: { route: { id: 'testNavigationOrigin' } }, | ||
to: { route: { id: 'testNavigationDestination' } }, | ||
}); | ||
|
||
// This should update the transaction name with the parameterized route: | ||
expect(mockedStartTransaction).toHaveBeenCalledTimes(1); | ||
expect(mockedStartTransaction).toHaveBeenCalledWith({ | ||
name: 'testNavigationDestination', | ||
op: 'navigation', | ||
metadata: { | ||
source: 'route', | ||
}, | ||
tags: { | ||
'routing.instrumentation': '@sentry/sveltekit', | ||
}, | ||
}); | ||
|
||
expect(returnedTransaction?.startChild).toHaveBeenCalledWith({ | ||
op: 'ui.sveltekit.routing', | ||
description: 'SvelteKit Route Change', | ||
}); | ||
|
||
expect(returnedTransaction?.setTag).toHaveBeenCalledWith('from', 'testNavigationOrigin'); | ||
|
||
// We emit `null` here to simulate the end of the navigation lifecycle | ||
// @ts-ignore this is fine | ||
navigating.set(null); | ||
|
||
expect(routingSpanFinishSpy).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
it("doesn't start a navigation transaction if navigation origin and destination are equal", () => { | ||
svelteKitRoutingInstrumentation(mockedStartTransaction, false, true); | ||
|
||
// We emit an update to the `navigating` store to simulate the SvelteKit navigation lifecycle | ||
// @ts-ignore This is fine because we testUtils/stores.ts defines `navigating` as a writable store | ||
navigating.set({ | ||
from: { route: { id: 'testRoute' } }, | ||
to: { route: { id: 'testRoute' } }, | ||
}); | ||
|
||
expect(mockedStartTransaction).toHaveBeenCalledTimes(0); | ||
}); | ||
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import { SDK_VERSION, WINDOW } from '@sentry/svelte'; | |
import { vi } from 'vitest'; | ||
|
||
import { BrowserTracing, init } from '../../src/client'; | ||
import { svelteKitRoutingInstrumentation } from '../../src/client/router'; | ||
|
||
const svelteInit = vi.spyOn(SentrySvelte, 'init'); | ||
|
||
|
@@ -87,6 +88,7 @@ describe('Sentry client SDK', () => { | |
// This is the closest we can get to unit-testing the `__SENTRY_TRACING__` tree-shaking guard | ||
// IRL, the code to add the integration would most likely be removed by the bundler. | ||
|
||
// @ts-ignore this is fine in the test | ||
globalThis.__SENTRY_TRACING__ = false; | ||
|
||
init({ | ||
|
@@ -100,24 +102,35 @@ describe('Sentry client SDK', () => { | |
expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); | ||
expect(browserTracing).toBeUndefined(); | ||
|
||
// @ts-ignore this is fine in the test | ||
delete globalThis.__SENTRY_TRACING__; | ||
}); | ||
|
||
// TODO: this test is only meaningful once we have a routing instrumentation which we always want to add | ||
// to a user-provided BrowserTracing integration (see NextJS SDK) | ||
it.skip('Merges the user-provided BrowserTracing integration with the automatically added one', () => { | ||
it('Merges a user-provided BrowserTracing integration with the automatically added one', () => { | ||
init({ | ||
dsn: 'https://[email protected]/1337', | ||
integrations: [new BrowserTracing({ tracePropagationTargets: ['myDomain.com'] })], | ||
integrations: [ | ||
new BrowserTracing({ tracePropagationTargets: ['myDomain.com'], startTransactionOnLocationChange: false }), | ||
], | ||
enableTracing: true, | ||
}); | ||
|
||
const integrationsToInit = svelteInit.mock.calls[0][0].integrations; | ||
const browserTracing = (getCurrentHub().getClient() as BrowserClient)?.getIntegrationById('BrowserTracing'); | ||
|
||
const browserTracing = (getCurrentHub().getClient() as BrowserClient)?.getIntegrationById( | ||
'BrowserTracing', | ||
) as BrowserTracing; | ||
const options = browserTracing.options; | ||
|
||
expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); | ||
expect(browserTracing).toBeDefined(); | ||
expect((browserTracing as BrowserTracing).options.tracePropagationTargets).toEqual(['myDomain.com']); | ||
|
||
// This shows that the user-configured options are still here | ||
expect(options.tracePropagationTargets).toEqual(['myDomain.com']); | ||
expect(options.startTransactionOnLocationChange).toBe(false); | ||
|
||
// But we force the routing instrumentation to be ours | ||
expect(options.routingInstrumentation).toEqual(svelteKitRoutingInstrumentation); | ||
}); | ||
}); | ||
}); | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import { writable } from 'svelte/store'; | ||
import { vi } from 'vitest'; | ||
|
||
export function setup() { | ||
// mock $app/stores because vitest can't resolve this import from SvelteKit. | ||
// Seems like $app/stores is only created at build time of a SvelteKit app. | ||
vi.mock('$app/stores', async () => { | ||
return { | ||
navigating: writable(), | ||
page: writable(), | ||
}; | ||
}); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.