Skip to content

Commit fcf90a2

Browse files
authored
test(opentelemetry-node): fix flaky instr-redis tests with fixed TestCollector.sortedSpans (#890)
The previous sorting of spans based on parentSpanId would fail if there were *3 or more* spans with (a) the same start time millisecond and (b) had parent/child relationships.
1 parent 9546d5b commit fcf90a2

File tree

1 file changed

+40
-19
lines changed

1 file changed

+40
-19
lines changed

packages/opentelemetry-node/test/testutils.js

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
// A dumping ground for testing utility functions.
99

10-
const assert = require('assert');
1110
const fs = require('fs');
1211
const {execFile} = require('child_process');
1312

@@ -381,26 +380,48 @@ class TestCollector {
381380
});
382381
});
383382

384-
return spans.sort((a, b) => {
385-
assert(typeof a.startTimeUnixNano === 'string');
386-
assert(typeof b.startTimeUnixNano === 'string');
387-
let aStartInt = BigInt(a.startTimeUnixNano);
388-
let bStartInt = BigInt(b.startTimeUnixNano);
389-
390-
if (aStartInt === bStartInt) {
391-
// Fast-created spans that start in the same millisecond cannot
392-
// reliably be sorted, because OTel JS currently doesn't have
393-
// sub-ms resolution. Attempt to improve the sorting by using
394-
// `spanId` and `parentSpanId`: a span cannot start before its
395-
// parent.
396-
if (a.parentSpanId && a.parentSpanId === b.spanId) {
397-
aStartInt += 1n;
398-
} else if (b.parentSpanId && b.parentSpanId === a.spanId) {
399-
bStartInt += 1n;
383+
// Fast-created spans that start in the same millisecond cannot reliably be
384+
// sorted, because OTel JS currently doesn't have sub-ms resolution. Attempt
385+
// to improve the sorting by using `spanId` and `parentSpanId`: a span
386+
// cannot start before its parent.
387+
const sortKeyFromSpanId = {};
388+
const spanFromSpanId = {};
389+
const childIdsFromSpanId = {};
390+
for (let s of spans) {
391+
sortKeyFromSpanId[s.spanId] = BigInt(s.startTimeUnixNano);
392+
spanFromSpanId[s.spanId] = s;
393+
childIdsFromSpanId[s.spanId] = [];
394+
}
395+
for (let s of spans) {
396+
if (s.parentSpanId && s.parentSpanId in childIdsFromSpanId) {
397+
childIdsFromSpanId[s.parentSpanId].push(s.spanId);
398+
}
399+
}
400+
let needAnotherPass;
401+
do {
402+
needAnotherPass = false;
403+
for (const [spanId, childIds] of Object.entries(
404+
childIdsFromSpanId
405+
)) {
406+
const sSortKey = sortKeyFromSpanId[spanId];
407+
for (let childId of childIds) {
408+
while (sortKeyFromSpanId[childId] <= sSortKey) {
409+
sortKeyFromSpanId[childId] += 1n;
410+
// If we bump a span's sortKey, then *its* children's
411+
// sortKeys might need a bump: do another pass.
412+
needAnotherPass = true;
413+
}
400414
}
401415
}
402-
403-
return aStartInt < bStartInt ? -1 : aStartInt > bStartInt ? 1 : 0;
416+
} while (needAnotherPass);
417+
// Uncomment this for debugging to show the used sort keys.
418+
// for (let s of spans) {
419+
// s.sortKey = sortKeyFromSpanId[s.spanId];
420+
// }
421+
return spans.sort((a, b) => {
422+
const aSortKey = sortKeyFromSpanId[a.spanId];
423+
const bSortKey = sortKeyFromSpanId[b.spanId];
424+
return aSortKey < bSortKey ? -1 : aSortKey > bSortKey ? 1 : 0;
404425
});
405426
}
406427

0 commit comments

Comments
 (0)