Serve Prometheus metrics behind SWWAF_METRICS_TOKEN (closes #23)

GET /_smallwebwaf/metrics answers in the Prometheus text format for a
request carrying SWWAF_METRICS_TOKEN, 401 without it and 404 while it is
unset. Every request under /_smallwebwaf/ but the health check now goes
through the checks and is answered where it would be forwarded, 404 for
any path but the metrics, so none reaches the app. SWWAF_METRICS_TOP_N
bounds the series by country, the rest counted as other.

Judgement call: a request answered at smallwebwaf's own endpoints is
neither forwarded nor refused in the client's history.
Deviation: go.mod and go.sum written by hand from the module proxy and
sum.golang.org, as go runs only through make.
Deviation: no metrics yet for state files read again after an edit or
edits set aside; that work is not merged.

Model: opus-5-5

README.md

@@ -284,10 +284,9 @@ entries by client address, with times in UTC.
 - `clients.json`: each client's two buckets in the minute, the hour and the day,
   and its history: when it was first and last seen, its country as last looked
   up and when, its requests, how many were forwarded and how many refused (one
-  `smallwebwaf` answered at its own endpoints is neither, unless it was refused
-  with `401` for a missing or wrong token), the body bytes in each direction,
-  its responses by status class and its offences by kind. Each client is on a
-  line of its own, so `grep` shows everything about one.
+  `smallwebwaf` answered at its own endpoints is neither), the body bytes in
+  each direction, its responses by status class and its offences by kind. Each
+  client is on a line of its own, so `grep` shows everything about one.
 - `lookups.json`: GeoJS's answers, one to a line, with when GeoJS gave each and
   when it was last used.
 
@@ -343,11 +342,9 @@ other request. No metric carries a client's address.
   series of their own, and the others are counted as `other`. A country that
   drops out of them loses its series, and its later requests count as `other`;
   one that comes into them gets a series that counts from then on.
-- `smallwebwaf_geojs_requests_total`: the requests to GeoJS;
-  `smallwebwaf_geojs_failures_total`: those that failed, an answer that leaves
-  out an address asked about included; and `smallwebwaf_geojs_unanswered_total`:
-  the requests whose client counted as coming from an unknown country because
-  GeoJS had not answered about it in time.
+- `smallwebwaf_geojs_requests_total`, `smallwebwaf_geojs_failures_total`, and
+  `smallwebwaf_geojs_unanswered_total`: the requests whose client counted as
+  coming from an unknown country because GeoJS had not answered in time.
 - `smallwebwaf_tracked_clients`: the clients in the table of clients.
 - `smallwebwaf_state_file_writes_total`,
   `smallwebwaf_state_file_write_failures_total`,

go.mod

@@ -10,7 +10,6 @@ require (
 require (
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
-	github.com/kylelemons/godebug v1.1.0 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
 	github.com/prometheus/client_model v0.6.2 // indirect
 	github.com/prometheus/common v0.70.1 // indirect

internal/lookup/lookup_test.go

@@ -13,7 +13,6 @@ import (
 	"testing/synctest"
 	"time"
 
-	"github.com/prometheus/client_golang/prometheus/testutil"
 	"sneak.berlin/go/smallwebwaf/internal/lookup"
 	"sneak.berlin/go/smallwebwaf/internal/metrics"
 )
@@ -351,40 +350,6 @@ func TestAtMost10000ClientsWait(t *testing.T) {
 	})
 }
 
-func TestClientsWithoutAnAnswerAreCounted(t *testing.T) {
-	t.Parallel()
-
-	synctest.Test(t, func(t *testing.T) {
-		m := metrics.New(1)
-		g := lookup.New(lookup.Params{
-			URL:        lookup.URL,
-			Now:        time.Now,
-			ProcessLog: slog.New(slog.DiscardHandler),
-			Metrics:    m,
-		})
-		g.SetTransport(&standIn{answers: failing})
-
-		clients := newClients()
-
-		// GeoJS fails, so the first client goes without an answer, and GeoJS
-		// is left alone for a second, which does not pass in this test.
-		wantCountry(t, g, clients(), "")
-		wantUnanswered(t, m, 1)
-
-		// Meanwhile each new client goes without one at once, while there is
-		// room for it among the 10,000 that may wait.
-		for range 9999 {
-			wantCountry(t, g, clients(), "")
-		}
-
-		wantUnanswered(t, m, 10000)
-
-		// One more, for which there is no room, goes without one too.
-		wantCountry(t, g, clients(), "")
-		wantUnanswered(t, m, 10001)
-	})
-}
-
 // How the stand-in for GeoJS answers.
 const (
 	answering               = iota
@@ -587,17 +552,6 @@ func wantAsked(t *testing.T, geojs *standIn, i int, want ...string) {
 	}
 }
 
