don't auto-wrap load functions if kit-tracing is enabled

Author: Lms24

packages/sveltekit/src/vite/autoInstrument.ts

@@ -27,6 +27,7 @@ export type AutoInstrumentSelection = {
 
 type AutoInstrumentPluginOptions = AutoInstrumentSelection & {
   debug: boolean;
+  onlyInstrumentClient: boolean;
 };
 
 /**
@@ -41,12 +42,26 @@ type AutoInstrumentPluginOptions = AutoInstrumentSelection & {
 export function makeAutoInstrumentationPlugin(options: AutoInstrumentPluginOptions): Plugin {
   const { load: wrapLoadEnabled, serverLoad: wrapServerLoadEnabled, debug } = options;
 
+  let isServerBuild: boolean | undefined = undefined;
+
   return {
     name: 'sentry-auto-instrumentation',
     // This plugin needs to run as early as possible, before the SvelteKit plugin virtualizes all paths and ids
     enforce: 'pre',
 
+    configResolved: config => {
+      // The SvelteKit plugins trigger additional builds within the main (SSR) build.
+      // We just need a mechanism to upload source maps only once.
+      // `config.build.ssr` is `true` for that first build and `false` in the other ones.
+      // Hence we can use it as a switch to upload source maps only once in main build.
+      isServerBuild = !!config.build.ssr;
+    },
+
     async load(id) {
+      if (options.onlyInstrumentClient && isServerBuild) {
+        return null;
+      }
+
       const applyUniversalLoadWrapper =
         wrapLoadEnabled &&
         /^\+(page|layout)\.(js|ts|mjs|mts)$/.test(path.basename(id)) &&
@@ -58,6 +73,12 @@ export function makeAutoInstrumentationPlugin(options: AutoInstrumentPluginOptio
         return getWrapperCode('wrapLoadWithSentry', `${id}${WRAPPED_MODULE_SUFFIX}`);
       }
 
+      if (options.onlyInstrumentClient) {
+        // Now that we've checked universal files, we can early return and avoid further
+        // regexp checks below for server-only files, in case `onlyInstrumentClient` is `true`.
+        return null;
+      }
+
       const applyServerLoadWrapper =
         wrapServerLoadEnabled &&
         /^\+(page|layout)\.server\.(js|ts|mjs|mts)$/.test(path.basename(id)) &&

packages/sveltekit/src/vite/sentryVitePlugins.ts

@@ -28,9 +28,13 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {}
     adapter: options.adapter || (await detectAdapter(options.debug)),
   };
 
+  const svelteConfig = await loadSvelteConfig();
+
   const sentryPlugins: Plugin[] = [];
 
   if (mergedOptions.autoInstrument) {
+    const kitTracingEnabled = !!svelteConfig.kit?.experimental?.tracing?.server;
+
     const pluginOptions: AutoInstrumentSelection = {
       load: true,
       serverLoad: true,
@@ -41,14 +45,14 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {}
       makeAutoInstrumentationPlugin({
         ...pluginOptions,
         debug: options.debug || false,
+        // if kit-internal tracing is enabled, we only want to instrument client-side code.
+        onlyInstrumentClient: kitTracingEnabled,
       }),
     );
   }
 
   const sentryVitePluginsOptions = generateVitePluginOptions(mergedOptions);
 
-  const svelteConfig = await loadSvelteConfig();
-
   if (mergedOptions.injectGlobalValues) {
     sentryPlugins.push(await makeGlobalValuesInjectionPlugin(svelteConfig, mergedOptions));
   }

packages/sveltekit/test/vite/autoInstrument.test.ts

@@ -41,7 +41,12 @@ describe('makeAutoInstrumentationPlugin()', () => {
   });
 
   it('returns the auto instrumentation plugin', async () => {
-    const plugin = makeAutoInstrumentationPlugin({ debug: true, load: true, serverLoad: true });
+    const plugin = makeAutoInstrumentationPlugin({
+      debug: true,
+      load: true,
+      serverLoad: true,
+      onlyInstrumentClient: false,
+    });
     expect(plugin.name).toEqual('sentry-auto-instrumentation');
     expect(plugin.enforce).toEqual('pre');
     expect(plugin.load).toEqual(expect.any(Function));
@@ -58,7 +63,12 @@ describe('makeAutoInstrumentationPlugin()', () => {
     'path/to/+layout.mjs',
   ])('transform %s files', (path: string) => {
     it('wraps universal load if `load` option is `true`', async () => {
-      const plugin = makeAutoInstrumentationPlugin({ debug: false, load: true, serverLoad: true });
+      const plugin = makeAutoInstrumentationPlugin({
+        debug: false,
+        load: true,
+        serverLoad: true,
+        onlyInstrumentClient: false,
+      });
       // @ts-expect-error this exists
       const loadResult = await plugin.load(path);
       expect(loadResult).toEqual(
@@ -74,6 +84,7 @@ describe('makeAutoInstrumentationPlugin()', () => {
         debug: false,
         load: false,
         serverLoad: false,
+        onlyInstrumentClient: false,
       });
       // @ts-expect-error this exists
       const loadResult = await plugin.load(path);
@@ -92,7 +103,12 @@ describe('makeAutoInstrumentationPlugin()', () => {
     'path/to/+layout.server.mjs',
   ])('transform %s files', (path: string) => {
     it('wraps universal load if `load` option is `true`', async () => {
-      const plugin = makeAutoInstrumentationPlugin({ debug: false, load: false, serverLoad: true });
+      const plugin = makeAutoInstrumentationPlugin({
+        debug: false,
+        load: false,
+        serverLoad: true,
+        onlyInstrumentClient: false,
+      });
       // @ts-expect-error this exists
       const loadResult = await plugin.load(path);
       expect(loadResult).toEqual(
@@ -108,12 +124,101 @@ describe('makeAutoInstrumentationPlugin()', () => {
         debug: false,
         load: false,
         serverLoad: false,
+        onlyInstrumentClient: false,
       });
       // @ts-expect-error this exists
       const loadResult = await plugin.load(path);
       expect(loadResult).toEqual(null);
     });
   });
+
+  describe('when `onlyInstrumentClient` is `true`', () => {
+    it.each([
+      // server-only files
+      'path/to/+page.server.ts',
+      'path/to/+layout.server.js',
+      // universal files
+      'path/to/+page.mts',
+      'path/to/+layout.mjs',
+    ])("doesn't wrap code in SSR build in %s", async (path: string) => {
+      const plugin = makeAutoInstrumentationPlugin({
+        debug: false,
+        load: true,
+        serverLoad: true,
+        onlyInstrumentClient: true,
+      });
+
+      // @ts-expect-error this exists and is callable
+      plugin.configResolved({
+        build: {
+          ssr: true,
+        },
+      });
+
+      // @ts-expect-error this exists
+      const loadResult = await plugin.load(path);
+
+      expect(loadResult).toEqual(null);
+    });
+
+    it.each(['path/to/+page.ts', 'path/to/+layout.js'])(
+      'wraps client-side code in universal files in %s',
+      async (path: string) => {
+        const plugin = makeAutoInstrumentationPlugin({
+          debug: false,
+          load: true,
+          serverLoad: true,
+          onlyInstrumentClient: true,
+        });
+
+        // @ts-expect-error this exists and is callable
+        plugin.configResolved({
+          build: {
+            ssr: false,
+          },
+        });
+
+        // @ts-expect-error this exists and is callable
+        const loadResult = await plugin.load(path);
+
+        expect(loadResult).toBe(
+          'import { wrapLoadWithSentry } from "@sentry/sveltekit";' +
+            `import * as userModule from "${path}?sentry-auto-wrap";` +
+            'export const load = userModule.load ? wrapLoadWithSentry(userModule.load) : undefined;' +
+            `export * from "${path}?sentry-auto-wrap";`,
+        );
+      },
+    );
+
+    /**
+     * This is a bit of a constructed case because in a client build, server-only files
+     * shouldn't even be passed into the load hook. But just to be extra careful, let's
+     * make sure we don't wrap server-only files in a client build.
+     */
+    it.each(['path/to/+page.server.ts', 'path/to/+layout.server.js'])(
+      "doesn't wrap client-side code in server-only files in %s",
+      async (path: string) => {
+        const plugin = makeAutoInstrumentationPlugin({
+          debug: false,
+          load: true,
+          serverLoad: true,
+          onlyInstrumentClient: true,
+        });
+
+        // @ts-expect-error this exists and is callable
+        plugin.configResolved({
+          build: {
+            ssr: false,
+          },
+        });
+
+        // @ts-expect-error this exists and is callable
+        const loadResult = await plugin.load(path);
+
+        expect(loadResult).toBe(null);
+      },
+    );
+  });
 });
 
 describe('canWrapLoad', () => {

packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts

@@ -200,6 +200,7 @@ describe('sentrySvelteKit()', () => {
       debug: true,
       load: true,
       serverLoad: false,
+      onlyInstrumentClient: false,
     });
   });
 });