Skip to content

feat(sveltekit): Update scope transactionName when pageload route name is updated #11406

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.

Sign up for GitHub

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 4 commits into from
Apr 4, 2024
Merged

feat(sveltekit): Update scope transactionName when pageload route name is updated #11406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
42535cd
feat(sveltekit): Update scope `transactionName` when pageload route n…
Lms24 Apr 3, 2024
0b87667
cleanup
Lms24 Apr 3, 2024
1d8f708
remove to unnecessary eslint ignores
Lms24 Apr 3, 2024
59465ff
fix e2e test
Lms24 Apr 4, 2024
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
3 changes: 3 additions & 0 deletions dev-packages/e2e-tests/test-applications/sveltekit-2/test/errors.client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ test.describe('client-side errors', () => {
);

expect(errorEvent.tags).toMatchObject({ runtime: 'browser' });

expect(errorEvent.transaction).toEqual('/client-error');
});

test('captures universal load error', async ({ page }) => {
Expand All @@ -52,5 +54,6 @@ test.describe('client-side errors', () => {
);

expect(errorEvent.tags).toMatchObject({ runtime: 'browser' });
expect(errorEvent.transaction).toEqual('/universal-load-error');
});
});
2 changes: 2 additions & 0 deletions packages/sveltekit/src/client/browserTracingIntegration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } fr
import {
WINDOW,
browserTracingIntegration as originalBrowserTracingIntegration,
getCurrentScope,
startBrowserTracingNavigationSpan,
startBrowserTracingPageLoadSpan,
startInactiveSpan,
Expand Down Expand Up @@ -65,6 +66,7 @@ function _instrumentPageload(client: Client): void {
if (routeId) {
pageloadSpan.updateName(routeId);
pageloadSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
getCurrentScope().setTransactionName(routeId);
}
});
}
Expand Down
27 changes: 23 additions & 4 deletions packages/sveltekit/test/client/browserTracingIntegration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ describe('browserTracingIntegration', () => {
...txnCtx,
updateName: vi.fn(),
setAttribute: vi.fn(),
setTag: vi.fn(),
};
return createdRootSpan;
});
Expand All @@ -52,7 +51,6 @@ describe('browserTracingIntegration', () => {
...txnCtx,
updateName: vi.fn(),
setAttribute: vi.fn(),
setTag: vi.fn(),
};
return createdRootSpan;
});
Expand Down Expand Up @@ -138,6 +136,29 @@ describe('browserTracingIntegration', () => {
expect(startBrowserTracingPageLoadSpanSpy).toHaveBeenCalledTimes(0);
});

it("updates the current scope's transactionName once it's resolved during pageload", () => {
const scopeSetTransactionNameSpy = vi.fn();

// @ts-expect-error - only returning a partial scope here, that's fine
vi.spyOn(SentrySvelte, 'getCurrentScope').mockImplementation(() => {
return {
setTransactionName: scopeSetTransactionNameSpy,
};
});

const integration = browserTracingIntegration();
// @ts-expect-error - the fakeClient doesn't satisfy Client but that's fine
integration.afterAllSetup(fakeClient);

// We emit an update to the `page` store to simulate the SvelteKit router lifecycle
// @ts-expect-error - page is a writable but the types say it's just readable
page.set({ route: { id: 'testRoute/:id' } });

// This should update the transaction name with the parameterized route:
expect(scopeSetTransactionNameSpy).toHaveBeenCalledTimes(3);
expect(scopeSetTransactionNameSpy).toHaveBeenLastCalledWith('testRoute/:id');
});

it("doesn't start a navigation span when `instrumentNavigation` is false", () => {
const integration = browserTracingIntegration({
instrumentNavigation: false,
Expand Down Expand Up @@ -185,7 +206,6 @@ describe('browserTracingIntegration', () => {
},
});

// eslint-disable-next-line deprecation/deprecation
expect(startInactiveSpanSpy).toHaveBeenCalledWith({
op: 'ui.sveltekit.routing',
name: 'SvelteKit Route Change',
Expand Down Expand Up @@ -248,7 +268,6 @@ describe('browserTracingIntegration', () => {
},
});

// eslint-disable-next-line deprecation/deprecation
expect(startInactiveSpanSpy).toHaveBeenCalledWith({
op: 'ui.sveltekit.routing',
name: 'SvelteKit Route Change',
Expand Down