ref: Hoist RequestData integration to @sentry/core (#9597)

Author: lforst

packages/core/src/index.ts

@@ -2,6 +2,7 @@ export type { ClientClass } from './sdk';
 export type { AsyncContextStrategy, Carrier, Layer, RunWithAsyncContextOptions } from './hub';
 export type { OfflineStore, OfflineTransportOptions } from './transports/offline';
 export type { ServerRuntimeClientOptions } from './server-runtime-client';
+export type { RequestDataIntegrationOptions } from './integrations/requestdata';
 
 export * from './tracing';
 export { createEventEnvelope } from './envelope';
@@ -56,6 +57,7 @@ export { hasTracingEnabled } from './utils/hasTracingEnabled';
 export { isSentryRequestUrl } from './utils/isSentryRequestUrl';
 export { DEFAULT_ENVIRONMENT } from './constants';
 export { ModuleMetadata } from './integrations/metadata';
+export { RequestData } from './integrations/requestdata';
 import * as Integrations from './integrations';
 
 export { Integrations };

packages/node/src/integrations/requestdata.ts renamed to packages/core/src/integrations/requestdata.ts

@@ -1,11 +1,6 @@
-// TODO (v8 or v9): Whenever this becomes a default integration for `@sentry/browser`, move this to `@sentry/core`. For
-// now, we leave it in `@sentry/integrations` so that it doesn't contribute bytes to our CDN bundles.
-
 import type { Event, EventProcessor, Hub, Integration, PolymorphicRequest, Transaction } from '@sentry/types';
-import { extractPathForTransaction } from '@sentry/utils';
-
-import type { AddRequestDataToEventOptions, TransactionNamingScheme } from '../requestdata';
-import { addRequestDataToEvent } from '../requestdata';
+import type { AddRequestDataToEventOptions, TransactionNamingScheme } from '@sentry/utils';
+import { addRequestDataToEvent, extractPathForTransaction } from '@sentry/utils';
 
 export type RequestDataIntegrationOptions = {
   /**
@@ -59,7 +54,7 @@ export class RequestData implements Integration {
   /**
    * @inheritDoc
    */
-  public name: string = RequestData.id;
+  public name: string;
 
   /**
    * Function for adding request data to event. Defaults to `addRequestDataToEvent` from `@sentry/node` for now, but
@@ -74,6 +69,7 @@ export class RequestData implements Integration {
    * @inheritDoc
    */
   public constructor(options: RequestDataIntegrationOptions = {}) {
+    this.name = RequestData.id;
     this._addRequestData = addRequestDataToEvent;
     this._options = {
       ...DEFAULT_OPTIONS,

packages/core/test/lib/integrations/requestdata.test.ts

@@ -0,0 +1,102 @@
+import type { RequestDataIntegrationOptions } from '@sentry/core';
+import { getCurrentHub, Hub, makeMain, RequestData } from '@sentry/core';
+import type { Event, EventProcessor } from '@sentry/types';
+import * as sentryUtils from '@sentry/utils';
+import type { IncomingMessage } from 'http';
+
+import { getDefaultTestClientOptions, TestClient } from '../../mocks/client';
+
+const addRequestDataToEventSpy = jest.spyOn(sentryUtils, 'addRequestDataToEvent');
+const requestDataEventProcessor = jest.fn();
+
+const headers = { ears: 'furry', nose: 'wet', tongue: 'spotted', cookie: 'favorite=zukes' };
+const method = 'wagging';
+const protocol = 'mutualsniffing';
+const hostname = 'the.dog.park';
+const path = '/by/the/trees/';
+const queryString = 'chase=me&please=thankyou';
+
+function initWithRequestDataIntegrationOptions(integrationOptions: RequestDataIntegrationOptions): void {
+  const setMockEventProcessor = (eventProcessor: EventProcessor) =>
+    requestDataEventProcessor.mockImplementationOnce(eventProcessor);
+
+  const requestDataIntegration = new RequestData({
+    ...integrationOptions,
+  });
+
+  const client = new TestClient(
+    getDefaultTestClientOptions({
+      dsn: 'https://[email protected]/12312012',
+      integrations: [requestDataIntegration],
+    }),
+  );
+  client.setupIntegrations = () => requestDataIntegration.setupOnce(setMockEventProcessor, getCurrentHub);
+  client.getIntegration = () => requestDataIntegration as any;
+
+  const hub = new Hub(client);
+
+  makeMain(hub);
+}
+
+describe('`RequestData` integration', () => {
+  let req: IncomingMessage, event: Event;
+
+  beforeEach(() => {
+    req = {
+      headers,
+      method,
+      protocol,
+      hostname,
+      originalUrl: `${path}?${queryString}`,
+    } as unknown as IncomingMessage;
+    event = { sdkProcessingMetadata: { request: req } };
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  describe('option conversion', () => {
+    it('leaves `ip` and `user` at top level of `include`', () => {
+      initWithRequestDataIntegrationOptions({ include: { ip: false, user: true } });
+
+      requestDataEventProcessor(event);
+
+      const passedOptions = addRequestDataToEventSpy.mock.calls[0][2];
+
+      expect(passedOptions?.include).toEqual(expect.objectContaining({ ip: false, user: true }));
+    });
+
+    it('moves `transactionNamingScheme` to `transaction` include', () => {
+      initWithRequestDataIntegrationOptions({ transactionNamingScheme: 'path' });
+
+      requestDataEventProcessor(event);
+
+      const passedOptions = addRequestDataToEventSpy.mock.calls[0][2];
+
+      expect(passedOptions?.include).toEqual(expect.objectContaining({ transaction: 'path' }));
+    });
+
+    it('moves `true` request keys into `request` include, but omits `false` ones', async () => {
+      initWithRequestDataIntegrationOptions({ include: { data: true, cookies: false } });
+
+      requestDataEventProcessor(event);
+
+      const passedOptions = addRequestDataToEventSpy.mock.calls[0][2];
+
+      expect(passedOptions?.include?.request).toEqual(expect.arrayContaining(['data']));
+      expect(passedOptions?.include?.request).not.toEqual(expect.arrayContaining(['cookies']));
+    });
+
+    it('moves `true` user keys into `user` include, but omits `false` ones', async () => {
+      initWithRequestDataIntegrationOptions({ include: { user: { id: true, email: false } } });
+
+      requestDataEventProcessor(event);
+
+      const passedOptions = addRequestDataToEventSpy.mock.calls[0][2];
+
+      expect(passedOptions?.include?.user).toEqual(expect.arrayContaining(['id']));
+      expect(passedOptions?.include?.user).not.toEqual(expect.arrayContaining(['email']));
+    });
+  });
+});

packages/node/src/handlers.ts

@@ -15,6 +15,7 @@ import {
   addRequestDataToTransaction,
   dropUndefinedKeys,
   extractPathForTransaction,
+  extractRequestData,
   isString,
   isThenable,
   logger,
@@ -23,7 +24,6 @@ import {
 import type * as http from 'http';
 
 import type { NodeClient } from './client';
-import { extractRequestData } from './requestdata';
 // TODO (v8 / XXX) Remove this import
 import type { ParseRequestOptions } from './requestDataDeprecated';
 import { isAutoSessionTrackingEnabled } from './sdk';

packages/node/src/index.ts

@@ -18,9 +18,8 @@ export type {
   Transaction,
   User,
 } from '@sentry/types';
-export type { AddRequestDataToEventOptions } from '@sentry/utils';
+export type { AddRequestDataToEventOptions, TransactionNamingScheme } from '@sentry/utils';
 
-export type { TransactionNamingScheme } from './requestdata';
 export type { NodeOptions } from './types';
 
 export {
@@ -72,7 +71,7 @@ export { autoDiscoverNodePerformanceMonitoringIntegrations } from './tracing';
 export { NodeClient } from './client';
 export { makeNodeTransport } from './transports';
 export { defaultIntegrations, init, defaultStackParser, getSentryRelease } from './sdk';
-export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from './requestdata';
+export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from '@sentry/utils';
 export { deepReadDirSync } from './utils';
 export { getModuleFromFilename } from './module';
 export { enableAnrDetection } from './anr';

packages/node/src/integrations/index.ts

@@ -5,6 +5,6 @@ export { OnUnhandledRejection } from './onunhandledrejection';
 export { Modules } from './modules';
 export { ContextLines } from './contextlines';
 export { Context } from './context';
-export { RequestData } from './requestdata';
+export { RequestData } from '@sentry/core';
 export { LocalVariables } from './localvariables';
 export { Undici } from './undici';

packages/node/src/requestDataDeprecated.ts

@@ -7,9 +7,8 @@
 /* eslint-disable deprecation/deprecation */
 /* eslint-disable @typescript-eslint/no-explicit-any */
 import type { Event, ExtractedNodeRequestData, PolymorphicRequest } from '@sentry/types';
-
-import type { AddRequestDataToEventOptions } from './requestdata';
-import { addRequestDataToEvent, extractRequestData as _extractRequestData } from './requestdata';
+import type { AddRequestDataToEventOptions } from '@sentry/utils';
+import { addRequestDataToEvent, extractRequestData as _extractRequestData } from '@sentry/utils';
 
 /**
  * @deprecated `Handlers.ExpressRequest` is deprecated and will be removed in v8. Use `PolymorphicRequest` instead.