metrics: Always track method label in uppercase (#3742)

* metrics: Always track method label in uppercase Signed-off-by: Dave Henderson <[email protected]> * Just use strings.ToUpper for clarity Signed-off-by: Dave Henderson <[email protected]>
caddyserver · Sep 23, 2020 · f197cec · f197cec
1 parent be6daa5
commit f197cec
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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