-
Notifications
You must be signed in to change notification settings - Fork 380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extra metrics #34
Extra metrics #34
Conversation
} | ||
|
||
module.exports = function() { | ||
var gauge = new Gauge('node_eventloop_lag_milliseconds', 'Lag of event loop in milliseconds.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could report this in nanoseconds instead?
Thanks, as always, a test for this would be great - i will probably mess stuff up sooner rather than later :) |
var gauge = new Gauge('node_active_handles_total', 'Number of active requests.'); | ||
|
||
return function() { | ||
gauge.set(process._getActiveRequests().length); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_getActiveRequests and _getActiveHandles are new stuff to me, tried googling it and didn't find much info in node documentation. Is it a good practice to depend on "private" functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the current recommended way. nodejs/node#1128 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could add an if
guard like for process.cpuUsage
in case it disappears in later nodes, if you want?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be great! Just to as you said to safe guard us for the future
@siimon Should be GTG now. Took way more time than it should because timers didn't work. Fix for that included in its own commit. |
Lets just add a safeguard and I'll merge right away! |
@siimon Done! |
Merged and published to npm with a new major version! |
Builds on top of #25. I'll rebase after that's merged (or you can just merge this).
See the last commit in this PR for the diff in the meantime.
Ideas for extra metrics very much welcome!
At work we show more of
memoryUsage
(but I think those are covered in #25, although just for Linux) andos.loadAvg[0]
, but I'm not sure if those are actually useful?