From 42535cd96e35b4190b24ba002474ddb70007b153 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 3 Apr 2024 17:40:09 +0200 Subject: [PATCH 1/4] feat(sveltekit): Update scope `transactionName` when pageload route name is updated --- .../sveltekit-2/test/errors.client.test.ts | 3 +++ .../opentelemetry/test/propagator.test.ts | 2 +- .../src/client/browserTracingIntegration.ts | 2 ++ .../client/browserTracingIntegration.test.ts | 25 +++++++++++++++++-- 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/test/errors.client.test.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2/test/errors.client.test.ts index 329ac33b7880..3a3d53012846 100644 --- a/dev-packages/e2e-tests/test-applications/sveltekit-2/test/errors.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/test/errors.client.test.ts @@ -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 }) => { @@ -52,5 +54,6 @@ test.describe('client-side errors', () => { ); expect(errorEvent.tags).toMatchObject({ runtime: 'browser' }); + expect(errorEvent.transaction).toEqual('universal-load-error'); }); }); diff --git a/packages/opentelemetry/test/propagator.test.ts b/packages/opentelemetry/test/propagator.test.ts index 244b0aa93d2b..946546359530 100644 --- a/packages/opentelemetry/test/propagator.test.ts +++ b/packages/opentelemetry/test/propagator.test.ts @@ -203,7 +203,7 @@ describe('SentryPropagator', () => { spanId: '6e0c63257de34c92', sampled: true, dsc: { - transaction: 'sampled-transaction', + transaction: 'users/123', sampled: 'false', trace_id: 'dsc_trace_id', public_key: 'dsc_public_key', diff --git a/packages/sveltekit/src/client/browserTracingIntegration.ts b/packages/sveltekit/src/client/browserTracingIntegration.ts index 716223a17c5c..d2a266e2cbea 100644 --- a/packages/sveltekit/src/client/browserTracingIntegration.ts +++ b/packages/sveltekit/src/client/browserTracingIntegration.ts @@ -3,6 +3,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } fr import { WINDOW, browserTracingIntegration as originalBrowserTracingIntegration, + getCurrentScope, startBrowserTracingNavigationSpan, startBrowserTracingPageLoadSpan, startInactiveSpan, @@ -65,6 +66,7 @@ function _instrumentPageload(client: Client): void { if (routeId) { pageloadSpan.updateName(routeId); pageloadSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + getCurrentScope().setTransactionName(routeId); } }); } diff --git a/packages/sveltekit/test/client/browserTracingIntegration.test.ts b/packages/sveltekit/test/client/browserTracingIntegration.test.ts index 315985433e66..05d3ff9fe764 100644 --- a/packages/sveltekit/test/client/browserTracingIntegration.test.ts +++ b/packages/sveltekit/test/client/browserTracingIntegration.test.ts @@ -40,7 +40,6 @@ describe('browserTracingIntegration', () => { ...txnCtx, updateName: vi.fn(), setAttribute: vi.fn(), - setTag: vi.fn(), }; return createdRootSpan; }); @@ -52,7 +51,6 @@ describe('browserTracingIntegration', () => { ...txnCtx, updateName: vi.fn(), setAttribute: vi.fn(), - setTag: vi.fn(), }; return createdRootSpan; }); @@ -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, From 0b87667127969d7f60cf1138739f37f97d7f59f6 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 3 Apr 2024 17:42:14 +0200 Subject: [PATCH 2/4] cleanup --- packages/opentelemetry/test/propagator.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry/test/propagator.test.ts b/packages/opentelemetry/test/propagator.test.ts index 946546359530..244b0aa93d2b 100644 --- a/packages/opentelemetry/test/propagator.test.ts +++ b/packages/opentelemetry/test/propagator.test.ts @@ -203,7 +203,7 @@ describe('SentryPropagator', () => { spanId: '6e0c63257de34c92', sampled: true, dsc: { - transaction: 'users/123', + transaction: 'sampled-transaction', sampled: 'false', trace_id: 'dsc_trace_id', public_key: 'dsc_public_key', From 1d8f70808fef513325b8be11e257bb5e1fd17fdf Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 3 Apr 2024 17:51:22 +0200 Subject: [PATCH 3/4] remove to unnecessary eslint ignores --- .../sveltekit/test/client/browserTracingIntegration.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/sveltekit/test/client/browserTracingIntegration.test.ts b/packages/sveltekit/test/client/browserTracingIntegration.test.ts index 05d3ff9fe764..cdbe093e201b 100644 --- a/packages/sveltekit/test/client/browserTracingIntegration.test.ts +++ b/packages/sveltekit/test/client/browserTracingIntegration.test.ts @@ -206,7 +206,6 @@ describe('browserTracingIntegration', () => { }, }); - // eslint-disable-next-line deprecation/deprecation expect(startInactiveSpanSpy).toHaveBeenCalledWith({ op: 'ui.sveltekit.routing', name: 'SvelteKit Route Change', @@ -269,7 +268,6 @@ describe('browserTracingIntegration', () => { }, }); - // eslint-disable-next-line deprecation/deprecation expect(startInactiveSpanSpy).toHaveBeenCalledWith({ op: 'ui.sveltekit.routing', name: 'SvelteKit Route Change', From 59465ff5d852085cd25d7ae30af3028fd389151a Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 4 Apr 2024 09:59:54 +0200 Subject: [PATCH 4/4] fix e2e test --- .../test-applications/sveltekit-2/test/errors.client.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/test/errors.client.test.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2/test/errors.client.test.ts index 3a3d53012846..796d34d7623a 100644 --- a/dev-packages/e2e-tests/test-applications/sveltekit-2/test/errors.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/test/errors.client.test.ts @@ -28,7 +28,7 @@ test.describe('client-side errors', () => { expect(errorEvent.tags).toMatchObject({ runtime: 'browser' }); - expect(errorEvent.transaction).toEqual('client-error'); + expect(errorEvent.transaction).toEqual('/client-error'); }); test('captures universal load error', async ({ page }) => { @@ -54,6 +54,6 @@ test.describe('client-side errors', () => { ); expect(errorEvent.tags).toMatchObject({ runtime: 'browser' }); - expect(errorEvent.transaction).toEqual('universal-load-error'); + expect(errorEvent.transaction).toEqual('/universal-load-error'); }); });