diff options
author | Dave Henderson <dhenderson@gmail.com> | 2020-09-17 23:46:24 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-09-17 21:46:24 -0600 |
commit | d16ede358a2ec049bda28bf37e79c9cfcaa64c29 (patch) | |
tree | 9304f4b06287fa2cbeafda019a70556a00e119ee /modules/caddyhttp/metrics.go | |
parent | c82c231ba7ec7c400e4d48ab943f57a6b29a2a2a (diff) |
metrics: Fix hidden panic while observing with bad exemplars (#3733)
* metrics: Fixing panic while observing with bad exemplars
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Minor cleanup
The server is already added to the context. So, we can simply use that
to get the server name, which is a field on the server.
* Add integration test for auto HTTP->HTTPS redirects
A test like this would have caught the problem in the first place
Co-authored-by: Matthew Holt <mholt@users.noreply.github.com>
Diffstat (limited to 'modules/caddyhttp/metrics.go')
-rw-r--r-- | modules/caddyhttp/metrics.go | 52 |
1 files changed, 18 insertions, 34 deletions
diff --git a/modules/caddyhttp/metrics.go b/modules/caddyhttp/metrics.go index 2b3b7c7..74be135 100644 --- a/modules/caddyhttp/metrics.go +++ b/modules/caddyhttp/metrics.go @@ -82,44 +82,38 @@ func initHTTPMetrics() { }, httpLabels) } -type ctxKeyServerName struct{} - // serverNameFromContext extracts the current server name from the context. -// Returns "UNKNOWN" if none is available (should probably never happen?) +// Returns "UNKNOWN" if none is available (should probably never happen). func serverNameFromContext(ctx context.Context) string { - srvName, ok := ctx.Value(ctxKeyServerName{}).(string) - if !ok { + srv, ok := ctx.Value(ServerCtxKey).(*Server) + if !ok || srv == nil || srv.name == "" { return "UNKNOWN" } - return srvName + return srv.name } type metricsInstrumentedHandler struct { - labels prometheus.Labels - statusLabels prometheus.Labels - mh MiddlewareHandler + handler string + mh MiddlewareHandler } -func newMetricsInstrumentedHandler(server, handler string, mh MiddlewareHandler) *metricsInstrumentedHandler { +func newMetricsInstrumentedHandler(handler string, mh MiddlewareHandler) *metricsInstrumentedHandler { httpMetrics.init.Do(func() { initHTTPMetrics() }) - labels := prometheus.Labels{"server": server, "handler": handler} - statusLabels := prometheus.Labels{"server": server, "handler": handler, "code": "", "method": ""} - return &metricsInstrumentedHandler{labels, statusLabels, mh} + return &metricsInstrumentedHandler{handler, mh} } func (h *metricsInstrumentedHandler) ServeHTTP(w http.ResponseWriter, r *http.Request, next Handler) error { - inFlight := httpMetrics.requestInFlight.With(h.labels) + server := serverNameFromContext(r.Context()) + labels := prometheus.Labels{"server": server, "handler": h.handler} + statusLabels := prometheus.Labels{"server": server, "handler": h.handler, "method": r.Method} + + inFlight := httpMetrics.requestInFlight.With(labels) inFlight.Inc() defer inFlight.Dec() - statusLabels := prometheus.Labels{"method": r.Method} - for k, v := range h.labels { - statusLabels[k] = v - } - start := time.Now() // This is a _bit_ of a hack - it depends on the ShouldBufferFunc always @@ -128,35 +122,25 @@ func (h *metricsInstrumentedHandler) ServeHTTP(w http.ResponseWriter, r *http.Re writeHeaderRecorder := ShouldBufferFunc(func(status int, header http.Header) bool { statusLabels["code"] = sanitizeCode(status) ttfb := time.Since(start).Seconds() - observeWithExemplar(statusLabels, httpMetrics.responseDuration, ttfb) + httpMetrics.responseDuration.With(statusLabels).Observe(ttfb) return false }) wrec := NewResponseRecorder(w, nil, writeHeaderRecorder) err := h.mh.ServeHTTP(wrec, r, next) dur := time.Since(start).Seconds() - httpMetrics.requestCount.With(h.labels).Inc() + httpMetrics.requestCount.With(labels).Inc() if err != nil { - httpMetrics.requestErrors.With(h.labels).Inc() + httpMetrics.requestErrors.With(labels).Inc() return err } - observeWithExemplar(statusLabels, httpMetrics.requestDuration, dur) - observeWithExemplar(statusLabels, httpMetrics.requestSize, float64(computeApproximateRequestSize(r))) + httpMetrics.requestDuration.With(statusLabels).Observe(dur) + httpMetrics.requestSize.With(statusLabels).Observe(float64(computeApproximateRequestSize(r))) httpMetrics.responseSize.With(statusLabels).Observe(float64(wrec.Size())) return nil } -func observeWithExemplar(l prometheus.Labels, o *prometheus.HistogramVec, value float64) { - obs := o.With(l) - if oe, ok := obs.(prometheus.ExemplarObserver); ok { - oe.ObserveWithExemplar(value, l) - return - } - // _should_ be a noop, but here just in case... - obs.Observe(value) -} - func sanitizeCode(code int) string { if code == 0 { return "200" |