Add collector for database/sql#DBStats

[johejo]

MaxIdleTimeClosed is not available below go1.15.
Therefore it is TODO.

Signed-off-by: Mitsuo Heijo [email protected]

[beorn7]

Thanks. This is a neat idea. I'm just not sure if it should live in the Go instrumentation client or if it should rather be a separate tool. Another question is if the metrics should be prefixed with go_.... While the database/sql package is Go-specific, those metrics aren't really coming from the Go runtime.

I would like to discuss that with the Prometheus community first. This kind of discussions happens on the prometheus-developers mailing list. @johejo would you like to start a thread there? Otherwise, I can do it.

[johejo]

Thank you.
I posted a new conversation.
https://groups.google.com/g/prometheus-developers/c/Zexpd_oRYRQ

[beorn7]

To put the conclusion from the mailing list thread here:

In general, it's fine to have this kind of collector here in prometheus/client_golang. Nobody voiced their opinion if we should have a separate collectors package. We already have four similar collectors directly in the prometheus package, and we cannot move them out without a breaking change. But at some point, we have to draw a line, so I'd say let's put it into a new collectors package, and move other collectors (like NewBuildInfoCollector) over in the next major release. (We could already now replicate the existing collectors and deprecate them.)

However, this PR is already the third implementation after https://github.com/dlmiddlecote/sqlstats and https://github.com/krpn/go-sql-db-stats . At first glance, the latter seems more elaborate, but it's still not following all best practices (e.g. counters don't end on _total). Same is true for this PR.

Next step would be to take the best of all three implementations and unite them here (could be in this PR or a new one) and also make sure naming conventions are honored (e.g. counter names end on _total, unit in the metric name where applicable, etc. see https://prometheus.io/docs/practices/naming/ ).

[johejo]

Fixed label naming.

[beorn7]

Hi @johejo , are you still working on this? If you feel it's ready for a detailed review, please let me know. (At first glance, I saw that you haven't moved the code into a new collectors package as discussed above.)

[johejo]

It's okay.
I will work this weekend.
Thanks.
@beorn7

[johejo]

I thought it would be better to have a separate PR for the dbstats collector and the new package.