Repository navigation
feat(supervisor): count fuel-set drops and sample latency on every outcome - #372
Merged
Merged
Conversation
…tcome DispatchOutcome::FuelSetFailed was logged and never counted. It now increments nexum_runtime_dispatch_dropped_total with reason "fuel_set_failed", beside the existing shutdown and rate_limited reasons. nexum_runtime_dispatch_latency_seconds was recorded in the success arm alone, so a fault, a trap or a deadline hit contributed no sample and p95 read low exactly when dispatch was failing. The histogram is now recorded once, after the match, for every dispatch that reached the guest, labelled outcome = ok, fault, trap or deadline. A deadline hit stays fatal like a trap and carries its own label so a blocked host call is legible apart from a guest trap. Cardinality is eleven buckets per module and outcome pair, over four fixed outcome values. Neither metric name is new, so the METRICS table is unchanged and the name guard is untouched. The exporter matches its bucket list on the bare metric name, which the addons test now records an outcome label to prove, and the NexumDispatchLatency alert sums by (module, le), so the added label costs neither its bounds nor its reading. Closes #368 AI Assistance: Claude Code used for implementation, tests and docs.
…ze its cardinality The METRICS table is the only in-band description an operator reads at /metrics, and its sibling entries name their labels. The dispatch latency HELP now names module and outcome like they do. The cardinality note in docs/production.md counted eleven buckets per module and outcome pair, which omits trigger_kind and the +Inf, _sum and _count series a Prometheus histogram also renders. It now states the multiplier the outcome label actually adds. The two capture_metrics tests that predate this branch built their current-thread runtime inline; they now use the block_on_current_thread helper the new tests introduced, so the file carries one form of the incantation. AI Assistance: Claude Code used for the red-team review and these fixes.
strum is already a workspace dependency this crate inherits, and nexum-runtime-wasm derives IntoStaticStr for exactly this: string labels on an enum. A hand-written match reinvented it and gave a second place for a variant to be added without its label. serialize_all covers every variant but Trapped, whose label is "trap" rather than the variant name. AI Assistance: claude-opus-5 used for the review pass.
The error counter hardcoded error_kind = "trap" in the arm that also handles a dispatch cut off at the deadline, while the latency histogram called the same event outcome = "deadline". A dashboard joining the two on what it took to be one failure got two answers. Both now read the same variant, so they cannot drift again. A deadline is a limit doing its job rather than a module bug, so NexumModuleTraps deliberately stops counting one, and the metric row and the alert say so. AI Assistance: claude-opus-5 used for the fix and the docs.
The runtime-builder incantation was open-coded eight times across four files before this branch, which then added a ninth as a local helper. It belongs beside capture_metrics, which is what makes it necessary: the recorder is thread-local, so work driven on another runtime's worker records into a different one and the capture comes back empty. Five of the eight now call the shared function. The three in event_loop.rs add start_paused(true) and are left alone, because another branch is in that file. Named block_on_current_thread rather than block_on: the supervisor tests already have a block_on that builds a chain block. AI Assistance: claude-opus-5 used for the review pass.
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
nexum_runtime_dispatch_latency_secondswas recorded in the success arm ofdispatch_toand nowhere else, so a fault, a trap or a deadline hit contributed no sample. It is now recorded once after the match, for every dispatch that reached the guest, carrying anoutcomelabel:ok,fault,trapordeadline.The deadline case was previously folded into the trap arm and invisible.
with_dispatch_deadlineis now bound before the error is converted, and the trap arm returns a newDispatchOutcome::Deadline. Health handling is byte-identical toTrapped, module death, poison verdict and panic record included; only the label differs.The fuel-set step moved into a
refuelhelper that keeps the existingerror!and incrementsnexum_runtime_dispatch_dropped_totalwithreason = "fuel_set_failed", beside theshutdownandrate_limitedvalues it already carried. The two pre-guest drops record no latency, since they never reached the guest, and carry their label as the drop reason instead, which single-sources both strings.No new metric name, so
nexum-runtime-metricsneeds no table entry and the name guard stays quiet.Why
Closes #368
p95 was biased low exactly when things went wrong, which is the reading an operator trusts least and needs most. A
FuelSetFaileddrop was logged and never counted.Cardinality, decided
Acceptable. Eleven buckets per
(module, outcome)pair over four fixed outcome values, so the series count grows by a constant factor of at most four on one histogram, and the outcome set cannot grow with operator or guest input. Recorded in the metrics table row rather than left implicit.Constraints confirmed rather than assumed
The exporter's bucket list uses
Matcher::Fullon the bare metric name, so a label cannot defeat it. That confirmation is executable rather than asserted:the_latency_histogram_renders_bucket_seriesincrates/nexum-runtime/src/addons.rsnow records anoutcomelabel and still asserts the_bucketseries atle="5".NexumDispatchLatencysums by(module, le), so it aggregates the new label away exactly as it already doestrigger_kind. No alert change needed.Testing
Every outcome is driven for real, using the adversarial fixtures rather than asserted in the abstract.
a_trap_and_a_success_each_record_a_labelled_latency_sample: fuel-bomb on chain 1, example on chain 100.a_deadline_hit_records_a_latency_sample_labelled_deadline: slow-host parked an hour past a one-second deadline, clock paused after boot so neither is waited out.a_fault_records_a_latency_sample_labelled_fault:max_state_bytes = 0makes flaky-bomb's first store write return a typed WIT fault, and the module stays alive.a_failed_fuel_set_is_counted_as_a_drop:refuelover aStoreon a fuel-less engine, which is the only wayset_fuelfails, asserting the reason, module and trigger kind.Gate: 257 tests across the three touched crates, all passing;
just buildthenjust test-e2e16 passed; clippy, rustdoc and doctests clean under-D warnings; noCargo.lockchurn.The two metrics now agree about one event
The error counter hardcoded
error_kind = "trap"in the arm that also handles a dispatch cut off at[limits.dispatch] deadline_secs, while this histogram calls that same eventoutcome = "deadline". A dashboard joining the two on what it took to be one failure got two answers.Both now read the same
DispatchOutcomevariant, so they cannot drift again.A deadline is a limit doing its job rather than a module bug, so
NexumModuleTrapsdeliberately stops counting one. The alert carries a comment saying so and pointing aterror_kind="deadline"for anyone who wants to alert on a module that should never reach its deadline. The metric row indocs/production.mdnames both values and says they are the same strings the histogram uses.a_deadline_hit_records_a_latency_sample_labelled_deadlineasserts both halves: the counter carriesdeadline, and no sample carriestrap.The label comes from a derive, not a match
DispatchOutcomederivesstrum::IntoStaticStrwithserialize_all = "snake_case", and one#[strum(serialize = "trap")]override onTrapped.strumis already a workspace dependency this crate inherits, andnexum-runtime-wasmalready derives it for the same purpose.The first revision hand-wrote a match from variant to string. It compiled and read fine, and it gave a variant a second place to be added without its label, which is exactly the drift the paragraph above describes.
The block_on helper is shared, not local
The first revision added a local
block_on_current_threadto the dispatch tests. The same runtime-builder incantation was already open-coded eight times across four files, so that would have made nine.It now lives beside
capture_metricsinnexum-runtime-testing, which is what makes it necessary in the first place: the recorder is thread-local, so work driven on another runtime's worker records into a different one and the capture comes back empty.nexum-runtime-testinggains therttokio feature it needs.Five of the eight call sites now use it. The three in
event_loop.rsaddstart_paused(true)and are deliberately left, because #373 is in that file and a sweep there would conflict for no gain. They are the obvious follow-on for whichever of the two lands second.It keeps the longer name rather than
block_on, because the supervisor tests already have ablock_onthat builds a chain block.Merge note
#371 touches
supervisor/dispatch.rsand the test module too. The two conflict only on adjacent test additions and a re-export list. Landing this one first leaves that one a trivial rebase.AI Assistance
Implementation: claude-opus-5. Red-team review: claude-opus-5. Verification: claude-opus-5. PR description: claude-opus-5.