django-prometheus icon indicating copy to clipboard operation
django-prometheus copied to clipboard

prometheus_client>=0.4.0's new Counter names is creating awkward names

Open crccheck opened this issue 5 years ago • 7 comments

Prometheus/OpenMetrics/prometheus_client has standardized on making counters end in _total: https://github.com/prometheus/client_python/releases/tag/v0.4.0

Counter time series will now always be exposed with _total, and counter metrics will have a _total suffix stripped. This is as the internal data model is now OpenMetrics, rather than Prometheus Text Format

This means all the total_by_xxx counters are now total_by_xxx_total. Which is awkward. Examples:

  • django_http_responses_total_by_templatename_total
  • django_http_requests_total_by_method_total
  • django_http_requests_total_by_view_transport_method_total
  • django_http_exceptions_total_by_type_total

I'm not sure what the new names should be, but hopefully someone has a good idea.

crccheck avatar Oct 09 '18 19:10 crccheck

my current opinion is to drop the first _total so they read like django_http_requests_by_view_transport_method_total

crccheck avatar Oct 09 '18 19:10 crccheck

I agree with your suggestion. I feel awkward about breaking timeseries history though. Maybe it's better to expose both, and remove the awkward name in a future major release? Wdyt?

On Wed, Oct 10, 2018, 03:53 Chris Chang [email protected] wrote:

my current opinion is to drop the first _total so they read like django_http_requests_by_view_transport_method_total

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/korfuri/django-prometheus/issues/84#issuecomment-428328241, or mute the thread https://github.com/notifications/unsubscribe-auth/ABEnp1XIkcSBtYDuWKHNPTYlCt4x7l8sks5ujP7LgaJpZM4XUAn3 .

korfuri avatar Oct 10 '18 01:10 korfuri

The downside to exporting both is it's a bigger load on Prometheus. There's probably a way to do the same thing using metric_relabel_configs in prometheus.yml. My Prometheus is managed by another team so I don't have the ability to try it out without spinning up my own Prometheus. In the past, I've had terrible luck getting that config to work but I always got it to work eventually.

I went ahead and kept my client version at 0.4.1 and just added a temporary query to my Grafana dashboard to cover the old name. It really sucks, but I've gotten used to metrics being ephemeral and screenshots being my only history. The default retention for Prom is 15 days so they think of things as being ephemeral too.

crccheck avatar Oct 10 '18 16:10 crccheck

Related: #88 upgrades the client. I requested some changes there but when it's done I'll release this as a breaking release of django-prometheus, with notes about breaking history.

korfuri avatar Oct 23 '18 09:10 korfuri

I would like to implement https://github.com/korfuri/django-prometheus/issues/57 to break down the metric into status code per view or whatever as well.. Maybe it's a good time to work a bit towards a breaking release in how metrics are named? (The company I work for is going to do a heavy switch towards an exclusive prometheus only monitoring solution atm. and I really need a solution for #57 as part of that project) Time for django-prometheus 2.0 ?

thomasf avatar Oct 30 '18 11:10 thomasf

That sounds good. Go ahead :)

On Tue, Oct 30, 2018, 19:38 Thomas Frössman [email protected] wrote:

I would like to implement #57 https://github.com/korfuri/django-prometheus/issues/57 to break down the metric into status code per view or whatever as well.. Maybe it's a good time to work a bit towards a breaking release in how metrics are named?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/korfuri/django-prometheus/issues/84#issuecomment-434267657, or mute the thread https://github.com/notifications/unsubscribe-auth/ABEnpzIA2zXKWpefDMp9iuBJF_YUyXSJks5uqDo1gaJpZM4XUAn3 .

korfuri avatar Oct 30 '18 16:10 korfuri

It's worth noting that this is already a breaking change. Some of our graphs needed to be modified to show both counters when upgrading.

stealthybox avatar Jan 11 '19 00:01 stealthybox