You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Child of #19, which scopes M5's observability but does not specify it. SPEC §17.2 names what to measure and §18.3 gives the targets those measurements are supposed to falsify; this is the endpoint-and-instrument-level spec for both.
Nothing exists today: no metrics crate, no #[instrument], 29 tracing:: calls total. So this is greenfield, and the ordering matters — the interesting decisions are below, and one of them is the reason this issue outranks the rest of M5's remaining scope.
1. The fork-case counter is the falsification instrument, not a nice-to-have
SPEC §17.2 calls the class-1/2/3 split from §9.2 "the single most important health metric — case 3 should be near zero", and §18.3 turns that into a falsifiable target: case-3 fork resolutions < 0.1% of federated events.
That is the number the entire architectural bet rests on. If case 3 is not rare in the wild, "no state resolution on the hot path" is false and we need to know from a counter rather than from a user complaint.
It is also a hard dependency of two other issues, which is why this one should land first:
M3: prove class-D fork handling with a Synapse interop rig #16 requires "assert the selected fast/bounded/full path via metrics" and "no test can pass by silently taking a more expensive path". That test cannot be written until the path is counted.
So the first slice is not a /metrics endpoint in the abstract — it is this counter, plus the smallest endpoint that can expose it.
2. A metric that never moves is worse than a missing one
The same argument as surface.rs and the admin API's honest-advertisement rule, and it bites harder here. An endpoint that 404s is obviously absent. A gauge exported as a flat 0 is indistinguishable on a dashboard from a healthy system, and the operator finds out it was never wired during the incident it was supposed to catch.
Therefore: every exported metric ships with a test that proves it moves — drive the operation, scrape, assert the delta. A metric without one does not get exported. This is the acceptance criterion that keeps this issue from becoming decoration.
3. Per-room detail does not go in labels
§17.2 says "per-room and per-shard metrics", and a literal reading means a room-ID label. That is the classic Prometheus cardinality failure: a server with 10,000 rooms and six per-room series becomes 60,000 series that mostly never change, and the scrape cost grows with the room count rather than with traffic.
The split this proposes:
Question
Where it is answered
"How is the server doing?"
/metrics — aggregate histograms and counters, bounded label sets
"How is this room doing?"
The admin API (#83), which already has per-room endpoints and an authorization model
"Which shard is hot?"
/metrics with a shard label — bounded by shard_count, which is config
Bounded labels only: shard, fork case, federation destination (capped, with an other bucket — a hostile or misconfigured peer must not be able to mint series), request route from the router's own template rather than the raw path.
4. /metrics is an operational surface, not a public one
Scrape output names rooms indirectly, exposes traffic volumes and reveals which peers this server talks to. It must not be reachable from the open internet by default. Options, in preference order:
A separate listener bound to 127.0.0.1 by default ([metrics] bind = …), which is what most operators expect and what makes a reverse-proxy config obvious.
On the main listener behind the AdminActor guard, for deployments that cannot run a second port.
Proposed: (1) by default, (2) available, and off entirely unless configured — consistent with builtin_oidc and [auth.delegated] being opt-in.
What to export
From §17.2, in dependency order. Each row is a slice; each ships with the test from §2.
This is the set #19's exit criteria name: "durability, federation backlog, sync watermark lag and unexpected fork paths".
Slice 4 — traces
OpenTelemetry spans for client request → executor → commit → fan-out, per §17.2. Deliberately last: traces are the most invasive change and the least useful without the counters to tell you which trace to go looking for.
Deliberately not in scope
A metrics library shootout. Pick one, note it in an ADR, move on. The exposition format is the contract, not the crate.
Child of #19, which scopes M5's observability but does not specify it. SPEC §17.2 names what to measure and §18.3 gives the targets those measurements are supposed to falsify; this is the endpoint-and-instrument-level spec for both.
Nothing exists today: no metrics crate, no
#[instrument], 29tracing::calls total. So this is greenfield, and the ordering matters — the interesting decisions are below, and one of them is the reason this issue outranks the rest of M5's remaining scope.1. The fork-case counter is the falsification instrument, not a nice-to-have
SPEC §17.2 calls the class-1/2/3 split from §9.2 "the single most important health metric — case 3 should be near zero", and §18.3 turns that into a falsifiable target: case-3 fork resolutions < 0.1% of federated events.
That is the number the entire architectural bet rests on. If case 3 is not rare in the wild, "no state resolution on the hot path" is false and we need to know from a counter rather than from a user complaint.
It is also a hard dependency of two other issues, which is why this one should land first:
So the first slice is not a
/metricsendpoint in the abstract — it is this counter, plus the smallest endpoint that can expose it.2. A metric that never moves is worse than a missing one
The same argument as
surface.rsand the admin API's honest-advertisement rule, and it bites harder here. An endpoint that 404s is obviously absent. A gauge exported as a flat0is indistinguishable on a dashboard from a healthy system, and the operator finds out it was never wired during the incident it was supposed to catch.Therefore: every exported metric ships with a test that proves it moves — drive the operation, scrape, assert the delta. A metric without one does not get exported. This is the acceptance criterion that keeps this issue from becoming decoration.
3. Per-room detail does not go in labels
§17.2 says "per-room and per-shard metrics", and a literal reading means a room-ID label. That is the classic Prometheus cardinality failure: a server with 10,000 rooms and six per-room series becomes 60,000 series that mostly never change, and the scrape cost grows with the room count rather than with traffic.
The split this proposes:
/metrics— aggregate histograms and counters, bounded label sets/metricswith ashardlabel — bounded byshard_count, which is configBounded labels only:
shard, forkcase, federationdestination(capped, with anotherbucket — a hostile or misconfigured peer must not be able to mint series), requestroutefrom the router's own template rather than the raw path.4.
/metricsis an operational surface, not a public oneScrape output names rooms indirectly, exposes traffic volumes and reveals which peers this server talks to. It must not be reachable from the open internet by default. Options, in preference order:
127.0.0.1by default ([metrics] bind = …), which is what most operators expect and what makes a reverse-proxy config obvious.AdminActorguard, for deployments that cannot run a second port.Proposed: (1) by default, (2) available, and off entirely unless configured — consistent with
builtin_oidcand[auth.delegated]being opt-in.What to export
From §17.2, in dependency order. Each row is a slice; each ships with the test from §2.
Slice 1 — the counter that matters
spindle_fork_resolutions_totalcase(1/2/3)spindle_events_appended_totalorigin(local/federated)spindle_build_infoversionPlus the endpoint and the config to serve it.
Slice 2 — the hot path
spindle_append_duration_secondsdurabilityspindle_http_requests_totalroute,method,statusspindle_http_request_duration_secondsroute§18.3's local-send p50/p99 targets become checkable here.
Slice 3 — the queues and the caches
spindle_federation_queue_depthdestination(capped)spindle_federation_transactions_totaloutcomespindle_state_node_cache_hits_total/_misses_totalspindle_sync_subscribersspindle_sync_watermark_lag_secondsThis is the set #19's exit criteria name: "durability, federation backlog, sync watermark lag and unexpected fork paths".
Slice 4 — traces
OpenTelemetry spans for client request → executor → commit → fan-out, per §17.2. Deliberately last: traces are the most invasive change and the least useful without the counters to tell you which trace to go looking for.
Deliberately not in scope
Acceptance
/metricsis off unless configured, and not on the public listener by default.docs/gains an operator page saying what each metric means and which SPEC target it validates — a metric nobody can interpret is not observability.