feat(sveltekit): Automatically add BrowserTracing integration (#7528)

Lms24 · web-flow · commit 8b8bb473c8f2 · 2023-03-21T09:26:44.000+01:00
With this patch, the `BrowserTracing` integration is automatically added to the client SvelteKit SDK initialization. As in NextJS, tree-shaking will still work by replacing the `__SENTRY_TRACING__` flag with `false` when bundling.
diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts
@@ -4,11 +4,11 @@ import type { BrowserOptions } from '@sentry/react';
 import { configureScope, init as reactInit, Integrations } from '@sentry/react';
 import { BrowserTracing, defaultRequestInstrumentationOptions } from '@sentry/tracing';
 import type { EventProcessor } from '@sentry/types';
+import { addOrUpdateIntegration } from '@sentry/utils';
 
 import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor';
 import { getVercelEnv } from '../common/getVercelEnv';
 import { buildMetadata } from '../common/metadata';
-import { addOrUpdateIntegration } from '../common/userIntegrations';
 import { nextRouterInstrumentation } from './performance';
 import { applyTunnelRouteOption } from './tunnelRoute';
 
diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts
@@ -4,15 +4,14 @@ import { RewriteFrames } from '@sentry/integrations';
 import type { NodeOptions } from '@sentry/node';
 import { configureScope, getCurrentHub, init as nodeInit, Integrations } from '@sentry/node';
 import type { EventProcessor } from '@sentry/types';
-import { escapeStringForRegex, logger } from '@sentry/utils';
+import type { IntegrationWithExclusionOption } from '@sentry/utils';
+import { addOrUpdateIntegration, escapeStringForRegex, logger } from '@sentry/utils';
 import * as domainModule from 'domain';
 import * as path from 'path';
 
 import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor';
 import { getVercelEnv } from '../common/getVercelEnv';
 import { buildMetadata } from '../common/metadata';
-import type { IntegrationWithExclusionOption } from '../common/userIntegrations';
-import { addOrUpdateIntegration } from '../common/userIntegrations';
 import { isBuild } from './utils/isBuild';
 
 export * from '@sentry/node';
diff --git a/packages/nextjs/test/clientSdk.test.ts b/packages/nextjs/test/clientSdk.test.ts
@@ -3,11 +3,11 @@ import * as SentryReact from '@sentry/react';
 import { WINDOW } from '@sentry/react';
 import { Integrations as TracingIntegrations } from '@sentry/tracing';
 import type { Integration } from '@sentry/types';
+import type { UserIntegrationsFunction } from '@sentry/utils';
 import { logger } from '@sentry/utils';
 import { JSDOM } from 'jsdom';
 
 import { init, Integrations, nextRouterInstrumentation } from '../src/client';
-import type { UserIntegrationsFunction } from '../src/common/userIntegrations';
 
 const { BrowserTracing } = TracingIntegrations;
 
diff --git a/packages/sveltekit/package.json b/packages/sveltekit/package.json
@@ -25,6 +25,7 @@
     "@sentry/svelte": "7.44.1",
     "@sentry/types": "7.44.1",
     "@sentry/utils": "7.44.1",
+    "@sentry-internal/tracing": "7.44.1",
     "magic-string": "^0.30.0"
   },
   "devDependencies": {
diff --git a/packages/sveltekit/src/client/sdk.ts b/packages/sveltekit/src/client/sdk.ts
@@ -1,18 +1,49 @@
+import { defaultRequestInstrumentationOptions } from '@sentry-internal/tracing';
+import { hasTracingEnabled } from '@sentry/core';
 import type { BrowserOptions } from '@sentry/svelte';
-import { configureScope, init as initSvelteSdk } from '@sentry/svelte';
+import { BrowserTracing, configureScope, init as initSvelteSdk } from '@sentry/svelte';
+import { addOrUpdateIntegration } from '@sentry/utils';
 
 import { applySdkMetadata } from '../common/metadata';
 
+// Treeshakable guard to remove all code related to tracing
+declare const __SENTRY_TRACING__: boolean;
+
 /**
  *
  * @param options
  */
 export function init(options: BrowserOptions): void {
   applySdkMetadata(options, ['sveltekit', 'svelte']);
 
+  addClientIntegrations(options);
+
   initSvelteSdk(options);
 
   configureScope(scope => {
     scope.setTag('runtime', 'browser');
   });
 }
+
+function addClientIntegrations(options: BrowserOptions): void {
+  let integrations = options.integrations || [];
+
+  // This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false",
+  // in which case everything inside will get treeshaken away
+  if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) {
+    if (hasTracingEnabled(options)) {
+      const defaultBrowserTracingIntegration = new BrowserTracing({
+        tracePropagationTargets: [...defaultRequestInstrumentationOptions.tracePropagationTargets],
+        // TODO: Add SvelteKit router instrumentations
+        // routingInstrumentation: sveltekitRoutingInstrumentation,
+      });
+
+      integrations = addOrUpdateIntegration(defaultBrowserTracingIntegration, integrations, {
+        // TODO: Add SvelteKit router instrumentations
+        // options.routingInstrumentation: sveltekitRoutingInstrumentation,
+      });
+    }
+  }
+
+  options.integrations = integrations;
+}
diff --git a/packages/sveltekit/test/client/sdk.test.ts b/packages/sveltekit/test/client/sdk.test.ts
@@ -1,9 +1,10 @@
 import { getCurrentHub } from '@sentry/core';
