8000 ref(serverless): Use `RequestData` integration in GCP wrapper (#5991) · vaind/sentry-javascript@0576852 · GitHub
[go: up one dir, main page]
Skip to content
/ sentry-javascript Public
forked from getsentry/sentry-javascript

Commit 0576852

Browse files
lobsterkatielobsterkatie
authored
ref(serverless): Use RequestData integration in GCP wrapper (getsentry#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: getsentry#5756
1 parent 284184e commit 0576852

File tree

5 files changed

+68
-25
lines changed

5 files changed

+68
-25
lines changed

‎packages/node/src/integrations/requestdata.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,11 @@ export class RequestData implements Integration {
119119
}
120120

121121
// The Express request handler takes a similar `include` option to that which can be passed to this integration.
122-
// If passed there, we store it in `sdkProcessingMetadata`. TODO(v8): Force express people to use this
122+
// If passed there, we store it in `sdkProcessingMetadata`. TODO(v8): Force express and GCP people to use this
123123
// integration, so that all of this passing and conversion isn't necessary
124124
const addRequestDataOptions =
125125
sdkProcessingMetadata.requestDataOptionsFromExpressHandler ||
126+
sdkProcessingMetadata.requestDataOptionsFromGCPWrapper ||
126127
convertReqDataIntegrationOptsToAddReqDataOpts(this._options);
127128

128129
const processedEvent = this._addRequestData(event, req, addRequestDataOptions);

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

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { getCurrentHub, Hub, makeMain } from '@sentry/core';
2-
import { Event, EventProcessor } from '@sentry/types';
2+
import { Event, EventProcessor, PolymorphicRequest } from '@sentry/types';
33
import * as http from 'http';
44

55
import { NodeClient } from '../../src/client';
@@ -102,8 +102,8 @@ describe('`RequestData` integration', () => {
102102
});
103103
});
104104

