From 49e2074f7419f54f8d84bd101083d3f26bfb5235 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Sat, 12 Nov 2016 18:58:43 +0100 Subject: [PATCH 1/2] Use Prometheus instrumentation conventions The `System` and `Subsystem` parameters are subject to removal. (x-ref: https://github.com/prometheus/client_golang/issues/240) All metrics should use base units, which is seconds in the duration case. Counters should always end in `_total` and metrics should avoid referring to potential label dimensions. Those should rather be mentioned in the documentation string. --- pkg/client/metrics/prometheus/prometheus.go | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/pkg/client/metrics/prometheus/prometheus.go b/pkg/client/metrics/prometheus/prometheus.go index 812b935ef59..52a2f9befcf 100644 --- a/pkg/client/metrics/prometheus/prometheus.go +++ b/pkg/client/metrics/prometheus/prometheus.go @@ -27,26 +27,22 @@ import ( "github.com/prometheus/client_golang/prometheus" ) -const restClientSubsystem = "rest_client" - var ( // requestLatency is a Prometheus Summary metric type partitioned by // "verb" and "url" labels. It is used for the rest client latency metrics. requestLatency = prometheus.NewSummaryVec( prometheus.SummaryOpts{ - Subsystem: restClientSubsystem, - Name: "request_latency_microseconds", - Help: "Request latency in microseconds. Broken down by verb and URL", - MaxAge: time.Hour, + Name: "rest_client_request_latency_seconds", + Help: "Request latency in seconds. Broken down by verb and URL.", + MaxAge: time.Hour, }, []string{"verb", "url"}, ) requestResult = prometheus.NewCounterVec( prometheus.CounterOpts{ - Subsystem: restClientSubsystem, - Name: "request_status_codes", - Help: "Number of http requests, partitioned by metadata", + Name: "rest_client_requests_total", + Help: "Number of HTTP requests, partitioned by status code, method, and host.", }, []string{"code", "method", "host"}, ) @@ -63,8 +59,7 @@ type latencyAdapter struct { } func (l *latencyAdapter) Observe(verb string, u url.URL, latency time.Duration) { - microseconds := float64(latency) / float64(time.Microsecond) - l.m.WithLabelValues(verb, u.String()).Observe(microseconds) + l.m.WithLabelValues(verb, u.String()).Observe(latency.Seconds()) } type resultAdapter struct { From 2b66b49a2fda94eab87296a24affa87e2c7103fc Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Mon, 14 Nov 2016 12:45:13 +0100 Subject: [PATCH 2/2] Use Histogram instead of Summary A histogram allows to aggregate by labels and calculate more comprehensive quantiles. --- pkg/client/metrics/prometheus/prometheus.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/client/metrics/prometheus/prometheus.go b/pkg/client/metrics/prometheus/prometheus.go index 52a2f9befcf..d9265f4ddc3 100644 --- a/pkg/client/metrics/prometheus/prometheus.go +++ b/pkg/client/metrics/prometheus/prometheus.go @@ -30,11 +30,11 @@ import ( var ( // requestLatency is a Prometheus Summary metric type partitioned by // "verb" and "url" labels. It is used for the rest client latency metrics. - requestLatency = prometheus.NewSummaryVec( - prometheus.SummaryOpts{ - Name: "rest_client_request_latency_seconds", - Help: "Request latency in seconds. Broken down by verb and URL.", - MaxAge: time.Hour, + requestLatency = prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "rest_client_request_latency_seconds", + Help: "Request latency in seconds. Broken down by verb and URL.", + Buckets: prometheus.ExponentialBuckets(0.001, 2, 10), }, []string{"verb", "url"}, ) @@ -55,7 +55,7 @@ func init() { } type latencyAdapter struct { - m *prometheus.SummaryVec + m *prometheus.HistogramVec } func (l *latencyAdapter) Observe(verb string, u url.URL, latency time.Duration) {