Rewrite plugin to add auto-instrument option

andreiborza · s1gr1d · commit 434cbf051aa0 · 2024-12-30T11:03:17.000+01:00
diff --git a/dev-packages/e2e-tests/test-applications/solidstart-spa/app.config.ts b/dev-packages/e2e-tests/test-applications/solidstart-spa/app.config.ts
@@ -1,9 +1,9 @@
-import { sentrySolidStartVite } from '@sentry/solidstart';
+import { withSentry } from '@sentry/solidstart';
 import { defineConfig } from '@solidjs/start/config';
 
-export default defineConfig({
-  ssr: false,
-  vite: {
-    plugins: [sentrySolidStartVite()],
-  },
-});
+export default defineConfig(
+  withSentry({
+    ssr: false,
+    middleware: './src/middleware.ts',
+  }),
+);
diff --git a/dev-packages/e2e-tests/test-applications/solidstart-spa/package.json b/dev-packages/e2e-tests/test-applications/solidstart-spa/package.json
@@ -3,9 +3,8 @@
   "version": "0.0.0",
   "scripts": {
     "clean": "pnpx rimraf node_modules pnpm-lock.yaml .vinxi .output",
-    "dev": "NODE_OPTIONS='--import ./src/instrument.server.mjs' vinxi dev",
-    "build": "vinxi build && sh ./post_build.sh",
-    "preview": "HOST=localhost PORT=3030 NODE_OPTIONS='--import ./src/instrument.server.mjs' vinxi start",
+    "build": "vinxi build && sh post_build.sh",
+    "preview": "HOST=localhost PORT=3030 vinxi start",
     "test:prod": "TEST_ENV=production playwright test",
     "test:build": "pnpm install && pnpm build",
     "test:assert": "pnpm test:prod"
diff --git a/dev-packages/e2e-tests/test-applications/solidstart-spa/src/instrument.server.ts b/dev-packages/e2e-tests/test-applications/solidstart-spa/src/instrument.server.ts
diff --git a/dev-packages/e2e-tests/test-applications/solidstart-spa/src/middleware.ts b/dev-packages/e2e-tests/test-applications/solidstart-spa/src/middleware.ts
@@ -0,0 +1,6 @@
+import { sentryBeforeResponseMiddleware } from '@sentry/solidstart';
+import { createMiddleware } from '@solidjs/start/middleware';
+
+export default createMiddleware({
+  onBeforeResponse: [sentryBeforeResponseMiddleware()],
+});
diff --git a/dev-packages/e2e-tests/test-applications/solidstart/app.config.ts b/dev-packages/e2e-tests/test-applications/solidstart/app.config.ts
@@ -2,22 +2,7 @@ import { withSentry } from '@sentry/solidstart';
 import { defineConfig } from '@solidjs/start/config';
 
 export default defineConfig(
-  withSentry(
-    {},
-    {
-      // Typically we want to default to ./src/instrument.sever.ts
-      // `withSentry` would then build and copy the file over to
-      // the .output folder, but since we can't use the production
-      // server for our e2e tests, we have to delete the build folders
-      // prior to using the dev server for our tests. Which also gets
-      // rid of the instrument.server.mjs file that we need to --import.
-      // Therefore, we specify the .mjs file here and to ensure
-      // `withSentry` gets its file to build and we continue to reference
-      // the file from the `src` folder for --import without needing to
-      // transpile before.
-      // This can be removed once we get the production server to work
-      // with our e2e tests.
-      instrumentation: './src/instrument.server.mjs',
-    },
-  ),
+  withSentry({
+    middleware: './src/middleware.ts',
+  }),
 );
diff --git a/dev-packages/e2e-tests/test-applications/solidstart/package.json b/dev-packages/e2e-tests/test-applications/solidstart/package.json
@@ -3,9 +3,8 @@
   "version": "0.0.0",
   "scripts": {
     "clean": "pnpx rimraf node_modules pnpm-lock.yaml .vinxi .output",
-    "dev": "NODE_OPTIONS='--import ./src/instrument.server.mjs' vinxi dev",
-    "build": "vinxi build && sh ./post_build.sh",
-    "preview": "HOST=localhost PORT=3030 NODE_OPTIONS='--import ./src/instrument.server.mjs' vinxi start",
+    "build": "vinxi build && sh post_build.sh",
+    "preview": "HOST=localhost PORT=3030 vinxi start",
     "test:prod": "TEST_ENV=production playwright test",
     "test:build": "pnpm install && pnpm build",
     "test:assert": "pnpm test:prod"
diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/instrument.server.ts b/dev-packages/e2e-tests/test-applications/solidstart/src/instrument.server.ts
diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/middleware.ts b/dev-packages/e2e-tests/test-applications/solidstart/src/middleware.ts
@@ -0,0 +1,6 @@
+import { sentryBeforeResponseMiddleware } from '@sentry/solidstart';
+import { createMiddleware } from '@solidjs/start/middleware';
+
+export default createMiddleware({
+  onBeforeResponse: [sentryBeforeResponseMiddleware()],
+});
diff --git a/packages/solidstart/src/config/addInstrumentation.ts b/packages/solidstart/src/config/addInstrumentation.ts
@@ -1,7 +1,16 @@
 import * as fs from 'fs';
 import * as path from 'path';
-import { consoleSandbox } from '@sentry/utils';
-import type { Nitro } from './types';
+import { consoleSandbox, flatten } from '@sentry/utils';
+import type { Nitro } from 'nitropack';
+import type { InputPluginOption } from 'rollup';
+import type { RollupConfig } from './types';
+import {
+  QUERY_END_INDICATOR,
+  SENTRY_FUNCTIONS_REEXPORT,
+  SENTRY_WRAPPED_ENTRY,
+  constructFunctionReExport,
+  removeSentryQueryFromPath,
+} from './utils';
 
 // Nitro presets for hosts that only host static files
 export const staticHostPresets = ['github_pages'];
@@ -42,54 +51,81 @@ export async function addInstrumentationFileToBuild(nitro: Nitro): Promise<void>
 }
 
 /**
- * Adds an `instrument.server.mjs` import to the top of the server entry file.
  *
- * This is meant as an escape hatch and should only be used in environments where
- * it's not possible to `--import` the file instead as it comes with a limited
- * tracing experience, only collecting http traces.
  */
-export async function experimental_addInstrumentationFileTopLevelImportToServerEntry(
-  serverDir: string,
-  preset: string,
-): Promise<void> {
+export async function addAutoInstrumentation(nitro: Nitro, config: RollupConfig): Promise<void> {
   // Static file hosts have no server component so there's nothing to do
-  if (staticHostPresets.includes(preset)) {
+  if (staticHostPresets.includes(nitro.options.preset)) {
     return;
   }
 
-  const instrumentationFile = path.resolve(serverDir, 'instrument.server.mjs');
-  const serverEntryFileName = serverFilePresets.includes(preset) ? 'server.mjs' : 'index.mjs';
-  const serverEntryFile = path.resolve(serverDir, serverEntryFileName);
+  const buildDir = nitro.options.buildDir;
+  const serverInstrumentationPath = path.resolve(buildDir, 'build', 'ssr', 'instrument.server.js');
 
-  try {
-    await fs.promises.access(instrumentationFile, fs.constants.F_OK);
-  } catch (error) {
-    consoleSandbox(() => {
-      // eslint-disable-next-line no-console
-      console.warn(
-        `[Sentry SolidStart withSentry] Failed to add \`${instrumentationFile}\` as top level import to \`${serverEntryFile}\`.`,
-        error,
-      );
-    });
-    return;
-  }
+  config.plugins.push({
+    name: 'sentry-solidstart-auto-instrument',
+    async resolveId(source, importer, options) {
+      if (source.includes('instrument.server.js')) {
+        return { id: source, moduleSideEffects: true };
+      }
 
-  try {
-    const content = await fs.promises.readFile(serverEntryFile, 'utf-8');
-    const updatedContent = `import './instrument.server.mjs';\n${content}`;
-    await fs.promises.writeFile(serverEntryFile, updatedContent);
+      if (source === 'import-in-the-middle/hook.mjs') {
+        // We are importing "import-in-the-middle" in the returned code of the `load()` function below
+        // By setting `moduleSideEffects` to `true`, the import is added to the bundle, although nothing is imported from it
+        // By importing "import-in-the-middle/hook.mjs", we can make sure this file is included, as not all node builders are including files imported with `module.register()`.
+        // Prevents the error "Failed to register ESM hook Error: Cannot find module 'import-in-the-middle/hook.mjs'"
+        return { id: source, moduleSideEffects: true, external: true };
+      }
 
-    consoleSandbox(() => {
-      // eslint-disable-next-line no-console
-      console.log(
-        `[Sentry SolidStart withSentry] Added \`${instrumentationFile}\` as top level import to \`${serverEntryFile}\`.`,
-      );
-    });
-  } catch (error) {
-    // eslint-disable-next-line no-console
-    console.warn(
-      `[Sentry SolidStart withSentry] An error occurred when trying to add \`${instrumentationFile}\` as top level import to \`${serverEntryFile}\`.`,
-      error,
-    );
-  }
+      if (options.isEntry && source.includes('.mjs') && !source.includes(`.mjs${SENTRY_WRAPPED_ENTRY}`)) {
+        const resolution = await this.resolve(source, importer, options);
+
+        // If it cannot be resolved or is external, just return it so that Rollup can display an error
+        if (!resolution || resolution?.external) return resolution;
+
+        const moduleInfo = await this.load(resolution);
+
+        moduleInfo.moduleSideEffects = true;
+
+        // The key `.` in `exportedBindings` refer to the exports within the file
+        const functionsToExport = flatten(Object.values(moduleInfo.exportedBindings || {})).filter(functionName =>
+          ['default', 'handler', 'server'].includes(functionName),
+        );
+
+        // The enclosing `if` already checks for the suffix in `source`, but a check in `resolution.id` is needed as well to prevent multiple attachment of the suffix
+        return resolution.id.includes(`.mjs${SENTRY_WRAPPED_ENTRY}`)
+          ? resolution.id
+          : resolution.id
+              // Concatenates the query params to mark the file (also attaches names of re-exports - this is needed for serverless functions to re-export the handler)
+              .concat(SENTRY_WRAPPED_ENTRY)
+              .concat(functionsToExport?.length ? SENTRY_FUNCTIONS_REEXPORT.concat(functionsToExport.join(',')) : '')
+              .concat(QUERY_END_INDICATOR);
+      }
+
+      return null;
+    },
+    load(id: string) {
+      if (id.includes(`.mjs${SENTRY_WRAPPED_ENTRY}`)) {
+        const entryId = removeSentryQueryFromPath(id);
+
+        // Mostly useful for serverless `handler` functions
+        const reExportedFunctions = id.includes(SENTRY_FUNCTIONS_REEXPORT)
+          ? constructFunctionReExport(id, entryId)
+          : '';
+
+        return [
+          // Regular `import` of the Sentry config
+          `import ${JSON.stringify(serverInstrumentationPath)};`,
+          // Dynamic `import()` for the previous, actual entry point.
+          // `import()` can be used for any code that should be run after the hooks are registered (https://nodejs.org/api/module.html#enabling)
+          `import(${JSON.stringify(entryId)});`,
+          // By importing "import-in-the-middle/hook.mjs", we can make sure this file wil be included, as not all node builders are including files imported with `module.register()`.
+          "import 'import-in-the-middle/hook.mjs';",
+          `${reExportedFunctions}`,
+        ].join('\n');
+      }
+
+      return null;
+    },
+  } satisfies InputPluginOption);
 }
diff --git a/packages/solidstart/src/config/index.ts b/packages/solidstart/src/config/index.ts
@@ -1,2 +1 @@
 export * from './withSentry';
-export type { Nitro, SentrySolidStartConfigOptions } from './types';
diff --git a/packages/solidstart/src/config/types.ts b/packages/solidstart/src/config/types.ts
@@ -1,14 +1,9 @@
 import type { defineConfig } from '@solidjs/start/config';
-// Types to avoid pulling in extra dependencies
-// These are non-exhaustive
-export type Nitro = {
-  options: {
-    buildDir: string;
-    output: {
-      serverDir: string;
-    };
-    preset: string;
-  };
+import type { Nitro } from 'nitropack';
+
+// Nitro does not export this type
+export type RollupConfig = {
+  plugins: unknown[];
 };
 
 export type SolidStartInlineConfig = Parameters<typeof defineConfig>[0];
@@ -19,17 +14,3 @@ export type SolidStartInlineServerConfig = {
     'rollup:before'?: (nitro: Nitro) => unknown;
   };
 };
-
-export type SentrySolidStartConfigOptions = {
-  /**
-   * Enabling basic server tracing can be used for environments where modifying the node option `--import` is not possible.
-   * However, enabling this option only supports limited tracing instrumentation. Only http traces will be collected (but no database-specific traces etc.).
-   *
-   * If this option is `true`, the Sentry SDK will import the instrumentation.server.ts|js file at the top of the server entry file to load the SDK on the server.
-   *
-   * **DO NOT** enable this option if you've already added the node option `--import` in your node start script. This would initialize Sentry twice on the server-side and leads to unexpected issues.
-   *
-   * @default false
-   */
-  experimental_basicServerTracing?: boolean;
-};
diff --git a/packages/solidstart/src/config/utils.ts b/packages/solidstart/src/config/utils.ts
@@ -0,0 +1,56 @@
+export const SENTRY_WRAPPED_ENTRY = '?sentry-query-wrapped-entry';
+export const SENTRY_FUNCTIONS_REEXPORT = '?sentry-query-functions-reexport=';
+export const QUERY_END_INDICATOR = 'SENTRY-QUERY-END';
+
+/**
+ * Strips the Sentry query part from a path.
+ * Example: example/path?sentry-query-wrapped-entry?sentry-query-functions-reexport=foo,SENTRY-QUERY-END -> /example/path
+ *
+ * Only exported for testing.
+ */
+export function removeSentryQueryFromPath(url: string): string {
+  // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor
+  const regex = new RegExp(`\\${SENTRY_WRAPPED_ENTRY}.*?\\${QUERY_END_INDICATOR}`);
+  return url.replace(regex, '');
+}
+
+/**
+ * Extracts and sanitizes function re-export query parameters from a query string.
+ * If it is a default export, it is not considered for re-exporting. This function is mostly relevant for re-exporting
+ * serverless `handler` functions.
+ *
+ * Only exported for testing.
+ */
+export function extractFunctionReexportQueryParameters(query: string): string[] {
+  // Regex matches the comma-separated params between the functions query
+  // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor
+  const regex = new RegExp(`\\${SENTRY_FUNCTIONS_REEXPORT}(.*?)\\${QUERY_END_INDICATOR}`);
+  const match = query.match(regex);
+
+  return match && match[1]
+    ? match[1]
+        .split(',')
+        .filter(param => param !== '')
+        // Sanitize, as code could be injected with another rollup plugin
+        .map((str: string) => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'))
+    : [];
+}
+
+/**
+ * Constructs a code snippet with function reexports (can be used in Rollup plugins)
+ */
+export function constructFunctionReExport(pathWithQuery: string, entryId: string): string {
+  const functionNames = extractFunctionReexportQueryParameters(pathWithQuery);
+
+  return functionNames.reduce(
+    (functionsCode, currFunctionName) =>
+      functionsCode.concat(
+        'async function reExport(...args) {\n' +
+          `  const res = await import(${JSON.stringify(entryId)});\n` +
+          `  return res.${currFunctionName}.call(this, ...args);\n` +
+          '}\n' +
+          `export { reExport as ${currFunctionName} };\n`,
+      ),
+    '',
+  );
+}
diff --git a/packages/solidstart/src/config/withSentry.ts b/packages/solidstart/src/config/withSentry.ts
@@ -1,10 +1,12 @@
+import type { Nitro } from 'nitropack';
 import { addSentryPluginToVite } from '../vite';
 import type { SentrySolidStartPluginOptions } from '../vite/types';
-import {
-  addInstrumentationFileToBuild,
-  experimental_addInstrumentationFileTopLevelImportToServerEntry,
-} from './addInstrumentation';
-import type { Nitro, SolidStartInlineConfig, SolidStartInlineServerConfig } from './types';
+import { addAutoInstrumentation, addInstrumentationFileToBuild } from './addInstrumentation';
+import type { RollupConfig, SolidStartInlineConfig, SolidStartInlineServerConfig } from './types';
+
+const defaultSentrySolidStartPluginOptions: SentrySolidStartPluginOptions = {
+  autoInstrument: true,
+};
 
 /**
  * Modifies the passed in Solid Start configuration with build-time enhancements such as
@@ -17,7 +19,7 @@ import type { Nitro, SolidStartInlineConfig, SolidStartInlineServerConfig } from
  */
 export const withSentry = (
   solidStartConfig: SolidStartInlineConfig = {},
-  sentrySolidStartPluginOptions: SentrySolidStartPluginOptions = {},
+  sentrySolidStartPluginOptions: SentrySolidStartPluginOptions = defaultSentrySolidStartPluginOptions,
 ): SolidStartInlineConfig => {
   const server = (solidStartConfig.server || {}) as SolidStartInlineServerConfig;
   const hooks = server.hooks || {};
@@ -26,31 +28,19 @@ export const withSentry = (
       ? (...args: unknown[]) => addSentryPluginToVite(solidStartConfig.vite(...args), sentrySolidStartPluginOptions)
       : addSentryPluginToVite(solidStartConfig.vite, sentrySolidStartPluginOptions);
 
-  let serverDir: string;
-  let buildPreset: string;
-
   return {
     ...solidStartConfig,
     vite,
     server: {
       ...server,
       hooks: {
         ...hooks,
-        async close() {
-          if (sentrySolidStartPluginOptions.experimental_basicServerTracing) {
-            await experimental_addInstrumentationFileTopLevelImportToServerEntry(serverDir, buildPreset);
-          }
-
-          // Run user provided hook
-          if (hooks.close) {
-            hooks.close();
+        async 'rollup:before'(nitro: Nitro, config: RollupConfig) {
+          if (sentrySolidStartPluginOptions.autoInstrument) {
+            await addAutoInstrumentation(nitro, config);
+          } else {
+            await addInstrumentationFileToBuild(nitro);
           }
-        },
-        async 'rollup:before'(nitro: Nitro) {
-          serverDir = nitro.options.output.serverDir;
-          buildPreset = nitro.options.preset;
-
-          await addInstrumentationFileToBuild(nitro);
 
           // Run user provided hook
           if (hooks['rollup:before']) {
diff --git a/packages/solidstart/src/vite/types.ts b/packages/solidstart/src/vite/types.ts
diff --git a/packages/solidstart/test/config/addInstrumentation.test.ts b/packages/solidstart/test/config/addInstrumentation.test.ts
diff --git a/packages/solidstart/test/config/withSentry.test.ts b/packages/solidstart/test/config/withSentry.test.ts

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1 @@`
`1`	`1`	`export * from './withSentry';`
`2`		`-export type { Nitro, SentrySolidStartConfigOptions } from './types';`