test(opentelemetry): deflake OTLP metrics HTTP export test with fake timers#9287
Conversation
Overall package sizeSelf size: 6.71 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.3.1 | 122.62 kB | 437.94 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 8c7d789 | Docs | Datadog PR Page | Give us feedback! |
38874fc to
2f3f1af
Compare
BenchmarksBenchmark execution time: 2026-07-10 18:14:38 Comparing candidate commit 8c7d789 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2315 metrics, 43 unstable metrics.
|
…timers `handles timeout and error without network` asserted `results.timeout` and `results.error` at a fixed 300ms wall-clock deadline, but both flags depend on two real periodic-exporter ticks (100ms interval) plus each request's real +10ms timeout/error callback. The second export's error handler fires around ~251ms, leaving only ~50ms of slack, so a modest event-loop stall on a loaded CI runner (GC/CPU contention) pushed it past 300ms and left `results.error` false — throwing `assert(results.error)` inside `Timeout._onTimeout`. Drive the two exports deterministically with sinon fake timers instead of racing wall-clock time: install `useFakeTimers()` before `setupMetrics()` so the reader's export interval is faked, then advance the clock explicitly for each export round-trip. The HTTP stub is unchanged. Verified: 25/25 clean runs and 8/8 under CPU contention; a 1.3s real event-loop stall injected into the window that broke the original still passes (it flaked at ~150ms). Full metrics.spec.js: 58 passing. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2f3f1af to
8c7d789
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9287 +/- ##
========================================
Coverage 96.56% 96.57%
========================================
Files 918 918
Lines 121564 121564
Branches 20634 20887 +253
========================================
+ Hits 117394 117396 +2
+ Misses 4170 4168 -2 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…timers (#9287) `handles timeout and error without network` asserted `results.timeout` and `results.error` at a fixed 300ms wall-clock deadline, but both flags depend on two real periodic-exporter ticks (100ms interval) plus each request's real +10ms timeout/error callback. The second export's error handler fires around ~251ms, leaving only ~50ms of slack, so a modest event-loop stall on a loaded CI runner (GC/CPU contention) pushed it past 300ms and left `results.error` false — throwing `assert(results.error)` inside `Timeout._onTimeout`. Drive the two exports deterministically with sinon fake timers instead of racing wall-clock time: install `useFakeTimers()` before `setupMetrics()` so the reader's export interval is faked, then advance the clock explicitly for each export round-trip. The HTTP stub is unchanged. Verified: 25/25 clean runs and 8/8 under CPU contention; a 1.3s real event-loop stall injected into the window that broke the original still passes (it flaked at ~150ms). Full metrics.spec.js: 58 passing. Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
…timers (#9287) `handles timeout and error without network` asserted `results.timeout` and `results.error` at a fixed 300ms wall-clock deadline, but both flags depend on two real periodic-exporter ticks (100ms interval) plus each request's real +10ms timeout/error callback. The second export's error handler fires around ~251ms, leaving only ~50ms of slack, so a modest event-loop stall on a loaded CI runner (GC/CPU contention) pushed it past 300ms and left `results.error` false — throwing `assert(results.error)` inside `Timeout._onTimeout`. Drive the two exports deterministically with sinon fake timers instead of racing wall-clock time: install `useFakeTimers()` before `setupMetrics()` so the reader's export interval is faked, then advance the clock explicitly for each export round-trip. The HTTP stub is unchanged. Verified: 25/25 clean runs and 8/8 under CPU contention; a 1.3s real event-loop stall injected into the window that broke the original still passes (it flaked at ~150ms). Full metrics.spec.js: 58 passing. Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
What does this PR do?
Rewrites the OpenTelemetry metrics test
handles timeout and error without networkto use sinon fake timers instead of racing real wall-clock timeouts. No production code changes.Motivation
The test flaked on
master(Node 24). It assertedresults.timeoutandresults.errorat a fixed 300ms deadline, but both flags depend on two real periodic-exporter ticks (100ms interval) plus each request's real +10ms timeout/error callback. The second export's error handler fires around ~251ms, leaving only ~50ms of slack, so a modest event-loop stall on a loaded runner (GC or CPU contention) pushed it past 300ms. Withresults.errorstill false,assert(results.error)threw insideTimeout._onTimeout.The fix installs
useFakeTimers()beforesetupMetrics()so the reader's export interval is faked, then advances the clock explicitly for each export round-trip. This matches the repo guidance to never rely on real time passing in unit tests. The HTTP stub is unchanged.Additional Notes
Verified: 25/25 clean runs and 8/8 under CPU contention. A 1.3s real event-loop stall injected into the window that broke the original still passes (the original flaked at ~150ms). Full
metrics.spec.js: 58 passing.🤖 Generated with Claude Code