From 0576852aee4ed2c0a8b76729f27aedeebbabea97 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 20 Oct 2022 06:31:54 -0700 Subject: [PATCH] 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: https://github.com/getsentry/sentry-javascript/issues/5756 --- packages/node/src/integrations/requestdata.ts | 3 +- .../test/integrations/requestdata.test.ts | 35 ++++++++++++++-- packages/serverless/src/gcpfunction/http.ts | 13 +++--- .../serverless/test/__mocks__/@sentry/node.ts | 2 + packages/serverless/test/gcpfunction.test.ts | 40 +++++++++++++------ 5 files changed, 68 insertions(+), 25 deletions(-) diff --git a/packages/node/src/integrations/requestdata.ts b/packages/node/src/integrations/requestdata.ts index 9f00d1bb4fe4..82817a2a2073 100644 --- a/packages/node/src/integrations/requestdata.ts +++ b/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); diff --git a/packages/node/test/integrations/requestdata.test.ts b/packages/node/test/integrations/requestdata.test.ts index 8ec956beac8b..270c04cdfa86 100644 --- a/packages/node/test/integrations/requestdata.test.ts +++ b/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): 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' })); + }); }); }); diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index 82e553fa0c2b..3b62d8301bf1 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/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 { - 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); }); diff --git a/packages/serverless/test/__mocks__/@sentry/node.ts b/packages/serverless/test/__mocks__/@sentry/node.ts index 7c544c14fcda..355065317ea7 100644 --- a/packages/serverless/test/__mocks__/@sentry/node.ts +++ b/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(), diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts index 757669387863..20f878565a56 100644 --- a/packages/serverless/test/gcpfunction.test.ts +++ b/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', () => {