fastly / fastly-exporter

A Prometheus exporter for the Fastly Real-time Analytics API
Apache License 2.0
99 stars 36 forks source link

remove tls_version="any" from fastly_rt_tls_total #121

Closed mrnetops closed 1 year ago

mrnetops commented 1 year ago

This is essentially including a total field in the metric itself which is violates

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

As a rule of thumb, either the sum() or the avg() over all dimensions of a given metric should be meaningful (though not necessarily useful).

As this essentially means that a simple sum(rate(fastly_rt_tls_total[1m])) by (service_name) ends up double counting the values (v10 + v11 + v12 + v13 + any) as seen in this normalized snippet

fastly_rt_tls_total{datacenter="YYZ",service_id="XXX",service_name="XXX",tls_version="any"} 11686
fastly_rt_tls_total{datacenter="YYZ",service_id="XXX",service_name="XXX",tls_version="v10"} 0
fastly_rt_tls_total{datacenter="YYZ",service_id="XXX",service_name="XXX",tls_version="v11"} 0
fastly_rt_tls_total{datacenter="YYZ",service_id="XXX",service_name="XXX",tls_version="v12"} 11686
fastly_rt_tls_total{datacenter="YYZ",service_id="XXX",service_name="XXX",tls_version="v13"} 0

We shouldn't be adding any total or sum subvalue that will impact totaling or summing the metric itself.

mrnetops commented 1 year ago

While we're at it, can we possibly rename the vXX format that admittedly matches the fastly documentation and data structure, but absolutely does not match how anybody sane would present the data?

i.e. nobody says tls v12 rather then tls 1.2, and this leads to needless overrides when trying to present the data.

In grafana for example, I need to break it into individual queries with explicit legend overrides, or try and find other override/formatting trickery to get presentation to make sense.

https://www.google.com/search?q=tls+v12 returns 395,000 results (most of which are actually 1.2 references) https://www.google.com/search?q=tls+1.2 returns 28,900,000 results

leklund commented 1 year ago

pushed a pre-release with this change https://github.com/fastly/fastly-exporter/releases/tag/v8.0.0-beta

leklund commented 7 months ago

@mrnetops This is officially released (at last): https://github.com/fastly/fastly-exporter/releases/tag/v8.0.0