Added basic GoodJob collectors

[AndersGM]

I need metrics for GoodJob, so I've given it a shot.

[SamSaffron]

looking good but I think there is a bit of fine tuning we need here:

see: https://prometheus.io/docs/practices/naming/

there is a total postfix missing and some of these should be Counter vs Gauge no?

[AndersGM]

@SamSaffron Thank you very much for taking your time to look at my PR!

I've never implemented my own metrics, so I might very well be wrong. I chose gauges for all of the metrics since data is read from the database, and jobs can be destroyed and retried at will. Consequently, the numbers can actually decrement for all the metrics, which isn't allowed for counters (AFAIK).

To get actual counters, I think we'll need to either extend GoodJob or count through callbacks.

Let me know what you think!

[SamSaffron]

be sure to read up about it, counters are "smart" they can go back to 0, it is fine, prom adjusts to it.

Gauges are for things where "rate" does not matter like memory.
Counters are for things where "rate" matters, like counting stuff.

[AndersGM]

be sure to read up about it, counters are "smart" they can go back to 0, it is fine, prom adjusts to it.

Yes - my point is that since, with my suggested implementation, we read data directly from the database, the numbers would never really be reset, but rather (potentially) decrease by some number. That could be when the user manually delete some of the jobs through GoodJob's user interface (so the number of succeeded jobs could go from 200 to 199). It could also be when jobs older than 14days are deleted through automatic clean ups. Counters can only ever increment or be reset to zero - never decrease :-)

I completely agree that counters would be favorable for the use case, however, we'll need to build on top of GoodJob (eg. count each time a job has completed). I can see there is already some work going on in GoodJob about adding stats, that we can then read from, but I think we'll wait quite some time before its ready, so this is best effort at the time of writing AFAIK :-)

[SamSaffron]

I see! will merge this then, naming is not ideal prom likes to recommend _total. But it seems reasonably cohesive with our existing naming

[AndersGM]

@SamSaffron Agree! At least _total is prohibited for gauge:

prometheus_exporter/lib/prometheus_exporter/metric/gauge.rb

Line 9 in 8d77c53

raise ArgumentError, "The metric name of gauge must not have _total suffix. Given: #{name}"

I will try and work on a better implementation in the GoodJob project.

[lauer]

@AndersGM did you find a solution to handle that those counters can decrease doing clean up script?
As I see it, the problem would persist in this new pr #302

[AndersGM]

@lauer I implemented metrics for GoodJob in this PR to address the issue: bensheldon/good_job#984 - haven't looked into it since. The file is now improved and moved to https://github.com/bensheldon/good_job/blob/main/lib/good_job/job_performer/metrics.rb to support MultiScheduler.

As far as I can see I forgot about it, and never actually implemented it in prometheus_exporter. Consequently, it is still just using the database counts. I'll try and look into using the metrics from GoodJob later today - it should be straight forward to use the metrics there :-)

[lauer]

okay, an other issue we have, is that the SQL queries around database count is showed when we are using rails console.