From f197cec7f3a599ca18807e7b7719ef7666cfdb70 Mon Sep 17 00:00:00 2001 From: Dave Henderson Date: Tue, 22 Sep 2020 22:10:34 -0400 Subject: [PATCH] metrics: Always track method label in uppercase (#3742) * metrics: Always track method label in uppercase Signed-off-by: Dave Henderson * Just use strings.ToUpper for clarity Signed-off-by: Dave Henderson --- admin.go | 5 ++-- metrics.go | 45 ++++++++++++++++++++++++++++++++++++ modules/caddyhttp/metrics.go | 7 ++++-- 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/admin.go b/admin.go index 4ce20998..b2eb5423 100644 --- a/admin.go +++ b/admin.go @@ -36,7 +36,6 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promhttp" "go.uber.org/zap" ) @@ -111,7 +110,7 @@ func (admin AdminConfig) newAdminHandler(addr NetworkAddress) adminHandler { addRouteWithMetrics := func(pattern string, handlerLabel string, h http.Handler) { labels := prometheus.Labels{"path": pattern, "handler": handlerLabel} - h = promhttp.InstrumentHandlerCounter( + h = instrumentHandlerCounter( adminMetrics.requestCount.MustCurryWith(labels), h, ) @@ -126,7 +125,7 @@ func (admin AdminConfig) newAdminHandler(addr NetworkAddress) adminHandler { labels := prometheus.Labels{ "path": pattern, "handler": handlerLabel, - "method": r.Method, + "method": strings.ToUpper(r.Method), } adminMetrics.requestErrors.With(labels).Inc() } diff --git a/metrics.go b/metrics.go index a7ff3b71..55fb0a31 100644 --- a/metrics.go +++ b/metrics.go @@ -1,6 +1,10 @@ package caddy import ( + "net/http" + "strconv" + "strings" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" ) @@ -30,3 +34,44 @@ var adminMetrics = struct { requestCount *prometheus.CounterVec requestErrors *prometheus.CounterVec }{} + +// Similar to promhttp.InstrumentHandlerCounter, but upper-cases method names +// instead of lower-casing them. +// +// Unlike promhttp.InstrumentHandlerCounter, this assumes a "code" and "method" +// label is present, and will panic otherwise. +func instrumentHandlerCounter(counter *prometheus.CounterVec, next http.Handler) http.HandlerFunc { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + d := newDelegator(w) + next.ServeHTTP(d, r) + counter.With(prometheus.Labels{ + "code": sanitizeCode(d.status), + "method": strings.ToUpper(r.Method), + }).Inc() + }) +} + +func newDelegator(w http.ResponseWriter) *delegator { + return &delegator{ + ResponseWriter: w, + } +} + +type delegator struct { + http.ResponseWriter + status int +} + +func (d *delegator) WriteHeader(code int) { + d.status = code + d.ResponseWriter.WriteHeader(code) +} + +func sanitizeCode(s int) string { + switch s { + case 0, 200: + return "200" + default: + return strconv.Itoa(s) + } +} diff --git a/modules/caddyhttp/metrics.go b/modules/caddyhttp/metrics.go index b764b903..3e5d6396 100644 --- a/modules/caddyhttp/metrics.go +++ b/modules/caddyhttp/metrics.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "strconv" + "strings" "sync" "time" @@ -108,7 +109,10 @@ func newMetricsInstrumentedHandler(handler string, mh MiddlewareHandler) *metric func (h *metricsInstrumentedHandler) ServeHTTP(w http.ResponseWriter, r *http.Request, next Handler) error { server := serverNameFromContext(r.Context()) labels := prometheus.Labels{"server": server, "handler": h.handler} - statusLabels := prometheus.Labels{"server": server, "handler": h.handler, "method": r.Method, "code": ""} + method := strings.ToUpper(r.Method) + // the "code" value is set later, but initialized here to eliminate the possibility + // of a panic + statusLabels := prometheus.Labels{"server": server, "handler": h.handler, "method": method, "code": ""} inFlight := httpMetrics.requestInFlight.With(labels) inFlight.Inc() @@ -154,7 +158,6 @@ func sanitizeCode(code int) string { return "200" } return strconv.Itoa(code) - } // taken from https://github.com/prometheus/client_golang/blob/6007b2b5cae01203111de55f753e76d8dac1f529/prometheus/promhttp/instrument_server.go#L298