From b1d456d8abf15616afb6c6893af90473b4941365 Mon Sep 17 00:00:00 2001 From: Dave Henderson Date: Mon, 21 Sep 2020 15:42:47 -0400 Subject: metrics: Fix panic when headers aren't written (#3737) Signed-off-by: Dave Henderson --- 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 74be135..b764b90 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 6ac591f..72f29a7 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 -- cgit v1.2.3