-// wantUnanswered checks how many requests m counts as having gone without
-// an answer from GeoJS.
-func wantUnanswered(t *testing.T, m *metrics.Metrics, want float64) {
-	t.Helper()
-
-	got := testutil.ToFloat64(m.GeoJSUnanswered)
-	if got != want {
-		t.Errorf("%v requests went without an answer, want %v", got, want)
-	}
-}
-
 // waitForRequests waits until g has done all it can before time passes,
 // checks that GeoJS has had count requests, and returns the addresses each
 // asked about.

internal/proxy/admin.go

@@ -10,9 +10,8 @@ import (
 
 // answerAdmin answers a request for smallwebwaf itself, under
 // /_smallwebwaf/, once it has passed the checks: GET MetricsPath with
-// SWWAF_METRICS_TOKEN gets the metrics, and without it is refused with
-// 401. Any other request gets 404, as the metrics do while
-// SWWAF_METRICS_TOKEN is unset.
+// SWWAF_METRICS_TOKEN gets the metrics, and without it 401. Any other
+// request gets 404, as the metrics do while SWWAF_METRICS_TOKEN is unset.
 func (rq *request) answerAdmin() {
 	rq.line.Action = requestlog.ActionAdmin
 	rq.startClientResponseTimeout()
@@ -24,10 +23,8 @@ func (rq *request) answerAdmin() {
 		http.Error(rq.out, http.StatusText(http.StatusNotFound), http.StatusNotFound)
 	case !hasToken(rq.in, token):
 		rq.out.Header().Set("WWW-Authenticate", "Bearer")
-		rq.answer(refusal{
-			status: http.StatusUnauthorized,
-			action: requestlog.ActionAdmin,
-		})
+		http.Error(rq.out, http.StatusText(http.StatusUnauthorized),
+			http.StatusUnauthorized)
 	default:
 		rq.h.metrics.ServeHTTP(rq.out, rq.in)
 	}

internal/proxy/history_test.go

@@ -86,24 +86,21 @@ func TestHealthEndpointIsNotInTheHistory(t *testing.T) {
 	}
 }
 
-func TestRequestForSmallwebwafIsRefusedOnlyWithoutTheToken(t *testing.T) {
+func TestRequestForSmallwebwafIsNeitherForwardedNorRefused(t *testing.T) {
 	t.Parallel()
 
 	app := startApp(t, func(http.ResponseWriter, *http.Request) {})
 	addr, out, server := startProxyWithClock(t, app.URL, "", time.Now,
 		map[string]string{metricsToken: token})
 
-	// The metrics and the 404 are neither forwarded nor refused; the 401
-	// is refused.
 	scrape(t, addr)
 	wantStatus(t, get(t, addr, "/_smallwebwaf/nothing"), http.StatusNotFound)
-	wantStatus(t, get(t, addr, proxy.MetricsPath), http.StatusUnauthorized)
-	out.requestLines(t, 3)
+	out.requestLines(t, 2)
 
 	history := historyOf(t, server, localhost)
-	if history.Requests != 3 || history.Forwarded != 0 || history.Refused != 1 {
+	if history.Requests != 2 || history.Forwarded != 0 || history.Refused != 0 {
 		t.Errorf("history counts %d requests, %d forwarded and %d refused, "+
-			"want 3, 0 and 1", history.Requests, history.Forwarded, history.Refused)
+			"want 2, 0 and 0", history.Requests, history.Forwarded, history.Refused)
 	}
 }
 

internal/proxy/limits_test.go

@@ -55,7 +55,6 @@ func TestRequestBodyLimit(t *testing.T) {
 			})
 			addr, out := startProxy(t, app.URL, map[string]string{
 				requestMaxBytes: sizeLimitSetting,
-				metricsToken:    token,
 			})
 
 			var body io.Reader = bytes.NewReader(make([]byte, tc.size))
@@ -67,13 +66,6 @@ func TestRequestBodyLimit(t *testing.T) {
 				tc.want)
 			wantLine(t, out.requestLine(t), tc.want, tc.action)
 
-			hits := 0
-			if tc.action == requestlog.ActionTooLarge {
-				hits = 1
-			}
-
-			wantLimitHits(t, addr, requestMaxBytes, hits)
-
 			if tc.refusedBeforeApp && calls.Load() != 0 {
 				t.Errorf("the app was called %d times, want never", calls.Load())
 			}
@@ -114,7 +106,6 @@ func TestResponseBodyLimit(t *testing.T) {
 			})
 			addr, out := startProxy(t, app.URL, map[string]string{
 				responseMaxBytes: sizeLimitSetting,
-				metricsToken:     token,
 			})
 
 			got := get(t, addr, "/download")
