Skip to content

Commit 3e87c22

Browse files
dreamorosidreamorosi
committed
fix: addressed review comments
1 parent bd1eb1b commit 3e87c22

File tree

2 files changed

+60
-126
lines changed

2 files changed

+60
-126
lines changed

‎packages/tracing/tests/e2e/tracer.test.ts‎

Lines changed: 47 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ import { Table, AttributeType, BillingMode } from 'aws-cdk-lib/aws-dynamodb';
1212
import { App, Duration, Stack, RemovalPolicy } from 'aws-cdk-lib';
1313
import { deployStack, destroyStack } from '../../../commons/tests/utils/cdk-cli';
1414
import * as AWS from 'aws-sdk';
15-
import { getTraces, getInvocationSubsegment } from '../helpers/tracesUtils';
16-
import type { ParsedDocument } from '../helpers/tracesUtils';
15+
import { getTraces, getInvocationSubsegment, splitSegmentsByName } from '../helpers/tracesUtils';
1716

1817
const xray = new AWS.XRay();
1918
const lambdaClient = new AWS.Lambda();
@@ -140,39 +139,26 @@ describe('Tracer integration tests', () => {
140139
const sortedTraces = await getTraces(xray, startTime, resourceArn, invocations, 5);
141140

142141
for (let i = 0; i < invocations; i++) {
143-
// Assert that the trace has the expected amount of segments
144142
expect(sortedTraces[i].Segments.length).toBe(5);
145143

146144
const invocationSubsegment = getInvocationSubsegment(sortedTraces[i]);
147145

148146
if (invocationSubsegment?.subsegments !== undefined) {
149147
expect(invocationSubsegment?.subsegments?.length).toBe(1);
148+
150149
const handlerSubsegment = invocationSubsegment?.subsegments[0];
151-
// Assert that the subsegment name is the expected one
152150
expect(handlerSubsegment.name).toBe('## index.handler');
151+
153152
if (handlerSubsegment?.subsegments !== undefined) {
154-
// Assert that there are three subsegments
155153
expect(handlerSubsegment?.subsegments?.length).toBe(3);
156154

157-
// Sort the subsegments by name
158-
const dynamoDBSubsegments: ParsedDocument[] = [];
159-
const httpSubsegment: ParsedDocument[] = [];
160-
const otherSegments: ParsedDocument[] = [];
161-
handlerSubsegment?.subsegments.forEach(subsegment => {
162-
if (subsegment.name === 'DynamoDB') {
163-
dynamoDBSubsegments.push(subsegment);
164-
} else if (subsegment.name === 'httpbin.org') {
165-
httpSubsegment.push(subsegment);
166-
} else {
167-
otherSegments.push(subsegment);
168-
}
169-
});
155+
const subsegments = splitSegmentsByName(handlerSubsegment.subsegments, [ 'DynamoDB', 'httpbin.org' ]);
170156
// Assert that there are exactly two subsegment with the name 'DynamoDB'
171-
expect(dynamoDBSubsegments.length).toBe(2);
157+
expect(subsegments.get('DynamoDB')?.length).toBe(2);
172158
// Assert that there is exactly one subsegment with the name 'httpbin.org'
173-
expect(httpSubsegment.length).toBe(1);
159+
expect(subsegments.get('httpbin.org')?.length).toBe(1);
174160
// Assert that there are exactly zero other subsegments
175-
expect(otherSegments.length).toBe(0);
161+
expect(subsegments.get('other')?.length).toBe(0);
176162

177163
const { annotations, metadata } = handlerSubsegment;
178164

@@ -232,32 +218,20 @@ describe('Tracer integration tests', () => {
232218

233219
if (invocationSubsegment?.subsegments !== undefined) {
234220
expect(invocationSubsegment?.subsegments?.length).toBe(1);
221+
235222
const handlerSubsegment = invocationSubsegment?.subsegments[0];
236-
// Assert that the subsegment name is the expected one
237223
expect(handlerSubsegment.name).toBe('## index.handler');
224+
238225
if (handlerSubsegment?.subsegments !== undefined) {
239-
// Assert that there're two subsegments
240226
expect(handlerSubsegment?.subsegments?.length).toBe(3);
241227

242-
// Sort the subsegments by name
243-
const dynamoDBSubsegments: ParsedDocument[] = [];
244-
const httpSubsegment: ParsedDocument[] = [];
245-
const otherSegments: ParsedDocument[] = [];
246-
handlerSubsegment?.subsegments.forEach(subsegment => {
247-
if (subsegment.name === 'DynamoDB') {
248-
dynamoDBSubsegments.push(subsegment);
249-
} else if (subsegment.name === 'httpbin.org') {
250-
httpSubsegment.push(subsegment);
251-
} else {
252-
otherSegments.push(subsegment);
253-
}
254-
});
228+
const subsegments = splitSegmentsByName(handlerSubsegment.subsegments, [ 'DynamoDB', 'httpbin.org' ]);
255229
// Assert that there are exactly two subsegment with the name 'DynamoDB'
256-
expect(dynamoDBSubsegments.length).toBe(2);
230+
expect(subsegments.get('DynamoDB')?.length).toBe(2);
257231
// Assert that there is exactly one subsegment with the name 'httpbin.org'
258-
expect(httpSubsegment.length).toBe(1);
232+
expect(subsegments.get('httpbin.org')?.length).toBe(1);
259233
// Assert that there are exactly zero other subsegments
260-
expect(otherSegments.length).toBe(0);
234+
expect(subsegments.get('other')?.length).toBe(0);
261235

262236
const { annotations, metadata } = handlerSubsegment;
263237

@@ -317,32 +291,20 @@ describe('Tracer integration tests', () => {
317291

318292
if (invocationSubsegment?.subsegments !== undefined) {
319293
expect(invocationSubsegment?.subsegments?.length).toBe(1);
294+
320295
const handlerSubsegment = invocationSubsegment?.subsegments[0];
321-
// Assert that the subsegment name is the expected one
322296
expect(handlerSubsegment.name).toBe('## index.handler');
297+
323298
if (handlerSubsegment?.subsegments !== undefined) {
324-
// Assert that there're two subsegments
325299
expect(handlerSubsegment?.subsegments?.length).toBe(3);
326300

327-
// Sort the subsegments by name
328-
const dynamoDBSubsegments: ParsedDocument[] = [];
329-
const httpSubsegment: ParsedDocument[] = [];
330-
const otherSegments: ParsedDocument[] = [];
331-
handlerSubsegment?.subsegments.forEach(subsegment => {
332-
if (subsegment.name === 'DynamoDB') {
333-
dynamoDBSubsegments.push(subsegment);
334-
} else if (subsegment.name === 'httpbin.org') {
335-
httpSubsegment.push(subsegment);
336-
} else {
337-
otherSegments.push(subsegment);
338-
}
339-
});
301+
const subsegments = splitSegmentsByName(handlerSubsegment.subsegments, [ 'DynamoDB', 'httpbin.org' ]);
340302
// Assert that there are exactly two subsegment with the name 'DynamoDB'
341-
expect(dynamoDBSubsegments.length).toBe(2);
303+
expect(subsegments.get('DynamoDB')?.length).toBe(2);
342304
// Assert that there is exactly one subsegment with the name 'httpbin.org'
343-
expect(httpSubsegment.length).toBe(1);
305+
expect(subsegments.get('httpbin.org')?.length).toBe(1);
344306
// Assert that there are exactly zero other subsegments
345-
expect(otherSegments.length).toBe(0);
307+
expect(subsegments.get('other')?.length).toBe(0);
346308

347309
const { annotations, metadata } = handlerSubsegment;
348310

@@ -425,38 +387,24 @@ describe('Tracer integration tests', () => {
425387

426388
if (invocationSubsegment?.subsegments !== undefined) {
427389
expect(invocationSubsegment?.subsegments?.length).toBe(1);
390+
428391
const handlerSubsegment = invocationSubsegment?.subsegments[0];
429-
// Assert that the subsegment name is the expected one
430392
expect(handlerSubsegment.name).toBe('## index.handler');
393+
431394
if (handlerSubsegment?.subsegments !== undefined) {
432-
// Assert that there are four subsegments
433395
expect(handlerSubsegment?.subsegments?.length).toBe(4);
434396

435-
// Sort the subsegments by name
436-
const dynamoDBSubsegments: ParsedDocument[] = [];
437-
const methodSubsegment: ParsedDocument[] = [];
438-
const httpSubsegment: ParsedDocument[] = [];
439-
const otherSegments: ParsedDocument[] = [];
440-
handlerSubsegment?.subsegments.forEach(subsegment => {
441-
if (subsegment.name === 'DynamoDB') {
442-
dynamoDBSubsegments.push(subsegment);
443-
} else if (subsegment.name === '### myMethod') {
444-
methodSubsegment.push(subsegment);
445-
} else if (subsegment.name === 'httpbin.org') {
446-
httpSubsegment.push(subsegment);
447-
} else {
448-
otherSegments.push(subsegment);
449-
}
450-
});
397+
const subsegments = splitSegmentsByName(handlerSubsegment.subsegments, [ 'DynamoDB', 'httpbin.org', '### myMethod' ]);
451398
// Assert that there are exactly two subsegment with the name 'DynamoDB'
452-
expect(dynamoDBSubsegments.length).toBe(2);
453-
// Assert that there is exactly one subsegment with the name '### myMethod'
454-
expect(methodSubsegment.length).toBe(1);
399+
expect(subsegments.get('DynamoDB')?.length).toBe(2);
455400
// Assert that there is exactly one subsegment with the name 'httpbin.org'
456-
expect(httpSubsegment.length).toBe(1);
401+
expect(subsegments.get('httpbin.org')?.length).toBe(1);
402+
// Assert that there is exactly one subsegment with the name '### myMethod'
403+
expect(subsegments.get('### myMethod')?.length).toBe(1);
457404
// Assert that there are exactly zero other subsegments
458-
expect(otherSegments.length).toBe(0);
405+
expect(subsegments.get('other')?.length).toBe(0);
459406

407+
const methodSubsegment = subsegments.get('### myMethod') || [];
460408
const { metadata } = methodSubsegment[0];
461409

462410
if (metadata !== undefined) {
@@ -527,38 +475,24 @@ describe('Tracer integration tests', () => {
527475

528476
if (invocationSubsegment?.subsegments !== undefined) {
529477
expect(invocationSubsegment?.subsegments?.length).toBe(1);
478+
530479
const handlerSubsegment = invocationSubsegment?.subsegments[0];
531-
// Assert that the subsegment name is the expected one
532480
expect(handlerSubsegment.name).toBe('## index.handler');
481+
533482
if (handlerSubsegment?.subsegments !== undefined) {
534-
// Assert that there are four subsegments
535483
expect(handlerSubsegment?.subsegments?.length).toBe(4);
536484

537-
// Sort the subsegments by name
538-
const dynamoDBSubsegments: ParsedDocument[] = [];
539-
const methodSubsegment: ParsedDocument[] = [];
540-
const httpSubsegment: ParsedDocument[] = [];
541-
const otherSegments: ParsedDocument[] = [];
542-
handlerSubsegment?.subsegments.forEach(subsegment => {
543-
if (subsegment.name === 'DynamoDB') {
544-
dynamoDBSubsegments.push(subsegment);
545-
} else if (subsegment.name === '### myMethod') {
546-
methodSubsegment.push(subsegment);
547-
} else if (subsegment.name === 'httpbin.org') {
548-
httpSubsegment.push(subsegment);
549-
} else {
550-
otherSegments.push(subsegment);
551-
}
552-
});
485+
const subsegments = splitSegmentsByName(handlerSubsegment.subsegments, [ 'DynamoDB', 'httpbin.org', '### myMethod' ]);
553486
// Assert that there are exactly two subsegment with the name 'DynamoDB'
554-
expect(dynamoDBSubsegments.length).toBe(2);
555-
// Assert that there is exactly one subsegment with the name '### myMethod'
556-
expect(methodSubsegment.length).toBe(1);
487+
expect(subsegments.get('DynamoDB')?.length).toBe(2);
557488
// Assert that there is exactly one subsegment with the name 'httpbin.org'
558-
expect(httpSubsegment.length).toBe(1);
489+
expect(subsegments.get('httpbin.org')?.length).toBe(1);
490+
// Assert that there is exactly one subsegment with the name '### myMethod'
491+
expect(subsegments.get('### myMethod')?.length).toBe(1);
559492
// Assert that there are exactly zero other subsegments
560-
expect(otherSegments.length).toBe(0);
493+
expect(subsegments.get('other')?.length).toBe(0);
561494

495+
const methodSubsegment = subsegments.get('### myMethod') || [];
562496
const { metadata } = methodSubsegment[0];
563497

564498
if (metadata !== undefined) {
@@ -629,38 +563,25 @@ describe('Tracer integration tests', () => {
629563

630564
if (invocationSubsegment?.subsegments !== undefined) {
631565
expect(invocationSubsegment?.subsegments?.length).toBe(1);
566+
632567
const handlerSubsegment = invocationSubsegment?.subsegments[0];
633-
// Assert that the subsegment name is the expected one
634568
expect(handlerSubsegment.name).toBe('## index.handler');
569+
635570
if (handlerSubsegment?.subsegments !== undefined) {
636-
// Assert that there are four subsegments
637571
expect(handlerSubsegment?.subsegments?.length).toBe(4);
638572

639-
// Sort the subsegments by name
640-
const dynamoDBSubsegments: ParsedDocument[] = [];
641-
const methodSubsegment: ParsedDocument[] = [];
642-
const httpSubsegment: ParsedDocument[] = [];
643-
const otherSegments: ParsedDocument[] = [];
644-
handlerSubsegment?.subsegments.forEach(subsegment => {
645-
if (subsegment.name === 'DynamoDB') {
646-
dynamoDBSubsegments.push(subsegment);
647-
} else if (subsegment.name === '### myMethod') {
648-
methodSubsegment.push(subsegment);
649-
} else if (subsegment.name === 'httpbin.org') {
650-
httpSubsegment.push(subsegment);
651-
} else {
652-
otherSegments.push(subsegment);
653-
}
654-
});
573+
const subsegments = splitSegmentsByName(handlerSubsegment.subsegments, [ 'DynamoDB', 'httpbin.org', '### myMethod' ]);
655574
// Assert that there are exactly two subsegment with the name 'DynamoDB'
656-
expect(dynamoDBSubsegments.length).toBe(2);
657-
// Assert that there is exactly one subsegment with the name '### myMethod'
658-
expect(methodSubsegment.length).toBe(1);
575+
expect(subsegments.get('DynamoDB')?.length).toBe(2);
659576
// Assert that there is exactly one subsegment with the name 'httpbin.org'
660-
expect(httpSubsegment.length).toBe(1);
577+
expect(subsegments.get('httpbin.org')?.length).toBe(1);
578+
// Assert that there is exactly one subsegment with the name '### myMethod'
579+
expect(subsegments.get('### myMethod')?.length).toBe(1);
661580
// Assert that there are exactly zero other subsegments
662-
expect(otherSegments.length).toBe(0);
581+
expect(subsegments.get('other')?.length).toBe(0);
582+
663583
// Assert that no response was captured on the subsegment
584+
const methodSubsegment = subsegments.get('### myMethod') || [];
664585
expect(methodSubsegment[0].hasOwnProperty('metadata')).toBe(false);
665586
} else {
666587
// Make test fail if the handlerSubsegment subsegment doesn't have any subsebment

‎packages/tracing/tests/helpers/tracesUtils.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,23 @@ const getInvocationSubsegment = (trace: ParsedTrace): ParsedDocument => {
135135
return invocationSubsegment;
136136
};
137137

138+
const splitSegmentsByName = (subsegments: ParsedDocument[], expectedNames: string[]): Map<string, ParsedDocument[]> => {
139+
const splitSegments: Map<string, ParsedDocument[]> = new Map([ ...expectedNames, 'other' ].map(name => [ name, [] ]));
140+
subsegments.forEach(subsegment => {
141+
const name = expectedNames.indexOf(subsegment.name) !== -1 ? subsegment.name : 'other';
142+
const newSegments = splitSegments.get(name) as ParsedDocument[];
143+
newSegments.push(subsegment);
144+
splitSegments.set(name, newSegments);
145+
});
146+
147+
return splitSegments;
148+
};
149+
138150
export {
139151
getTraces,
140152
getFunctionSegment,
141153
getInvocationSubsegment,
154+
splitSegmentsByName
142155
};
143156

144157
export type {

0 commit comments

Comments
 (0)