Skip to content

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 5 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/sveltekit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
},
"devDependencies": {
"@sveltejs/kit": "^1.11.0",
"svelte": "^3.44.0",
"typescript": "^4.9.3",
"vite": "4.0.0"
},
Expand Down
20 changes: 8 additions & 12 deletions packages/sveltekit/rollup.npm.config.js
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'],
},
}),
);
111 changes: 111 additions & 0 deletions packages/sveltekit/src/client/router.ts
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;
}

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();
}
routingSpan = activeTransaction.startChild({
op: 'ui.sveltekit.routing',
description: 'SvelteKit Route Change',
});
activeTransaction.setTag('from', routeOrigin);
}
});
}
12 changes: 5 additions & 7 deletions packages/sveltekit/src/client/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import { defaultRequestInstrumentationOptions } from '@sentry-internal/tracing';
import { hasTracingEnabled } from '@sentry/core';
import type { BrowserOptions } from '@sentry/svelte';
import { BrowserTracing, configureScope, init as initSvelteSdk } from '@sentry/svelte';
import { addOrUpdateIntegration } from '@sentry/utils';

import { applySdkMetadata } from '../common/metadata';
import { svelteKitRoutingInstrumentation } from './router';

// Treeshakable guard to remove all code related to tracing
declare const __SENTRY_TRACING__: boolean;

/**
* Initialize the client side of the Sentry SvelteKit SDK.
*
* @param options
* @param options Configuration options for the SDK.
*/
export function init(options: BrowserOptions): void {
applySdkMetadata(options, ['sveltekit', 'svelte']);
Expand All @@ -33,14 +34,11 @@ function addClientIntegrations(options: BrowserOptions): void {
if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) {
if (hasTracingEnabled(options)) {
const defaultBrowserTracingIntegration = new BrowserTracing({
tracePropagationTargets: [...defaultRequestInstrumentationOptions.tracePropagationTargets],
// TODO: Add SvelteKit router instrumentations
// routingInstrumentation: sveltekitRoutingInstrumentation,
routingInstrumentation: svelteKitRoutingInstrumentation,
});

integrations = addOrUpdateIntegration(defaultBrowserTracingIntegration, integrations, {
// TODO: Add SvelteKit router instrumentations
// options.routingInstrumentation: sveltekitRoutingInstrumentation,
'options.routingInstrumentation': svelteKitRoutingInstrumentation,
});
}
}
Expand Down
139 changes: 139 additions & 0 deletions packages/sveltekit/test/client/router.test.ts
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);
});
});
25 changes: 19 additions & 6 deletions packages/sveltekit/test/client/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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({
Expand All @@ -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);
});
});
});
Expand Down
13 changes: 13 additions & 0 deletions packages/sveltekit/test/vitest.setup.ts
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(),
};
});
}
Loading