A metrics endpoint that cannot map your API, and a limiter that cannot be spoofed

Contribution Date
Contribution Project
Contribution Details
A metrics endpoint that cannot map your API, and a limiter that cannot be spoofed Go-live items 3 and 4, paired because they are both "the thing that has been declared and never wired". ## `/ready`, and what it deliberately does not check `/health` already answers liveness and says nothing on purpose. `/ready` answers the other question — should traffic come here — by touching what a request touches: Postgres and the object store, **both** checked rather than short-circuiting on the first failure, because during a wide outage "the database is down" tells you nothing about whether the store is also down. It skips the search index, and that is a decision rather than an omission: a tenant's index opens lazily and is rebuildable, so failing readiness over it would pull a replica out of rotation for something that stops no upload, download or metadata write. ## `/metrics` is fail-closed, and 404 rather than 401 No `server.metrics_token`, no endpoint. A wrong token gets the same 404, so a scan cannot tell a deployment with metrics switched off from one behind a credential. The argument is already written in this repo against `/health`: it is the first thing anybody scans, and metrics are worse than a version string — route templates are a map of the entire API surface and per-route counts are a usage profile. The default that ends up on the public internet is the one that serves openly and trusts a firewall. ## Cardinality is the whole design The registry is hand-written in `dam-telemetry`: four series, about two hundred visible lines, no global recorder to install and no ordering question about when. If the metric set outgrows one screen the `metrics` crates are the right answer and this should be deleted rather than extended. The route label is axum's `MatchedPath` — the template — and never the URI. A label per asset id is a million series and a monitoring system that falls over, so there is a case asserting a thousand requests through one template stay one series. An unmatched request collapses to a single bucket, because labelling 404s with their URIs would let anybody create unbounded series by requesting random paths. Status is a class rather than a code: the questions asked of it are answered by the class, and the code is in the log. `damrs_jobs` is refreshed on scrape rather than on a timer — queue depth is a fact about the database, not something this process accumulates — and a failure reading it does not fail the scrape. An endpoint that 500s because one gauge is unavailable goes dark exactly when the database is the thing going wrong. `state="dead"` is the series worth alerting on: a worker failing every derivative is indistinguishable from outside from one with nothing to do. Lock poisoning is recovered from rather than propagated. The worst case is a counter off by one, and letting the observability layer panic the process is backwards. ## The limiter protects the public routes and nothing else `governor` has been a declared dependency commented "per-tenant rate limiting" and never called. It now covers `/d/{token}`, `/share/{token}` and `/portal/{key}` — the routes that take no API key — keyed by client address. The authenticated API is deliberately not address-keyed. A company sits behind one or two egress addresses, so that would be a limit on the *customer*: the whole art department sharing a bucket and one bulk upload starving everybody's thumbnails. Authenticated traffic has a revocable credential and per-tenant quotas, which are the right instruments. Burst is separate from the sustained rate because a grid loads sixty thumbnails at once, and a limiter tuned only on the rate throttles the first screen every user ever sees. Off by default: a guessed number either does nothing or throttles a real page load, and neither is discovered until it is in front of users. `X-Forwarded-For` is trusted only as far as `trusted_proxy_hops` says, counting from the **right**, because those entries are what each proxy appended and a client cannot forge them. Taking the leftmost entry — the usual mistake — lets anybody claim a fresh bucket per request, or put somebody else's address in and exhaust theirs. There is a case for each hop count and one for a header shorter than claimed, which falls back to the socket rather than promoting a client-supplied value. ## One correction The middleware first took `ConnectInfo` by value while its own comment claimed a missing address was allowed. A required extractor *rejects*, so a forgotten `into_make_service_with_connect_info` in `damd` would have been a 500 on every public route rather than a lenient one — the comment described the opposite of the code. It reads the extension directly now, and a case asserts both halves: engaged when a peer is present, allowing when it is not. Verified live: three requests through the burst then 429s with `Retry-After` on the public route, six of six 200s on the authenticated API, `/ready` reporting both dependencies, and `/metrics` 404 without a token and correct with one — including `route="/assets/{asset_id}"` for a request to `/assets/nope`. `dam-api` also gains the `cfg_attr(test, allow(...))` every other crate carries; it had no unit tests until the throttle got some.
Contribution Author
Bassam Ismail
Files count
0
Patches count
1