ref(serverless): Use RequestData integration in GCP wrapper (#5991)

This switches the GCP wrapper in the serverless SDK to use the new `RequestData` integration for adding request data to events rather than the current event processor. 

Ref: #5756

Author: lobsterkatie

packages/node/src/integrations/requestdata.ts

@@ -119,10 +119,11 @@ export class RequestData implements Integration {
       }
 
       // The Express request handler takes a similar `include` option to that which can be passed to this integration.
-      // If passed there, we store it in `sdkProcessingMetadata`. TODO(v8): Force express people to use this
+      // If passed there, we store it in `sdkProcessingMetadata`. TODO(v8): Force express and GCP people to use this
       // integration, so that all of this passing and conversion isn't necessary
       const addRequestDataOptions =
         sdkProcessingMetadata.requestDataOptionsFromExpressHandler ||
+        sdkProcessingMetadata.requestDataOptionsFromGCPWrapper ||
         convertReqDataIntegrationOptsToAddReqDataOpts(this._options);
 
       const processedEvent = this._addRequestData(event, req, addRequestDataOptions);

packages/node/test/integrations/requestdata.test.ts

@@ -1,5 +1,5 @@
 import { getCurrentHub, Hub, makeMain } from '@sentry/core';
-import { Event, EventProcessor } from '@sentry/types';
+import { Event, EventProcessor, PolymorphicRequest } from '@sentry/types';
 import * as http from 'http';
 
 import { NodeClient } from '../../src/client';
@@ -102,8 +102,8 @@ describe('`RequestData` integration', () => {
     });
   });
 
