Skip to content

Commit ea85419

Browse files
authored
ref(serverless): Convert integrations to functional approach (#10329)
This PR updates the serverless integrations to use the functional approach, and rewrites all of the serverless unit tests. In the unit test rewrite we remove the usage of global mocks and instead do per file mocks. I dislike the amount of mocks that still exist, but to prevent any regressions I only tried to translate the test code.
1 parent 9690d7d commit ea85419

File tree

12 files changed

+541
-396
lines changed

12 files changed

+541
-396
lines changed

packages/serverless/src/awslambda.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { existsSync } from 'fs';
22
import { hostname } from 'os';
33
import { basename, resolve } from 'path';
44
import { types } from 'util';
5-
/* eslint-disable max-lines */
65
import type { NodeOptions, Scope } from '@sentry/node';
76
import { SDK_VERSION } from '@sentry/node';
87
import {
@@ -19,12 +18,12 @@ import {
1918
} from '@sentry/node';
2019
import type { Integration, Options, SdkMetadata, Span } from '@sentry/types';
2120
import { isString, logger } from '@sentry/utils';
22-
// 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
2321
import type { Context, Handler } from 'aws-lambda';
2422
import { performance } from 'perf_hooks';
2523

2624
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
27-
import { AWSServices } from './awsservices';
25+
import { awsServicesIntegration } from './awsservices';
26+
2827
import { DEBUG_BUILD } from './debug-build';
2928
import { markEventUnhandled } from './utils';
3029

@@ -71,12 +70,12 @@ export interface WrapperOptions {
7170
export const defaultIntegrations: Integration[] = [
7271
// eslint-disable-next-line deprecation/deprecation
7372
...nodeDefaultIntegrations,
74-
new AWSServices({ optional: true }),
73+
awsServicesIntegration({ optional: true }),
7574
];
7675

7776
/** Get the default integrations for the AWSLambda SDK. */
7877
export function getDefaultIntegrations(options: Options): Integration[] {
79-
return [...getNodeDefaultIntegrations(options), new AWSServices({ optional: true })];
78+
return [...getNodeDefaultIntegrations(options), awsServicesIntegration({ optional: true })];
8079
}
8180

8281
interface AWSLambdaOptions extends NodeOptions {

packages/serverless/src/awsservices.ts

Lines changed: 57 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
2-
import { startInactiveSpan } from '@sentry/node';
3-
import type { Integration, Span } from '@sentry/types';
1+
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, convertIntegrationFnToClass, defineIntegration } from '@sentry/core';
2+
import { getClient, startInactiveSpan } from '@sentry/node';
3+
import type { Client, Integration, IntegrationClass, IntegrationFn, Span } from '@sentry/types';
44
import { fill } from '@sentry/utils';
55
// 'aws-sdk/global' import is expected to be type-only so it's erased in the final .js file.
66
// 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,73 @@ interface AWSService {
1616
serviceIdentifier: string;
1717
}
1818

19-
/** AWS service requests tracking */
20-
export class AWSServices implements Integration {
21-
/**
22-
* @inheritDoc
23-
*/
24-
public static id: string = 'AWSServices';
19+
const INTEGRATION_NAME = 'AWSServices';
2520

26-
/**
27-
* @inheritDoc
28-
*/
29-
public name: string;
21+
const SETUP_CLIENTS = new WeakMap<Client, boolean>();
3022

31-
private readonly _optional: boolean;
23+
const _awsServicesIntegration = ((options: { optional?: boolean } = {}) => {
24+
const optional = options.optional || false;
25+
return {
26+
name: INTEGRATION_NAME,
27+
setupOnce() {
28+
try {
29+
// eslint-disable-next-line @typescript-eslint/no-var-requires
30+
const awsModule = require('aws-sdk/global') as typeof AWS;
31+
fill(awsModule.Service.prototype, 'makeRequest', wrapMakeRequest);
32+
} catch (e) {
33+
if (!optional) {
34+
throw e;
35+
}
36+
}
37+
},
38+
setup(client) {
39+
SETUP_CLIENTS.set(client, true);
40+
},
41+
};
42+
}) satisfies IntegrationFn;
3243

33-
public constructor(options: { optional?: boolean } = {}) {
34-
this.name = AWSServices.id;
44+
export const awsServicesIntegration = defineIntegration(_awsServicesIntegration);
3545

36-
this._optional = options.optional || false;
37-
}
46+
/**
47+
* AWS Service Request Tracking.
48+
*
49+
* @deprecated Use `awsServicesIntegration()` instead.
50+
*/
51+
// eslint-disable-next-line deprecation/deprecation
52+
export const AWSServices = convertIntegrationFnToClass(
53+
INTEGRATION_NAME,
54+
awsServicesIntegration,
55+
) as IntegrationClass<Integration>;
3856

39-
/**
40-
* @inheritDoc
41-
*/
42-
public setupOnce(): void {
43-
try {
44-
// eslint-disable-next-line @typescript-eslint/no-var-requires
45-
const awsModule = require('aws-sdk/global') as typeof AWS;
46-
fill(awsModule.Service.prototype, 'makeRequest', wrapMakeRequest);
47-
} catch (e) {
48-
if (!this._optional) {
49-
throw e;
50-
}
51-
}
52-
}
53-
}
57+
// eslint-disable-next-line deprecation/deprecation
58+
export type AWSServices = typeof AWSServices;
5459

55-
/** */
60+
/**
61+
* Patches AWS SDK request to create `http.client` spans.
62+
*/
5663
function wrapMakeRequest<TService extends AWSService, TResult>(
5764
orig: MakeRequestFunction<GenericParams, TResult>,
5865
): MakeRequestFunction<GenericParams, TResult> {
5966
return function (this: TService, operation: string, params?: GenericParams, callback?: MakeRequestCallback<TResult>) {
6067
let span: Span | undefined;
6168
const req = orig.call(this, operation, params);
62-
req.on('afterBuild', () => {
63-
span = startInactiveSpan({
64-
name: describe(this, operation, params),
65-
op: 'http.client',
66-
attributes: {
67-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.serverless',
68-
},
69+
70+
if (SETUP_CLIENTS.has(getClient() as Client)) {
71+
req.on('afterBuild', () => {
72+
span = startInactiveSpan({
73+
name: describe(this, operation, params),
74+
op: 'http.client',
75+
attributes: {
76+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.serverless',
77+
},
78+
});
6979
});
70-
});
71-
req.on('complete', () => {
72-
if (span) {
73-
span.end();
74-
}
75-
});
80+
req.on('complete', () => {
81+
if (span) {
82+
span.end();
83+
}
84+
});
85+
}
7686

7787
if (callback) {
7888
req.send(callback);

packages/serverless/src/gcpfunction/index.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import {
77
} from '@sentry/node';
88
import type { Integration, Options, SdkMetadata } from '@sentry/types';
99

10-
import { GoogleCloudGrpc } from '../google-cloud-grpc';
11-
import { GoogleCloudHttp } from '../google-cloud-http';
10+
import { googleCloudGrpcIntegration } from '../google-cloud-grpc';
11+
import { googleCloudHttpIntegration } from '../google-cloud-http';
1212

1313
export * from './http';
1414
export * from './events';
@@ -18,16 +18,16 @@ export * from './cloud_events';
1818
export const defaultIntegrations: Integration[] = [
1919
// eslint-disable-next-line deprecation/deprecation
2020
...defaultNodeIntegrations,
21-
new GoogleCloudHttp({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing.
22-
new GoogleCloudGrpc({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing.
21+
googleCloudHttpIntegration({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing.
22+
googleCloudGrpcIntegration({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing.
2323
];
2424

2525
/** Get the default integrations for the GCP SDK. */
2626
export function getDefaultIntegrations(options: Options): Integration[] {
2727
return [
2828
...getDefaultNodeIntegrations(options),
29-
new GoogleCloudHttp({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing.
30-
new GoogleCloudGrpc({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing.
29+
googleCloudHttpIntegration({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing.
30+
googleCloudGrpcIntegration({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing.
3131
];
3232
}
3333

packages/serverless/src/google-cloud-grpc.ts

Lines changed: 50 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
import type { EventEmitter } from 'events';
2-
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
2+
import {
3+
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
4+
convertIntegrationFnToClass,
5+
defineIntegration,
6+
getClient,
7+
} from '@sentry/core';
38
import { startInactiveSpan } from '@sentry/node';
4-
import type { Integration } from '@sentry/types';
9+
import type { Client, Integration, IntegrationClass, IntegrationFn } from '@sentry/types';
510
import { fill } from '@sentry/utils';
611

712
interface GrpcFunction extends CallableFunction {
@@ -26,45 +31,52 @@ interface Stub {
2631
[key: string]: GrpcFunctionObject;
2732
}
2833

29-
/** Google Cloud Platform service requests tracking for GRPC APIs */
30-
export class GoogleCloudGrpc implements Integration {
31-
/**
32-
* @inheritDoc
33-
*/
34-
public static id: string = 'GoogleCloudGrpc';
34+
const SERVICE_PATH_REGEX = /^(\w+)\.googleapis.com$/;
3535

36-
/**
37-
* @inheritDoc
38-
*/
39-
public name: string;
36+
const INTEGRATION_NAME = 'GoogleCloudGrpc';
4037

41-
private readonly _optional: boolean;
38+
const SETUP_CLIENTS = new WeakMap<Client, boolean>();
4239

43-
public constructor(options: { optional?: boolean } = {}) {
44-
this.name = GoogleCloudGrpc.id;
40+
const _googleCloudGrpcIntegration = ((options: { optional?: boolean } = {}) => {
41+
const optional = options.optional || false;
42+
return {
43+
name: INTEGRATION_NAME,
44+
setupOnce() {
45+
try {
46+
// eslint-disable-next-line @typescript-eslint/no-var-requires
47+
const gaxModule = require('google-gax');
48+
fill(
49+
gaxModule.GrpcClient.prototype, // eslint-disable-line @typescript-eslint/no-unsafe-member-access
50+
'createStub',
51+
wrapCreateStub,
52+
);
53+
} catch (e) {
54+
if (!optional) {
55+
throw e;
56+
}
57+
}
58+
},
59+
setup(client) {
60+
SETUP_CLIENTS.set(client, true);
61+
},
62+
};
63+
}) satisfies IntegrationFn;
4564

46-
this._optional = options.optional || false;
47-
}
65+
export const googleCloudGrpcIntegration = defineIntegration(_googleCloudGrpcIntegration);
4866

49-
/**
50-
* @inheritDoc
51-
*/
52-
public setupOnce(): void {
53-
try {
54-
// eslint-disable-next-line @typescript-eslint/no-var-requires
55-
const gaxModule = require('google-gax');
56-
fill(
57-
gaxModule.GrpcClient.prototype, // eslint-disable-line @typescript-eslint/no-unsafe-member-access
58-
'createStub',
59-
wrapCreateStub,
60-
);
61-
} catch (e) {
62-
if (!this._optional) {
63-
throw e;
64-
}
65-
}
66-
}
67-
}
67+
/**
68+
* Google Cloud Platform service requests tracking for GRPC APIs.
69+
*
70+
* @deprecated Use `googleCloudGrpcIntegration()` instead.
71+
*/
72+
// eslint-disable-next-line deprecation/deprecation
73+
export const GoogleCloudGrpc = convertIntegrationFnToClass(
74+
INTEGRATION_NAME,
75+
googleCloudGrpcIntegration,
76+
) as IntegrationClass<Integration>;
77+
78+
// eslint-disable-next-line deprecation/deprecation
79+
export type GoogleCloudGrpc = typeof GoogleCloudGrpc;
6880

6981
/** Returns a wrapped function that returns a stub with tracing enabled */
7082
function wrapCreateStub(origCreate: CreateStubFunc): CreateStubFunc {
@@ -105,7 +117,7 @@ function fillGrpcFunction(stub: Stub, serviceIdentifier: string, methodName: str
105117
(orig: GrpcFunction): GrpcFunction =>
106118
(...args) => {
107119
const ret = orig.apply(stub, args);
108-
if (typeof ret?.on !== 'function') {
120+
if (typeof ret?.on !== 'function' || !SETUP_CLIENTS.has(getClient() as Client)) {
109121
return ret;
110122
}
111123
const span = startInactiveSpan({
@@ -127,6 +139,6 @@ function fillGrpcFunction(stub: Stub, serviceIdentifier: string, methodName: str
127139

128140
/** Identifies service by its address */
129141
function identifyService(servicePath: string): string {
130-
const match = servicePath.match(/^(\w+)\.googleapis.com$/);
142+
const match = servicePath.match(SERVICE_PATH_REGEX);
131143
return match ? match[1] : servicePath;
132144
}

0 commit comments

Comments
 (0)