Skip to content

Commit 77192f3

Browse files
authored
fix: SDK will report warning for unclosed spans (#635)
1 parent dda2409 commit 77192f3

File tree

6 files changed

+60
-3
lines changed

6 files changed

+60
-3
lines changed

node/packages/aws-lambda-sdk/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"version": "0.14.11",
55
"author": "Serverless, Inc.",
66
"dependencies": {
7-
"@serverless/sdk": "^0.5.5",
7+
"@serverless/sdk": "^0.5.6",
88
"@serverless/sdk-schema": "^0.15.3",
99
"d": "^1.0.1",
1010
"ext": "^1.7.0",

node/packages/aws-lambda-sdk/test/fixtures/lambdas/esm-sdk/index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ export const handler = async () => {
2323
sdk.setTag('user.tag', `example:${invocationId}`);
2424

2525
console.warn('Consoled warning', 12, true);
26+
27+
sdk._createTraceSpan('custom.not.closed');
28+
2629
return {
2730
name: sdk.name,
2831
version: sdk.version,

node/packages/aws-lambda-sdk/test/fixtures/lambdas/sdk.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ module.exports.handler = async (event) => {
2727
sdk.setTag('user.tag', `example:${invocationId}`);
2828

2929
console.warn('Consoled warning', 12, true);
30+
31+
sdk._createTraceSpan('custom.not.closed');
32+
3033
return {
3134
name: sdk.name,
3235
version: sdk.version,

node/packages/aws-lambda-sdk/test/integration/index.test.js

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,25 +413,29 @@ describe('integration', function () {
413413
{ name: 'telemetry.error.generated.v1', type: 'ERROR_TYPE_CAUGHT_USER' },
414414
{ name: 'telemetry.warning.generated.v1', type: 'WARNING_TYPE_USER' },
415415
{ name: 'telemetry.warning.generated.v1', type: 'WARNING_TYPE_USER' },
416+
{ name: 'telemetry.warning.generated.v1', type: 'WARNING_TYPE_SDK_INTERNAL' },
416417
],
417418
test: ({ invocationsData }) => {
418419
for (const [index, { trace, responsePayload }] of invocationsData.entries()) {
419420
const { spans, events, customTags } = trace;
420421
let awsLambdaInvocationSpan;
422+
const rootSpan = spans[0];
421423
if (index === 0) {
422424
awsLambdaInvocationSpan = spans[2];
423425
expect(spans.map(({ name }) => name)).to.deep.equal([
424426
'aws.lambda',
425427
'aws.lambda.initialization',
426428
'aws.lambda.invocation',
427429
'user.span',
430+
'custom.not.closed',
428431
]);
429432
} else {
430433
awsLambdaInvocationSpan = spans[1];
431434
expect(spans.map(({ name }) => name)).to.deep.equal([
432435
'aws.lambda',
433436
'aws.lambda.invocation',
434437
'user.span',
438+
'custom.not.closed',
435439
]);
436440
}
437441
const payload = JSON.parse(responsePayload.raw);
@@ -508,6 +512,20 @@ describe('integration', function () {
508512
},
509513
},
510514
},
515+
{
516+
traceId: rootSpan.traceId,
517+
spanId: rootSpan.id,
518+
eventName: 'telemetry.warning.generated.v1',
519+
customTags: JSON.stringify({}),
520+
customFingerprint: 'SDK_SPAN_NOT_CLOSED',
521+
tags: {
522+
warning: {
523+
message:
524+
"Serverless SDK Warning: Following trace spans didn't end before end of lambda invocation: custom.not.closed\n",
525+
type: 3,
526+
},
527+
},
528+
},
511529
]);
512530
}
513531
},
@@ -1778,7 +1796,15 @@ describe('integration', function () {
17781796
'aws.lambda.invocation',
17791797
]);
17801798
},
1799+
// The internal captured events may not actually get published until a following
1800+
// invocation. So we will leverage the hasOrphanedSpans flag to indicate
1801+
// that an acceptable event list for this test should be an empty array _or_
1802+
// the capturedEvents listed below.
17811803
hasOrphanedSpans: true,
1804+
capturedEvents: [
1805+
// Warning generated by the SDK when the span fails to close
1806+
{ name: 'telemetry.warning.generated.v1', type: 'WARNING_TYPE_SDK_INTERNAL' },
1807+
],
17821808
},
17831809
},
17841810
],
@@ -1842,7 +1868,7 @@ describe('integration', function () {
18421868
if (testResult.error) throw testResult.error;
18431869
log.debug('%s test result: %o', testConfig.name, testResult);
18441870
const afterTimestamp = resolveNanosecondsTimestamp() + 2000000000; // 2 seconds after
1845-
const { expectedOutcome, capturedEvents } = testConfig;
1871+
const { expectedOutcome, capturedEvents, hasOrphanedSpans } = testConfig;
18461872
const { invocationsData } = testResult;
18471873
if (
18481874
expectedOutcome === 'success' ||
@@ -1951,7 +1977,12 @@ describe('integration', function () {
19511977
]);
19521978
}
19531979
}
1954-
if (capturedEvents) expect(normalizedEvents).deep.equal(capturedEvents);
1980+
if (
1981+
(capturedEvents && !hasOrphanedSpans) ||
1982+
(capturedEvents && hasOrphanedSpans && normalizedEvents.length > 0)
1983+
) {
1984+
expect(normalizedEvents).deep.equal(capturedEvents);
1985+
}
19551986
}
19561987
}
19571988
if (testConfig.test) {

node/packages/aws-lambda-sdk/test/unit/internal-extension/index.test.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,7 @@ describe('internal-extension/index.test.js', () => {
818818
'aws.lambda.initialization',
819819
'aws.lambda.invocation',
820820
'user.span',
821+
'custom.not.closed',
821822
]);
822823
expect(result.name).to.equal(pkgJson.name);
823824
expect(result.version).to.equal(pkgJson.version);
@@ -841,6 +842,7 @@ describe('internal-extension/index.test.js', () => {
841842
delete event.timestampUnixNano;
842843
return event;
843844
};
845+
const { id: SDK_ERROR_SPAN_ID } = spans[0];
844846
const { traceId, spanId } = events[0];
845847
expect(events.map(normalizeEvent)).to.deep.equal([
846848
{
@@ -893,6 +895,20 @@ describe('internal-extension/index.test.js', () => {
893895
},
894896
},
895897
},
898+
{
899+
traceId,
900+
spanId: SDK_ERROR_SPAN_ID,
901+
eventName: 'telemetry.warning.generated.v1',
902+
customFingerprint: 'SDK_SPAN_NOT_CLOSED',
903+
customTags: JSON.stringify({}),
904+
tags: {
905+
warning: {
906+
message:
907+
"Serverless SDK Warning: Following trace spans didn't end before end of lambda invocation: custom.not.closed\n",
908+
type: 3,
909+
},
910+
},
911+
},
896912
]);
897913
});
898914

