justwatchcom
Add option to have /healthz probes to be tied to database(s) connectivity
It could be useful to fail sql-exporter by means of K8s health probing by simply checking all configured DB connections could be established, failing /healthz probing otherwise.
Hello, any chance this feature request could be merged? We'd pretty much appreciate it for our setup. Thank you.
Hey, I'm not sure it's a good idea to kill the service if one database can't connect. That way your metrics would drop out for all databases just because one database has a connection issue / being restarted.
A better way to do that would be to write an alert based on the absence of a metric, or an outdated timestamp if you export one through a metric.
Hi @dewey, thanks for the quick reply. The point of this behavior is that we have some database password rotation carried out regularly and having this feature enabled allows auto-restart of the sql_exporter container (we use K8s for container provisioning) which all of a sudden loses ability to connect to the database as the respective secret has changed.
I can imagine a situation when sql_exporter has only jobs configured for night metrics scraping, thus such metrics would be outdated during the day and nobody would figure out there's something wrong with sql_exporter as it doesn't need to connect to DB until night hours.
Thanks for describing your use case @alsin, I understand your reasoning! In this case I still think it's not a good fit to merge as for your use case it would work well, but for others that could be very unexpected.
I'd maybe look into triggering a restart of the service through other means in that case.
...but for others that could be very unexpected.
That's why my PR suggests an optional behavior which is off by default and to use it one should intentionally enable it by setting the db.connectivity-as-healthz flag to true. Otherwise it stays off and the healthcheck endpoint works just as previously.
As an alternative we could require all databases to be unreachable and only after that try to kill the service.
I think that would make more sense. The code would also need to be formatted a bit, I think there's a gofmt missing as there's a lot of new lines being introduced.
@dewey Please, take a look again. I've changed it the way we agreed on. I also ran gofmt on the changes.