From d1c036e1b5f0d281c51c78985eb50b0f6f2d5b7d Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 24 Jan 2024 13:43:42 -0500 Subject: [PATCH 1/6] ref(serverless): Convert AWSServices to function --- packages/serverless/src/awsservices.ts | 103 ++++++++++-------- .../serverless/test/__mocks__/@sentry/node.ts | 54 --------- packages/serverless/test/awsservices.test.ts | 80 ++++++++++++-- 3 files changed, 123 insertions(+), 114 deletions(-) delete mode 100644 packages/serverless/test/__mocks__/@sentry/node.ts diff --git a/packages/serverless/src/awsservices.ts b/packages/serverless/src/awsservices.ts index 36a789c52632..4e30983b2089 100644 --- a/packages/serverless/src/awsservices.ts +++ b/packages/serverless/src/awsservices.ts @@ -1,6 +1,6 @@ -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; -import { startInactiveSpan } from '@sentry/node'; -import type { Integration, Span } from '@sentry/types'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, convertIntegrationFnToClass, defineIntegration } from '@sentry/core'; +import { getClient, startInactiveSpan } from '@sentry/node'; +import type { Client, Integration, IntegrationClass, IntegrationFn, Span } from '@sentry/types'; import { fill } from '@sentry/utils'; // 'aws-sdk/global' import is expected to be type-only so it's erased in the final .js file. // When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here. @@ -16,63 +16,70 @@ interface AWSService { serviceIdentifier: string; } -/** AWS service requests tracking */ -export class AWSServices implements Integration { - /** - * @inheritDoc - */ - public static id: string = 'AWSServices'; +const INTEGRATION_NAME = 'AWSServices'; - /** - * @inheritDoc - */ - public name: string; +export const SETUP_CLIENTS = new WeakMap(); - private readonly _optional: boolean; - - public constructor(options: { optional?: boolean } = {}) { - this.name = AWSServices.id; +const _awsServicesIntegration = ((options: { optional?: boolean } = {}) => { + const optional = options.optional || false; + return { + name: INTEGRATION_NAME, + setupOnce() { + try { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const awsModule = require('aws-sdk/global') as typeof AWS; + fill(awsModule.Service.prototype, 'makeRequest', wrapMakeRequest); + } catch (e) { + if (!optional) { + throw e; + } + } + }, + setup(client) { + SETUP_CLIENTS.set(client, true); + }, + }; +}) satisfies IntegrationFn; - this._optional = options.optional || false; - } +export const awsServicesIntegration = defineIntegration(_awsServicesIntegration); - /** - * @inheritDoc - */ - public setupOnce(): void { - try { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const awsModule = require('aws-sdk/global') as typeof AWS; - fill(awsModule.Service.prototype, 'makeRequest', wrapMakeRequest); - } catch (e) { - if (!this._optional) { - throw e; - } - } - } -} +/** + * AWS Service Request Tracking. + * + * @deprecated Use `awsServicesIntegration()` instead. + */ +// eslint-disable-next-line deprecation/deprecation +export const AWSServices = convertIntegrationFnToClass( + INTEGRATION_NAME, + awsServicesIntegration, +) as IntegrationClass; -/** */ +/** + * Patches AWS SDK request to create `http.client` spans. + */ function wrapMakeRequest( orig: MakeRequestFunction, ): MakeRequestFunction { return function (this: TService, operation: string, params?: GenericParams, callback?: MakeRequestCallback) { let span: Span | undefined; const req = orig.call(this, operation, params); - req.on('afterBuild', () => { - span = startInactiveSpan({ - name: describe(this, operation, params), - op: 'http.client', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.serverless', - }, + + if (SETUP_CLIENTS.has(getClient() as Client)) { + req.on('afterBuild', () => { + span = startInactiveSpan({ + name: describe(this, operation, params), + op: 'http.client', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.serverless', + }, + }); }); - }); - req.on('complete', () => { - if (span) { - span.end(); - } - }); + req.on('complete', () => { + if (span) { + span.end(); + } + }); + } if (callback) { req.send(callback); diff --git a/packages/serverless/test/__mocks__/@sentry/node.ts b/packages/serverless/test/__mocks__/@sentry/node.ts deleted file mode 100644 index 5181b8a0a535..000000000000 --- a/packages/serverless/test/__mocks__/@sentry/node.ts +++ /dev/null @@ -1,54 +0,0 @@ -const origSentry = jest.requireActual('@sentry/node'); -export const defaultIntegrations = origSentry.defaultIntegrations; // eslint-disable-line @typescript-eslint/no-unsafe-member-access -export const getDefaultIntegrations = origSentry.getDefaultIntegrations; // 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 = { - Warning: 'warning', -}; -export const continueTrace = origSentry.continueTrace; - -export const fakeScope = { - addEventProcessor: jest.fn(), - setTag: jest.fn(), - setContext: jest.fn(), - setSpan: jest.fn(), - setSDKProcessingMetadata: jest.fn(), - setPropagationContext: jest.fn(), -}; -export const fakeSpan = { - end: jest.fn(), - setHttpStatus: jest.fn(), -}; -export const init = jest.fn(); -export const addGlobalEventProcessor = jest.fn(); -export const getCurrentScope = jest.fn(() => fakeScope); -export const captureException = jest.fn(); -export const captureMessage = jest.fn(); -export const withScope = jest.fn(cb => cb(fakeScope)); -export const flush = jest.fn(() => Promise.resolve()); -export const getClient = jest.fn(() => ({})); -export const startSpanManual = jest.fn((ctx, callback: (span: any) => any) => callback(fakeSpan)); -export const startInactiveSpan = jest.fn(() => fakeSpan); - -export const resetMocks = (): void => { - fakeSpan.end.mockClear(); - fakeSpan.setHttpStatus.mockClear(); - - fakeScope.addEventProcessor.mockClear(); - fakeScope.setTag.mockClear(); - fakeScope.setContext.mockClear(); - fakeScope.setSpan.mockClear(); - - init.mockClear(); - addGlobalEventProcessor.mockClear(); - - captureException.mockClear(); - captureMessage.mockClear(); - withScope.mockClear(); - flush.mockClear(); - getClient.mockClear(); - startSpanManual.mockClear(); -}; diff --git a/packages/serverless/test/awsservices.test.ts b/packages/serverless/test/awsservices.test.ts index b18b1d8dd9af..94660b329a0e 100644 --- a/packages/serverless/test/awsservices.test.ts +++ b/packages/serverless/test/awsservices.test.ts @@ -3,20 +3,51 @@ import * as AWS from 'aws-sdk'; import * as nock from 'nock'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; -import { AWSServices } from '../src/awsservices'; +import { awsServicesIntegration } from '../src/awsservices'; -describe('AWSServices', () => { - beforeAll(() => { - new AWSServices().setupOnce(); +const mockSpanEnd = jest.fn(); +const mockStartInactiveSpan = jest.fn(spanArgs => ({ ...spanArgs })); + +jest.mock('@sentry/node', () => { + return { + ...jest.requireActual('@sentry/node'), + startInactiveSpan: (ctx: unknown) => { + mockStartInactiveSpan(ctx); + return { end: mockSpanEnd }; + }, + }; +}); + +describe('awsServicesIntegration', () => { + const mockClient = new SentryNode.NodeClient({ + tracesSampleRate: 1.0, + integrations: [], + dsn: 'https://withAWSServices@domain/123', + transport: () => SentryNode.createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), + stackParser: () => [], }); - afterEach(() => { - // @ts-expect-error see "Why @ts-expect-error" note - SentryNode.resetMocks(); + + const integration = awsServicesIntegration(); + mockClient.addIntegration(integration); + + const mockClientWithoutIntegration = new SentryNode.NodeClient({ + tracesSampleRate: 1.0, + integrations: [], + dsn: 'https://withoutAWSServices@domain/123', + transport: () => SentryNode.createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), + stackParser: () => [], }); + afterAll(() => { nock.restore(); }); + beforeEach(() => { + SentryNode.setCurrentClient(mockClient); + mockSpanEnd.mockClear(); + mockStartInactiveSpan.mockClear(); + }); + describe('S3 tracing', () => { const s3 = new AWS.S3({ accessKeyId: '-', secretAccessKey: '-' }); @@ -24,15 +55,22 @@ describe('AWSServices', () => { nock('https://foo.s3.amazonaws.com').get('/bar').reply(200, 'contents'); const data = await s3.getObject({ Bucket: 'foo', Key: 'bar' }).promise(); expect(data.Body?.toString('utf-8')).toEqual('contents'); - expect(SentryNode.startInactiveSpan).toBeCalledWith({ + expect(mockStartInactiveSpan).toBeCalledWith({ op: 'http.client', attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.serverless', }, name: 'aws.s3.getObject foo', }); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); + + expect(mockSpanEnd).toHaveBeenCalledTimes(1); + }); + + test('getObject with integration-less client', async () => { + SentryNode.setCurrentClient(mockClientWithoutIntegration); + nock('https://foo.s3.amazonaws.com').get('/bar').reply(200, 'contents'); + await s3.getObject({ Bucket: 'foo', Key: 'bar' }).promise(); + expect(mockStartInactiveSpan).not.toBeCalled(); }); test('getObject with callback', done => { @@ -43,7 +81,7 @@ describe('AWSServices', () => { expect(data.Body?.toString('utf-8')).toEqual('contents'); done(); }); - expect(SentryNode.startInactiveSpan).toBeCalledWith({ + expect(mockStartInactiveSpan).toBeCalledWith({ op: 'http.client', name: 'aws.s3.getObject foo', attributes: { @@ -51,6 +89,16 @@ describe('AWSServices', () => { }, }); }); + + test('getObject with callback with integration-less client', done => { + SentryNode.setCurrentClient(mockClientWithoutIntegration); + expect.assertions(1); + nock('https://foo.s3.amazonaws.com').get('/bar').reply(200, 'contents'); + s3.getObject({ Bucket: 'foo', Key: 'bar' }, () => { + done(); + }); + expect(mockStartInactiveSpan).not.toBeCalled(); + }); }); describe('Lambda', () => { @@ -60,13 +108,21 @@ describe('AWSServices', () => { nock('https://lambda.eu-north-1.amazonaws.com').post('/2015-03-31/functions/foo/invocations').reply(201, 'reply'); const data = await lambda.invoke({ FunctionName: 'foo' }).promise(); expect(data.Payload?.toString('utf-8')).toEqual('reply'); - expect(SentryNode.startInactiveSpan).toBeCalledWith({ + expect(mockStartInactiveSpan).toBeCalledWith({ op: 'http.client', attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.serverless', }, name: 'aws.lambda.invoke foo', }); + expect(mockSpanEnd).toHaveBeenCalledTimes(1); + }); + + test('invoke with integration-less client', async () => { + SentryNode.setCurrentClient(mockClientWithoutIntegration); + nock('https://lambda.eu-north-1.amazonaws.com').post('/2015-03-31/functions/foo/invocations').reply(201, 'reply'); + await lambda.invoke({ FunctionName: 'foo' }).promise(); + expect(mockStartInactiveSpan).not.toBeCalled(); }); }); }); From fb75d2e23e09181b77b68959d5970797db0996ae Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 24 Jan 2024 15:03:40 -0500 Subject: [PATCH 2/6] ref(serverless): Fix awslambda tests --- packages/serverless/src/awslambda.ts | 9 +- packages/serverless/src/index.ts | 3 +- packages/serverless/test/awslambda.test.ts | 203 ++++++++++++--------- 3 files changed, 125 insertions(+), 90 deletions(-) diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index 89086e7a1c77..10f0e0e29c81 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -2,7 +2,6 @@ import { existsSync } from 'fs'; import { hostname } from 'os'; import { basename, resolve } from 'path'; import { types } from 'util'; -/* eslint-disable max-lines */ import type { NodeOptions, Scope } from '@sentry/node'; import { SDK_VERSION } from '@sentry/node'; import { @@ -19,12 +18,12 @@ import { } from '@sentry/node'; import type { Integration, Options, SdkMetadata, Span } from '@sentry/types'; import { isString, logger } from '@sentry/utils'; -// NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil import type { Context, Handler } from 'aws-lambda'; import { performance } from 'perf_hooks'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; -import { AWSServices } from './awsservices'; +import { awsServicesIntegration } from './awsservices'; + import { DEBUG_BUILD } from './debug-build'; import { markEventUnhandled } from './utils'; @@ -71,12 +70,12 @@ export interface WrapperOptions { export const defaultIntegrations: Integration[] = [ // eslint-disable-next-line deprecation/deprecation ...nodeDefaultIntegrations, - new AWSServices({ optional: true }), + awsServicesIntegration({ optional: true }), ]; /** Get the default integrations for the AWSLambda SDK. */ export function getDefaultIntegrations(options: Options): Integration[] { - return [...getNodeDefaultIntegrations(options), new AWSServices({ optional: true })]; + return [...getNodeDefaultIntegrations(options), awsServicesIntegration({ optional: true })]; } interface AWSLambdaOptions extends NodeOptions { diff --git a/packages/serverless/src/index.ts b/packages/serverless/src/index.ts index 3fef9fb94283..8db1e4ba5be0 100644 --- a/packages/serverless/src/index.ts +++ b/packages/serverless/src/index.ts @@ -3,7 +3,8 @@ import * as AWSLambda from './awslambda'; import * as GCPFunction from './gcpfunction'; export { AWSLambda, GCPFunction }; -export { AWSServices } from './awsservices'; +// eslint-disable-next-line deprecation/deprecation +export { AWSServices, awsServicesIntegration } from './awsservices'; // TODO(v8): We have to explicitly export these because of the namespace exports // above. This is because just doing `export * from '@sentry/node'` will not diff --git a/packages/serverless/test/awslambda.test.ts b/packages/serverless/test/awslambda.test.ts index de851cda8bbb..57feede5a102 100644 --- a/packages/serverless/test/awslambda.test.ts +++ b/packages/serverless/test/awslambda.test.ts @@ -1,12 +1,59 @@ -// NOTE: I have no idea how to fix this right now, and don't want to waste more time, as it builds just fine — Kamil import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; -import * as SentryNode from '@sentry/node'; + import type { Event } from '@sentry/types'; import type { Callback, Handler } from 'aws-lambda'; -import * as Sentry from '../src'; +import { init, wrapHandler } from '../src/awslambda'; + +const mockSpanEnd = jest.fn(); +const mockStartInactiveSpan = jest.fn((...spanArgs) => ({ ...spanArgs })); +const mockStartSpanManual = jest.fn((...spanArgs) => ({ ...spanArgs })); +const mockFlush = jest.fn((...args) => Promise.resolve(args)); +const mockWithScope = jest.fn(); +const mockCaptureMessage = jest.fn(); +const mockCaptureException = jest.fn(); +const mockInit = jest.fn(); + +const mockScope = { + setTag: jest.fn(), + setContext: jest.fn(), + addEventProcessor: jest.fn(), +}; -const { wrapHandler } = Sentry.AWSLambda; +jest.mock('@sentry/node', () => { + const original = jest.requireActual('@sentry/node'); + return { + ...original, + init: (options: unknown) => { + mockInit(options); + }, + startInactiveSpan: (...args: unknown[]) => { + mockStartInactiveSpan(...args); + return { end: mockSpanEnd }; + }, + startSpanManual: (...args: unknown[]) => { + mockStartSpanManual(...args); + mockSpanEnd(); + return original.startSpanManual(...args); + }, + getCurrentScope: () => { + return mockScope; + }, + flush: (...args: unknown[]) => { + return mockFlush(...args); + }, + withScope: (fn: (scope: unknown) => void) => { + mockWithScope(fn); + fn(mockScope); + }, + captureMessage: (...args: unknown[]) => { + mockCaptureMessage(...args); + }, + captureException: (...args: unknown[]) => { + mockCaptureException(...args); + }, + }; +}); // Default `timeoutWarningLimit` is 500ms so leaving some space for it to trigger when necessary const DEFAULT_EXECUTION_TIME = 100; @@ -34,21 +81,18 @@ const fakeCallback: Callback = (err, result) => { }; function expectScopeSettings() { - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.addEventProcessor).toBeCalledTimes(1); + expect(mockScope.addEventProcessor).toBeCalledTimes(1); // Test than an event processor to add `transaction` is registered for the scope - // @ts-expect-error see "Why @ts-expect-error" note - const eventProcessor = SentryNode.fakeScope.addEventProcessor.mock.calls[0][0]; + const eventProcessor = mockScope.addEventProcessor.mock.calls[0][0]; const event: Event = {}; eventProcessor(event); expect(event).toEqual({ transaction: 'functionName' }); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.setTag).toBeCalledWith('server_name', expect.anything()); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.setTag).toBeCalledWith('url', 'awslambda:///functionName'); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.setContext).toBeCalledWith( + expect(mockScope.setTag).toBeCalledWith('server_name', expect.anything()); + + expect(mockScope.setTag).toBeCalledWith('url', 'awslambda:///functionName'); + + expect(mockScope.setContext).toBeCalledWith( 'aws.lambda', expect.objectContaining({ aws_request_id: 'awsRequestId', @@ -58,8 +102,8 @@ function expectScopeSettings() { remaining_time_in_millis: 100, }), ); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.setContext).toBeCalledWith( + + expect(mockScope.setContext).toBeCalledWith( 'aws.cloudwatch.logs', expect.objectContaining({ log_group: 'logGroupName', @@ -73,11 +117,8 @@ describe('AWSLambda', () => { fakeEvent = { fortySix: 'o_O', }; - }); - afterEach(() => { - // @ts-expect-error see "Why @ts-expect-error" note - SentryNode.resetMocks(); + jest.clearAllMocks(); }); describe('wrapHandler() options', () => { @@ -88,7 +129,7 @@ describe('AWSLambda', () => { const wrappedHandler = wrapHandler(handler, { flushTimeout: 1337 }); await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - expect(SentryNode.flush).toBeCalledWith(1337); + expect(mockFlush).toBeCalledWith(1337); }); test('captureTimeoutWarning enabled (default)', async () => { @@ -100,10 +141,9 @@ describe('AWSLambda', () => { const wrappedHandler = wrapHandler(handler); await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - expect(Sentry.withScope).toBeCalledTimes(1); - expect(Sentry.captureMessage).toBeCalled(); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.setTag).toBeCalledWith('timeout', '1s'); + expect(mockWithScope).toBeCalledTimes(1); + expect(mockCaptureMessage).toBeCalled(); + expect(mockScope.setTag).toBeCalledWith('timeout', '1s'); }); test('captureTimeoutWarning disabled', async () => { @@ -117,10 +157,9 @@ describe('AWSLambda', () => { }); await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - expect(Sentry.withScope).toBeCalledTimes(0); - expect(Sentry.captureMessage).not.toBeCalled(); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.setTag).not.toBeCalledWith('timeout', '1s'); + expect(mockWithScope).toBeCalledTimes(0); + expect(mockCaptureMessage).not.toBeCalled(); + expect(mockScope.setTag).not.toBeCalledWith('timeout', '1s'); }); test('captureTimeoutWarning with configured timeoutWarningLimit', async () => { @@ -149,16 +188,15 @@ describe('AWSLambda', () => { fakeCallback, ); - expect(Sentry.captureMessage).toBeCalled(); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.setTag).toBeCalledWith('timeout', '1m40s'); + expect(mockCaptureMessage).toBeCalled(); + expect(mockScope.setTag).toBeCalledWith('timeout', '1m40s'); }); test('captureAllSettledReasons disabled (default)', async () => { const handler = () => Promise.resolve([{ status: 'rejected', reason: new Error() }]); const wrappedHandler = wrapHandler(handler, { flushTimeout: 1337 }); await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - expect(SentryNode.captureException).toBeCalledTimes(0); + expect(mockCaptureException).toBeCalledTimes(0); }); test('captureAllSettledReasons enable', async () => { @@ -172,9 +210,9 @@ describe('AWSLambda', () => { ]); const wrappedHandler = wrapHandler(handler, { flushTimeout: 1337, captureAllSettledReasons: true }); await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - expect(SentryNode.captureException).toHaveBeenNthCalledWith(1, error, expect.any(Function)); - expect(SentryNode.captureException).toHaveBeenNthCalledWith(2, error2, expect.any(Function)); - expect(SentryNode.captureException).toBeCalledTimes(2); + expect(mockCaptureException).toHaveBeenNthCalledWith(1, error, expect.any(Function)); + expect(mockCaptureException).toHaveBeenNthCalledWith(2, error2, expect.any(Function)); + expect(mockCaptureException).toBeCalledTimes(2); }); // "wrapHandler() ... successful execution" tests the default of startTrace enabled @@ -185,11 +223,10 @@ describe('AWSLambda', () => { const wrappedHandler = wrapHandler(handler, { startTrace: false }); await wrappedHandler(fakeEvent, fakeContext, fakeCallback); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.addEventProcessor).toBeCalledTimes(0); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.setTag).toBeCalledTimes(0); - expect(SentryNode.startSpanManual).toBeCalledTimes(0); + expect(mockScope.addEventProcessor).toBeCalledTimes(0); + + expect(mockScope.setTag).toBeCalledTimes(0); + expect(mockStartSpanManual).toBeCalledTimes(0); }); }); @@ -214,11 +251,11 @@ describe('AWSLambda', () => { }; expect(rv).toStrictEqual(42); - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalledWith(2000); + + expect(mockSpanEnd).toBeCalled(); + expect(mockFlush).toBeCalledWith(2000); }); test('unsuccessful execution', async () => { @@ -243,12 +280,12 @@ describe('AWSLambda', () => { metadata: {}, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalledWith(2000); + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); + + expect(mockSpanEnd).toBeCalled(); + expect(mockFlush).toBeCalledWith(2000); } }); @@ -273,7 +310,7 @@ describe('AWSLambda', () => { }; const handler: Handler = (_event, _context, callback) => { - expect(SentryNode.startSpanManual).toBeCalledWith( + expect(mockStartSpanManual).toBeCalledWith( expect.objectContaining({ parentSpanId: '1121201211212012', parentSampled: false, @@ -326,12 +363,12 @@ describe('AWSLambda', () => { metadata: { dynamicSamplingContext: {} }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); - expect(SentryNode.captureException).toBeCalledWith(e, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalled(); + expect(mockCaptureException).toBeCalledWith(e, expect.any(Function)); + + expect(mockSpanEnd).toBeCalled(); + expect(mockFlush).toBeCalled(); } }); }); @@ -357,11 +394,11 @@ describe('AWSLambda', () => { }; expect(rv).toStrictEqual(42); - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalled(); + + expect(mockSpanEnd).toBeCalled(); + expect(mockFlush).toBeCalled(); }); test('event and context are correctly passed to the original handler', async () => { @@ -397,12 +434,12 @@ describe('AWSLambda', () => { metadata: {}, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalled(); + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); + + expect(mockSpanEnd).toBeCalled(); + expect(mockFlush).toBeCalled(); } }); @@ -414,9 +451,7 @@ describe('AWSLambda', () => { const wrappedHandler = wrapHandler(handler); - jest.spyOn(Sentry, 'flush').mockImplementationOnce(async () => { - throw new Error(); - }); + mockFlush.mockImplementationOnce(() => Promise.reject(new Error('wat'))); await expect(wrappedHandler(fakeEvent, fakeContext, fakeCallback)).resolves.toBe('some string'); }); @@ -443,11 +478,11 @@ describe('AWSLambda', () => { }; expect(rv).toStrictEqual(42); - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalled(); + + expect(mockSpanEnd).toBeCalled(); + expect(mockFlush).toBeCalled(); }); test('event and context are correctly passed to the original handler', async () => { @@ -483,12 +518,12 @@ describe('AWSLambda', () => { metadata: {}, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); expectScopeSettings(); - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalled(); + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); + + expect(mockSpanEnd).toBeCalled(); + expect(mockFlush).toBeCalled(); } }); }); @@ -505,9 +540,9 @@ describe('AWSLambda', () => { try { await wrappedHandler(fakeEvent, fakeContext, fakeCallback); } catch (e) { - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - const scopeFunction = SentryNode.captureException.mock.calls[0][1]; + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); + + const scopeFunction = mockCaptureException.mock.calls[0][1]; const event: Event = { exception: { values: [{}] } }; let evtProcessor: ((e: Event) => Event) | undefined = undefined; scopeFunction({ addEventProcessor: jest.fn().mockImplementation(proc => (evtProcessor = proc)) }); @@ -523,9 +558,9 @@ describe('AWSLambda', () => { describe('init()', () => { test('calls Sentry.init with correct sdk info metadata', () => { - Sentry.AWSLambda.init({}); + init({}); - expect(Sentry.init).toBeCalledWith( + expect(mockInit).toBeCalledWith( expect.objectContaining({ _metadata: { sdk: { @@ -534,10 +569,10 @@ describe('AWSLambda', () => { packages: [ { name: 'npm:@sentry/serverless', - version: '6.6.6', + version: expect.any(String), }, ], - version: '6.6.6', + version: expect.any(String), }, }, }), From e0b971d61b5fb247dd50892e29574bbadb068709 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 24 Jan 2024 15:36:55 -0500 Subject: [PATCH 3/6] ref(serverless): Convert GoogleCloudGrpc to function --- packages/serverless/src/awsservices.ts | 2 +- packages/serverless/src/gcpfunction/index.ts | 6 +- packages/serverless/src/google-cloud-grpc.ts | 85 ++++++++++--------- packages/serverless/test/awsservices.test.ts | 18 ++-- .../serverless/test/google-cloud-grpc.test.ts | 44 +++++++--- 5 files changed, 93 insertions(+), 62 deletions(-) diff --git a/packages/serverless/src/awsservices.ts b/packages/serverless/src/awsservices.ts index 4e30983b2089..161b225c5ce0 100644 --- a/packages/serverless/src/awsservices.ts +++ b/packages/serverless/src/awsservices.ts @@ -18,7 +18,7 @@ interface AWSService { const INTEGRATION_NAME = 'AWSServices'; -export const SETUP_CLIENTS = new WeakMap(); +const SETUP_CLIENTS = new WeakMap(); const _awsServicesIntegration = ((options: { optional?: boolean } = {}) => { const optional = options.optional || false; diff --git a/packages/serverless/src/gcpfunction/index.ts b/packages/serverless/src/gcpfunction/index.ts index 3907e84aabc8..4528828044ec 100644 --- a/packages/serverless/src/gcpfunction/index.ts +++ b/packages/serverless/src/gcpfunction/index.ts @@ -7,7 +7,7 @@ import { } from '@sentry/node'; import type { Integration, Options, SdkMetadata } from '@sentry/types'; -import { GoogleCloudGrpc } from '../google-cloud-grpc'; +import { googleCloudGrpcIntegration } from '../google-cloud-grpc'; import { GoogleCloudHttp } from '../google-cloud-http'; export * from './http'; @@ -19,7 +19,7 @@ export const defaultIntegrations: Integration[] = [ // eslint-disable-next-line deprecation/deprecation ...defaultNodeIntegrations, new GoogleCloudHttp({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing. - new GoogleCloudGrpc({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing. + googleCloudGrpcIntegration({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing. ]; /** Get the default integrations for the GCP SDK. */ @@ -27,7 +27,7 @@ export function getDefaultIntegrations(options: Options): Integration[] { return [ ...getDefaultNodeIntegrations(options), new GoogleCloudHttp({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing. - new GoogleCloudGrpc({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing. + googleCloudGrpcIntegration({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing. ]; } diff --git a/packages/serverless/src/google-cloud-grpc.ts b/packages/serverless/src/google-cloud-grpc.ts index ba2ddb038a03..9124ec9d3bcf 100644 --- a/packages/serverless/src/google-cloud-grpc.ts +++ b/packages/serverless/src/google-cloud-grpc.ts @@ -1,7 +1,12 @@ import type { EventEmitter } from 'events'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + convertIntegrationFnToClass, + defineIntegration, + getClient, +} from '@sentry/core'; import { startInactiveSpan } from '@sentry/node'; -import type { Integration } from '@sentry/types'; +import type { Client, Integration, IntegrationClass, IntegrationFn } from '@sentry/types'; import { fill } from '@sentry/utils'; interface GrpcFunction extends CallableFunction { @@ -26,45 +31,49 @@ interface Stub { [key: string]: GrpcFunctionObject; } -/** Google Cloud Platform service requests tracking for GRPC APIs */ -export class GoogleCloudGrpc implements Integration { - /** - * @inheritDoc - */ - public static id: string = 'GoogleCloudGrpc'; +const SERVICE_PATH_REGEX = /^(\w+)\.googleapis.com$/; - /** - * @inheritDoc - */ - public name: string; +const INTEGRATION_NAME = 'GoogleCloudGrpc'; - private readonly _optional: boolean; +const SETUP_CLIENTS = new WeakMap(); - public constructor(options: { optional?: boolean } = {}) { - this.name = GoogleCloudGrpc.id; +const _googleCloudGrpcIntegration = ((options: { optional?: boolean } = {}) => { + const optional = options.optional || false; + return { + name: INTEGRATION_NAME, + setupOnce() { + try { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const gaxModule = require('google-gax'); + fill( + gaxModule.GrpcClient.prototype, // eslint-disable-line @typescript-eslint/no-unsafe-member-access + 'createStub', + wrapCreateStub, + ); + } catch (e) { + if (!optional) { + throw e; + } + } + }, + setup(client) { + SETUP_CLIENTS.set(client, true); + }, + }; +}) satisfies IntegrationFn; - this._optional = options.optional || false; - } +export const googleCloudGrpcIntegration = defineIntegration(_googleCloudGrpcIntegration); - /** - * @inheritDoc - */ - public setupOnce(): void { - try { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const gaxModule = require('google-gax'); - fill( - gaxModule.GrpcClient.prototype, // eslint-disable-line @typescript-eslint/no-unsafe-member-access - 'createStub', - wrapCreateStub, - ); - } catch (e) { - if (!this._optional) { - throw e; - } - } - } -} +/** + * Google Cloud Platform service requests tracking for GRPC APIs. + * + * @deprecated Use `googleCloudGrpcIntegration()` instead. + */ +// eslint-disable-next-line deprecation/deprecation +export const GoogleCloudGrpc = convertIntegrationFnToClass( + INTEGRATION_NAME, + googleCloudGrpcIntegration, +) as IntegrationClass; /** Returns a wrapped function that returns a stub with tracing enabled */ function wrapCreateStub(origCreate: CreateStubFunc): CreateStubFunc { @@ -105,7 +114,7 @@ function fillGrpcFunction(stub: Stub, serviceIdentifier: string, methodName: str (orig: GrpcFunction): GrpcFunction => (...args) => { const ret = orig.apply(stub, args); - if (typeof ret?.on !== 'function') { + if (typeof ret?.on !== 'function' || !SETUP_CLIENTS.has(getClient() as Client)) { return ret; } const span = startInactiveSpan({ @@ -127,6 +136,6 @@ function fillGrpcFunction(stub: Stub, serviceIdentifier: string, methodName: str /** Identifies service by its address */ function identifyService(servicePath: string): string { - const match = servicePath.match(/^(\w+)\.googleapis.com$/); + const match = servicePath.match(SERVICE_PATH_REGEX); return match ? match[1] : servicePath; } diff --git a/packages/serverless/test/awsservices.test.ts b/packages/serverless/test/awsservices.test.ts index 94660b329a0e..bbfc3240c3ac 100644 --- a/packages/serverless/test/awsservices.test.ts +++ b/packages/serverless/test/awsservices.test.ts @@ -1,4 +1,4 @@ -import * as SentryNode from '@sentry/node'; +import { NodeClient, createTransport, setCurrentClient } from '@sentry/node'; import * as AWS from 'aws-sdk'; import * as nock from 'nock'; @@ -19,22 +19,22 @@ jest.mock('@sentry/node', () => { }); describe('awsServicesIntegration', () => { - const mockClient = new SentryNode.NodeClient({ + const mockClient = new NodeClient({ tracesSampleRate: 1.0, integrations: [], dsn: 'https://withAWSServices@domain/123', - transport: () => SentryNode.createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), stackParser: () => [], }); const integration = awsServicesIntegration(); mockClient.addIntegration(integration); - const mockClientWithoutIntegration = new SentryNode.NodeClient({ + const mockClientWithoutIntegration = new NodeClient({ tracesSampleRate: 1.0, integrations: [], dsn: 'https://withoutAWSServices@domain/123', - transport: () => SentryNode.createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), stackParser: () => [], }); @@ -43,7 +43,7 @@ describe('awsServicesIntegration', () => { }); beforeEach(() => { - SentryNode.setCurrentClient(mockClient); + setCurrentClient(mockClient); mockSpanEnd.mockClear(); mockStartInactiveSpan.mockClear(); }); @@ -67,7 +67,7 @@ describe('awsServicesIntegration', () => { }); test('getObject with integration-less client', async () => { - SentryNode.setCurrentClient(mockClientWithoutIntegration); + setCurrentClient(mockClientWithoutIntegration); nock('https://foo.s3.amazonaws.com').get('/bar').reply(200, 'contents'); await s3.getObject({ Bucket: 'foo', Key: 'bar' }).promise(); expect(mockStartInactiveSpan).not.toBeCalled(); @@ -91,7 +91,7 @@ describe('awsServicesIntegration', () => { }); test('getObject with callback with integration-less client', done => { - SentryNode.setCurrentClient(mockClientWithoutIntegration); + setCurrentClient(mockClientWithoutIntegration); expect.assertions(1); nock('https://foo.s3.amazonaws.com').get('/bar').reply(200, 'contents'); s3.getObject({ Bucket: 'foo', Key: 'bar' }, () => { @@ -119,7 +119,7 @@ describe('awsServicesIntegration', () => { }); test('invoke with integration-less client', async () => { - SentryNode.setCurrentClient(mockClientWithoutIntegration); + setCurrentClient(mockClientWithoutIntegration); nock('https://lambda.eu-north-1.amazonaws.com').post('/2015-03-31/functions/foo/invocations').reply(201, 'reply'); await lambda.invoke({ FunctionName: 'foo' }).promise(); expect(mockStartInactiveSpan).not.toBeCalled(); diff --git a/packages/serverless/test/google-cloud-grpc.test.ts b/packages/serverless/test/google-cloud-grpc.test.ts index 8c0e0866bf0c..c2cd1b3167db 100644 --- a/packages/serverless/test/google-cloud-grpc.test.ts +++ b/packages/serverless/test/google-cloud-grpc.test.ts @@ -5,15 +5,28 @@ import { EventEmitter } from 'events'; import * as fs from 'fs'; import * as path from 'path'; import { PubSub } from '@google-cloud/pubsub'; -import * as SentryNode from '@sentry/node'; import * as http2 from 'http2'; import * as nock from 'nock'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; -import { GoogleCloudGrpc } from '../src/google-cloud-grpc'; +import { NodeClient, createTransport, setCurrentClient } from '@sentry/node'; +import { googleCloudGrpcIntegration } from '../src/google-cloud-grpc'; const spyConnect = jest.spyOn(http2, 'connect'); +const mockSpanEnd = jest.fn(); +const mockStartInactiveSpan = jest.fn(spanArgs => ({ ...spanArgs })); + +jest.mock('@sentry/node', () => { + return { + ...jest.requireActual('@sentry/node'), + startInactiveSpan: (ctx: unknown) => { + mockStartInactiveSpan(ctx); + return { end: mockSpanEnd }; + }, + }; +}); + /** Fake HTTP2 stream */ class FakeStream extends EventEmitter { public rstCode: number = 0; @@ -71,18 +84,24 @@ function mockHttp2Session(): FakeSession { } describe('GoogleCloudGrpc tracing', () => { - beforeAll(() => { - new GoogleCloudGrpc().setupOnce(); + const mockClient = new NodeClient({ + tracesSampleRate: 1.0, + integrations: [], + dsn: 'https://withAWSServices@domain/123', + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), + stackParser: () => [], }); + const integration = googleCloudGrpcIntegration(); + mockClient.addIntegration(integration); + beforeEach(() => { nock('https://www.googleapis.com').post('/oauth2/v4/token').reply(200, []); + setCurrentClient(mockClient); + mockSpanEnd.mockClear(); + mockStartInactiveSpan.mockClear(); }); - afterEach(() => { - // @ts-expect-error see "Why @ts-expect-error" note - SentryNode.resetMocks(); - spyConnect.mockClear(); - }); + afterAll(() => { nock.restore(); spyConnect.mockRestore(); @@ -116,18 +135,21 @@ describe('GoogleCloudGrpc tracing', () => { resolveTxt.mockReset(); }); + afterAll(async () => { + await pubsub.close(); + }); + test('publish', async () => { mockHttp2Session().mockUnaryRequest(Buffer.from('00000000120a1031363337303834313536363233383630', 'hex')); const resp = await pubsub.topic('nicetopic').publish(Buffer.from('data')); expect(resp).toEqual('1637084156623860'); - expect(SentryNode.startInactiveSpan).toBeCalledWith({ + expect(mockStartInactiveSpan).toBeCalledWith({ op: 'grpc.pubsub', attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.grpc.serverless', }, name: 'unary call publish', }); - await pubsub.close(); }); }); }); From f4fa59315937b361c4d29c232770467d08ba7654 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 24 Jan 2024 15:45:06 -0500 Subject: [PATCH 4/6] ref(serverless): Convert GoogleCloudHttp to function --- packages/serverless/src/gcpfunction/index.ts | 6 +- packages/serverless/src/google-cloud-http.ts | 91 ++++++++++--------- .../serverless/test/google-cloud-http.test.ts | 41 +++++++-- 3 files changed, 83 insertions(+), 55 deletions(-) diff --git a/packages/serverless/src/gcpfunction/index.ts b/packages/serverless/src/gcpfunction/index.ts index 4528828044ec..50556634c5fc 100644 --- a/packages/serverless/src/gcpfunction/index.ts +++ b/packages/serverless/src/gcpfunction/index.ts @@ -8,7 +8,7 @@ import { import type { Integration, Options, SdkMetadata } from '@sentry/types'; import { googleCloudGrpcIntegration } from '../google-cloud-grpc'; -import { GoogleCloudHttp } from '../google-cloud-http'; +import { googleCloudHttpIntegration } from '../google-cloud-http'; export * from './http'; export * from './events'; @@ -18,7 +18,7 @@ export * from './cloud_events'; export const defaultIntegrations: Integration[] = [ // eslint-disable-next-line deprecation/deprecation ...defaultNodeIntegrations, - new GoogleCloudHttp({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing. + googleCloudHttpIntegration({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing. googleCloudGrpcIntegration({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing. ]; @@ -26,7 +26,7 @@ export const defaultIntegrations: Integration[] = [ export function getDefaultIntegrations(options: Options): Integration[] { return [ ...getDefaultNodeIntegrations(options), - new GoogleCloudHttp({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing. + googleCloudHttpIntegration({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing. googleCloudGrpcIntegration({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing. ]; } diff --git a/packages/serverless/src/google-cloud-http.ts b/packages/serverless/src/google-cloud-http.ts index 769519013785..dfe0fd1a50b2 100644 --- a/packages/serverless/src/google-cloud-http.ts +++ b/packages/serverless/src/google-cloud-http.ts @@ -1,9 +1,12 @@ -// '@google-cloud/common' import is expected to be type-only so it's erased in the final .js file. -// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here. import type * as common from '@google-cloud/common'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + convertIntegrationFnToClass, + defineIntegration, + getClient, +} from '@sentry/core'; import { startInactiveSpan } from '@sentry/node'; -import type { Integration } from '@sentry/types'; +import type { Client, Integration, IntegrationClass, IntegrationFn } from '@sentry/types'; import { fill } from '@sentry/utils'; type RequestOptions = common.DecorateRequestOptions; @@ -13,53 +16,57 @@ interface RequestFunction extends CallableFunction { (reqOpts: RequestOptions, callback: ResponseCallback): void; } -/** Google Cloud Platform service requests tracking for RESTful APIs */ -export class GoogleCloudHttp implements Integration { - /** - * @inheritDoc - */ - public static id: string = 'GoogleCloudHttp'; +const INTEGRATION_NAME = 'GoogleCloudHttp'; - /** - * @inheritDoc - */ - public name: string; +const SETUP_CLIENTS = new WeakMap(); - private readonly _optional: boolean; - - public constructor(options: { optional?: boolean } = {}) { - this.name = GoogleCloudHttp.id; +const _googleCloudHttpIntegration = ((options: { optional?: boolean } = {}) => { + const optional = options.optional || false; + return { + name: INTEGRATION_NAME, + setupOnce() { + try { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const commonModule = require('@google-cloud/common') as typeof common; + fill(commonModule.Service.prototype, 'request', wrapRequestFunction); + } catch (e) { + if (!optional) { + throw e; + } + } + }, + setup(client) { + SETUP_CLIENTS.set(client, true); + }, + }; +}) satisfies IntegrationFn; - this._optional = options.optional || false; - } +export const googleCloudHttpIntegration = defineIntegration(_googleCloudHttpIntegration); - /** - * @inheritDoc - */ - public setupOnce(): void { - try { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const commonModule = require('@google-cloud/common') as typeof common; - fill(commonModule.Service.prototype, 'request', wrapRequestFunction); - } catch (e) { - if (!this._optional) { - throw e; - } - } - } -} +/** + * Google Cloud Platform service requests tracking for RESTful APIs. + * + * @deprecated Use `googleCloudHttpIntegration()` instead. + */ +// eslint-disable-next-line deprecation/deprecation +export const GoogleCloudHttp = convertIntegrationFnToClass( + INTEGRATION_NAME, + googleCloudHttpIntegration, +) as IntegrationClass; /** Returns a wrapped function that makes a request with tracing enabled */ function wrapRequestFunction(orig: RequestFunction): RequestFunction { return function (this: common.Service, reqOpts: RequestOptions, callback: ResponseCallback): void { const httpMethod = reqOpts.method || 'GET'; - const span = startInactiveSpan({ - name: `${httpMethod} ${reqOpts.uri}`, - op: `http.client.${identifyService(this.apiEndpoint)}`, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.serverless', - }, - }); + const span = SETUP_CLIENTS.has(getClient() as Client) + ? startInactiveSpan({ + name: `${httpMethod} ${reqOpts.uri}`, + op: `http.client.${identifyService(this.apiEndpoint)}`, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.serverless', + }, + }) + : undefined; orig.call(this, reqOpts, (...args: Parameters) => { if (span) { span.end(); diff --git a/packages/serverless/test/google-cloud-http.test.ts b/packages/serverless/test/google-cloud-http.test.ts index 748841e58579..6d64bd68624f 100644 --- a/packages/serverless/test/google-cloud-http.test.ts +++ b/packages/serverless/test/google-cloud-http.test.ts @@ -1,25 +1,46 @@ import * as fs from 'fs'; import * as path from 'path'; import { BigQuery } from '@google-cloud/bigquery'; -import * as SentryNode from '@sentry/node'; import * as nock from 'nock'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; -import { GoogleCloudHttp } from '../src/google-cloud-http'; +import { NodeClient, createTransport, setCurrentClient } from '@sentry/node'; +import { googleCloudHttpIntegration } from '../src/google-cloud-http'; + +const mockSpanEnd = jest.fn(); +const mockStartInactiveSpan = jest.fn(spanArgs => ({ ...spanArgs })); + +jest.mock('@sentry/node', () => { + return { + ...jest.requireActual('@sentry/node'), + startInactiveSpan: (ctx: unknown) => { + mockStartInactiveSpan(ctx); + return { end: mockSpanEnd }; + }, + }; +}); describe('GoogleCloudHttp tracing', () => { - beforeAll(() => { - new GoogleCloudHttp().setupOnce(); + const mockClient = new NodeClient({ + tracesSampleRate: 1.0, + integrations: [], + dsn: 'https://withAWSServices@domain/123', + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), + stackParser: () => [], }); + + const integration = googleCloudHttpIntegration(); + mockClient.addIntegration(integration); + beforeEach(() => { nock('https://www.googleapis.com') .post('/oauth2/v4/token') .reply(200, '{"access_token":"a.b.c","expires_in":3599,"token_type":"Bearer"}'); + setCurrentClient(mockClient); + mockSpanEnd.mockClear(); + mockStartInactiveSpan.mockClear(); }); - afterEach(() => { - // @ts-expect-error see "Why @ts-expect-error" note - SentryNode.resetMocks(); - }); + afterAll(() => { nock.restore(); }); @@ -51,14 +72,14 @@ describe('GoogleCloudHttp tracing', () => { ); const resp = await bigquery.query('SELECT true AS foo'); expect(resp).toEqual([[{ foo: true }]]); - expect(SentryNode.startInactiveSpan).toBeCalledWith({ + expect(mockStartInactiveSpan).toBeCalledWith({ op: 'http.client.bigquery', name: 'POST /jobs', attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.serverless', }, }); - expect(SentryNode.startInactiveSpan).toBeCalledWith({ + expect(mockStartInactiveSpan).toBeCalledWith({ op: 'http.client.bigquery', name: expect.stringMatching(/^GET \/queries\/.+/), attributes: { From d3006bea61ccd8d8220f20fe7038aa056332a105 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 24 Jan 2024 16:28:34 -0500 Subject: [PATCH 5/6] ref(serverless): Fix GCPFunction tests --- packages/serverless/test/gcpfunction.test.ts | 202 +++++++++++-------- 1 file changed, 119 insertions(+), 83 deletions(-) diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts index 8fb51d3bf368..d4cefc7b3cb1 100644 --- a/packages/serverless/test/gcpfunction.test.ts +++ b/packages/serverless/test/gcpfunction.test.ts @@ -1,9 +1,8 @@ import * as domain from 'domain'; -import * as SentryNode from '@sentry/node'; import type { Event, Integration } from '@sentry/types'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; -import * as Sentry from '../src'; + import { wrapCloudEventFunction, wrapEventFunction, wrapHttpFunction } from '../src/gcpfunction'; import type { CloudEventFunction, @@ -15,10 +14,65 @@ import type { Response, } from '../src/gcpfunction/general'; +import { init } from '../src/gcpfunction'; + +const mockStartInactiveSpan = jest.fn((...spanArgs) => ({ ...spanArgs })); +const mockStartSpanManual = jest.fn((...spanArgs) => ({ ...spanArgs })); +const mockFlush = jest.fn((...args) => Promise.resolve(args)); +const mockWithScope = jest.fn(); +const mockCaptureMessage = jest.fn(); +const mockCaptureException = jest.fn(); +const mockInit = jest.fn(); + +const mockScope = { + setTag: jest.fn(), + setContext: jest.fn(), + addEventProcessor: jest.fn(), + setSDKProcessingMetadata: jest.fn(), +}; + +const mockSpan = { + end: jest.fn(), +}; + +jest.mock('@sentry/node', () => { + const original = jest.requireActual('@sentry/node'); + return { + ...original, + init: (options: unknown) => { + mockInit(options); + }, + startInactiveSpan: (...args: unknown[]) => { + mockStartInactiveSpan(...args); + return mockSpan; + }, + startSpanManual: (...args: unknown[]) => { + mockStartSpanManual(...args); + mockSpan.end(); + return original.startSpanManual(...args); + }, + getCurrentScope: () => { + return mockScope; + }, + flush: (...args: unknown[]) => { + return mockFlush(...args); + }, + withScope: (fn: (scope: unknown) => void) => { + mockWithScope(fn); + fn(mockScope); + }, + captureMessage: (...args: unknown[]) => { + mockCaptureMessage(...args); + }, + captureException: (...args: unknown[]) => { + mockCaptureException(...args); + }, + }; +}); + describe('GCPFunction', () => { - afterEach(() => { - // @ts-expect-error see "Why @ts-expect-error" note - SentryNode.resetMocks(); + beforeEach(() => { + jest.clearAllMocks(); }); async function handleHttp(fn: HttpFunction, trace_headers: { [key: string]: string } | null = null): Promise { @@ -89,7 +143,7 @@ describe('GCPFunction', () => { const wrappedHandler = wrapHttpFunction(handler, { flushTimeout: 1337 }); await handleHttp(wrappedHandler); - expect(SentryNode.flush).toBeCalledWith(1337); + expect(mockFlush).toBeCalledWith(1337); }); }); @@ -112,12 +166,9 @@ describe('GCPFunction', () => { metadata: {}, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.setHttpStatus).toBeCalledWith(200); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalledWith(2000); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockSpan.end).toBeCalled(); + expect(mockFlush).toBeCalledWith(2000); }); test('incoming trace headers are correctly parsed and used', async () => { @@ -149,7 +200,7 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); }); test('capture error', async () => { @@ -178,11 +229,10 @@ describe('GCPFunction', () => { metadata: { dynamicSamplingContext: {} }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalled(); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); + expect(mockSpan.end).toBeCalled(); + expect(mockFlush).toBeCalled(); }); test('should not throw when flush rejects', async () => { @@ -203,7 +253,7 @@ describe('GCPFunction', () => { const mockEnd = jest.fn(); const response = { end: mockEnd } as unknown as Response; - jest.spyOn(Sentry, 'flush').mockImplementationOnce(async () => { + mockFlush.mockImplementationOnce(async () => { throw new Error(); }); @@ -216,7 +266,7 @@ describe('GCPFunction', () => { // 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 () => { - Sentry.GCPFunction.init({}); + init({}); const handler: HttpFunction = (_req, res) => { res.end(); @@ -225,13 +275,12 @@ describe('GCPFunction', () => { await handleHttp(wrappedHandler); - const initOptions = (SentryNode.init as unknown as jest.SpyInstance).mock.calls[0]; + const initOptions = (mockInit as unknown as jest.SpyInstance).mock.calls[0]; const defaultIntegrations = initOptions[0].defaultIntegrations.map((i: Integration) => i.name); expect(defaultIntegrations).toContain('RequestData'); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.setSDKProcessingMetadata).toHaveBeenCalledWith({ + expect(mockScope.setSDKProcessingMetadata).toHaveBeenCalledWith({ request: { method: 'POST', url: '/path?q=query', @@ -259,10 +308,9 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalledWith(2000); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockSpan.end).toBeCalled(); + expect(mockFlush).toBeCalledWith(2000); }); test('capture error', async () => { @@ -282,11 +330,10 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalled(); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); + expect(mockSpan.end).toBeCalled(); + expect(mockFlush).toBeCalled(); }); }); @@ -310,10 +357,9 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalledWith(2000); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockSpan.end).toBeCalled(); + expect(mockFlush).toBeCalledWith(2000); }); test('capture error', async () => { @@ -337,11 +383,10 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalled(); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); + expect(mockSpan.end).toBeCalled(); + expect(mockFlush).toBeCalled(); }); }); @@ -362,10 +407,9 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalledWith(2000); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockSpan.end).toBeCalled(); + expect(mockFlush).toBeCalledWith(2000); }); test('capture error', async () => { @@ -385,11 +429,10 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalled(); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); + expect(mockSpan.end).toBeCalled(); + expect(mockFlush).toBeCalled(); }); test('capture exception', async () => { @@ -409,8 +452,8 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); }); }); @@ -422,10 +465,9 @@ describe('GCPFunction', () => { const wrappedHandler = wrapEventFunction(handler); await expect(handleEvent(wrappedHandler)).rejects.toThrowError(error); - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); - // @ts-expect-error just mocking around... - const scopeFunction = SentryNode.captureException.mock.calls[0][1]; + const scopeFunction = mockCaptureException.mock.calls[0][1]; const event: Event = { exception: { values: [{}] } }; let evtProcessor: ((e: Event) => Event) | undefined = undefined; scopeFunction({ addEventProcessor: jest.fn().mockImplementation(proc => (evtProcessor = proc)) }); @@ -442,8 +484,7 @@ describe('GCPFunction', () => { const handler: EventFunction = (_data, _context) => 42; const wrappedHandler = wrapEventFunction(handler); await handleEvent(wrappedHandler); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.setContext).toBeCalledWith('gcp.function.context', { + expect(mockScope.setContext).toBeCalledWith('gcp.function.context', { eventType: 'event.type', resource: 'some.resource', }); @@ -466,10 +507,9 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalledWith(2000); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockSpan.end).toBeCalled(); + expect(mockFlush).toBeCalledWith(2000); }); test('capture error', async () => { @@ -489,11 +529,10 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalled(); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); + expect(mockSpan.end).toBeCalled(); + expect(mockFlush).toBeCalled(); }); }); @@ -514,10 +553,9 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalledWith(2000); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockSpan.end).toBeCalled(); + expect(mockFlush).toBeCalledWith(2000); }); test('capture error', async () => { @@ -537,11 +575,10 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeSpan.end).toBeCalled(); - expect(SentryNode.flush).toBeCalled(); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); + expect(mockSpan.end).toBeCalled(); + expect(mockFlush).toBeCalled(); }); test('capture exception', async () => { @@ -561,8 +598,8 @@ describe('GCPFunction', () => { }, }; - expect(SentryNode.startSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); - expect(SentryNode.captureException).toBeCalledWith(error, expect.any(Function)); + expect(mockStartSpanManual).toBeCalledWith(fakeTransactionContext, expect.any(Function)); + expect(mockCaptureException).toBeCalledWith(error, expect.any(Function)); }); }); @@ -570,15 +607,14 @@ describe('GCPFunction', () => { const handler: CloudEventFunction = _context => 42; const wrappedHandler = wrapCloudEventFunction(handler); await handleCloudEvent(wrappedHandler); - // @ts-expect-error see "Why @ts-expect-error" note - expect(SentryNode.fakeScope.setContext).toBeCalledWith('gcp.function.context', { type: 'event.type' }); + expect(mockScope.setContext).toBeCalledWith('gcp.function.context', { type: 'event.type' }); }); describe('init()', () => { test('calls Sentry.init with correct sdk info metadata', () => { - Sentry.GCPFunction.init({}); + init({}); - expect(Sentry.init).toBeCalledWith( + expect(mockInit).toBeCalledWith( expect.objectContaining({ _metadata: { sdk: { @@ -587,10 +623,10 @@ describe('GCPFunction', () => { packages: [ { name: 'npm:@sentry/serverless', - version: '6.6.6', + version: expect.any(String), }, ], - version: '6.6.6', + version: expect.any(String), }, }, }), From 6b2a1747fc9f5804eb71e02d34463d2acca10884 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 25 Jan 2024 16:44:50 -0500 Subject: [PATCH 6/6] re-export type --- packages/serverless/src/awsservices.ts | 3 +++ packages/serverless/src/google-cloud-grpc.ts | 3 +++ packages/serverless/src/google-cloud-http.ts | 3 +++ 3 files changed, 9 insertions(+) diff --git a/packages/serverless/src/awsservices.ts b/packages/serverless/src/awsservices.ts index 161b225c5ce0..84e6ec93ff25 100644 --- a/packages/serverless/src/awsservices.ts +++ b/packages/serverless/src/awsservices.ts @@ -54,6 +54,9 @@ export const AWSServices = convertIntegrationFnToClass( awsServicesIntegration, ) as IntegrationClass; +// eslint-disable-next-line deprecation/deprecation +export type AWSServices = typeof AWSServices; + /** * Patches AWS SDK request to create `http.client` spans. */ diff --git a/packages/serverless/src/google-cloud-grpc.ts b/packages/serverless/src/google-cloud-grpc.ts index 9124ec9d3bcf..1423e2b3ad5a 100644 --- a/packages/serverless/src/google-cloud-grpc.ts +++ b/packages/serverless/src/google-cloud-grpc.ts @@ -75,6 +75,9 @@ export const GoogleCloudGrpc = convertIntegrationFnToClass( googleCloudGrpcIntegration, ) as IntegrationClass; +// eslint-disable-next-line deprecation/deprecation +export type GoogleCloudGrpc = typeof GoogleCloudGrpc; + /** Returns a wrapped function that returns a stub with tracing enabled */ function wrapCreateStub(origCreate: CreateStubFunc): CreateStubFunc { return async function (this: unknown, ...args: Parameters) { diff --git a/packages/serverless/src/google-cloud-http.ts b/packages/serverless/src/google-cloud-http.ts index dfe0fd1a50b2..ee300e78a010 100644 --- a/packages/serverless/src/google-cloud-http.ts +++ b/packages/serverless/src/google-cloud-http.ts @@ -54,6 +54,9 @@ export const GoogleCloudHttp = convertIntegrationFnToClass( googleCloudHttpIntegration, ) as IntegrationClass; +// eslint-disable-next-line deprecation/deprecation +export type GoogleCloudHttp = typeof GoogleCloudHttp; + /** Returns a wrapped function that makes a request with tracing enabled */ function wrapRequestFunction(orig: RequestFunction): RequestFunction { return function (this: common.Service, reqOpts: RequestOptions, callback: ResponseCallback): void {