Split process request and process handles by type

[yvasiyarov]

Hello @siimon
In my project I found it quite useful to monitor not only total number of active requests and handles(process._getActiveRequests()/process._getActiveHandles()), but also split them by type.

If you see that number of active handles is growing but you dont know the handle type - it did not help you much track down the problem.

# HELP nodejs_active_handles Number of active handles grouped by handle type.
# TYPE nodejs_active_handles gauge
nodejs_active_handles{type="WriteStream"} 2 1556027670456
nodejs_active_handles{type="Server"} 2 1556027670456
nodejs_active_handles{type="Socket"} 2 1556027670456

With this patch you will see what exactly is going wrong with your node js app.
I kept original nodejs_active_handles_total/nodejs_active_requests_total metrics for backward compatibility. So there is no breaking changes in this pull request

[yvasiyarov]

sorry, let me fix the linter errors

[SimenB]

We don't need a total and by type, you can just not select by labels

[yvasiyarov]

@SimenB I kept the separate total metric to keep backward compatibility.
If I will drop then guys who rely on this metric can loose their alerts and their graphics will be broken. I think its better to avoid breaking changes if possible.

[SimenB]

since we did not have labels before, I don't think it's a breaking change to add them. unless people group by labels, it'll look the exact same

[yvasiyarov]

@SimenB metric names are different.
Original "total" gauge has name "nodejs_active_requests_total" and "nodejs_active_handles_total"
New gauges has name "nodejs_active_requests" and "nodejs_active_handles"

So everybody has to explicitly change prometheus queries from nodejs_active_requests_total to sum(nodejs_active_requests).

[SimenB]

Yes, my point is that we don't need new metrics since these should just be labels, right?

[yvasiyarov]

@SimenB if I use same name (nodejs_active_requests_total and nodejs_active_handles_total)
then everyone have to change query from nodejs_active_handles_total to sum(nodejs_active_handles_total). And this is breaking change.

My point was to avoid make breaking changes.

[SimenB]

Right, I agree this seems best then. Thanks!

[yvasiyarov]

@SimenB please let me know if proposed changes can be accepted into your project or I have to live with my own fork

[akopichin]

I think it's a useful feature.

[SimenB]

Code LGTM, but I think this might deserve a note in the docs? To explain the difference.

I'm without a computer for the weekend, so unable to publish this. So I'll leave that to the other maintainers :)

[yvasiyarov]

@SimenB I've updated CHANGELOG and make metrics documentation more detailed

[siimon]

I think it looks good to me as well!

[yvasiyarov]

Thank you, guys!

[akopichin]

@siimon Can you publish this version to npm please?

[SimenB]

I can do that

[SimenB]

https://github.com/siimon/prom-client/releases/tag/v11.4.0

[akopichin]

Thank you! ;)