-  describe('usage with express request handler', () => {
-    it('uses options from request handler', async () => {
+  describe('usage with express request handler and GCP wrapper', () => {
+    it('uses options from Express request handler', async () => {
       const sentryRequestMiddleware = requestHandler({ include: { transaction: 'methodPath' } });
       const res = new http.ServerResponse(req);
       const next = jest.fn();
@@ -120,5 +120,34 @@ describe('`RequestData` integration', () => {
       // `transaction` matches the request middleware's option, not the integration's option
       expect(passedOptions?.include).toEqual(expect.objectContaining({ transaction: 'methodPath' }));
     });
+
+    it('uses options from GCP wrapper', async () => {
+      type GCPHandler = (req: PolymorphicRequest, res: http.ServerResponse) => void;
+      const mockGCPWrapper = (origHandler: GCPHandler, options: Record<string, unknown>): GCPHandler => {
+        const wrappedHandler: GCPHandler = (req, res) => {
+          getCurrentHub().getScope()?.setSDKProcessingMetadata({
+            request: req,
+            requestDataOptionsFromGCPWrapper: options,
+          });
+          origHandler(req, res);
+        };
+        return wrappedHandler;
+      };
+
+      const wrappedGCPFunction = mockGCPWrapper(jest.fn(), { include: { transaction: 'methodPath' } });
+      const res = new http.ServerResponse(req);
+
+      initWithRequestDataIntegrationOptions({ transactionNamingScheme: 'path' });
+
+      wrappedGCPFunction(req, res);
+
+      await getCurrentHub().getScope()!.applyToEvent(event, {});
+      requestDataEventProcessor(event);
+
+      const passedOptions = addRequestDataToEventSpy.mock.calls[0][2];
+
+      // `transaction` matches the GCP wrapper's option, not the integration's option
+      expect(passedOptions?.include).toEqual(expect.objectContaining({ transaction: 'methodPath' }));
+    });
   });
 });

packages/serverless/src/gcpfunction/http.ts

@@ -1,10 +1,4 @@
-import {
-  addRequestDataToEvent,
-  AddRequestDataToEventOptions,
-  captureException,
-  flush,
-  getCurrentHub,
-} from '@sentry/node';
+import { AddRequestDataToEventOptions, captureException, flush, getCurrentHub } from '@sentry/node';
 import { extractTraceparentData } from '@sentry/tracing';
 import { baggageHeaderToDynamicSamplingContext, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils';
 
@@ -97,7 +91,10 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
     // since functions-framework creates a domain for each incoming request.
     // So adding of event processors every time should not lead to memory bloat.
     hub.configureScope(scope => {
-      scope.addEventProcessor(event => addRequestDataToEvent(event, req, options.addRequestDataToEventOptions));
+      scope.setSDKProcessingMetadata({
+        request: req,
+        requestDataOptionsFromGCPWrapper: options.addRequestDataToEventOptions,
+      });
       // We put the transaction on the scope so users can attach children to it
       scope.setSpan(transaction);
     });

packages/serverless/test/__mocks__/@sentry/node.ts

@@ -1,6 +1,7 @@
 const origSentry = jest.requireActual('@sentry/node');
 export const defaultIntegrations = origSentry.defaultIntegrations; // eslint-disable-line @typescript-eslint/no-unsafe-member-access
 export const Handlers = origSentry.Handlers; // eslint-disable-line @typescript-eslint/no-unsafe-member-access
+export const Integrations = origSentry.Integrations;
 export const addRequestDataToEvent = origSentry.addRequestDataToEvent;
 export const SDK_VERSION = '6.6.6';
 export const Severity = {
@@ -20,6 +21,7 @@ export const fakeScope = {
   setContext: jest.fn(),
   setSpan: jest.fn(),
   getTransaction: jest.fn(() => fakeTransaction),
+  setSDKProcessingMetadata: jest.fn(),
 };
 export const fakeSpan = {
   finish: jest.fn(),

packages/serverless/test/gcpfunction.test.ts

@@ -1,4 +1,4 @@
-import { Event } from '@sentry/types';
+import * as SentryNode from '@sentry/node';
 import * as domain from 'domain';
 
 import * as Sentry from '../src';
@@ -230,23 +230,37 @@ describe('GCPFunction', () => {
     });
   });
 
-  test('wrapHttpFunction request data', async () => {
-    expect.assertions(6);
+  // This tests that the necessary pieces are in place for request data to get added to event - the `RequestData`
+  // integration is included in the defaults and the necessary data is stored in `sdkProcessingMetadata`. The
+  // integration's tests cover testing that it uses that data correctly.
+  test('wrapHttpFunction request data prereqs', async () => {
+    expect.assertions(2);
+
+    Sentry.GCPFunction.init({});
 
     const handler: HttpFunction = (_req, res) => {
       res.end();
     };
-    const wrappedHandler = wrapHttpFunction(handler);
-    const event: Event = {};
-    // @ts-ignore see "Why @ts-ignore" note
-    Sentry.fakeScope.addEventProcessor.mockImplementation(cb => cb(event));
+    const wrappedHandler = wrapHttpFunction(handler, { addRequestDataToEventOptions: { include: { ip: true } } });
+
     await handleHttp(wrappedHandler);
-    expect(event.transaction).toEqual('POST /path');
-    expect(event.request?.method).toEqual('POST');
-    expect(event.request?.url).toEqual('http://hostname/path?q=query');
-    expect(event.request?.query_string).toEqual('q=query');
-    expect(event.request?.headers).toEqual({ host: 'hostname', 'content-type': 'application/json' });
-    expect(event.request?.data).toEqual('{"foo":"bar"}');
+
+    expect(SentryNode.init).toHaveBeenCalledWith(
+      expect.objectContaining({
+        defaultIntegrations: expect.arrayContaining([expect.any(SentryNode.Integrations.RequestData)]),
+      }),
+    );
+
+    // @ts-ignore see "Why @ts-ignore" note
+    expect(Sentry.fakeScope.setSDKProcessingMetadata).toHaveBeenCalledWith({
+      request: {
+        method: 'POST',
+        url: '/path?q=query',
+        headers: { host: 'hostname', 'content-type': 'application/json' },
+        body: { foo: 'bar' },
+      },
+      requestDataOptionsFromGCPWrapper: { include: { ip: true } },
+    });
   });
 
   describe('wrapEventFunction() without callback', () => {