From 0a7b0a1f816cdef9136fae6eb17abe907c6946df Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 18 Jul 2025 12:00:34 -0700 Subject: [PATCH 1/2] test(opentelemetry-node): fix flaky instr-redis tests with fixed TestCollector.sortedSpans 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. This case was happening frequently for me in local testing of instr-redis-4.test.js --- packages/opentelemetry-node/test/testutils.js | 56 +++++++++++++------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/packages/opentelemetry-node/test/testutils.js b/packages/opentelemetry-node/test/testutils.js index 26f2ee92..69daf37b 100644 --- a/packages/opentelemetry-node/test/testutils.js +++ b/packages/opentelemetry-node/test/testutils.js @@ -381,26 +381,46 @@ class TestCollector { }); }); - return spans.sort((a, b) => { - assert(typeof a.startTimeUnixNano === 'string'); - assert(typeof b.startTimeUnixNano === 'string'); - let aStartInt = BigInt(a.startTimeUnixNano); - let bStartInt = BigInt(b.startTimeUnixNano); - - if (aStartInt === bStartInt) { - // Fast-created spans that start in the same millisecond cannot - // reliably be sorted, because OTel JS currently doesn't have - // sub-ms resolution. Attempt to improve the sorting by using - // `spanId` and `parentSpanId`: a span cannot start before its - // parent. - if (a.parentSpanId && a.parentSpanId === b.spanId) { - aStartInt += 1n; - } else if (b.parentSpanId && b.parentSpanId === a.spanId) { - bStartInt += 1n; + // Fast-created spans that start in the same millisecond cannot reliably be + // sorted, because OTel JS currently doesn't have sub-ms resolution. Attempt + // to improve the sorting by using `spanId` and `parentSpanId`: a span + // cannot start before its parent. + const sortKeyFromSpanId = {}; + const spanFromSpanId = {}; + const childIdsFromSpanId = {}; + for (let s of spans) { + sortKeyFromSpanId[s.spanId] = BigInt(s.startTimeUnixNano); + spanFromSpanId[s.spanId] = s; + childIdsFromSpanId[s.spanId] = []; + } + for (let s of spans) { + if (s.parentSpanId && s.parentSpanId in childIdsFromSpanId) { + childIdsFromSpanId[s.parentSpanId].push(s.spanId) + } + } + let needAnotherPass; + do { + needAnotherPass = false; + for (const [spanId, childIds] of Object.entries(childIdsFromSpanId)) { + const sSortKey = sortKeyFromSpanId[spanId]; + for (let childId of childIds) { + while (sortKeyFromSpanId[childId] <= sSortKey) { + sortKeyFromSpanId[childId] += 1n; + // If we bump a span's sortKey, then *its* children's + // sortKeys might need a bump: do another pass. + needAnotherPass = true; + } } } - - return aStartInt < bStartInt ? -1 : aStartInt > bStartInt ? 1 : 0; + } while (needAnotherPass); + // Uncomment this for debugging to show the used sort keys. + // for (let s of spans) { + // s.sortKey = sortKeyFromSpanId[s.spanId]; + // } + return spans.sort((a, b) => { + const aSortKey = sortKeyFromSpanId[a.spanId]; + const bSortKey = sortKeyFromSpanId[b.spanId]; + return aSortKey < bSortKey ? -1 : aSortKey > bSortKey ? 1 : 0; }); } From 261953f9c53f5f1579acdbb699115abf6e0ea92d Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Fri, 18 Jul 2025 12:03:13 -0700 Subject: [PATCH 2/2] lint:fix --- packages/opentelemetry-node/test/testutils.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-node/test/testutils.js b/packages/opentelemetry-node/test/testutils.js index 69daf37b..09e906d4 100644 --- a/packages/opentelemetry-node/test/testutils.js +++ b/packages/opentelemetry-node/test/testutils.js @@ -7,7 +7,6 @@ // A dumping ground for testing utility functions. -const assert = require('assert'); const fs = require('fs'); const {execFile} = require('child_process'); @@ -395,13 +394,15 @@ class TestCollector { } for (let s of spans) { if (s.parentSpanId && s.parentSpanId in childIdsFromSpanId) { - childIdsFromSpanId[s.parentSpanId].push(s.spanId) + childIdsFromSpanId[s.parentSpanId].push(s.spanId); } } let needAnotherPass; do { needAnotherPass = false; - for (const [spanId, childIds] of Object.entries(childIdsFromSpanId)) { + for (const [spanId, childIds] of Object.entries( + childIdsFromSpanId + )) { const sSortKey = sortKeyFromSpanId[spanId]; for (let childId of childIds) { while (sortKeyFromSpanId[childId] <= sSortKey) {