@@ -1017,13 +1033,15 @@ describe('internal-extension/index.test.js', () => {
10171033
{ name: 'aws.lambda.initialization', input: undefined, output: undefined },
10181034
{ name: 'user.span', input: undefined, output: undefined },
10191035
{ name: 'aws.lambda.invocation', input: undefined, output: undefined },
1036+
{ name: 'custom.not.closed', input: undefined, output: undefined },
10201037
{ name: 'aws.lambda', input: undefined, output: undefined },
10211038
],
10221039
traceEvents: [
10231040
{ eventName: 'telemetry.error.generated.v1' },
10241041
{ eventName: 'telemetry.error.generated.v1' },
10251042
{ eventName: 'telemetry.warning.generated.v1' },
10261043
{ eventName: 'telemetry.warning.generated.v1' },
1044+
{ eventName: 'telemetry.warning.generated.v1' },
10271045
],
10281046
});
10291047
});

node/packages/aws-lambda-sdk/trace-spans/aws-lambda.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const TraceSpan = require('@serverless/sdk/lib/trace-span');
4+
const reportWarning = require('@serverless/sdk/lib/report-warning');
45

56
const immutableTags = {
67
'aws.lambda.name': process.env.AWS_LAMBDA_FUNCTION_NAME,
@@ -28,6 +29,7 @@ const awsLambdaSpan = new TraceSpan('aws.lambda', {
2829
startTime: EvalError.$serverlessAwsLambdaInitializationStartTime,
2930
immediateDescendants: ['aws.lambda.initialization'],
3031
tags: immutableTags,
32+
_reportWarning: reportWarning,
3133
});
3234

3335
if (process.env.AWS_LAMBDA_INITIALIZATION_TYPE === 'on-demand') {

0 commit comments

Comments
 (0)