105-
describe('usage with express request handler', () => {
106-
it('uses options from request handler', async () => {
105+
describe('usage with express request handler and GCP wrapper', () => {
106+
it('uses options from Express request handler', async () => {
107107
const sentryRequestMiddleware = requestHandler({ include: { transaction: 'methodPath' } });
108108
const res = new http.ServerResponse(req);
109109
const next = jest.fn();
@@ -120,5 +120,34 @@ describe('`RequestData` integration', () => {
120120
// `transaction` matches the request middleware's option, not the integration's option
121121
expect(passedOptions?.include).toEqual(expect.objectContaining({ transaction: 'methodPath' }));
122122
});
123+
124+
it('uses options from GCP wrapper', async () => {
125+
type GCPHandler = (req: PolymorphicRequest, res: http.ServerResponse) => void;
126+
const mockGCPWrapper = (origHandler: GCPHandler, options: Record<string, unknown>): GCPHandler => {
127+
const wrappedHandler: GCPHandler = (req, res) => {
128+
getCurrentHub().getScope()?.setSDKProcessingMetadata({
129+
request: req,
130+
requestDataOptionsFromGCPWrapper: options,
131+
});
132+
origHandler(req, res);
133+
};
134+
return wrappedHandler;
135+
};
136+
137+
const wrappedGCPFunction = mockGCPWrapper(jest.fn(), { include: { transaction: 'methodPath' } });
138+
const res = new http.ServerResponse(req);
139+
140+
initWithRequestDataIntegrationOptions({ transactionNamingScheme: 'path' });
141+
142+
wrappedGCPFunction(req, res);
143+
144+
await getCurrentHub().getScope()!.applyToEvent(event, {});
145+
requestDataEventProcessor(event);
146+
147+
const passedOptions = addRequestDataToEventSpy.mock.calls[0][2];
148+
149+
// `transaction` matches the GCP wrapper's option, not the integration's option
150+
expect(passedOptions?.include).toEqual(expect.objectContaining({ transaction: 'methodPath' }));
151+
});
123152
});
124153
});

‎packages/serverless/src/gcpfunction/http.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
1-
import {
2-
addRequestDataToEvent,
3-
AddRequestDataToEventOptions,
4-
captureException,
5-
flush,
6-
getCurrentHub,
7-
} from '@sentry/node';
1+
import { AddRequestDataToEventOptions, captureException, flush, getCurrentHub } from '@sentry/node';
82
import { extractTraceparentData } from '@sentry/tracing';
93
import { baggageHeaderToDynamicSamplingContext, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils';
104

@@ -97,7 +91,10 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
9791
// since functions-framework creates a domain for each incoming request.
9892
// So adding of event processors every time should not lead to memory bloat.
9993
hub.configureScope(scope => {
100-
scope.addEventProcessor(event => addRequestDataToEvent(event, req, options.addRequestDataToEventOptions));
94+
scope.setSDKProcessingMetadata({
95+
request: req,
96+
requestDataOptionsFromGCPWrapper: options.addRequestDataToEventOptions,
97+
});
10198
// We put the transaction on the scope so users can attach children to it
10299
scope.setSpan(transaction);
103100
});

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
const origSentry = jest.requireActual('@sentry/node');
22
export const defaultIntegrations = origSentry.defaultIntegrations; // eslint-disable-line @typescript-eslint/no-unsafe-member-access
33
export const Handlers = origSentry.Handlers; // eslint-disable-line @typescript-eslint/no-unsafe-member-access
4+
export const Integrations = origSentry.Integrations;
45
export const addRequestDataToEvent = origSentry.addRequestDataToEvent;
56
export const SDK_VERSION = '6.6.6';
67
export const Severity = {
@@ -20,6 +21,7 @@ export const fakeScope = {
2021
setContext: jest.fn(),
2122
setSpan: jest.fn(),
2223
getTransaction: jest.fn(() => fakeTransaction),
24+
setSDKProcessingMetadata: jest.fn(),
2325
};
2426
export const fakeSpan = {
2527
finish: jest.fn(),

‎packages/serverless/test/gcpfunction.test.ts

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Event } from '@sentry/types';
1+
import * as SentryNode from '@sentry/node';
22
import * as domain from 'domain';
33

44
import * as Sentry from '../src';
@@ -230,23 +230,37 @@ describe('GCPFunction', () => {
230230
});
231231
});
232232

233-
test('wrapHttpFunction request data', async () => {
234-
expect.assertions(6);
233+
// This tests that the necessary pieces are in place for request data to get added to event - the `RequestData`
234+
// integration is included in the defaults and the necessary data is stored in `sdkProcessingMetadata`. The
235+
// integration's tests cover testing that it uses that data correctly.
236+
test('wrapHttpFunction request data prereqs', async () => {
237+
expect.assertions(2);
238+
239+
Sentry.GCPFunction.init({});
235240

236241
const handler: HttpFunction = (_req, res) => {
237242
res.end();
238243
};
239-
const wrappedHandler = wrapHttpFunction(handler);
240-
const event: Event = {};
241-
// @ts-ignore see "Why @ts-ignore" note
242-
Sentry.fakeScope.addEventProcessor.mockImplementation(cb => cb(event));
244+
const wrappedHandler = wrapHttpFunction(handler, { addRequestDataToEventOptions: { include: { ip: true } } });
245+
243246
await handleHttp(wrappedHandler);
244-
expect(event.transaction).toEqual('POST /path');
245-
expect(event.request?.method).toEqual('POST');
246-
expect(event.request?.url).toEqual('http://hostname/path?q=query');
247-
expect(event.request?.query_string).toEqual('q=query');
248-
expect(event.request?.headers).toEqual({ host: 'hostname', 'content-type': 'application/json' });
249-
expect(event.request?.data).toEqual('{"foo":"bar"}');
247+
248+
expect(SentryNode.init).toHaveBeenCalledWith(
249+
expect.objectContaining({
250+
defaultIntegrations: expect.arrayContaining([expect.any(SentryNode.Integrations.RequestData)]),
251+
}),
252+
);
253+
254+
// @ts-ignore see "Why @ts-ignore" note
255+
expect(Sentry.fakeScope.setSDKProcessingMetadata).toHaveBeenCalledWith({
256+
request: {
257+
method: 'POST',
258+
url: '/path?q=query',
259+
headers: { host: 'hostname', 'content-type': 'application/json' },
260+
body: { foo: 'bar' },
261+
},
262+
requestDataOptionsFromGCPWrapper: { include: { ip: true } },
263+
});
250264
});
251265

252266
describe('wrapEventFunction() without callback', () => {

0 commit comments

Comments
 (0)
0