diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts index 85bd765c9c44..0e3922a2b758 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts @@ -6,4 +6,7 @@ Sentry.init({ tunnel: `http://localhost:3031/`, // proxy server tracesSampleRate: 1.0, sendDefaultPii: true, + + // Ensure it also works with passing the integration + integrations: [Sentry.browserTracingIntegration()], }); diff --git a/packages/nextjs/src/client/browserTracingIntegration.ts b/packages/nextjs/src/client/browserTracingIntegration.ts index af8f59f53b6f..171ecef7efd2 100644 --- a/packages/nextjs/src/client/browserTracingIntegration.ts +++ b/packages/nextjs/src/client/browserTracingIntegration.ts @@ -5,7 +5,7 @@ import { startBrowserTracingNavigationSpan, startBrowserTracingPageLoadSpan, } from '@sentry/react'; -import type { Integration, StartSpanOptions } from '@sentry/types'; +import type { StartSpanOptions } from '@sentry/types'; import { nextRouterInstrumentation } from '../index.client'; /** @@ -42,7 +42,7 @@ export class BrowserTracing extends OriginalBrowserTracing { */ export function browserTracingIntegration( options?: Parameters[0], -): Integration { +): ReturnType { const browserTracingIntegrationInstance = originalBrowserTracingIntegration({ // eslint-disable-next-line deprecation/deprecation tracingOrigins: @@ -61,8 +61,16 @@ export function browserTracingIntegration( instrumentPageLoad: false, }); + const fullOptions = { + ...browserTracingIntegrationInstance.options, + instrumentPageLoad: true, + instrumentNavigation: true, + ...options, + }; + return { ...browserTracingIntegrationInstance, + options: fullOptions, afterAllSetup(client) { const startPageloadCallback = (startSpanOptions: StartSpanOptions): void => { startBrowserTracingPageLoadSpan(client, startSpanOptions); @@ -80,7 +88,7 @@ export function browserTracingIntegration( nextRouterInstrumentation( () => undefined, false, - options?.instrumentNavigation, + fullOptions.instrumentNavigation, startPageloadCallback, startNavigationCallback, ); @@ -90,7 +98,7 @@ export function browserTracingIntegration( // eslint-disable-next-line deprecation/deprecation nextRouterInstrumentation( () => undefined, - options?.instrumentPageLoad, + fullOptions.instrumentPageLoad, false, startPageloadCallback, startNavigationCallback, diff --git a/packages/nextjs/test/clientSdk.test.ts b/packages/nextjs/test/clientSdk.test.ts index 1ac61f687e88..a670f64f2d0c 100644 --- a/packages/nextjs/test/clientSdk.test.ts +++ b/packages/nextjs/test/clientSdk.test.ts @@ -1,4 +1,10 @@ -import { BaseClient, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, getActiveSpan, spanToJSON } from '@sentry/core'; +import { + BaseClient, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + getActiveSpan, + getIsolationScope, + spanToJSON, +} from '@sentry/core'; import * as SentryReact from '@sentry/react'; import type { BrowserClient } from '@sentry/react'; import { browserTracingIntegration } from '@sentry/react'; @@ -7,7 +13,13 @@ import type { Integration } from '@sentry/types'; import { logger } from '@sentry/utils'; import { JSDOM } from 'jsdom'; -import { BrowserTracing, breadcrumbsIntegration, init, nextRouterInstrumentation } from '../src/client'; +import { + BrowserTracing, + breadcrumbsIntegration, + browserTracingIntegration as nextjsBrowserTracingIntegration, + init, + nextRouterInstrumentation, +} from '../src/client'; const reactInit = jest.spyOn(SentryReact, 'init'); const captureEvent = jest.spyOn(BaseClient.prototype, 'captureEvent'); @@ -35,6 +47,11 @@ function findIntegrationByName(integrations: Integration[] = [], name: string): const TEST_DSN = 'https://public@dsn.ingest.sentry.io/1337'; describe('Client init()', () => { + beforeEach(() => { + getCurrentScope().clear(); + getIsolationScope().clear(); + }); + afterEach(() => { jest.clearAllMocks(); WINDOW.__SENTRY__.hub = undefined; @@ -141,9 +158,16 @@ describe('Client init()', () => { }); it('forces correct router instrumentation if user provides `browserTracingIntegration`', () => { + const beforeStartSpan = jest.fn(options => options); init({ dsn: TEST_DSN, - integrations: [browserTracingIntegration({ finalTimeout: 10 })], + integrations: [ + browserTracingIntegration({ + finalTimeout: 10, + instrumentNavigation: false, + beforeStartSpan, + }), + ], enableTracing: true, }); @@ -156,14 +180,58 @@ describe('Client init()', () => { // It is a "new" browser tracing integration expect(typeof integration?.afterAllSetup).toBe('function'); + // the hooks is correctly invoked + expect(beforeStartSpan).toHaveBeenCalledTimes(1); + expect(beforeStartSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: '/', + op: 'pageload', + }), + ); + + // it correctly starts the page load span + expect(getActiveSpan()).toBeDefined(); + expect(spanToJSON(getActiveSpan()!).data?.[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]).toEqual( + 'auto.pageload.nextjs.app_router_instrumentation', + ); + // This shows that the user-configured options are still here - expect(integration?.options?.finalTimeout).toEqual(10); + expect(integration?.options.finalTimeout).toEqual(10); + expect(integration?.options.instrumentNavigation).toBe(false); + expect(integration?.options.instrumentPageLoad).toBe(true); + }); - // it is the svelte kit variety + it('forces correct router instrumentation if user provides Next.js `browserTracingIntegration` ', () => { + init({ + dsn: TEST_DSN, + integrations: [ + nextjsBrowserTracingIntegration({ + finalTimeout: 10, + instrumentNavigation: false, + }), + ], + enableTracing: true, + }); + + const client = getClient()!; + // eslint-disable-next-line deprecation/deprecation + const integration = client.getIntegrationByName>('BrowserTracing'); + + expect(integration).toBeDefined(); + + // It is a "new" browser tracing integration + expect(typeof integration?.afterAllSetup).toBe('function'); + + // it correctly starts the pageload span expect(getActiveSpan()).toBeDefined(); expect(spanToJSON(getActiveSpan()!).data?.[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]).toEqual( 'auto.pageload.nextjs.app_router_instrumentation', ); + + // This shows that the user-configured options are still here + expect(integration?.options.finalTimeout).toEqual(10); + expect(integration?.options.instrumentNavigation).toBe(false); + expect(integration?.options.instrumentPageLoad).toBe(true); }); it('forces correct router instrumentation if user provides `BrowserTracing` in a function', () => { @@ -187,10 +255,10 @@ describe('Client init()', () => { expect(browserTracingIntegration?.options).toEqual( expect.objectContaining({ + startTransactionOnPageLoad: true, + startTransactionOnLocationChange: false, // eslint-disable-next-line deprecation/deprecation routingInstrumentation: nextRouterInstrumentation, - // This proves it's still the user's copy - startTransactionOnLocationChange: false, }), ); }); diff --git a/packages/sveltekit/src/client/browserTracingIntegration.ts b/packages/sveltekit/src/client/browserTracingIntegration.ts index 5e80cdc92f28..7204e2186da4 100644 --- a/packages/sveltekit/src/client/browserTracingIntegration.ts +++ b/packages/sveltekit/src/client/browserTracingIntegration.ts @@ -9,7 +9,7 @@ import { startBrowserTracingPageLoadSpan, startInactiveSpan, } from '@sentry/svelte'; -import type { Client, Integration, Span } from '@sentry/types'; +import type { Client, Span } from '@sentry/types'; import { svelteKitRoutingInstrumentation } from './router'; /** @@ -36,7 +36,7 @@ export class BrowserTracing extends OriginalBrowserTracing { */ export function browserTracingIntegration( options: Parameters[0] = {}, -): Integration { +): ReturnType { const integration = { ...originalBrowserTracingIntegration({ ...options, @@ -45,16 +45,24 @@ export function browserTracingIntegration( }), }; + const fullOptions = { + ...integration.options, + instrumentPageLoad: true, + instrumentNavigation: true, + ...options, + }; + return { ...integration, + options: fullOptions, afterAllSetup: client => { integration.afterAllSetup(client); - if (options.instrumentPageLoad !== false) { + if (fullOptions.instrumentPageLoad) { _instrumentPageload(client); } - if (options.instrumentNavigation !== false) { + if (fullOptions.instrumentNavigation) { _instrumentNavigations(client); } }, diff --git a/packages/sveltekit/test/client/sdk.test.ts b/packages/sveltekit/test/client/sdk.test.ts index 96d177a5cd84..747e4c901417 100644 --- a/packages/sveltekit/test/client/sdk.test.ts +++ b/packages/sveltekit/test/client/sdk.test.ts @@ -1,10 +1,21 @@ -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, getActiveSpan, getClient, getCurrentScope, spanToJSON } from '@sentry/core'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + getActiveSpan, + getClient, + getCurrentScope, + getIsolationScope, + spanToJSON, +} from '@sentry/core'; import type { BrowserClient } from '@sentry/svelte'; import * as SentrySvelte from '@sentry/svelte'; import { SDK_VERSION, WINDOW, browserTracingIntegration } from '@sentry/svelte'; import { vi } from 'vitest'; -import { BrowserTracing, init } from '../../src/client'; +import { + BrowserTracing, + browserTracingIntegration as sveltekitBrowserTracingIntegration, + init, +} from '../../src/client'; import { svelteKitRoutingInstrumentation } from '../../src/client/router'; const svelteInit = vi.spyOn(SentrySvelte, 'init'); @@ -16,6 +27,11 @@ describe('Sentry client SDK', () => { WINDOW.__SENTRY__.hub = undefined; }); + beforeEach(() => { + getCurrentScope().clear(); + getIsolationScope().clear(); + }); + it('adds SvelteKit metadata to the SDK options', () => { expect(svelteInit).not.toHaveBeenCalled(); @@ -99,7 +115,7 @@ describe('Sentry client SDK', () => { init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', // eslint-disable-next-line deprecation/deprecation - integrations: [new BrowserTracing({ finalTimeout: 10 })], + integrations: [new BrowserTracing({ finalTimeout: 10, startTransactionOnLocationChange: false })], enableTracing: true, }); @@ -118,12 +134,14 @@ describe('Sentry client SDK', () => { // But we force the routing instrumentation to be ours // eslint-disable-next-line deprecation/deprecation expect(options.routingInstrumentation).toEqual(svelteKitRoutingInstrumentation); + expect(options.startTransactionOnPageLoad).toEqual(true); + expect(options.startTransactionOnLocationChange).toEqual(false); }); it('Merges a user-provided browserTracingIntegration with the automatically added one', () => { init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [browserTracingIntegration({ finalTimeout: 10 })], + integrations: [browserTracingIntegration({ finalTimeout: 10, instrumentNavigation: false })], enableTracing: true, }); @@ -131,21 +149,50 @@ describe('Sentry client SDK', () => { getClient()?.getIntegrationByName>( 'BrowserTracing', ); - const options = browserTracing?.options; expect(browserTracing).toBeDefined(); // It is a "new" browser tracing integration expect(typeof browserTracing?.afterAllSetup).toBe('function'); + // it is the svelte kit variety + expect(getActiveSpan()).toBeDefined(); + expect(spanToJSON(getActiveSpan()!).data?.[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]).toEqual( + 'auto.pageload.sveltekit', + ); + // This shows that the user-configured options are still here - expect(options?.finalTimeout).toEqual(10); + expect(browserTracing?.options.finalTimeout).toEqual(10); + expect(browserTracing?.options.instrumentPageLoad).toEqual(true); + expect(browserTracing?.options.instrumentNavigation).toEqual(false); + }); - // it is the svelte kit variety + it('forces correct router instrumentation if user provides Sveltekit `browserTracingIntegration` ', () => { + init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [sveltekitBrowserTracingIntegration({ finalTimeout: 10, instrumentNavigation: false })], + enableTracing: true, + }); + + const client = getClient()!; + // eslint-disable-next-line deprecation/deprecation + const integration = client.getIntegrationByName>('BrowserTracing'); + + expect(integration).toBeDefined(); + + // It is a "new" browser tracing integration + expect(typeof integration?.afterAllSetup).toBe('function'); + + // it correctly starts the pageload span expect(getActiveSpan()).toBeDefined(); expect(spanToJSON(getActiveSpan()!).data?.[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]).toEqual( 'auto.pageload.sveltekit', ); + + // This shows that the user-configured options are still here + expect(integration?.options.finalTimeout).toEqual(10); + expect(integration?.options.instrumentNavigation).toBe(false); + expect(integration?.options.instrumentPageLoad).toBe(true); }); }); });