A dependency-free Prometheus /metrics endpoint (#91) - #199
Open
aaylward wants to merge 11 commits into
Open
Conversation
The first work item of #91. The runtime bundles no telemetry SDK by design, but the Prometheus text exposition format needs no client library at all -- it is a few lines of text over HTTP -- so this backend costs zero new dependencies and lives in :server directly. MetricsRegistry aggregates the existing Observe hooks into three families: requests_total{method,operation,status}, request_duration_seconds{method, operation} as a histogram, and requests_in_flight. MetricsEndpoint serves them; RecordMetrics is Observe wired to a registry, built on Observe rather than beside it so request timing keeps one implementation and the scraped numbers cannot drift from the logged ones. Composing the endpoint outside the recorder -- the documented order -- lets scrapes answer without inflating the request rate they report. Cardinality is the failure mode a metrics endpoint actually dies of, so the label set is bounded by construction rather than by convention. `target` is never a label: it carries path parameters and query strings, so one series per distinct URL is one series per request id, and `operation` is the bounded stand-in the router stamps from the model. `method` arrives off the wire, so a loop of `curl -X <random>` would otherwise mint a series per invented verb -- anything outside the standard set collapses to "other", case-sensitively, so a rejected "get" is not reported as served GET traffic. A series cap backstops anything unforeseen (a hand-written handler stamping its own operation) and counts each refused observation once in smithy_metrics_observations_dropped_total, so the limit is something to alert on rather than discover as an OOM. Status is the exact code rather than a class: bounded either way, and `{status=~"5.."}` recovers the class at query time while the reverse direction loses information that matters at 3am. Scope is the server side; the client-side registry #91 also asks for waits on that issue's open hook-shape question, since AttemptObservation carries no duration today and item 3 sequences hook changes separately. Tested: 21 unit tests over the registry and the composed middleware -- exposition format (HELP/TYPE, cumulative buckets, +Inf, sum/count), sub-millisecond latencies surviving the microsecond hook, concurrent recording, method clamping, the series cap's exact accounting, label escaping, and in-flight pairing including the throwing-handler path -- plus two tests over a real Beast socket for the scrape and for HEAD reporting the GET's length. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU
The operation label is the part only this level can check. In-tree the endpoint is driven by hand-written handlers that stamp HttpResponse::operation themselves, so those tests would keep passing if the generated router stopped stamping it -- and the label would go empty for every request in every real deployment, collapsing per-operation dashboards into one anonymous bucket. Three cases over a real socket against the generated Todo service, composed from consumer code against the published targets: the model's operation names reach the counters and the histogram, three scrapes add no series of their own and leave nothing in flight, and an unrouted request counts under an empty operation without its target leaking into a label. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU
MoonBase forked this document and developed it further; this brings the additions back, adapted rather than copied. New rules: fold review feedback into the PR it came from; question the request itself, not just how to build it; panel after pushing, on four lenses including altitude, with read-only agents enforced structurally; TDD as the default rather than bug-fixes-only; mutation checking is not a substitute for writing the test first; test through the objects production uses; watch for tests that do not actually run; CI is cheaper than model tokens. Adds a "Writing it down" section covering comments, commit messages and PR bodies, and turns the verification list into a table naming each command and the CI job that gates it. Adapted, not transplanted: MoonBase's receipts are replaced with this repo's, its squash-merge rationale for terse commit messages is replaced with the merge-commit one that actually applies here, and its "no ADRs, no CHANGELOG" section is dropped since both exist here. Two new receipts are local: #130's stash-vs-cancel question dissolving on contact with the transports, and `make verify` reporting success through a pipe while lint aborted on a missing buildifier. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU
The registry only accepted RequestObservation, so an application metric had
nowhere to go: a consumer wanting orders_processed_total needed a second
registry behind a second endpoint, and a Prometheus target scrapes one
endpoint. NewCounter/NewGauge/NewHistogram mint typed families served
alongside the built-in HTTP ones.
The registry keeps owning what corrupts a scrape when it goes wrong: label
values are escaped, labels are sorted so {a,b} and {b,a} are one series
rather than two, the per-family cap attributes overruns to
smithy_metrics_observations_dropped_total{metric="..."}, and an invalid or
colliding metric name aborts at registration. Each of those otherwise
produces output Prometheus rejects in full, with nothing in-process to
notice. Handles share ownership of their family, so one outliving its
registry is inert rather than dangling.
Also fixes the exposition's last line losing its terminating newline when no
application families follow.
The consumer example now emits a labelled counter, a histogram and a gauge
from its handler, proving they reach the same scrape as the generated
router's operation labels.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU
A series nobody has touched is absent from the scrape, so a counter's first exported sample is its first event's value — and increase() has nothing earlier to measure against, which hides that event for good and leaves the panel reading zero. Declare() materializes a series at its baseline; idempotent, cap-respecting, and it never disturbs one that has events. A declared histogram is genuinely empty rather than an observation of zero, so rate(_sum)/rate(_count) stays unbiased — writing the exposition directly avoids the bias a record-only API would have to accept. Ported from MoonBase futility/otel (#1323, #1384). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU
BeastServerTransport answers over-limit requests itself, while the parser is still reading, so RecordMetrics never sees them and a 413/431 flood is invisible in the counters. RecordRejections feeds Options::on_rejected; generic in the rejection type, so :server keeps no Beast dependency. They count and nothing more. No latency is filed: a request refused at parse time has no service latency, and zeros would drag rate(_sum)/rate(_count) down, flattering the panel during exactly the flood it should expose. The gauge stays untouched, since such a request was never in flight. A method that never parsed is labeled "unparsed" rather than "other" -- a 431 can fire mid-headers, and that is a different diagnosis from an invented verb. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU
HealthEndpoint built its response without stamping HttpResponse::operation, so every probe reached Observe — and so any metrics or logging backend — as the empty operation, which is what 404/405/400 dispatch failures already report. An orchestrator polls liveness every few seconds, making it usually the highest-volume path a service has, so the 404 rate was buried under probe volume, the probe's latency contaminated the service duration histogram, and no query could separate them. MetricsEndpoint had the same hole. Both now report their composed path. It is fixed at composition, never off the wire, so this costs one series per composed endpoint rather than reopening the cardinality question `target` was excluded for. Composing probes outside RecordMetrics still leaves them uncounted; the difference is that either choice is now expressible. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU
Two changes to MetricsRegistry, both about it being someone else's decision how this gets used. Off by default. MetricsOptions::enabled is false, and off means absent rather than idle: RecordMetrics and MetricsEndpoint compose to the identity, so a disabled registry puts no wrapper on the request path — no timing, no lock, not even a call frame — and /metrics reaches the router like any other unmodeled path. An empty 200 there would read to Prometheus as a live target reporting no series, which is what a service whose metrics have gone silent also looks like. Handles are inert rather than unusable so application code never branches on the flag, but their arguments are still built at the call site, so enabled() is exposed for a hot one. Registration is deliberately not conditional: an invalid name, a type collision or a bad ladder aborts at startup either way, so turning metrics on in production is never the first time those checks run. Configurable exposition, because the format is a contract with whatever is already scraping. MetricsOptions::Aura() is a transcription of MoonBase's shared HTTP vocabulary — the five http_server_* families and their pinned descriptions, service_name/http_method/route, the unmatched and /health route sentinels, CUSTOM and (unparsed) for methods, and the microsecond ladder its three emitter rails pin equal — so a service here can replace an aura, yodel or server_pal one without touching a dashboard. Success and failure are derived from the same status-keyed tally the total sums rather than counted separately, so the three cannot disagree; the in-flight gauge is keyed by method and never by route, which is where every rail leaves it because it moves before dispatch. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU
The families were smithy_http_requests_total, smithy_http_request_duration_seconds and smithy_http_requests_in_flight. Smithy is the IDL the service is described in — not a property of the request being counted, and not something a dashboard reading HTTP traffic has any reason to know. The prefix named the wrong thing. They are now http_requests_total, http_request_duration_seconds and http_requests_in_flight, which is Prometheus's own conventional naming, and the registry's drop counter is metrics_observations_dropped_total. That name is configurable now too, so nothing in the exposition is hardcoded any more. Deliberately not http_server_*: that stem belongs to the Aura preset, and keeping the two dialects distinct is what lets a test assert one is not being emitted alongside the other. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU
The previous two commits built a configurable exposition with a preset for MoonBase's vocabulary. Those services are the consumers, so the configurability was surface with no user: what it actually offered was a way for one service to drift off a contract the fleet shares, and that drift is silent — prom_proxy renders an empty panel, which reads as a quiet service rather than a misconfigured one. So there is one exposition now, transcribed from what //domains/platform/libs/otel_contract pins across MoonBase's Java, Rust and C++ rails: the five http_server_* families with their descriptions, service_name/http_method/route, the unmatched and /health route sentinels, CUSTOM and (unparsed) for methods, and the microsecond ladder. MetricsOptions is down to three fields that are still genuine choices — whether it runs, who it says it is, and the cardinality cap. service_name is required when enabled, and empty aborts. Every dashboard query selects on it, so a service reporting the empty string is scraped, stored, and absent from all of them: success everywhere except the panel nobody is watching yet. The transport tests found this the moment it landed, which is the argument for it. NewHistogram no longer defaults its buckets. The built-in ladder is microseconds and shaped for request latency; inheriting it silently for a histogram of bytes would produce a chart whose bins mean nothing. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU
Read across aura/middleware_test.cc and futility/otel/http_metrics_test.cc for cases those rails found the hard way. Four were missing here, each catching a class of drift the per-value assertions miss: - No series carries the old `method` spelling. futility needed this pin because that rail alone had historically spelled it `method`; a single stray one forks every dashboard series for the service, and the panel renders empty rather than wrong. Swept across all five families so a family added later cannot quietly reintroduce it. - A query string does not defeat the /health route. Orchestrators poll with one, and if it pushed the probe off the route, prom_proxy's subtraction would stop matching and probe volume would rejoin the serving numbers. - Scanner paths collapse into one series. The cap already backstops this, but only after the damage; collapsing at the label is what stops it starting. - Invented methods collapse on the gauge too, not just the counters. The gauge is keyed by method and moves at request start, so it needs the same normalization — a separate code path from the one the existing method tests cover. Two divergences found and deliberately not changed. futility puts status_code/result/error_type on its failure counter and histogram; yodel and server_pal do not, and otel_contract pins only the route sentinels cross-rail, so the leaner set stays. futility splits success at >=200 && <400 while yodel and server_pal both use <400, which is what is implemented here. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The first work item of #91, server side.
MetricsRegistryaggregates the existingObservehooks;MetricsEndpointserves them in the Prometheus text exposition format, which needs no client library — so this costs zero new dependencies and lives in:serverdirectly.RecordMetricsisObservewired to a registry, built on it rather than beside it so request timing keeps one implementation.Off unless asked for, and absent rather than idle when off
MetricsOptions::enableddefaults to false. Disabled,RecordMetricsandMetricsEndpointcompose to the identity — not a wrapper that checks a flag, but no wrapper at all, so a served request runs the call chain it would have run had this never been written./metricsthen reaches the router like any other unmodeled path and 404s; an empty 200 there would read to Prometheus as a live target reporting no series, which is indistinguishable from a service whose metrics have gone silent. Handles from a disabled registry are inert rather than unusable, so application code never branches on the flag; only their arguments still cost anything, soenabled()is exposed for a hot call site whose labels are expensive to build.Registration is deliberately not conditional on the flag. An invalid metric name, a type collision, or a missing
service_nameaborts at startup either way (ADR-0009), so switching metrics on in production is never the first time those checks run.The exposition is MoonBase's, and is not configurable
Five families labeled by
service_name,http_method,route:http_server_requests_totalhttp_server_requests_success_totalhttp_server_requests_failure_totalhttp_server_requests_active_gaugeroute)http_server_request_duration_microsecondsplus
metrics_observations_dropped_total, the registry's own health.This is not a vocabulary invented here. It is MoonBase's shared HTTP serving contract, spoken identically by its Java (yodel), Rust (server_pal) and C++ (futility/otel, behind aura) emitters and pinned across them by
//domains/platform/libs/otel_contract— names, descriptions, label sets, route sentinels and bucket boundaries alike. Those services are who scrapes this, and prom_proxy queries exactly these names with exactly these labels.An earlier revision of this branch made all of it configurable with a preset. That was surface with no user, and worse: a knob is a way for one service to drift off a contract the fleet shares, and the drift is silent — the panel renders empty, which looks like a quiet service rather than a misconfigured one.
MetricsOptionsis now three fields that are still genuine choices: whether it runs, who it says it is, and the cardinality cap.Things the diff doesn't show:
route!="/health"matcher passes a series without the label through untouched — which is what makes the same filter safe on it.service_nameis required when enabled, and empty aborts. Every dashboard query selects on it, so a service reporting the empty string is scraped, stored, and absent from all of them: success everywhere except the panel nobody is watching yet. The transport tests found this the moment it landed.targetis never a label — path parameters and query strings make it one series per request id. An unrouted request reportsunmatchedrather than the empty string, becauseroute!="/health"matches the empty string and unrouted traffic would silently join the serving figures.http_methodarrives off the wire, so anything outside the nine RFC 9110 verbs collapses toCUSTOM(case-sensitively — a rejectedgetmust not report as servedGET), and a request rejected before its method parsed reports(unparsed). A per-family cap backstops the rest, attributed onmetrics_observations_dropped_total{metric="..."}.NewHistogramhas no default buckets. The built-in ladder is microseconds and shaped for request latency; inheriting it silently for a histogram of bytes would produce a chart whose bins mean nothing.Folded in: probes stop reporting as the unrouted sentinel
HealthEndpointbuilt its response without stampingHttpResponse::operation, so every probe reachedObserve— and so any backend, not just this one — under the same value 404/405/400 dispatch failures use. An orchestrator polls liveness every few seconds, making it usually the highest-volume path a service has, so the 404 rate was buried under probe volume, the probe's latency contaminated the service duration histogram, and no query could separate them.MetricsEndpointhad the same hole.Both now report their composed path. It is fixed at composition, never off the wire, so this costs one series per composed endpoint rather than reopening the cardinality question
targetwas excluded for. It is also the prerequisite for prom_proxy's fleet-wideroute!="/health"subtraction: composeHealthEndpoint()on its default path and insideRecordMetrics, and the Probes tile finds it.Testing
Unit tests over the registry, the application families, and the composed middleware: exposition format (HELP/TYPE, cumulative buckets,
+Inf, sum/count), sub-millisecond latencies surviving the microsecond hook, concurrent recording, method clamping, cap accounting, label escaping and ordering, registration aborts, a handle outliving its registry, and in-flight pairing including the throwing-handler path.The
ExpositionContractsuite pins the MoonBase literals — the five names with their descriptions, the label set, the outcome split at 400, the gauge keyed by method and never by route, the route and method sentinels, and the 15-bound microsecond ladder. Those tests are what stops this rail drifting off the pin, and silence is how such a drift would otherwise show up.The
DisabledRegistrysuite pins the other half. The identity-composition test uses a plain function as the terminal sostd::function::targetis non-null exactly while nothing has wrapped it, and asserts the enabled case does wrap — without that second half the first proves nothing.Two tests over a real Beast socket. Nine in the out-of-tree consumer module, driving the generated router so the
routelabel is the model's and not a fixture's: the outcome counters and duration under the model's route, probes separable from unrouted traffic, application metrics on the same scrape, declared series at zero before traffic, and the disabled path 404ing while the service still works.Locally:
make verify(127 tests, lockfiles, codegen, goldens, clang-format),make noexcept, gcc ASan and TSan, clang-tidy, buildifier via npx, and the 14-test consumer suite. Two sandbox gaps, both verified another way:make lint's buildifier has no system binary here (run CI's way vianpx @bazel/buildifier@8.2.1it passes), and clang-tidy cannot parsebeast_transport.ccwithout Boost headers, which CI installs — every remaining finding is downstream of that onefile not found, and the files this PR touches are clean.Deliberately not covered
The client-side registry #91 item 1 also asks for.
AttemptObservationcarries no duration, so the per-attempt latency histogram can't be built fromObserveAttemptstoday — that's the open API-shape question in the issue's own comment, and item 3 sequences hook changes separately and first. Items 2–4 remain, so this does not close #91.Second item carried: one commit updates
docs/working-agreement.mdfrom MoonBase's fork of it (adapted, not copied — its receipts, squash-merge rationale, and "no ADRs/CHANGELOG" section are replaced or dropped). Unrelated to the metrics work, kept here by request.Follow-up filed: #201, on removing
smithyfrom runtime surfaces that have nothing to do with the IDL. The metric names were the visible instance and are fixed here; the namespace, include root and Bazel labels are a breaking rename that needs a decision first.Checklist
bazel test //...and(cd codegen && gradle build spotlessCheck)pass locally🤖 Generated with Claude Code
https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU