Label request metrics with standard HTTP methods only (closes #274)
check / check (push) Waiting to run
check / check (push) Waiting to run
The metrics middleware labelled each request with its method as sent, and Go's HTTP server accepts any method name that is a valid token, so each made-up method a client sent added series that were never removed. The method label is now the request's method only for the nine standard methods, and "other" for any other. The middleware hands the metrics library a copy of the request carrying the label and serves the next handler the original, so handlers still see the method the client sent. Model: opus-5-5
This commit was merged in pull request #277.
This commit is contained in:
@@ -685,7 +685,9 @@ See `configs/config.example.yml` for all options with defaults.
|
||||
`metrics.username` and `metrics.password` are set. A request's `handler` label
|
||||
is the pattern of the route it matches, such as `/v1/image/*`, never its path.
|
||||
Requests that match no route share the label `unmatched`: those the router
|
||||
answers 404 or 405, and CORS preflights
|
||||
answers 404 or 405, and CORS preflights. A request's `method` label is its
|
||||
method when that is `GET`, `HEAD`, `POST`, `PUT`, `PATCH`, `DELETE`,
|
||||
`OPTIONS`, `CONNECT` or `TRACE`, and `other` for any other method
|
||||
- **Logging**: stdlib slog
|
||||
|
||||
## Entrypoints
|
||||
|
||||
@@ -30,6 +30,15 @@ P2: security: per-origin rate limiting
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-10-15 the request metrics record any method that is not a standard one as
|
||||
`other` (closes #274): their `method` label was each request's method as sent,
|
||||
and Go's HTTP server accepts any method name, so a client sending made-up
|
||||
methods added series to the Prometheus registry that were never removed. The
|
||||
label is now the request's method only for `GET`, `HEAD`, `POST`, `PUT`,
|
||||
`PATCH`, `DELETE`, `OPTIONS`, `CONNECT` and `TRACE`. The metrics library reads
|
||||
the method from the request it is handed, so the metrics middleware hands it a
|
||||
copy of the request carrying the label; the handlers still see the method the
|
||||
client sent.
|
||||
- 2026-10-15 logging out ends every login session (closes #256): a login session
|
||||
was a cookie with nothing kept on the server, so `/logout` removed it from one
|
||||
browser and a copy of it kept working for its 30 days. Every login is made
|
||||
|
||||
@@ -220,7 +220,10 @@ func (s *Middleware) CORS() func(http.Handler) http.Handler {
|
||||
// pattern of the chi route a request matches, such as /v1/image/*, or
|
||||
// "unmatched" for a request that matches none, so the routes fix how many
|
||||
// values the label has. It is never the path, with which any client could
|
||||
// add series to the registry without limit.
|
||||
// add series to the registry without limit. Its method label is the
|
||||
// request's method when that is a standard one, such as GET, and "other"
|
||||
// for any other: Go's HTTP server accepts any method name, so a label per
|
||||
// method would let a client add series the same way.
|
||||
func (s *Middleware) Metrics() func(http.Handler) http.Handler {
|
||||
mdlw := ghmm.New(ghmm.Config{
|
||||
Recorder: metrics.NewRecorder(metrics.Config{
|
||||
@@ -230,7 +233,19 @@ func (s *Middleware) Metrics() func(http.Handler) http.Handler {
|
||||
|
||||
return func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
std.Handler(routePattern(r), mdlw, next).ServeHTTP(w, r)
|
||||
// The metrics library takes the method label from the request
|
||||
// it is handed, so it is handed a shallow copy of r whose
|
||||
// method is the label. next is served r itself, through the
|
||||
// response writer the library wraps to see the status and size.
|
||||
labelled := r.WithContext(r.Context())
|
||||
labelled.Method = methodLabel(r.Method)
|
||||
|
||||
serveNext := http.HandlerFunc(
|
||||
func(wrapped http.ResponseWriter, _ *http.Request) {
|
||||
next.ServeHTTP(wrapped, r)
|
||||
})
|
||||
|
||||
std.Handler(routePattern(r), mdlw, serveNext).ServeHTTP(w, labelled)
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -275,6 +290,20 @@ func routePattern(r *http.Request) string {
|
||||
return pattern
|
||||
}
|
||||
|
||||
// methodLabel returns method when it is one of the standard methods net/http
|
||||
// names: GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, CONNECT and TRACE.
|
||||
// For any other method it returns "other".
|
||||
func methodLabel(method string) string {
|
||||
switch method {
|
||||
case http.MethodGet, http.MethodHead, http.MethodPost, http.MethodPut,
|
||||
http.MethodPatch, http.MethodDelete, http.MethodOptions,
|
||||
http.MethodConnect, http.MethodTrace:
|
||||
return method
|
||||
default:
|
||||
return "other"
|
||||
}
|
||||
}
|
||||
|
||||
// MetricsAuth returns a basic auth middleware for the metrics endpoint.
|
||||
func (s *Middleware) MetricsAuth() func(http.Handler) http.Handler {
|
||||
return basicauth.New(
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -314,6 +315,58 @@ func requestCountsByHandler(
|
||||
return counts
|
||||
}
|
||||
|
||||
// TestMetricsGivesMadeUpMethodsOneLabel checks that the metrics middleware
|
||||
// gives requests with methods that are not standard ones a single method
|
||||
// label, "other", and still serves each request with the method it was sent
|
||||
// with. With a label per method, any client could add series to the
|
||||
// registry without limit by making up a new method for each request.
|
||||
func TestMetricsGivesMadeUpMethodsOneLabel(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// The line /metrics shows once both made-up methods have been served.
|
||||
const want = `http_request_duration_seconds_count{` +
|
||||
`code="200",handler="unmatched",method="other",service=""} 2`
|
||||
|
||||
registry := prometheus.NewRegistry()
|
||||
mw := &Middleware{
|
||||
log: slog.Default(),
|
||||
config: &config.Config{},
|
||||
metricsRegistry: registry,
|
||||
}
|
||||
|
||||
var served []string
|
||||
|
||||
handler := mw.Metrics()(http.HandlerFunc(
|
||||
func(w http.ResponseWriter, r *http.Request) {
|
||||
served = append(served, r.Method)
|
||||
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
|
||||
methods := []string{"FROBNICATE", "WIBBLE"}
|
||||
for _, method := range methods {
|
||||
handler.ServeHTTP(httptest.NewRecorder(), httptest.NewRequestWithContext(
|
||||
t.Context(), method, "/", nil))
|
||||
}
|
||||
|
||||
t.Logf("methods the handler was served: %v", served)
|
||||
|
||||
if !slices.Equal(served, methods) {
|
||||
t.Errorf("methods the handler was served = %v, want %v",
|
||||
served, methods)
|
||||
}
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
promhttp.HandlerFor(registry, promhttp.HandlerOpts{}).ServeHTTP(rec,
|
||||
httptest.NewRequestWithContext(
|
||||
t.Context(), http.MethodGet, "/metrics", nil))
|
||||
|
||||
if !strings.Contains(rec.Body.String(), want) {
|
||||
t.Errorf("/metrics does not count both made-up methods as one; "+
|
||||
"want the line %q in:\n%s", want, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoggingLeavesOutSubmittedSigningKey checks that a login, a POST /
|
||||
// whose form carries the signing key, leaves no trace of the key in the
|
||||
// request's log line.
|
||||
|
||||
Reference in New Issue
Block a user