feat(remix)!: Remove autoInstrumentRemix option

Author: Luca Forstner

dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/instrument.mjs

@@ -6,6 +6,5 @@ Sentry.init({
   environment: 'qa', // dynamic sampling bias to keep transactions
   dsn: process.env.E2E_TEST_DSN,
   tunnel: 'http://localhost:3031/', // proxy server
-  autoInstrumentRemix: true, // auto instrument Remix
   integrations: [Sentry.nativeNodeFetchIntegration()],
 });

dev-packages/e2e-tests/test-applications/create-remix-app-express/instrument.mjs

@@ -7,7 +7,6 @@ Sentry.init({
   dsn: process.env.E2E_TEST_DSN,
   tunnel: 'http://localhost:3031/', // proxy server
   sendDefaultPii: true, // Testing the FormData
-  autoInstrumentRemix: true, // auto instrument Remix
   captureActionFormDataKeys: {
     file: true,
     text: true,

dev-packages/e2e-tests/test-applications/create-remix-app-v2/instrument.server.cjs

@@ -5,5 +5,4 @@ Sentry.init({
   environment: 'qa', // dynamic sampling bias to keep transactions
   dsn: process.env.E2E_TEST_DSN,
   tunnel: 'http://localhost:3031/', // proxy server
-  autoInstrumentRemix: true, // auto instrument Remix
 });

packages/remix/src/index.server.ts

@@ -153,8 +153,7 @@ export function getRemixDefaultIntegrations(options: RemixOptions): Integration[
   return [
     ...getDefaultNodeIntegrations(options as NodeOptions).filter(integration => integration.name !== 'Http'),
     httpIntegration(),
-    // eslint-disable-next-line deprecation/deprecation
-    options.autoInstrumentRemix ? remixIntegration() : undefined,
+    remixIntegration(),
   ].filter(int => int) as Integration[];
 }
 
@@ -186,7 +185,7 @@ export function init(options: RemixOptions): NodeClient | undefined {
 
   const client = nodeInit(options as NodeOptions);
 
-  instrumentServer(options);
+  instrumentServer();
 
   return client;
 }

packages/remix/src/utils/instrumentServer.ts

@@ -1,42 +1,28 @@
-/* eslint-disable max-lines */
-import type { RequestEventData, Span, TransactionSource, WrappedFunction } from '@sentry/core';
+import type { RequestEventData } from '@sentry/core';
 import {
-  SEMANTIC_ATTRIBUTE_SENTRY_OP,
-  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
-  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
   continueTrace,
   fill,
-  getActiveSpan,
   getClient,
-  getRootSpan,
   getTraceData,
   hasTracingEnabled,
   isNodeEnv,
   loadModule,
   logger,
-  setHttpStatus,
-  spanToJSON,
-  startSpan,
   winterCGRequestToRequestData,
   withIsolationScope,
 } from '@sentry/core';
 import { DEBUG_BUILD } from './debug-build';
-import { captureRemixServerException, errorHandleDataFunction, errorHandleDocumentRequestFunction } from './errors';
-import type { RemixOptions } from './remixOptions';
-import { createRoutes, getTransactionName } from './utils';
+import { captureRemixServerException } from './errors';
 import { extractData, isDeferredData, isResponse, isRouteErrorResponse, json } from './vendor/response';
 import type {
   AppData,
   AppLoadContext,
   CreateRequestHandlerFunction,
   DataFunction,
   DataFunctionArgs,
-  EntryContext,
-  HandleDocumentRequestFunction,
   RemixRequest,
   RequestHandler,
   ServerBuild,
-  ServerRoute,
   ServerRouteManifest,
 } from './vendor/types';
 
@@ -94,93 +80,6 @@ export function wrapHandleErrorWithSentry(
   };
 }
 
-function makeWrappedDocumentRequestFunction(autoInstrumentRemix?: boolean) {
-  return function (origDocumentRequestFunction: HandleDocumentRequestFunction): HandleDocumentRequestFunction {
-    return async function (
-      this: unknown,
-      request: Request,
-      responseStatusCode: number,
-      responseHeaders: Headers,
-      context: EntryContext,
-      loadContext?: Record<string, unknown>,
-    ): Promise<Response> {
-      const documentRequestContext = {
-        request,
-        responseStatusCode,
-        responseHeaders,
-        context,
-        loadContext,
-      };
-
-      if (!autoInstrumentRemix) {
-        const activeSpan = getActiveSpan();
-        const rootSpan = activeSpan && getRootSpan(activeSpan);
-
-        const name = rootSpan ? spanToJSON(rootSpan).description : undefined;
-
-        return startSpan(
-          {
-            // If we don't have a root span, `onlyIfParent` will lead to the span not being created anyhow
-            // So we don't need to care too much about the fallback name, it's just for typing purposes....
-            name: name || '<unknown>',
-            onlyIfParent: true,
-            attributes: {
-              method: request.method,
-              url: request.url,
-              [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.remix',
-              [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.remix.document_request',
-            },
-          },
-          () => {
-            return errorHandleDocumentRequestFunction.call(this, origDocumentRequestFunction, documentRequestContext);
-          },
-        );
-      } else {
-        return errorHandleDocumentRequestFunction.call(this, origDocumentRequestFunction, documentRequestContext);
-      }
-    };
-  };
-}
-
-function makeWrappedDataFunction(
-  origFn: DataFunction,
-  id: string,
-  name: 'action' | 'loader',
-  autoInstrumentRemix?: boolean,
-): DataFunction {
-  return async function (this: unknown, args: DataFunctionArgs): Promise<Response | AppData> {
-    if (!autoInstrumentRemix) {
-      return startSpan(
-        {
-          op: `function.remix.${name}`,
-          name: id,
-          attributes: {
-            [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.remix',
-            name,
-          },
-        },
-        (span: Span) => {
-          return errorHandleDataFunction.call(this, origFn, name, args, span);
-        },
-      );
-    } else {
-      return errorHandleDataFunction.call(this, origFn, name, args);
-    }
-  };
-}
-
-const makeWrappedAction =
-  (id: string, autoInstrumentRemix?: boolean) =>
-  (origAction: DataFunction): DataFunction => {
-    return makeWrappedDataFunction(origAction, id, 'action', autoInstrumentRemix);
-  };
-
-const makeWrappedLoader =
-  (id: string, autoInstrumentRemix?: boolean) =>
-  (origLoader: DataFunction): DataFunction => {
-    return makeWrappedDataFunction(origLoader, id, 'loader', autoInstrumentRemix);
-  };
-
 function getTraceAndBaggage(): {
   sentryTrace?: string;
   sentryBaggage?: string;
@@ -241,33 +140,14 @@ function makeWrappedRootLoader() {
   };
 }
 
-function wrapRequestHandler(
-  origRequestHandler: RequestHandler,
-  build: ServerBuild | (() => ServerBuild | Promise<ServerBuild>),
-  autoInstrumentRemix: boolean,
-): RequestHandler {
-  let resolvedBuild: ServerBuild;
-  let routes: ServerRoute[];
-  let name: string;
-  let source: TransactionSource;
-
+function wrapRequestHandler(origRequestHandler: RequestHandler): RequestHandler {
   return async function (this: unknown, request: RemixRequest, loadContext?: AppLoadContext): Promise<Response> {
     const upperCaseMethod = request.method.toUpperCase();
     // We don't want to wrap OPTIONS and HEAD requests
     if (upperCaseMethod === 'OPTIONS' || upperCaseMethod === 'HEAD') {
       return origRequestHandler.call(this, request, loadContext);
     }
 
-    if (!autoInstrumentRemix) {
-      if (typeof build === 'function') {
-        resolvedBuild = await build();
-      } else {
-        resolvedBuild = build;
-      }
-
-      routes = createRoutes(resolvedBuild.routes);
-    }
-
     return withIsolationScope(async isolationScope => {
       const options = getClient()?.getOptions();
 
@@ -279,13 +159,6 @@ function wrapRequestHandler(
         DEBUG_BUILD && logger.warn('Failed to normalize Remix request');
       }
 
-      if (!autoInstrumentRemix) {
-        const url = new URL(request.url);
-        [name, source] = getTransactionName(routes, url);
-
-        isolationScope.setTransactionName(name);
-      }
-
       isolationScope.setSDKProcessingMetadata({ normalizedRequest });
 
       if (!options || !hasTracingEnabled(options)) {
@@ -298,62 +171,20 @@ function wrapRequestHandler(
           baggage: request.headers.get('baggage') || '',
         },
         async () => {
-          if (!autoInstrumentRemix) {
-            return startSpan(
-              {
-                name,
-                attributes: {
-                  [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.remix',
-                  [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
-                  [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
-                  method: request.method,
-                },
-              },
-              async span => {
-                const res = (await origRequestHandler.call(this, request, loadContext)) as Response;
-
-                if (isResponse(res)) {
-                  setHttpStatus(span, res.status);
-                }
-
-                return res;
-              },
-            );
-          }
-
           return (await origRequestHandler.call(this, request, loadContext)) as Response;
         },
       );
     });
   };
 }
 
-function instrumentBuildCallback(build: ServerBuild, autoInstrumentRemix: boolean): ServerBuild {
+function instrumentBuildCallback(build: ServerBuild): ServerBuild {
   const routes: ServerRouteManifest = {};
   const wrappedEntry = { ...build.entry, module: { ...build.entry.module } };
 
-  // Not keeping boolean flags like it's done for `requestHandler` functions,
-  // Because the build can change between build and runtime.
-  // So if there is a new `loader` or`action` or `documentRequest` after build.
-  // We should be able to wrap them, as they may not be wrapped before.
-  const defaultExport = wrappedEntry.module.default as undefined | WrappedFunction;
-  if (defaultExport && !defaultExport.__sentry_original__) {
-    fill(wrappedEntry.module, 'default', makeWrappedDocumentRequestFunction(autoInstrumentRemix));
-  }
-
   for (const [id, route] of Object.entries(build.routes)) {
     const wrappedRoute = { ...route, module: { ...route.module } };
 
-    const routeAction = wrappedRoute.module.action as undefined | WrappedFunction;
-    if (routeAction && !routeAction.__sentry_original__) {
-      fill(wrappedRoute.module, 'action', makeWrappedAction(id, autoInstrumentRemix));
-    }
-
-    const routeLoader = wrappedRoute.module.loader as undefined | WrappedFunction;
-    if (routeLoader && !routeLoader.__sentry_original__) {
-      fill(wrappedRoute.module, 'loader', makeWrappedLoader(id, autoInstrumentRemix));
-    }
-
     // Entry module should have a loader function to provide `sentry-trace` and `baggage`
     // They will be available for the root `meta` function as `data.sentryTrace` and `data.sentryBaggage`
     if (!wrappedRoute.parentId) {
@@ -376,48 +207,43 @@ function instrumentBuildCallback(build: ServerBuild, autoInstrumentRemix: boolea
  */
 export function instrumentBuild(
   build: ServerBuild | (() => ServerBuild | Promise<ServerBuild>),
-  options: RemixOptions,
 ): ServerBuild | (() => ServerBuild | Promise<ServerBuild>) {
-  // eslint-disable-next-line deprecation/deprecation
-  const autoInstrumentRemix = options?.autoInstrumentRemix || false;
-
   if (typeof build === 'function') {
     return function () {
       const resolvedBuild = build();
 
       if (resolvedBuild instanceof Promise) {
         return resolvedBuild.then(build => {
-          return instrumentBuildCallback(build, autoInstrumentRemix);
+          return instrumentBuildCallback(build);
         });
       } else {
-        return instrumentBuildCallback(resolvedBuild, autoInstrumentRemix);
+        return instrumentBuildCallback(resolvedBuild);
       }
     };
   } else {
-    return instrumentBuildCallback(build, autoInstrumentRemix);
+    return instrumentBuildCallback(build);
   }
 }
 
-const makeWrappedCreateRequestHandler = (options: RemixOptions) =>
+const makeWrappedCreateRequestHandler = () =>
   function (origCreateRequestHandler: CreateRequestHandlerFunction): CreateRequestHandlerFunction {
     return function (
       this: unknown,
       build: ServerBuild | (() => Promise<ServerBuild>),
       ...args: unknown[]
     ): RequestHandler {
-      const newBuild = instrumentBuild(build, options);
+      const newBuild = instrumentBuild(build);
       const requestHandler = origCreateRequestHandler.call(this, newBuild, ...args);
 
-      // eslint-disable-next-line deprecation/deprecation
-      return wrapRequestHandler(requestHandler, newBuild, options.autoInstrumentRemix || false);
+      return wrapRequestHandler(requestHandler);
     };
   };
 
 /**
  * Monkey-patch Remix's `createRequestHandler` from `@remix-run/server-runtime`
  * which Remix Adapters (https://remix.run/docs/en/v1/api/remix) use underneath.
  */
-export function instrumentServer(options: RemixOptions): void {
+export function instrumentServer(): void {
   const pkg = loadModule<{
     createRequestHandler: CreateRequestHandlerFunction;
   }>('@remix-run/server-runtime');
@@ -428,5 +254,5 @@ export function instrumentServer(options: RemixOptions): void {
     return;
   }
 
-  fill(pkg, 'createRequestHandler', makeWrappedCreateRequestHandler(options));
+  fill(pkg, 'createRequestHandler', makeWrappedCreateRequestHandler());
 }

packages/remix/src/utils/remixOptions.ts

@@ -4,22 +4,4 @@ import type { BrowserOptions } from '@sentry/react';
 
 export type RemixOptions = (Options | BrowserOptions | NodeOptions) & {
   captureActionFormDataKeys?: Record<string, string | boolean>;
-} & (
-    | {
-        /**
-         * Enables OpenTelemetry Remix instrumentation.
-         *
-         * Note: This option will be the default behavior and will be removed in the next major version.
-         */
-        autoInstrumentRemix?: true;
-      }
-    | {
-        /**
-         * Enables OpenTelemetry Remix instrumentation
-         *
-         * @deprecated Setting this option to `false` is deprecated as the next major version will default to behaving as if this option were `true` and the option itself will be removed.
-         * It is recommended to set this option to `true`.
-         */
-        autoInstrumentRemix?: false;
-      }
-  );
+};

packages/remix/test/integration/app/entry.server.tsx

@@ -5,7 +5,6 @@ if (process.env.USE_OTEL !== '1') {
     dsn: 'https://public@dsn.ingest.sentry.io/1337',
     tracesSampleRate: 1,
     tracePropagationTargets: ['example.org'],
-    autoInstrumentRemix: false,
   });
 }
 

packages/remix/test/integration/instrument.server.mjs

@@ -4,5 +4,4 @@ Sentry.init({
   dsn: 'https://public@dsn.ingest.sentry.io/1337',
   tracesSampleRate: 1,
   tracePropagationTargets: ['example.org'],
-  autoInstrumentRemix: true,
 });