Skip to content

Commit 2f3f544

Browse files
authored
ref(serverless): Use new span APIs for serverless (#10235)
Most of the changes here are with tests, otherwise we swap to use `startInactiveSpan` everywhere. IMO it might be more correct to refactor to use `startSpanManual`, but that can come later, I want to keep same behavior for now.
1 parent 76378af commit 2f3f544

File tree

9 files changed

+34
-103
lines changed

9 files changed

+34
-103
lines changed

packages/serverless/src/awsservices.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getCurrentScope } from '@sentry/node';
1+
import { startInactiveSpan } from '@sentry/node';
22
import type { Integration, Span } from '@sentry/types';
33
import { fill } from '@sentry/utils';
44
// 'aws-sdk/global' import is expected to be type-only so it's erased in the final .js file.
@@ -57,19 +57,13 @@ function wrapMakeRequest<TService extends AWSService, TResult>(
5757
): MakeRequestFunction<GenericParams, TResult> {
5858
return function (this: TService, operation: string, params?: GenericParams, callback?: MakeRequestCallback<TResult>) {
5959
let span: Span | undefined;
60-
const scope = getCurrentScope();
61-
// eslint-disable-next-line deprecation/deprecation
62-
const transaction = scope.getTransaction();
6360
const req = orig.call(this, operation, params);
6461
req.on('afterBuild', () => {
65-
if (transaction) {
66-
// eslint-disable-next-line deprecation/deprecation
67-
span = transaction.startChild({
68-
description: describe(this, operation, params),
69-
op: 'http.client',
70-
origin: 'auto.http.serverless',
71-
});
72-
}
62+
span = startInactiveSpan({
63+
name: describe(this, operation, params),
64+
op: 'http.client',
65+
origin: 'auto.http.serverless',
66+
});
7367
});
7468
req.on('complete', () => {
7569
if (span) {

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

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { EventEmitter } from 'events';
2-
import { getCurrentScope } from '@sentry/node';
3-
import type { Integration, Span } from '@sentry/types';
2+
import { startInactiveSpan } from '@sentry/node';
3+
import type { Integration } from '@sentry/types';
44
import { fill } from '@sentry/utils';
55

66
interface GrpcFunction extends CallableFunction {
@@ -107,18 +107,11 @@ function fillGrpcFunction(stub: Stub, serviceIdentifier: string, methodName: str
107107
if (typeof ret?.on !== 'function') {
108108
return ret;
109109
}
110-
let span: Span | undefined;
111-
const scope = getCurrentScope();
112-
// eslint-disable-next-line deprecation/deprecation
113-
const transaction = scope.getTransaction();
114-
if (transaction) {
115-
// eslint-disable-next-line deprecation/deprecation
116-
span = transaction.startChild({
117-
description: `${callType} ${methodName}`,
118-
op: `grpc.${serviceIdentifier}`,
119-
origin: 'auto.grpc.serverless',
120-
});
121-
}
110+
const span = startInactiveSpan({
111+
name: `${callType} ${methodName}`,
112+
op: `grpc.${serviceIdentifier}`,
113+
origin: 'auto.grpc.serverless',
114+
});
122115
ret.on('status', () => {
123116
if (span) {
124117
span.end();

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// '@google-cloud/common' import is expected to be type-only so it's erased in the final .js file.
22
// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
33
import type * as common from '@google-cloud/common';
4-
import { getCurrentScope } from '@sentry/node';
5-
import type { Integration, Span } from '@sentry/types';
4+
import { startInactiveSpan } from '@sentry/node';
5+
import type { Integration } from '@sentry/types';
66
import { fill } from '@sentry/utils';
77

88
type RequestOptions = common.DecorateRequestOptions;
@@ -51,19 +51,12 @@ export class GoogleCloudHttp implements Integration {
5151
/** Returns a wrapped function that makes a request with tracing enabled */
5252
function wrapRequestFunction(orig: RequestFunction): RequestFunction {
5353
return function (this: common.Service, reqOpts: RequestOptions, callback: ResponseCallback): void {
54-
let span: Span | undefined;
55-
const scope = getCurrentScope();
56-
// eslint-disable-next-line deprecation/deprecation
57-
const transaction = scope.getTransaction();
58-
if (transaction) {
59-
const httpMethod = reqOpts.method || 'GET';
60-
// eslint-disable-next-line deprecation/deprecation
61-
span = transaction.startChild({
62-
description: `${httpMethod} ${reqOpts.uri}`,
63-
op: `http.client.${identifyService(this.apiEndpoint)}`,
64-
origin: 'auto.http.serverless',
65-
});
66-
}
54+
const httpMethod = reqOpts.method || 'GET';
55+
const span = startInactiveSpan({
56+
name: `${httpMethod} ${reqOpts.uri}`,
57+
op: `http.client.${identifyService(this.apiEndpoint)}`,
58+
origin: 'auto.http.serverless',
59+
});
6760
orig.call(this, reqOpts, (...args: Parameters<ResponseCallback>) => {
6861
if (span) {
6962
span.end();

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

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,13 @@ export const fakeScope = {
1414
setTag: jest.fn(),
1515
setContext: jest.fn(),
1616
setSpan: jest.fn(),
17-
getTransaction: jest.fn(() => fakeTransaction),
1817
setSDKProcessingMetadata: jest.fn(),
1918
setPropagationContext: jest.fn(),
2019
};
2120
export const fakeSpan = {
2221
end: jest.fn(),
2322
setHttpStatus: jest.fn(),
2423
};
25-
export const fakeTransaction = {
26-
end: jest.fn(),
27-
setHttpStatus: jest.fn(),
28-
startChild: jest.fn(() => fakeSpan),
29-
};
3024
export const init = jest.fn();
3125
export const addGlobalEventProcessor = jest.fn();
3226
export const getCurrentScope = jest.fn(() => fakeScope);
@@ -36,19 +30,16 @@ export const withScope = jest.fn(cb => cb(fakeScope));
3630
export const flush = jest.fn(() => Promise.resolve());
3731
export const getClient = jest.fn(() => ({}));
3832
export const startSpanManual = jest.fn((ctx, callback: (span: any) => any) => callback(fakeSpan));
33+
export const startInactiveSpan = jest.fn(() => fakeSpan);
3934

4035
export const resetMocks = (): void => {
41-
fakeTransaction.setHttpStatus.mockClear();
42-
fakeTransaction.end.mockClear();
43-
fakeTransaction.startChild.mockClear();
4436
fakeSpan.end.mockClear();
4537
fakeSpan.setHttpStatus.mockClear();
4638

4739
fakeScope.addEventProcessor.mockClear();
4840
fakeScope.setTag.mockClear();
4941
fakeScope.setContext.mockClear();
5042
fakeScope.setSpan.mockClear();
51-
fakeScope.getTransaction.mockClear();
5243

5344
init.mockClear();
5445
addGlobalEventProcessor.mockClear();

packages/serverless/test/awslambda.test.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,6 @@ import * as Sentry from '../src';
88

99
const { wrapHandler } = Sentry.AWSLambda;
1010

11-
/**
12-
* Why @ts-expect-error some Sentry.X calls
13-
*
14-
* A hack-ish way to contain everything related to mocks in the same __mocks__ file.
15-
* Thanks to this, we don't have to do more magic than necessary. Just add and export desired method and assert on it.
16-
*/
17-
1811
// Default `timeoutWarningLimit` is 500ms so leaving some space for it to trigger when necessary
1912
const DEFAULT_EXECUTION_TIME = 100;
2013
let fakeEvent: { [key: string]: unknown };

packages/serverless/test/awsservices.test.ts

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,6 @@ import * as nock from 'nock';
44

55
import { AWSServices } from '../src/awsservices';
66

7-
/**
8-
* Why @ts-expect-error some Sentry.X calls
9-
*
10-
* A hack-ish way to contain everything related to mocks in the same __mocks__ file.
11-
* Thanks to this, we don't have to do more magic than necessary. Just add and export desired method and assert on it.
12-
*/
13-
147
describe('AWSServices', () => {
158
beforeAll(() => {
169
new AWSServices().setupOnce();
@@ -30,11 +23,10 @@ describe('AWSServices', () => {
3023
nock('https://foo.s3.amazonaws.com').get('/bar').reply(200, 'contents');
3124
const data = await s3.getObject({ Bucket: 'foo', Key: 'bar' }).promise();
3225
expect(data.Body?.toString('utf-8')).toEqual('contents');
33-
// @ts-expect-error see "Why @ts-expect-error" note
34-
expect(SentryNode.fakeTransaction.startChild).toBeCalledWith({
26+
expect(SentryNode.startInactiveSpan).toBeCalledWith({
3527
op: 'http.client',
3628
origin: 'auto.http.serverless',
37-
description: 'aws.s3.getObject foo',
29+
name: 'aws.s3.getObject foo',
3830
});
3931
// @ts-expect-error see "Why @ts-expect-error" note
4032
expect(SentryNode.fakeSpan.end).toBeCalled();
@@ -48,11 +40,10 @@ describe('AWSServices', () => {
4840
expect(data.Body?.toString('utf-8')).toEqual('contents');
4941
done();
5042
});
51-
// @ts-expect-error see "Why @ts-expect-error" note
52-
expect(SentryNode.fakeTransaction.startChild).toBeCalledWith({
43+
expect(SentryNode.startInactiveSpan).toBeCalledWith({
5344
op: 'http.client',
5445
origin: 'auto.http.serverless',
55-
description: 'aws.s3.getObject foo',
46+
name: 'aws.s3.getObject foo',
5647
});
5748
});
5849
});
@@ -64,11 +55,10 @@ describe('AWSServices', () => {
6455
nock('https://lambda.eu-north-1.amazonaws.com').post('/2015-03-31/functions/foo/invocations').reply(201, 'reply');
6556
const data = await lambda.invoke({ FunctionName: 'foo' }).promise();
6657
expect(data.Payload?.toString('utf-8')).toEqual('reply');
67-
// @ts-expect-error see "Why @ts-expect-error" note
68-
expect(SentryNode.fakeTransaction.startChild).toBeCalledWith({
58+
expect(SentryNode.startInactiveSpan).toBeCalledWith({
6959
op: 'http.client',
7060
origin: 'auto.http.serverless',
71-
description: 'aws.lambda.invoke foo',
61+
name: 'aws.lambda.invoke foo',
7262
});
7363
});
7464
});

packages/serverless/test/gcpfunction.test.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,6 @@ import type {
1414
Request,
1515
Response,
1616
} from '../src/gcpfunction/general';
17-
/**
18-
* Why @ts-expect-error some Sentry.X calls
19-
*
20-
* A hack-ish way to contain everything related to mocks in the same __mocks__ file.
21-
* Thanks to this, we don't have to do more magic than necessary. Just add and export desired method and assert on it.
22-
*/
2317

2418
describe('GCPFunction', () => {
2519
afterEach(() => {

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

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,6 @@ import * as nock from 'nock';
1111

1212
import { GoogleCloudGrpc } from '../src/google-cloud-grpc';
1313

14-
/**
15-
* Why @ts-expect-error some Sentry.X calls
16-
*
17-
* A hack-ish way to contain everything related to mocks in the same __mocks__ file.
18-
* Thanks to this, we don't have to do more magic than necessary. Just add and export desired method and assert on it.
19-
*/
20-
2114
const spyConnect = jest.spyOn(http2, 'connect');
2215

2316
/** Fake HTTP2 stream */
@@ -126,11 +119,10 @@ describe('GoogleCloudGrpc tracing', () => {
126119
mockHttp2Session().mockUnaryRequest(Buffer.from('00000000120a1031363337303834313536363233383630', 'hex'));
127120
const resp = await pubsub.topic('nicetopic').publish(Buffer.from('data'));
128121
expect(resp).toEqual('1637084156623860');
129-
// @ts-expect-error see "Why @ts-expect-error" note
130-
expect(SentryNode.fakeTransaction.startChild).toBeCalledWith({
122+
expect(SentryNode.startInactiveSpan).toBeCalledWith({
131123
op: 'grpc.pubsub',
132124
origin: 'auto.grpc.serverless',
133-
description: 'unary call publish',
125+
name: 'unary call publish',
134126
});
135127
await pubsub.close();
136128
});

packages/serverless/test/google-cloud-http.test.ts

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,6 @@ import * as nock from 'nock';
66

77
import { GoogleCloudHttp } from '../src/google-cloud-http';
88

9-
/**
10-
* Why @ts-expect-error some Sentry.X calls
11-
*
12-
* A hack-ish way to contain everything related to mocks in the same __mocks__ file.
13-
* Thanks to this, we don't have to do more magic than necessary. Just add and export desired method and assert on it.
14-
*/
15-
169
describe('GoogleCloudHttp tracing', () => {
1710
beforeAll(() => {
1811
new GoogleCloudHttp().setupOnce();
@@ -57,17 +50,15 @@ describe('GoogleCloudHttp tracing', () => {
5750
);
5851
const resp = await bigquery.query('SELECT true AS foo');
5952
expect(resp).toEqual([[{ foo: true }]]);
60-
// @ts-expect-error see "Why @ts-expect-error" note
61-
expect(SentryNode.fakeTransaction.startChild).toBeCalledWith({
53+
expect(SentryNode.startInactiveSpan).toBeCalledWith({
6254
op: 'http.client.bigquery',
6355
origin: 'auto.http.serverless',
64-
description: 'POST /jobs',
56+
name: 'POST /jobs',
6557
});
66-
// @ts-expect-error see "Why @ts-expect-error" note
67-
expect(SentryNode.fakeTransaction.startChild).toBeCalledWith({
58+
expect(SentryNode.startInactiveSpan).toBeCalledWith({
6859
op: 'http.client.bigquery',
6960
origin: 'auto.http.serverless',
70-
description: expect.stringMatching(/^GET \/queries\/.+/),
61+
name: expect.stringMatching(/^GET \/queries\/.+/),
7162
});
7263
});
7364
});

0 commit comments

Comments
 (0)