export RQ statistics as prometheus metrics #666
Conversation
terencehonles
force-pushed
the
export-prometheus-metrics
branch
3 times, most recently
from
288bbf5 to
30e3eb4
Compare
terencehonles
force-pushed
the
export-prometheus-metrics
branch
5 times, most recently
from
79f6394 to
70f07f8
Compare
terencehonles
force-pushed
the
export-prometheus-metrics
branch
2 times, most recently
from
de7de29 to
84c84bf
Compare
django_rq/metrics_collector.py
Outdated
|
|
||
| with self.summary.time(): | ||
| rq_workers = GaugeMetricFamily('rq_workers', 'RQ workers', labels=['name', 'state', 'queues']) | ||
| rq_workers_success = CounterMetricFamily('rq_workers_success', 'RQ workers success count', labels=['name', 'queues']) |
There was a problem hiding this comment.
Can you rename rq_workers_success to successful_job_count?
There was a problem hiding this comment.
I thought the names should all be prefixed by "rq_", but I agree this is about the jobs not the workers so I renamed it to rq_job_successful_total. I used that prefix to keep both success/failure counts to be prefixed by "rq_job_" and used "total" instead of "count" because Prometheus' naming conventions suggest that the unit should be a suffix and "total" is preferred over count:
See: https://prometheus.io/docs/practices/naming/ & prometheus/docs#2465 (comment)
django_rq/metrics_collector.py
Outdated
| rq_workers = GaugeMetricFamily('rq_workers', 'RQ workers', labels=['name', 'state', 'queues']) | ||
| rq_workers_success = CounterMetricFamily('rq_workers_success', 'RQ workers success count', labels=['name', 'queues']) | ||
| rq_workers_failed = CounterMetricFamily('rq_workers_failed', 'RQ workers fail count', labels=['name', 'queues']) | ||
| rq_workers_working_time = CounterMetricFamily('rq_workers_working_time', 'RQ workers spent seconds', labels=['name', 'queues']) |
There was a problem hiding this comment.
For the same reasons in the thread #666 (comment) I went with rq_working_seconds_total
|
I don't mind merging this in, but could you document this feature? A simple unit test to show that this view works would also be great. |
terencehonles
force-pushed
the
export-prometheus-metrics
branch
from
84c84bf to
8066be6
Compare
There was a problem hiding this comment.
I don't mind merging this in, but could you document this feature? A simple unit test to show that this view works would also be great.
Sorry for the delay getting this updated, but here it is. The tests took a bit of finagling to get working, but I hope this is acceptable (running tests twice).
You can let me know if you have a strong preference about the metric names, but I reviewed what I put and adjusted them to match the Prometheus naming conventions (they were close but could use some updating), and I factored in the comments you made.
django_rq/metrics_collector.py
Outdated
|
|
||
| with self.summary.time(): | ||
| rq_workers = GaugeMetricFamily('rq_workers', 'RQ workers', labels=['name', 'state', 'queues']) | ||
| rq_workers_success = CounterMetricFamily('rq_workers_success', 'RQ workers success count', labels=['name', 'queues']) |
There was a problem hiding this comment.
I thought the names should all be prefixed by "rq_", but I agree this is about the jobs not the workers so I renamed it to rq_job_successful_total. I used that prefix to keep both success/failure counts to be prefixed by "rq_job_" and used "total" instead of "count" because Prometheus' naming conventions suggest that the unit should be a suffix and "total" is preferred over count:
See: https://prometheus.io/docs/practices/naming/ & prometheus/docs#2465 (comment)
django_rq/metrics_collector.py
Outdated
| rq_workers = GaugeMetricFamily('rq_workers', 'RQ workers', labels=['name', 'state', 'queues']) | ||
| rq_workers_success = CounterMetricFamily('rq_workers_success', 'RQ workers success count', labels=['name', 'queues']) | ||
| rq_workers_failed = CounterMetricFamily('rq_workers_failed', 'RQ workers fail count', labels=['name', 'queues']) | ||
| rq_workers_working_time = CounterMetricFamily('rq_workers_working_time', 'RQ workers spent seconds', labels=['name', 'queues']) |
There was a problem hiding this comment.
For the same reasons in the thread #666 (comment) I went with rq_working_seconds_total
terencehonles
force-pushed
the
export-prometheus-metrics
branch
from
8066be6 to
3dda38a
Compare
terencehonles
force-pushed
the
export-prometheus-metrics
branch
from
3dda38a to
0463893
Compare
|
It looks like the test failures are related to the fix I added in #700 |
This change follows the implementation of
rq_exporterand the prometheus client to export the RQ statistics as a Django view.fixes: #503