From b1d456d8abf15616afb6c6893af90473b4941365 Mon Sep 17 00:00:00 2001
From: Dave Henderson <dhenderson@gmail.com>
Date: Mon, 21 Sep 2020 15:42:47 -0400
Subject: [PATCH] metrics: Fix panic when headers aren't written (#3737)
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
---
modules/caddyhttp/metrics.go | 10 +++++++++-
modules/caddyhttp/metrics_test.go | 18 ++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/modules/caddyhttp/metrics.go b/modules/caddyhttp/metrics.go
index 74be135f1..b764b9036 100644
--- a/modules/caddyhttp/metrics.go
+++ b/modules/caddyhttp/metrics.go
@@ -108,7 +108,7 @@ 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}
+ statusLabels := prometheus.Labels{"server": server, "handler": h.handler, "method": r.Method, "code": ""}
inFlight := httpMetrics.requestInFlight.With(labels)
inFlight.Inc()
@@ -134,6 +134,14 @@ func (h *metricsInstrumentedHandler) ServeHTTP(w http.ResponseWriter, r *http.Re
return err
}
+ // If the code hasn't been set yet, and we didn't encounter an error, we're
+ // probably falling through with an empty handler.
+ if statusLabels["code"] == "" {
+ // we still sanitize it, even though it's likely to be 0. A 200 is
+ // returned on fallthrough so we want to reflect that.
+ statusLabels["code"] = sanitizeCode(wrec.Status())
+ }
+
httpMetrics.requestDuration.With(statusLabels).Observe(dur)
httpMetrics.requestSize.With(statusLabels).Observe(float64(computeApproximateRequestSize(r)))
httpMetrics.responseSize.With(statusLabels).Observe(float64(wrec.Size()))
diff --git a/modules/caddyhttp/metrics_test.go b/modules/caddyhttp/metrics_test.go
index 6ac591ff6..72f29a707 100644
--- a/modules/caddyhttp/metrics_test.go
+++ b/modules/caddyhttp/metrics_test.go
@@ -57,6 +57,24 @@ func TestMetricsInstrumentedHandler(t *testing.T) {
if err := ih.ServeHTTP(w, r, h); err != nil {
t.Errorf("Received unexpected error: %w", err)
}
+
+ // an empty handler - no errors, no header written
+ mh = middlewareHandlerFunc(func(w http.ResponseWriter, r *http.Request, h Handler) error {
+ return nil
+ })
+ ih = newMetricsInstrumentedHandler("empty", mh)
+ r = httptest.NewRequest("GET", "/", nil)
+ w = httptest.NewRecorder()
+
+ if err := ih.ServeHTTP(w, r, h); err != nil {
+ t.Errorf("Received unexpected error: %w", err)
+ }
+ if actual := w.Result().StatusCode; actual != 200 {
+ t.Errorf("Not same: expected status code %#v, but got %#v", 200, actual)
+ }
+ if actual := w.Result().Header; len(actual) != 0 {
+ t.Errorf("Not empty: expected headers to be empty, but got %#v", actual)
+ }
}
type middlewareHandlerFunc func(http.ResponseWriter, *http.Request, Handler) error