@@ -132,13 +123,6 @@ func TestResponseBodyLimit(t *testing.T) {
 			if line.UpstreamStatus != http.StatusOK {
 				t.Errorf("log line has upstream_status %d", line.UpstreamStatus)
 			}
-
-			hits := 0
-			if tc.action == requestlog.ActionTooLarge {
-				hits = 1
-			}
-
-			wantLimitHits(t, addr, responseMaxBytes, hits)
 		})
 	}
 }

internal/proxy/metrics_test.go

@@ -1,6 +1,7 @@
 package proxy_test
 
 import (
+	"bytes"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -243,6 +244,34 @@ func TestMetricsCountLimitsAndBans(t *testing.T) {
 	wantMetric(t, metrics, "smallwebwaf_tracked_clients", 3)
 }
 
+func TestMetricsCountSizeAndTimeLimits(t *testing.T) {
+	t.Parallel()
+
+	// The app never answers /hang, so the timeout runs out however slowly
+	// the test runs.
+	app := startApp(t, func(_ http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/hang" {
+			<-r.Context().Done()
+		}
+	})
+	addr, out := startProxy(t, app.URL, map[string]string{
+		metricsToken:            token,
+		requestMaxBytes:         sizeLimitSetting,
+		upstreamResponseTimeout: "100ms",
+	})
+
+	body := bytes.NewReader(make([]byte, 2*sizeLimit))
+	wantStatus(t, do(t, newRequest(t, http.MethodPost, addr, "/", body)),
+		http.StatusRequestEntityTooLarge)
+	wantStatus(t, get(t, addr, "/hang"), http.StatusGatewayTimeout)
+	out.requestLines(t, 2)
+
+	metrics := scrape(t, addr)
+	hits := "smallwebwaf_size_and_time_limit_hits_total"
+	wantMetric(t, metrics, hits+`{limit="SWWAF_REQUEST_MAX_BYTES"}`, 1)
+	wantMetric(t, metrics, hits+`{limit="SWWAF_UPSTREAM_RESPONSE_TIMEOUT"}`, 1)
+}
+
 func TestMetricsByCountryKeepTheBusiestAndCountTheRestAsOther(t *testing.T) {
 	t.Parallel()
 
@@ -435,21 +464,3 @@ func wantNoSeries(t *testing.T, metrics, series string) {
 		t.Errorf("there is a series %s", series)
 	}
 }
-
-// wantLimitHits checks that the metrics of smallwebwaf at addr count hits
-// requests that passed the size or time limit of the setting limit, with
-// no series for it when hits is 0.
-func wantLimitHits(t *testing.T, addr, limit string, hits int) {
-	t.Helper()
-
-	series := `smallwebwaf_size_and_time_limit_hits_total{limit="` + limit + `"}`
-	metrics := scrape(t, addr)
-
-	if hits == 0 {
-		wantNoSeries(t, metrics, series)
-
-		return
-	}
-
-	wantMetric(t, metrics, series, float64(hits))
-}

internal/proxy/timeouts_test.go

@@ -28,9 +28,7 @@ func TestRequestTimeouts(t *testing.T) {
 
 	for _, tc := range []struct {
 		name string
-		// limit is the setting set to shortTimeout, which runs out; long
-		// is one set to longTimeoutSetting, which does not, or "".
-		limit, long string
+		env  map[string]string
 		// appTakesNothing has the app never read, while the client sends
 		// as fast as it can; otherwise the app reads, and the client
 		// stops sending halfway.
@@ -38,26 +36,30 @@ func TestRequestTimeouts(t *testing.T) {
 		want            int
 	}{
 		{
-			name:  "client request timeout, waiting on the client",
-			limit: clientRequestTimeout,
-			want:  http.StatusRequestTimeout,
+			name: "client request timeout, waiting on the client",
+			env:  map[string]string{clientRequestTimeout: shortTimeoutSetting},
+			want: http.StatusRequestTimeout,
 		},
 		{
-			name:  "upstream request timeout, waiting on the client",
-			limit: upstreamRequestTimeout,
-			long:  clientRequestTimeout,
-			want:  http.StatusRequestTimeout,
+			name: "upstream request timeout, waiting on the client",
+			env: map[string]string{
+				upstreamRequestTimeout: shortTimeoutSetting,
+				clientRequestTimeout:   longTimeoutSetting,
+			},
+			want: http.StatusRequestTimeout,
 		},
 		{
 			name:            "upstream request timeout, waiting on the app",
-			limit:           upstreamRequestTimeout,
+			env:             map[string]string{upstreamRequestTimeout: shortTimeoutSetting},
 			appTakesNothing: true,
 			want:            http.StatusGatewayTimeout,
 		},
 		{
-			name:            "client request timeout, waiting on the app",
-			limit:           clientRequestTimeout,
-			long:            upstreamRequestTimeout,
+			name: "client request timeout, waiting on the app",
+			env: map[string]string{
+				clientRequestTimeout:   shortTimeoutSetting,
+				upstreamRequestTimeout: longTimeoutSetting,
+			},
 			appTakesNothing: true,
 			want:            http.StatusGatewayTimeout,
 		},
@@ -82,12 +84,7 @@ func TestRequestTimeouts(t *testing.T) {
 				appURL, sendRequest = app.URL, sendPartOfBody
 			}
 
-			env := map[string]string{tc.limit: shortTimeoutSetting, metricsToken: token}
-			if tc.long != "" {
-				env[tc.long] = longTimeoutSetting
-			}
-
-			addr, out := startProxy(t, appURL, env)
+			addr, out := startProxy(t, appURL, tc.env)
 			start := time.Now()
 			got := readResponse(t, sendRequest(t, addr))
 			wantTimedOut(t, start)
@@ -108,7 +105,6 @@ func TestRequestTimeouts(t *testing.T) {
 
 			wantStatus(t, got, want)
 			wantLine(t, out.requestLine(t), want, requestlog.ActionTimedOut)
-			wantLimitHits(t, addr, tc.limit, 1)
 		})
 	}
 }
