search

discourse / prometheus_exporter

A framework for collecting and aggregating prometheus metrics
MIT License
525 stars 153 forks source link

rename metrics to match prometheus official naming convention #182

Closed Shimokuni closed 2 years ago

Shimokuni commented 2 years ago

Hello.

In this gem, there are some metrics which have _total suffix name but actually its metrics type is Gauge.

e.g. Unicorn process metrics

Type Name Description
Gauge unicorn_workers_total Number of unicorn workers
Gauge unicorn_active_workers_total Number of active unicorn workers
Gauge unicorn_request_backlog_total Number of requests waiting to be processed by a unicorn worker

https://github.com/discourse/prometheus_exporter#metrics-collected-by-unicorn-instrumentation

According to prometheus official guide for metric names, the metrics which are named with _total suffix must be Counter, not Gauge.

This can be a problem when integrating with other services which infers the metric type based on prometheus naming conventions. e.g. New Relic prometheus remote write https://docs.newrelic.com/docs/integrations/prometheus-integrations/install-configure-remote-write/set-your-prometheus-remote-write-integration/#mapping

Of course we can use relabelConfig, but It takes effort...

What do you think about renaming the metrics to match the formal Promethean naming convention? Can I write PR? It will be breaking change, but I think it's the right change.

SamSaffron commented 2 years ago

Sure I am fine with a rename here, I guess we need a major version change cause we could break people.

unicorn_workers, unicorn_active_workers unicorn_request_backlog, be sure to add a note to changelog and review the readme for accuracy.

Shimokuni commented 2 years ago

PR ready. https://github.com/discourse/prometheus_exporter/pull/184 Could you review?