search

psyinfra / prometheus-eaton-ups-exporter

A Prometheus exporter for (some) Eaton UPSs
ISC License
11 stars 5 forks source link

percent should be "ratio" #11

Closed aqw closed 3 years ago

aqw commented 3 years ago

Prometheus best practice is to use "ratio": rather than percent. https://prometheus.io/docs/practices/naming/#base-units

The field names should be updated to end in _ratio rather than _percent. And the values should be changed accordingly.

mathisloevenich commented 3 years ago

So would this mean ups_output_percent_load_in_percent should rather be ups_output_percent_load_ratio ?

aqw commented 3 years ago

Correct. And the value itself should be (for example) 0.82 instead of 82.

mathisloevenich commented 3 years ago

Ah okay, I've just exposed the data as it is given by the API. Is it a common thing to change the format of the scraped data?

mathisloevenich commented 3 years ago

Done

aqw commented 3 years ago

For something as simple as this, it definitely makes sense. If we normalize the data flowing into Prometheus, it makes it much easier to reuse graphs and compare across systems.

However, in other situations, we ignore it. For example, Prometheus suggests using joules rather than watts. IMO, it's not worth it to adjust our watt metrics into joules, so we'll leave them as they are. :-)

nhjjr commented 3 years ago

Sorry to bug again on this issue, but it's better to prefix metrics with eaton_ups_* instead of ups_*, because this exporter only works for eaton UPS's.

mathisloevenich commented 3 years ago

Alright I will make the changes

nhjjr commented 3 years ago

Alex has addressed this in PR #15 already. If you can validate it and merge it to master, then we can run it and I'll set up the accompanying Eaton UPS Dashboard.

mathisloevenich commented 3 years ago

I just saw it.

mathisloevenich commented 3 years ago

Merged