@@ -202,7 +198,6 @@ func TestAppTooSlowToAnswer(t *testing.T) {
 	})
 	addr, out := startProxy(t, app.URL, map[string]string{
 		upstreamResponseTimeout: shortTimeoutSetting,
-		metricsToken:            token,
 	})
 
 	start := time.Now()
@@ -218,8 +213,6 @@ func TestAppTooSlowToAnswer(t *testing.T) {
 		t.Errorf("log line has upstream_status %v for an app that never answered",
 			line.fields["upstream_status"])
 	}
-
-	wantLimitHits(t, addr, upstreamResponseTimeout, 1)
 }
 
 func TestAppTooSlowToFinishItsAnswer(t *testing.T) {
@@ -268,7 +261,6 @@ func TestClientTooSlowToTakeTheAnswer(t *testing.T) {
 	})
 	addr, out := startProxy(t, app.URL, map[string]string{
 		clientResponseTimeout: shortTimeoutSetting,
-		metricsToken:          token,
 	})
 
 	start := time.Now()
@@ -280,7 +272,6 @@ func TestClientTooSlowToTakeTheAnswer(t *testing.T) {
 	line := out.requestLine(t)
 	wantTimedOut(t, start)
 	wantLine(t, line, http.StatusOK, requestlog.ActionTimedOut)
-	wantLimitHits(t, addr, clientResponseTimeout, 1)
 }
 
 func TestClosesAnIdleConnection(t *testing.T) {

internal/ratelimit/history_test.go

@@ -23,8 +23,8 @@ func TestHistoryKeepsEveryRequest(t *testing.T) {
 		{Forwarded: true, Status: 502, ResponseBytes: 12},
 		// Closed without an answer: refused, and no response.
 		{Refused: true, Status: 0},
-		// Answered 404 at smallwebwaf's own endpoints: neither forwarded
-		// nor refused.
+		// Answered at smallwebwaf's own endpoints: neither forwarded nor
+		// refused.
 		{Status: 404},
 	} {
 		limiter.AddToHistory(client, start.Add(time.Duration(i)*time.Minute), r)

internal/ratelimit/ratelimit.go

@@ -71,9 +71,8 @@ type History struct {
 	Country  string    `json:"country,omitempty"`
 	LookedUp time.Time `json:"looked_up,omitzero"`
 	// Requests are all the client's requests: Forwarded those passed to
-	// the app, Refused those refused before anything reached it, a 401 at
-	// smallwebwaf's own endpoints included, and neither the others
-	// smallwebwaf answered there.
+	// the app, Refused those refused before anything reached it, and
+	// neither those smallwebwaf answered at its own endpoints.
 	Requests  int64 `json:"requests"`
 	Forwarded int64 `json:"forwarded"`
 	Refused   int64 `json:"refused"`
@@ -106,9 +105,8 @@ type Request struct {
 	// Country is the client's country, when the request looked it up.
 	Country string
 	// Forwarded is true for a request passed to the app, Refused for one
-	// refused before anything reached it, a 401 at smallwebwaf's own
-	// endpoints included. Both are false for any other request smallwebwaf
-	// answered there.
+	// refused before anything reached it. Both are false for a request
+	// smallwebwaf answered at its own endpoints.
 	Forwarded bool
 	Refused   bool
 	// Status is what the client was sent, 0 if nothing was.