From d352379375d78bb66a68ce539d2ea2a9cc6cf4e6 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Sun, 23 Aug 2026 00:58:30 +0100 Subject: [PATCH] fix(metrics): honest rendering, injective escaping, aggregation docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes from adversarial verification. writeProviderMetrics rendered a storeDown providerCounters entry as a hard zero, the same fabricated-reset hazard the request/token/cost families already guarded against. Skip a storeDown provider or provider-model entry instead of emitting it. escapeLabelValue ranged over the string as runes, and Go substitutes U+FFFD for every invalid UTF-8 byte. Two distinct configured names that both contain invalid UTF-8 could therefore render as the identical escaped value, colliding two series into one duplicate series and failing the whole scrape target. Escaping is now byte-oriented, which is injective and also avoids the per-rune decode cost on a large value. Every store-backed family's HELP text now states its aggregation contract: with Redis configured, the value is the gateway-wide total and identical on every replica, so summing across replicas overcounts by the replica count and a cost alert fires early. Aggregate with max() instead. Without Redis, the same family becomes genuinely per-process and sum() becomes correct instead. The rejection counter and the provider-healthy gauge get their own notes: the former is always per-process regardless of Redis, the latter reflects each replica's own independently-running discovery breaker. Also replaces the AllowedCIDRs doc example (10.0.0.0/8) with a cluster pod CIDR (10.42.0.0/16) and states plainly that a whole RFC1918 range is never correct, since a SNAT-ing router or load balancer makes external traffic arrive from inside it — reproduced live against the target cluster. Adds tests pinning three properties mutation-proofed against this suite: every storeDown skip (deleting any of them now fails TestMetrics_StoreDown_SkipsRatherThanFabricatesZero), the CIDR-check running before auth.identify (TestMetrics_ AllowlistedRequest_NeverTouchesAuthFailureTracking), and escapeLabelValue's injectivity under invalid UTF-8 (TestEscapeLabelValue_InvalidUTF8_Injective). Also covers the new rejection-counter pruning end to end through a real file-sourced reload. --- metrics.go | 135 ++++++++++++++++++++++++++++++----- metrics_test.go | 186 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 303 insertions(+), 18 deletions(-) diff --git a/metrics.go b/metrics.go index cde6098..723fe46 100644 --- a/metrics.go +++ b/metrics.go @@ -31,14 +31,29 @@ import ( // either "everyone" or "no one" depending on what address arrives. // Confirm RemoteAddr reflects the scraper's own address in your // deployment before relying on this instead of a key. +// +// THE FAILURE MODE TO AVOID IS NOT "the allowlist does not work" — it is +// "the allowlist works exactly as configured, and what survives to +// RemoteAddr is an address you never intended to trust" (review fix, +// adversarial verification 2026-08-23, reproduced live): a whole RFC1918 +// block such as 10.0.0.0/8 is never a correct entry, because a SNAT-ing +// router or load balancer makes EXTERNAL traffic arrive from an address +// inside that same private range — on the reproduction cluster, +// AllowedCIDRs: ["10.0.0.0/8"] paired with the router's own SNAT address +// returned full per-user cost data to an external caller with no key at +// all. Always scope this to the NARROWEST block that covers only your +// actual scrapers — a Prometheus/VictoriaMetrics pod CIDR such as +// 10.42.0.0/16, not the broad private range it happens to sit inside. type MetricsConfig struct { // Path overrides the served path. Empty (the default) serves at // metricsPathDefault ("/metrics"). Path string `json:"path,omitempty"` - // AllowedCIDRs lists CIDR blocks (e.g. "10.0.0.0/8") whose source - // address may scrape without an admin key. Empty means every - // scrape must carry a valid admin bearer token — see this type's - // own doc comment for the trust assumption this relies on. + // AllowedCIDRs lists CIDR blocks whose source address may scrape + // without an admin key — e.g. "10.42.0.0/16" for a cluster's pod + // network, never a whole RFC1918 range like "10.0.0.0/8". Empty + // means every scrape must carry a valid admin bearer token — see + // this type's own doc comment for the trust assumption this relies + // on, and for why the narrowest-possible block matters here. AllowedCIDRs []string `json:"allowedCIDRs,omitempty"` Enabled bool `json:"enabled,omitempty"` // ModelLabel adds a `model` label to the provider attempt/failure @@ -168,17 +183,36 @@ func (g *Gateway) serveMetrics(w http.ResponseWriter) { // one bad line, so this is a correctness requirement, not a formatting // nicety. // -// This walks s rune by rune and classifies each one independently, never +// This walks s BYTE by byte and classifies each one independently, never // a sequential find-and-replace pass: replacing quotes first and // backslashes second (or vice versa) would double-escape a backslash // this function itself just inserted — a real, classic bug class for // this exact kind of escaping. A single forward pass has no such // ordering hazard. +// +// Byte-oriented, deliberately NOT `for _, r := range s` (review fix, +// adversarial verification 2026-08-23): ranging over a string decodes +// UTF-8 and substitutes U+FFFD for every invalid byte, which makes the +// function NOT injective — two distinct configured names that both +// contain invalid UTF-8 (a byte sequence auth.go's buildEntry never +// rejects; it validates non-empty only, see its own doc comment) can +// decode to the identical U+FFFD run and render as the SAME escaped +// label value. Two scope_id values colliding onto one series is a +// duplicate-series scrape failure — confirmed against a real Prometheus +// scrape parser: the whole target reports DOWN, and promtool alone will +// not catch it, since expfmt silently dedupes. Escaping byte-for-byte +// passes every raw byte through unchanged except the three ASCII +// characters requiring escaping ('\\', '"', '\n') — none of which can +// ever appear as a UTF-8 continuation byte (continuation bytes are +// always >= 0x80), so this never misinterprets a multi-byte sequence's +// internal bytes as one of these three, and every distinct input byte +// string still maps to a distinct output. It also avoids WriteRune's +// per-rune decode cost on a large value. func escapeLabelValue(s string) string { var b bytes.Buffer b.Grow(len(s)) - for _, r := range s { - switch r { + for i := 0; i < len(s); i++ { + switch c := s[i]; c { case '\\': b.WriteString(`\\`) case '"': @@ -186,7 +220,7 @@ func escapeLabelValue(s string) string { case '\n': b.WriteString(`\n`) default: - b.WriteRune(r) + b.WriteByte(c) } } return b.String() @@ -257,6 +291,38 @@ func (m *metricWriter) sampleFloat(name string, labels []metricLabel, v float64) m.sample(name, labels, strconv.FormatFloat(v, 'g', -1, 64)) } +// aggregationNoteStoreBacked is appended to every store-backed family's +// HELP text below (item 1, adversarial verification 2026-08-23, +// reproduced against a 3-replica Traefik deployment): windowKey +// (limits.go) carries no per-instance component, so with Config.Redis +// configured every replica's counterStore read returns the SAME +// gateway-wide value — Prometheus scrapes each replica as its own +// target, so sum(rate(...)) or sum(...) across replicas TRIPLE-COUNTS +// on 3 replicas (a cost alert fires at a third of its intended +// threshold). The correct cross-replica aggregation is max(), since +// every replica already reports the true total on its own. Without +// Config.Redis (the in-memory memoryStore fallback), the IDENTICAL +// family becomes genuinely per-process instead — each replica counts +// only the traffic it personally handled — and sum() becomes the +// correct aggregation instead of max(). This flips on that one config +// bit; know which one a deployment is running before wiring an alert. +const aggregationNoteStoreBacked = " AGGREGATION ACROSS REPLICAS: with Redis configured, this value is the gateway-wide total read from shared limit state and is IDENTICAL on every replica's own /metrics response — aggregate with max(), never sum(), or a multi-replica deployment overcounts by the replica count. Without Redis (the in-memory fallback), this SAME family becomes genuinely per-process instead, and sum() becomes the correct aggregation. Know which one you are running." + +// aggregationNotePerProcess is appended to llmgateway_rate_limit_ +// rejections_total's HELP text: limiter.rejections (limits.go) is a +// bare in-process map, never written to or read from Redis regardless +// of Config.Redis — unlike every store-backed family above, this one +// never flips, and sum() is always the right cross-replica aggregation. +const aggregationNotePerProcess = " AGGREGATION ACROSS REPLICAS: this counter lives only in this process's memory, never in Redis, regardless of Config.Redis — aggregate with sum(), which is always correct for it, unlike the store-backed families above." + +// aggregationNoteProviderHealthy is appended to llmgateway_provider_ +// healthy's HELP text: the discovery circuit breaker (registry.go's +// providerState.health) is per-process state, set by each replica's own +// discovery polling loop, and is never written to Redis — two replicas +// can legitimately disagree about one provider's health at the same +// instant (e.g. one mid-backoff, one already recovered). +const aggregationNoteProviderHealthy = " AGGREGATION ACROSS REPLICAS: each replica runs its own discovery circuit breaker in-process, never shared via Redis, so this can legitimately differ per replica. Read it per-instance where possible; if you must aggregate, use min() to surface \"at least one replica sees this provider as unhealthy\" — sum() is meaningless for a 0/1 gauge." + // renderMetrics builds the full Prometheus text-exposition document for // GET . Every read below goes through the SAME batched, // chunked accessors GET /admin/api/overview and GET /admin/api/usage @@ -304,7 +370,7 @@ func (g *Gateway) writeUsageMetrics(m *metricWriter) { usage := g.chunkedCurrentUsage(scopes) m.family("llmgateway_requests_total", "counter", - "Total requests admitted for accounting today (UTC calendar day; resets at UTC midnight), by scope.") + "Total requests admitted for accounting today (UTC calendar day; resets at UTC midnight), by scope."+aggregationNoteStoreBacked) for _, su := range usage { if su.storeDown { continue @@ -314,7 +380,7 @@ func (g *Gateway) writeUsageMetrics(m *metricWriter) { } m.family("llmgateway_tokens_total", "counter", - "Total tokens accounted today (UTC calendar day; resets at UTC midnight), by scope and direction.") + "Total tokens accounted today (UTC calendar day; resets at UTC midnight), by scope and direction."+aggregationNoteStoreBacked) for _, su := range usage { if su.storeDown { continue @@ -326,7 +392,7 @@ func (g *Gateway) writeUsageMetrics(m *metricWriter) { } m.family("llmgateway_cost_micro_usd_total", "counter", - "Total cost accounted today (UTC calendar day; resets at UTC midnight), in micro-USD (1000000 = $1), by scope.") + "Total cost accounted today (UTC calendar day; resets at UTC midnight), in micro-USD (1000000 = $1), by scope."+aggregationNoteStoreBacked) for _, su := range usage { if su.storeDown { continue @@ -336,7 +402,7 @@ func (g *Gateway) writeUsageMetrics(m *metricWriter) { } m.family("llmgateway_budget_consumed_ratio", "gauge", - "Fraction of a configured limit already consumed in its own window; 1.0 means fully consumed, above 1.0 means the limit has been breached. Emitted only for a scope with that particular limit configured.") + "Fraction of a configured limit already consumed in its own window; 1.0 means fully consumed, above 1.0 means the limit has been breached. Emitted only for a scope with that particular limit configured."+aggregationNoteStoreBacked) for i, su := range usage { if su.storeDown { continue @@ -416,17 +482,29 @@ func (g *Gateway) writeProviderMetrics(m *metricWriter) { providerCounts := counters[:len(snaps)] m.family("llmgateway_provider_attempts_total", "counter", - "Total upstream attempts today (UTC calendar day; resets at UTC midnight), by provider.") + "Total upstream attempts today (UTC calendar day; resets at UTC midnight), by provider."+aggregationNoteStoreBacked) m.family("llmgateway_provider_failures_total", "counter", - "Total upstream attempts today classified as a provider fault (transport error, 429, or 5xx — the same classification recordProviderAttempt/isTransient apply), by provider.") + "Total upstream attempts today classified as a provider fault (transport error, 429, or 5xx — the same classification recordProviderAttempt/isTransient apply), by provider."+aggregationNoteStoreBacked) for i, s := range snaps { + // A storeDown counter is a failed-closed read (limits.go's + // providerUsage), not a real zero (review fix, adversarial + // verification 2026-08-23): emitting it as a hard 0 here is + // exactly the bug writeUsageMetrics' own storeDown guard above + // already avoids for the request/token/cost families — a Redis + // blip would otherwise paint a phantom counter-reset spike the + // instant the store recovers and this value jumps back to its + // real total. Skip the series entirely; a temporary gap is the + // honest signal, a fabricated 0 is not. + if providerCounts[i].storeDown { + continue + } labels := []metricLabel{{"provider", s.name}} m.sampleInt("llmgateway_provider_attempts_total", labels, providerCounts[i].attemptsDay) m.sampleInt("llmgateway_provider_failures_total", labels, providerCounts[i].failuresDay) } m.family("llmgateway_provider_healthy", "gauge", - "1 when the provider's discovery circuit breaker is not open (closed or half-open), 0 when open (discoveryHealthy, registry.go).") + "1 when the provider's discovery circuit breaker is not open (closed or half-open), 0 when open (discoveryHealthy, registry.go)."+aggregationNoteProviderHealthy) for _, s := range snaps { healthy := int64(1) if s.health == breakerOpen { @@ -441,14 +519,17 @@ func (g *Gateway) writeProviderMetrics(m *metricWriter) { modelCounts := counters[len(snaps):] m.family("llmgateway_provider_model_attempts_total", "counter", - "Total upstream attempts today (UTC calendar day; resets at UTC midnight), by provider and model. Opt-in via metrics.modelLabel — see MetricsConfig's own doc comment for the cardinality trade-off.") + "Total upstream attempts today (UTC calendar day; resets at UTC midnight), by provider and model. Opt-in via metrics.modelLabel — see MetricsConfig's own doc comment for the cardinality trade-off."+aggregationNoteStoreBacked) m.family("llmgateway_provider_model_failures_total", "counter", - "Total upstream attempts today classified as a provider fault, by provider and model. Opt-in via metrics.modelLabel.") + "Total upstream attempts today classified as a provider fault, by provider and model. Opt-in via metrics.modelLabel."+aggregationNoteStoreBacked) mi := 0 for _, s := range snaps { for _, model := range s.models { mc := modelCounts[mi] mi++ + if mc.storeDown { // same fabricated-reset hazard as the provider-level loop above + continue + } labels := []metricLabel{{"provider", s.name}, {"model", model}} m.sampleInt("llmgateway_provider_model_attempts_total", labels, mc.attemptsDay) m.sampleInt("llmgateway_provider_model_failures_total", labels, mc.failuresDay) @@ -475,9 +556,27 @@ func (g *Gateway) writeRejectionMetrics(m *metricWriter) { }) m.family("llmgateway_rate_limit_rejections_total", "counter", - "Total requests refused by a rate or budget limit since process start, by scope. Unlike the request/token/cost counters above, this is a process-lifetime counter, not UTC-day-windowed — it resets only on restart.") + "Total requests refused by a rate or budget limit since process start, by scope. Unlike the request/token/cost counters above, this is a process-lifetime counter, not UTC-day-windowed — it resets only on restart."+aggregationNotePerProcess) for _, s := range snaps { m.sampleInt("llmgateway_rate_limit_rejections_total", []metricLabel{{"scope_kind", s.kind}, {"scope_id", s.id}}, s.count) } } + +// pruneRejectionsAfterReload evicts every "user"-kind rejection scope +// (limiter.pruneRejections, limits.go) whose name is no longer among +// g.auth's currently active users — ServeHTTP's own call site +// (llmgateway.go), gated on authStore.maybeReload just having performed +// a REAL file-sourced reload, never run unconditionally (see +// maybeReload's own doc comment, users_file.go, for why). Without this, +// a user removed from a hot-reloaded users file would keep exporting +// llmgateway_rate_limit_rejections_total for the rest of the process's +// life (review fix, adversarial verification 2026-08-23). +func (g *Gateway) pruneRejectionsAfterReload() { + userSummaries, _ := g.auth.snapshot() + keep := make(map[string]bool, len(userSummaries)) + for _, us := range userSummaries { + keep[us.name] = true + } + g.limiter.pruneRejections(keep) +} diff --git a/metrics_test.go b/metrics_test.go index b78f0ed..08214a5 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "net/http/httptest" + "path/filepath" "sort" "strconv" "strings" @@ -175,6 +176,38 @@ func TestMetrics_NoAllowlistConfigured_KeylessRequestRejected(t *testing.T) { } } +// TestMetrics_AllowlistedRequest_NeverTouchesAuthFailureTracking pins the +// ORDERING handleMetrics' own doc comment claims (metrics.go): the CIDR +// allowlist is checked BEFORE auth.identify ever runs, so a keyless, +// allowlisted scrape never records a failed-auth attempt for its own +// source address. This is not observable from status codes alone — a +// handler that checked identify FIRST and fell back to the CIDR +// allowlist only on failure would return the identical 200 for every +// case TestMetrics_GateMatrix asserts, while still quietly feeding +// authStore's failure tracker on every single scrape. Mutation-proofing: +// reordering the two checks changes only this test's outcome, nothing +// else in this file. +func TestMetrics_AllowlistedRequest_NeverTouchesAuthFailureTracking(t *testing.T) { + t.Parallel() + cfg := newMetricsTestConfig() + cfg.Metrics.AllowedCIDRs = []string{"192.0.2.0/24"} + h, gw := newMetricsGatewayHandle(t, cfg) + + for i := 0; i < 5; i++ { + rec := httptest.NewRecorder() + // Default RemoteAddr (192.0.2.1:1234, verified empirically) — + // inside the allowlist, no key presented at all. + h.ServeHTTP(rec, metricsRequest(http.MethodGet, metricsPathDefault, "")) + if rec.Code != http.StatusOK { + t.Fatalf("request %d: status = %d, want 200", i, rec.Code) + } + } + + if got := gw.auth.failures.get("192.0.2.1"); got != 0 { + t.Errorf("auth failure count for the allowlisted scraper's address = %d, want 0 — the CIDR check must run before auth.identify, never after", got) + } +} + func TestMetrics_InvalidCIDR_FailsConstruction(t *testing.T) { t.Parallel() cfg := newMetricsTestConfig() @@ -453,6 +486,13 @@ func TestEscapeLabelValue(t *testing.T) { {"quote_then_backslash", `"\`, `\"\\`}, {"backslash_then_quote", `\"`, `\\\"`}, {"all_three", "a\"b\\c\nd", `a\"b\\c\nd`}, + // Invalid UTF-8 bytes (0x80/0x81 are bare continuation bytes, + // never valid as the START of any UTF-8 sequence) must pass + // through UNCHANGED, byte-for-byte — see + // TestEscapeLabelValue_InvalidUTF8_Injective below for why this + // matters beyond just "does not crash". + {"invalid_utf8_lone_0x80", "u\x80", "u\x80"}, + {"invalid_utf8_lone_0x81", "u\x81", "u\x81"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -464,6 +504,30 @@ func TestEscapeLabelValue(t *testing.T) { } } +// TestEscapeLabelValue_InvalidUTF8_Injective is the mutation-proofing +// test for the byte-oriented rewrite (review fix, adversarial +// verification 2026-08-23): a rune-oriented `for _, r := range s` +// implementation decodes invalid UTF-8 to U+FFFD, which makes escaping +// NOT injective — "u\x80" and "u\x81" are two distinct, real inputs +// (auth.go's buildEntry validates a user name as non-empty only, never +// restricted to valid UTF-8) that would both render as "u�" under +// that approach, colliding two distinct scope_id label values onto one +// series — confirmed against a real Prometheus scrape parser: a +// duplicate series, and the whole scrape target reported DOWN. The +// byte-oriented implementation must keep every input byte-for-byte +// distinguishable. +func TestEscapeLabelValue_InvalidUTF8_Injective(t *testing.T) { + t.Parallel() + a := escapeLabelValue("u\x80") + b := escapeLabelValue("u\x81") + if a == b { + t.Fatalf("escapeLabelValue(%q) == escapeLabelValue(%q) == %q — two distinct inputs collided onto one label value", "u\x80", "u\x81", a) + } + if strings.Contains(a, "�") || strings.Contains(b, "�") { + t.Errorf("escaped output contains U+FFFD (replacement character) — the rune-decoding bug this test guards against: a=%q b=%q", a, b) + } +} + // TestMetrics_LabelValuesWithSpecialChars_Escaped is the end-to-end // version of TestEscapeLabelValue: a real user name containing a double // quote, a backslash, and a newline (auth.go's buildEntry doc comment @@ -689,6 +753,69 @@ func TestMetrics_ProviderHealthy_ReflectsOpenBreaker(t *testing.T) { assertSample(t, samples, "llmgateway_provider_healthy", map[string]string{"provider": "openai"}, "0") } +// --- store outage: skip, never fabricate a zero --- + +// TestMetrics_StoreDown_SkipsRatherThanFabricatesZero is the mutation- +// proofing test for every `if su.storeDown { continue }` / +// `if mc.storeDown { continue }` guard in metrics.go: deleting any of +// them makes this test fail. Forces the SAME limiter every family in +// this file reads from into a fail-closed store outage (alwaysErrStore, +// failOpen=false — routes_unified_test.go's own stub), then asserts +// every store-backed family emits NO samples at all rather than a +// fabricated 0 (review fix, adversarial verification 2026-08-23: +// without the guards, a Redis blip reads as a hard 0 mid-outage, then +// jumps back to the real total on recovery — Prometheus treats that as +// a counter reset followed by a spurious full-total "increase"). +// llmgateway_provider_healthy is deliberately excluded from the "must be +// absent" list: it comes from the in-process discovery breaker, never +// the counterStore, so a store outage must NOT affect it — asserted +// present here for exactly that reason. +func TestMetrics_StoreDown_SkipsRatherThanFabricatesZero(t *testing.T) { + t.Parallel() + cfg := newMetricsTestConfig() + cfg.Metrics.ModelLabel = true + cfg.Users.Inline[0].Limits = &LimitsConfig{RequestsPerDay: 100} + h, gw := newMetricsGatewayHandle(t, cfg) + + gw.limiter.store = alwaysErrStore{} + gw.limiter.failOpen = false + + rec := httptest.NewRecorder() + h.ServeHTTP(rec, metricsRequest(http.MethodGet, metricsPathDefault, "sk-admin1")) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) + } + + _, samples := parsePrometheusText(t, rec.Body.Bytes()) + mustBeAbsent := []string{ + "llmgateway_requests_total", + "llmgateway_tokens_total", + "llmgateway_cost_micro_usd_total", + "llmgateway_budget_consumed_ratio", + "llmgateway_provider_attempts_total", + "llmgateway_provider_failures_total", + "llmgateway_provider_model_attempts_total", + "llmgateway_provider_model_failures_total", + } + for _, s := range samples { + for _, absent := range mustBeAbsent { + if s.name == absent { + t.Errorf("sample %s%v present during a store outage — must be skipped, not fabricated as 0", s.name, s.labels) + } + } + } + + var providerHealthyPresent bool + for _, s := range samples { + if s.name == "llmgateway_provider_healthy" { + providerHealthyPresent = true + } + } + if !providerHealthyPresent { + t.Error("llmgateway_provider_healthy missing — this family reads in-process breaker state, not the store, and must still render during a store outage") + } +} + // --- rate-limit rejections --- // TestMetrics_RateLimitRejections_CountedByScope drives a real 429 @@ -745,6 +872,65 @@ func TestLimiter_RejectionSnapshot_StoreDown(t *testing.T) { } } +// TestMetrics_PruneRejectionsAfterReload_EndToEnd is the end-to-end proof +// for item 7 (adversarial verification, 2026-08-23): a file-sourced user +// removed via a real users-file hot-reload (auth.go's replaceFileUsers, +// driven through ServeHTTP exactly as production traffic drives it, not +// a direct limiter call) must stop exporting +// llmgateway_rate_limit_rejections_total, proving the wiring from +// ServeHTTP through authStore.maybeReload's new return value to +// Gateway.pruneRejectionsAfterReload actually runs, not just the +// pruning logic in isolation (TestLimiter_PruneRejections_ +// RemovesDeletedUserScope, limits_test.go). +func TestMetrics_PruneRejectionsAfterReload_EndToEnd(t *testing.T) { + t.Parallel() + dir := t.TempDir() + fp := filepath.Join(dir, "users.json") + writeUsersDoc(t, fp, []*UserConfig{ + {Name: "alice", Group: "g", APIKey: "sk-alice", Limits: &LimitsConfig{RequestsPerMinute: 1}}, + }, time.Time{}) + + cfg := newMetricsTestConfig() + cfg.Users = &UsersConfig{ + File: fp, + Inline: []*UserConfig{{Name: "admin1", Group: "g", APIKey: "sk-admin1", Admin: true}}, + } + h, gw := newMetricsGatewayHandle(t, cfg) + + body := map[string]any{"model": "gpt-test", "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + h.ServeHTTP(httptest.NewRecorder(), newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body)) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body)) + if rec.Code != http.StatusTooManyRequests { + t.Fatalf("status = %d, want 429 (RequestsPerMinute:1 exhausted)", rec.Code) + } + + rec = httptest.NewRecorder() + h.ServeHTTP(rec, metricsRequest(http.MethodGet, metricsPathDefault, "sk-admin1")) + _, samples := parsePrometheusText(t, rec.Body.Bytes()) + assertSample(t, samples, "llmgateway_rate_limit_rejections_total", map[string]string{"scope_kind": "user", "scope_id": "alice"}, "1") + + // Remove alice from the users file and force ServeHTTP's own + // maybeReload call to pick it up — mirrors users_file_test.go's own + // clock-advance idiom (newReloadableAuthStore). + clock := &fakeClock{now: time.Now()} + gw.auth.nowFn = clock.Now + writeUsersDoc(t, fp, nil, time.Now().Add(time.Hour)) + clock.Advance(reloadEvery + time.Second) + + rec = httptest.NewRecorder() + h.ServeHTTP(rec, metricsRequest(http.MethodGet, metricsPathDefault, "sk-admin1")) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) + } + _, samples = parsePrometheusText(t, rec.Body.Bytes()) + for _, s := range samples { + if s.name == "llmgateway_rate_limit_rejections_total" && s.labels["scope_id"] == "alice" { + t.Errorf("alice's rejection series is still present after being removed from the users file and reloaded: %+v", s) + } + } +} + // --- budget-consumed ratio --- func TestMetrics_BudgetConsumedRatio_ReflectsUsageVsLimit(t *testing.T) {