src: add event loop based metrics into node#62935
Conversation
| } | ||
|
|
||
| uv_close(reinterpret_cast<uv_handle_t*>(&prepare_handle_), | ||
| PrepareCloseCB); |
There was a problem hiding this comment.
This should call the superclass's method instead of duplicating it
There was a problem hiding this comment.
Calling HandleWrap::Close() would just uv_close check_handle_ immediately, run HandleWrap::OnClose and treat the wrap as fully torn down.
This would leave prepare_handle_ on need of a uv_close, so you’d risk a libuv handle leak or teardown in the wrong order relative to HandleWrap’s lifecycle.
There was a problem hiding this comment.
Hi @addaleax , I initially misunderstood your comment. I've refactored the Close method to use the superclass Close method instead of duplicating the shared logic.
edb66ea to
e7093c8
Compare
6f5cdca to
bc0a59c
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62935 +/- ##
==========================================
- Coverage 89.65% 89.64% -0.02%
==========================================
Files 708 708
Lines 220413 220546 +133
Branches 42275 42290 +15
==========================================
+ Hits 197607 197702 +95
- Misses 14658 14675 +17
- Partials 8148 8169 +21
🚀 New features to boost your workflow:
|
bc0a59c to
27ead1e
Compare
add a samplePerIteration option to monitorEventLoopDelay that records event loop delay from libuv event loop iterations instead of the existing timer interval sampler. The default remains the interval based, uses of monitorEventLoopDelay() will still behave the same unless the samplePerIteration options is passed through. Signed-off-by: Pablo Erhard Hernandez <pablo.erhardhernandez@datadoghq.com>
Add test coverage for the per iteration event loop delay histogram start and stop callbacks Use disitinct debug tracking keys for the event loop delay histogram callbacks.
Use the super Close method instead of duplicating code
27ead1e to
3b21617
Compare
Adds an opt in into monitorEventLoopDelay(), this option adds support for per iteration sampling using libuv hooks, while preserving the existing timer based approach.
When the new samplePerIteration option is set to true, monitorEventLoopDelay() samples directly from event loop iterations instead of using the interval timer. When samplePerIteration is omitted or false, the existing interval based implementation remains unchanged, including resolution handling and the older interval based trace counters.
Motivation
The interval based implementation has two limitations that the new option addresses:
This is an alternative implementation to the one in #62934
This PR directly addresses the following issue #56064