feat: Update requestDataIntegration handling

packages/astro/src/index.server.ts

@@ -11,8 +11,6 @@ export {
   addBreadcrumb,
   addEventProcessor,
   addIntegration,
-  // eslint-disable-next-line deprecation/deprecation
-  addRequestDataToEvent,
   amqplibIntegration,
   anrIntegration,
   disableAnrDetectionForCallback,
@@ -33,13 +31,10 @@ export {
   cron,
   dataloaderIntegration,
   dedupeIntegration,
-  DEFAULT_USER_INCLUDES,
   defaultStackParser,
   endSession,
   expressErrorHandler,
   expressIntegration,
-  // eslint-disable-next-line deprecation/deprecation
-  extractRequestData,
   extraErrorDataIntegration,
   fastifyIntegration,
   flush,

packages/aws-serverless/src/index.ts

@@ -42,11 +42,6 @@ export {
   flush,
   close,
   getSentryRelease,
-  // eslint-disable-next-line deprecation/deprecation
-  addRequestDataToEvent,
-  DEFAULT_USER_INCLUDES,
-  // eslint-disable-next-line deprecation/deprecation
-  extractRequestData,
   createGetModuleFromFilename,
   anrIntegration,
   disableAnrDetectionForCallback,

packages/bun/src/index.ts

@@ -18,7 +18,6 @@ export type {
   Thread,
   User,
 } from '@sentry/core';
-export type { AddRequestDataToEventOptions } from '@sentry/core';
 
 export {
   addEventProcessor,
@@ -64,11 +63,6 @@ export {
   flush,
   close,
   getSentryRelease,
-  // eslint-disable-next-line deprecation/deprecation
-  addRequestDataToEvent,
-  DEFAULT_USER_INCLUDES,
-  // eslint-disable-next-line deprecation/deprecation
-  extractRequestData,
   createGetModuleFromFilename,
   anrIntegration,
   disableAnrDetectionForCallback,

packages/cloudflare/src/index.ts

@@ -18,7 +18,6 @@ export type {
   Thread,
   User,
 } from '@sentry/core';
-export type { AddRequestDataToEventOptions } from '@sentry/core';
 
 export type { CloudflareOptions } from './client';
 

packages/core/src/index.ts

@@ -3,7 +3,6 @@ export type { AsyncContextStrategy } from './asyncContext/types';
 export type { Carrier } from './carrier';
 export type { OfflineStore, OfflineTransportOptions } from './transports/offline';
 export type { ServerRuntimeClientOptions } from './server-runtime-client';
-export type { RequestDataIntegrationOptions } from './integrations/requestdata';
 export type { IntegrationIndex } from './integration';
 
 export * from './tracing';

packages/core/src/integrations/requestdata.ts

@@ -1,105 +1,53 @@
 import { defineIntegration } from '../integration';
-import type { IntegrationFn } from '../types-hoist';
-import {
-  type AddRequestDataToEventOptions,
-  addNormalizedRequestDataToEvent,
-  addRequestDataToEvent,
-} from '../utils-hoist/requestdata';
-
-export type RequestDataIntegrationOptions = {
-  /**
-   * Controls what data is pulled from the request and added to the event
-   */
-  include?: {
-    cookies?: boolean;
-    data?: boolean;
-    headers?: boolean;
-    ip?: boolean;
-    query_string?: boolean;
-    url?: boolean;
-    user?:
-      | boolean
-      | {
-          id?: boolean;
-          username?: boolean;
-          email?: boolean;
-        };
-  };
+import type { Event, IntegrationFn, RequestEventData } from '../types-hoist';
+import { parseCookie } from '../utils-hoist/cookie';
+import { getClientIPAddress, ipHeaderNames } from '../utils-hoist/vendor/getIpAddress';
+
+interface RequestDataIncludeOptions {
+  cookies?: boolean;
+  data?: boolean;
+  headers?: boolean;
+  ip?: boolean;
+  query_string?: boolean;
+  url?: boolean;
+}
 
+type RequestDataIntegrationOptions = {
   /**
-   * Whether to identify transactions by parameterized path, parameterized path with method, or handler name.
-   * @deprecated This option does not do anything anymore, and will be removed in v9.
+   * Controls what data is pulled from the request and added to the event.
    */
-  transactionNamingScheme?: 'path' | 'methodPath' | 'handler';
+  include?: RequestDataIncludeOptions;
 };
 
-const DEFAULT_OPTIONS = {
-  include: {
-    cookies: true,
-    data: true,
-    headers: true,
-    ip: false,
-    query_string: true,
-    url: true,
-    user: {
-      id: true,
-      username: true,
-      email: true,
-    },
-  },
-  transactionNamingScheme: 'methodPath' as const,
-};
+const DEFAULT_INCLUDE = {
+  cookies: true,
+  data: true,
+  headers: true,
+  ip: false,
+  query_string: true,
+  url: true,
+} satisfies RequestDataIncludeOptions;
 
 const INTEGRATION_NAME = 'RequestData';
 
 const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) => {
-  const _options: Required<RequestDataIntegrationOptions> = {
-    ...DEFAULT_OPTIONS,
-    ...options,
-    include: {
-      ...DEFAULT_OPTIONS.include,
-      ...options.include,
-      user:
-        options.include && typeof options.include.user === 'boolean'
-          ? options.include.user
-          : {
-              ...DEFAULT_OPTIONS.include.user,
-              // Unclear why TS still thinks `options.include.user` could be a boolean at this point
-              ...((options.include || {}).user as Record<string, boolean>),
-            },
-    },
+  const include = {
+    ...DEFAULT_INCLUDE,
+    ...options.include,
   };
 
   return {
     name: INTEGRATION_NAME,
     processEvent(event) {
-      // Note: In the long run, most of the logic here should probably move into the request data utility functions. For
-      // the moment it lives here, though, until https://github.com/getsentry/sentry-javascript/issues/5718 is addressed.
-      // (TL;DR: Those functions touch many parts of the repo in many different ways, and need to be cleaned up. Once
-      // that's happened, it will be easier to add this logic in without worrying about unexpected side effects.)
-
       const { sdkProcessingMetadata = {} } = event;
-      const { request, normalizedRequest } = sdkProcessingMetadata;
-
-      const addRequestDataOptions = convertReqDataIntegrationOptsToAddReqDataOpts(_options);
+      const { normalizedRequest, ipAddress } = sdkProcessingMetadata;
 
-      // If this is set, it takes precedence over the plain request object
       if (normalizedRequest) {
-        // Some other data is not available in standard HTTP requests, but can sometimes be augmented by e.g. Express or Next.js
-        const ipAddress = request ? request.ip || (request.socket && request.socket.remoteAddress) : undefined;
-        const user = request ? request.user : undefined;
-
-        addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress, user }, addRequestDataOptions);
+        addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress }, include);
         return event;
       }
 
-      // TODO(v9): Eventually we can remove this fallback branch and only rely on the normalizedRequest above
-      if (!request) {
-        return event;
-      }
-
-      // eslint-disable-next-line deprecation/deprecation
-      return addRequestDataToEvent(event, request, addRequestDataOptions);
+      return event;
     },
   };
 }) satisfies IntegrationFn;
@@ -110,45 +58,75 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) =
  */
 export const requestDataIntegration = defineIntegration(_requestDataIntegration);
 