+import type { BrowserClient } from '@sentry/svelte';
 import * as SentrySvelte from '@sentry/svelte';
 import { SDK_VERSION, WINDOW } from '@sentry/svelte';
 import { vi } from 'vitest';
 
-import { init } from '../../src/client/sdk';
+import { BrowserTracing, init } from '../../src/client';
 
 const svelteInit = vi.spyOn(SentrySvelte, 'init');
 
@@ -47,5 +48,77 @@ describe('Sentry client SDK', () => {
       // @ts-ignore need access to protected _tags attribute
       expect(currentScope._tags).toEqual({ runtime: 'browser' });
     });
+
+    describe('automatically added integrations', () => {
+      it.each([
+        ['tracesSampleRate', { tracesSampleRate: 0 }],
+        ['tracesSampler', { tracesSampler: () => 1.0 }],
+        ['enableTracing', { enableTracing: true }],
+      ])('adds the BrowserTracing integration if tracing is enabled via %s', (_, tracingOptions) => {
+        init({
+          dsn: 'https://public@dsn.ingest.sentry.io/1337',
+          ...tracingOptions,
+        });
+
+        const integrationsToInit = svelteInit.mock.calls[0][0].integrations;
+        const browserTracing = (getCurrentHub().getClient() as BrowserClient)?.getIntegrationById('BrowserTracing');
+
+        expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
+        expect(browserTracing).toBeDefined();
+      });
+
+      it.each([
+        ['enableTracing', { enableTracing: false }],
+        ['no tracing option set', {}],
+      ])("doesn't add the BrowserTracing integration if tracing is disabled via %s", (_, tracingOptions) => {
+        init({
+          dsn: 'https://public@dsn.ingest.sentry.io/1337',
+          ...tracingOptions,
+        });
+
+        const integrationsToInit = svelteInit.mock.calls[0][0].integrations;
+        const browserTracing = (getCurrentHub().getClient() as BrowserClient)?.getIntegrationById('BrowserTracing');
+
+        expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
+        expect(browserTracing).toBeUndefined();
+      });
+
+      it("doesn't add the BrowserTracing integration if `__SENTRY_TRACING__` is set to false", () => {
+        // 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.
+
+        globalThis.__SENTRY_TRACING__ = false;
+
+        init({
+          dsn: 'https://public@dsn.ingest.sentry.io/1337',
+          enableTracing: true,
+        });
+
+        const integrationsToInit = svelteInit.mock.calls[0][0].integrations;
+        const browserTracing = (getCurrentHub().getClient() as BrowserClient)?.getIntegrationById('BrowserTracing');
+
+        expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
+        expect(browserTracing).toBeUndefined();
+
+        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', () => {
+        init({
+          dsn: 'https://public@dsn.ingest.sentry.io/1337',
+          integrations: [new BrowserTracing({ tracePropagationTargets: ['myDomain.com'] })],
+          enableTracing: true,
+        });
+
+        const integrationsToInit = svelteInit.mock.calls[0][0].integrations;
+        const browserTracing = (getCurrentHub().getClient() as BrowserClient)?.getIntegrationById('BrowserTracing');
+
+        expect(integrationsToInit).toContainEqual(expect.objectContaining({ name: 'BrowserTracing' }));
+        expect(browserTracing).toBeDefined();
+        expect((browserTracing as BrowserTracing).options.tracePropagationTargets).toEqual(['myDomain.com']);
+      });
+    });
   });
 });
diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts
@@ -27,3 +27,4 @@ export * from './clientreport';
 export * from './ratelimit';
 export * from './baggage';
 export * from './url';
+export * from './userIntegrations';
diff --git a/packages/utils/src/userIntegrations.ts b/packages/utils/src/userIntegrations.ts
@@ -2,11 +2,6 @@ import type { Integration } from '@sentry/types';
 
 export type UserIntegrationsFunction = (integrations: Integration[]) => Integration[];
 export type UserIntegrations = Integration[] | UserIntegrationsFunction;
-
-type ForcedIntegrationOptions = {
-  [keyPath: string]: unknown;
-};
-
 export type IntegrationWithExclusionOption = Integration & {
   /**
    * Allow the user to exclude this integration by not returning it from a function provided as the `integrations` option
@@ -16,6 +11,10 @@ export type IntegrationWithExclusionOption = Integration & {
   allowExclusionByUser?: boolean;
 };
 
+type ForcedIntegrationOptions = {
+  [keyPath: string]: unknown;
+};
+
 /**
  * Recursively traverses an object to update an existing nested key.
  * Note: The provided key path must include existing properties,
diff --git a/packages/utils/test/userIntegrations.test.ts b/packages/utils/test/userIntegrations.test.ts
@@ -1,8 +1,5 @@
-import type {
-  IntegrationWithExclusionOption as Integration,
-  UserIntegrations,
-} from '../../src/common/userIntegrations';
-import { addOrUpdateIntegration } from '../../src/common/userIntegrations';
+import type { IntegrationWithExclusionOption as Integration, UserIntegrations } from '../src/userIntegrations';
+import { addOrUpdateIntegration } from '../src/userIntegrations';
 
 type MockIntegrationOptions = {
   name: string;
