fix(metrics): exclude review and schedule dwell from outbound terminal latency - #818
Merged
Merged
Conversation
…l latency
e2a_outbound_terminal_latency_seconds measured from messages.created_at, so
any wait the caller deliberately asked for was recorded as e2a latency:
- a HITL hold sits in pending_review until a human (or the TTL sweep)
resolves it. ApproveAndAccept updates the row in place and never touches
created_at, so an approved hold's sample carried the reviewer's entire
dwell. Every hold approved more than 5 minutes after drafting was an
automatic miss against the "terminal within 5 min of acceptance >= 99%"
SLO in docs/observability.md.
- a scheduled send waits for its fire time by construction, so any send
scheduled further out than the window missed it too.
This is the same class of error the webhook eventual-delivery SLI already
avoids by excluding endpoint_failure: the target measures e2a's own behavior,
and a reviewer taking an hour is not e2a being slow. It burned the hosted
error budget for real — a reviewer clearing a queue of held messages put a
cluster of them past the 300s bucket and paged the prod SLO alert on
2026-08-03.
Introduce submissionAnchor(acceptedAt, scheduledAt, reviewedAt) = the latest
of the three: the instant the message became ELIGIBLE to submit. Both terminal
emission paths use it -- the send worker inline and the terminal reconciler
for stranded rows -- so a message settled by the sweep is measured the same
way as one settled inline. Zero/nil values are inert, so an ordinary send
still anchors at created_at and the SLI keeps reporting real pipeline latency.
Taking the latest of the gates mirrors pastRetryHorizon's existing
max(accept, scheduled) reasoning. SendJob.AcceptedAt and the 72h retry horizon
are deliberately NOT changed -- that is F2 in docs/design/hitl-ttl-async-send.md
and stays open; this commit is observability-only, with no behavior change to
send, retry, or terminal outcomes.
Tests assert the review dwell and the schedule delay are excluded on both the
sent and failure arms, that an unheld/unscheduled message is unchanged, and
that the reconciler and worker anchors agree. Each fails without the fix.
The anchor fix was provable only where SendJob is hand-built: every
worker-level latency test drives a fake store, and the reconciler's new
tests assert the anchor helper rather than its use. Mutation-checked, both
production seams were unprotected — deleting the ReviewedAt/ScheduledAt
mapping in outboundSendStore.ClaimSend, or reverting the reconciler's two
emit sites to candidate.acceptedAt, left `go test -tags integration` green
across internal/agent, internal/identity and internal/outboundsend while
the SLI silently returned to charging the reviewer's dwell to e2a.
Add a store-backed test at each seam:
- agent: a real DeliverOutbound row gated on reviewed_at (a hold) or
scheduled_at (a schedule), claimed through ClaimSend and sent by a real
SendWorker, asserting the recorded latency is the 30s since the gate and
not the 2h since drafting. Fails on either mapping being dropped.
- outboundsend: two stranded rows gated off their own job's finalized_at
and settled by the reconciler, asserting ~30s. Fails both when the emit
site reverts to acceptedAt and when the candidate query stops selecting
the gate columns.
Also close the documentation gaps the anchor opens. The Prometheus HELP
string, both Metrics interfaces and the metric catalog still described the
old created_at baseline. And the d > 0 guard is now reachable for real: a
scheduled send cancelled before its fire time settles ahead of its own
anchor, as can an approve submitted inside the app/DB clock skew (the
anchor is Postgres now(), occurred_at is this process). Those samples are
dropped by design — the message never became eligible — and the drop is
pessimistic, since both sides of the ratio lose it. Say so, and downgrade
"exactly once" to "at most once" where the histogram is described. The SLO
section is renamed eligibility→terminal, keeping the hosted alert's
historical name and noting the one-time step down to annotate at release.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Summary
e2a_outbound_terminal_latency_secondsmeasured frommessages.created_at, so any wait the caller deliberately asked for was recorded as e2a latency. A HITL hold sits inpending_reviewuntil a human (or the TTL sweep) resolves it —ApproveAndAcceptupdates the row in place and never touchescreated_at— so an approved hold's sample carried the reviewer's entire dwell. Every hold approved more than 5 minutes after drafting was an automatic miss against the "terminal within 5 min of acceptance ≥ 99%" SLO indocs/observability.md. Scheduled sends have the same defect: any send scheduled further out than the window missed it by construction.This is the same class of error the webhook eventual-delivery SLI already avoids by excluding
endpoint_failure— the target measures e2a's own behavior, and a reviewer taking an hour is not e2a being slow.It burned the hosted error budget for real: on 2026-08-03 a reviewer clearing a queue of held messages put a cluster of them past the 300s bucket and paged the prod
outbound acceptance→terminal latencyalert at 11.46% against a 1% budget.The fix introduces
submissionAnchor(acceptedAt, scheduledAt, reviewedAt)— the latest of the three, i.e. the instant the message became eligible to submit. Both terminal emission paths use it (the send worker inline, and the terminal reconciler for stranded rows) so a message settled by the sweep is measured identically to one settled inline. Taking the latest of the gates mirrorspastRetryHorizon's existingmax(accept, scheduled)reasoning.Operational risk
Observability-only. No change to send, retry, or terminal outcomes, and no API or client surface is touched.
scheduled_atandreviewed_atare inert, so an ordinary send still anchors atcreated_atand the SLI keeps reporting real pipeline latency. That path has an explicit regression test.SendJob.AcceptedAtand the 72h retry horizon are deliberately not changed. That is F2 indocs/design/hitl-ttl-async-send.mdand stays open; an addendum there records the split so the two anchors don't get conflated later.scheduled_at,reviewed_at) on a query already reading that row — no new scan, no index implication.Once released, the hosted
outbound_terminal_latencyalert query needs no change — it becomes correct as written. Expect the reported late fraction on that SLI to drop for any deployment using HITL or scheduled sends, because those samples were never e2a latency to begin with.Test plan
go build ./...cleango vetclean oninternal/outboundsend,internal/identity,internal/agentgo test -short ./internal/outboundsend/ ./internal/identity/ ./internal/agent/ ./internal/telemetry/— all passj.AcceptedAt); the unchanged-ordinary-send test correctly still passessubmissionAnchortable test: gate ordering, a gate before accept is inert, zero accept with a hold🤖 Generated with Claude Code
https://claude.ai/code/session_016zA4AGZSEVk9jdBfyCkL1v
Generated by Claude Code