-/** Convert this integration's options to match what `addRequestDataToEvent` expects */
-/** TODO: Can possibly be deleted once https://github.com/getsentry/sentry-javascript/issues/5718 is fixed */
-function convertReqDataIntegrationOptsToAddReqDataOpts(
-  integrationOptions: Required<RequestDataIntegrationOptions>,
-): AddRequestDataToEventOptions {
-  const {
-    // eslint-disable-next-line deprecation/deprecation
-    transactionNamingScheme,
-    include: { ip, user, ...requestOptions },
-  } = integrationOptions;
-
-  const requestIncludeKeys: string[] = ['method'];
-  for (const [key, value] of Object.entries(requestOptions)) {
-    if (value) {
-      requestIncludeKeys.push(key);
+/**
+ * Add already normalized request data to an event.
+ * This mutates the passed in event.
+ */
+function addNormalizedRequestDataToEvent(
+  event: Event,
+  req: RequestEventData,
+  // Data that should not go into `event.request` but is somehow related to requests
+  additionalData: { ipAddress?: string },
+  include: RequestDataIncludeOptions,
+): void {
+  event.request = {
+    ...event.request,
+    ...extractNormalizedRequestData(req, include),
+  };
+
+  if (include.ip) {
+    const ip = (req.headers && getClientIPAddress(req.headers)) || additionalData.ipAddress;
+    if (ip) {
+      event.user = {
+        ...event.user,
+        ip_address: ip,
+      };
     }
   }
+}
 
-  let addReqDataUserOpt;
-  if (user === undefined) {
-    addReqDataUserOpt = true;
-  } else if (typeof user === 'boolean') {
-    addReqDataUserOpt = user;
-  } else {
-    const userIncludeKeys: string[] = [];
-    for (const [key, value] of Object.entries(user)) {
-      if (value) {
-        userIncludeKeys.push(key);
-      }
+function extractNormalizedRequestData(
+  normalizedRequest: RequestEventData,
+  include: RequestDataIncludeOptions,
+): RequestEventData {
+  const requestData: RequestEventData = {};
+  const headers = { ...normalizedRequest.headers };
+
+  if (include.headers) {
+    requestData.headers = headers;
+
+    // Remove the Cookie header in case cookie data should not be included in the event
+    if (!include.cookies) {
+      delete (headers as { cookie?: string }).cookie;
+    }
+
+    // Remove IP headers in case IP data should not be included in the event
+    if (!include.ip) {
+      ipHeaderNames.forEach(ipHeaderName => {
+        // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
+        delete (headers as Record<string, unknown>)[ipHeaderName];
+      });
     }
-    addReqDataUserOpt = userIncludeKeys;
   }
 
-  return {
-    include: {
-      ip,
-      user: addReqDataUserOpt,
-      request: requestIncludeKeys.length !== 0 ? requestIncludeKeys : undefined,
-      transaction: transactionNamingScheme,
-    },
-  };
+  requestData.method = normalizedRequest.method;
+
+  if (include.url) {
+    requestData.url = normalizedRequest.url;
+  }
+
+  if (include.cookies) {
+    const cookies = normalizedRequest.cookies || (headers && headers.cookie ? parseCookie(headers.cookie) : undefined);
+    requestData.cookies = cookies || {};
+  }
+
+  if (include.query_string) {
+    requestData.query_string = normalizedRequest.query_string;
+  }
+
+  if (include.data) {
+    requestData.data = normalizedRequest.data;
+  }
+
+  return requestData;
 }

packages/core/src/scope.ts

@@ -5,13 +5,15 @@ import type {
   Client,
   Context,
   Contexts,
+  DynamicSamplingContext,
   Event,
   EventHint,
   EventProcessor,
   Extra,
   Extras,
   Primitive,
   PropagationContext,
+  RequestEventData,
   Session,
   SeverityLevel,
   Span,
@@ -58,6 +60,12 @@ export interface SdkProcessingMetadata {
   requestSession?: {
     status: 'ok' | 'errored' | 'crashed';
   };
+  normalizedRequest?: RequestEventData;
+  dynamicSamplingContext?: Partial<DynamicSamplingContext>;
+  capturedSpanScope?: Scope;
+  capturedSpanIsolationScope?: Scope;
+  spanCountBeforeProcessing?: number;
+  ipAddress?: string;
 }
 
 /**
@@ -537,10 +545,8 @@ export class Scope {
 
   /**
    * Add data which will be accessible during event processing but won't get sent to Sentry.
-   *
-   * TODO(v9): We should type this stricter, so that e.g. `normalizedRequest` is strictly typed.
    */
-  public setSDKProcessingMetadata(newData: { [key: string]: unknown }): this {
+  public setSDKProcessingMetadata(newData: SdkProcessingMetadata): this {
     this._sdkProcessingMetadata = merge(this._sdkProcessingMetadata, newData, 2);
     return this;
   }

packages/core/src/types-hoist/event.ts

@@ -1,15 +1,13 @@
-import type { CaptureContext, Scope } from '../scope';
+import type { CaptureContext, SdkProcessingMetadata } from '../scope';
 import type { Attachment } from './attachment';
 import type { Breadcrumb } from './breadcrumb';
 import type { Contexts } from './context';
 import type { DebugMeta } from './debugMeta';
-import type { DynamicSamplingContext } from './envelope';
 import type { Exception } from './exception';
 import type { Extras } from './extra';
 import type { Measurements } from './measurement';
 import type { Mechanism } from './mechanism';
 import type { Primitive } from './misc';
-import type { PolymorphicRequest } from './polymorphics';
 import type { RequestEventData } from './request';
 import type { SdkInfo } from './sdkinfo';
 import type { SeverityLevel } from './severity';
@@ -54,14 +52,7 @@ export interface Event {
   debug_meta?: DebugMeta;
   // A place to stash data which is needed at some point in the SDK's event processing pipeline but which shouldn't get sent to Sentry
   // Note: This is considered internal and is subject to change in minors
-  sdkProcessingMetadata?: { [key: string]: unknown } & {
-    request?: PolymorphicRequest;
-    normalizedRequest?: RequestEventData;
-    dynamicSamplingContext?: Partial<DynamicSamplingContext>;
-    capturedSpanScope?: Scope;
-    capturedSpanIsolationScope?: Scope;
-    spanCountBeforeProcessing?: number;
-  };
+  sdkProcessingMetadata?: SdkProcessingMetadata;
   transaction_info?: {
     source: TransactionSource;
   };