Repository navigation
fix(o11y): box and drain robustness follow-ups (DEV-3144) - #401
Merged
Merged
Conversation
The ingest panel groups by outcome only, so ingest_timeout and ingest_error drops read as plain 'dropped'. A second panel groups dropped points by reason (blob9). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
wrangler dev's O11Y_LOKI_STATE is miniflare's R2 sim, not the MinIO the box writes state/wakes/<id>/clean to, so every local wake resolved unclean and its inbox keys were redrained on each wake. Local mode now does a SigV4-signed path-style HEAD against MinIO; production keeps the R2 binding head. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A hung InboxWriter left the collect request hanging because its catch only covers throws. Collect now uses ingestWithDeadline like the other routes, so a timeout or rejection answers 503 + Retry-After with an accounted drop point. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… refresh The SDK re-applies the outbound interception for an already-running container inside the constructor without awaiting or catching it, so a rejection after ctx.abort() or a deploy was an unhandled rejection. GrafanaBox now overrides the (TS-private) method, logs o11y.ae_outbound.degraded, and still returns the SDK promise so start() keeps its fail-open behaviour. The stub models the unawaited constructor call, and a test pins the method against the package. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…t wait in o11y-local Nothing in CI built the GrafanaBox or ran stop-roundtrip.mjs. The o11y-local spec's fixed 60 s API worker wait is shorter than a cold Tier-2 image build, and kept waiting after the worker had already died. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…nterception The degraded fallback only console.errored, so nothing alerted. It now also writes an o11y.ae_degraded point (reason start|reload) through the Worker's own AE binding, and the ae-outbound-degraded rule fires on any point in 2 h. A new metric keeps the o11y.wake outcome panels free of non-wake points. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… timeout with the boot wait Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… box up drainStep ended its chain when drains were paused, and nothing restarted it, so an awake box (visit wake) never resumed draining after the cron cleared the pause. When #finishDrain leaves the box up for an active visitor, the step now reschedules itself every 60 s; a quiet or stopped box still ends the chain. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
drainStep retried every second until the 4 h cap, and its readiness probes renew the idle clock, so a box whose Loki kept answering 503 never idle-stopped. It now persists when the box first went not-ready, backs the gap off (1 s, 5 s, 30 s), and after 10 minutes writes an o11y.drain error point and runs the usual quiet-stop decision, leaving a box with a visitor up. A stopped container ends the chain instead of polling forever. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ed resume, not-ready backoff)
…me paths cost nothing A forged exception body could use up an inbox object's whole map-read budget (4 bodies x 8 new keys = 32), and real exceptions packed later in the same object were then reported over_cap and left minified. The drain now lists sourcemaps/<service.version>/ once per distinct version (at most 8 per call) and admits only keys the listing shows; a list that throws falls back to the caps and is reported as list_error. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… pushing it unsymbolicated A getMap throw became a permanent fetch_error: the frames stayed minified and the key was pushed that way, and a replay after an unclean stop pushed a different body, missing Loki's dedupe. The read is now retried twice in the call; if it still fails the key is deferred (map_fetch_error, like a failed inbox read) so the batch continues and nothing is pushed for it. The ledger has no attempt counter, so the deferral is bounded by the key's inbox hour: after 6 hours the key pushes as it is. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…image ships drain.ts matches Loki's stream-limit 429 by its message. The only record of that wording was a hard-coded string in a test, so a Loki bump could change it unnoticed and the 429 would be retried, where a retry can answer 204 with the excess streams dropped. The test now reads the Dockerfile's grafana/loki tag and fails until the wording is re-verified and the constant moved. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ility and attempt budget Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
setup-node v5 reads the root packageManager field and tries to restore a pnpm cache, but this job never installs pnpm, so it failed before compose. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
stop-roundtrip.mjs runs compose down -v at the end unless O11Y_ROUNDTRIP_KEEP=1, so the collect step found an empty project. The always() step still tears down. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…he deadline Browser metric and example.* points are written only for accepted items. After a deadline miss the client retry is a duplicate and writes none, so a late commit lost its points. The abandoned call is now kept alive with waitUntil and writes the points of its accepted items once. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A MinIO that accepted the connection and never answered hung resolveWakes in InboxWriter. The abort throws like any unexpected status, so the wake stays retryable. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
exitCode stays null when the child is killed by a signal, so the boot wait polled to its timeout instead of failing fast. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… starve the drain A key deferred by a failing map or inbox read stayed first in key order, so each step re-read it and drained fewer new keys, and a batch of only deferred keys ended the chain before later keys were reached. The box now keeps a per-wake set of deferred keys, passes it to nextWrittenKeys, and with a visitor present rechecks slowly with a cleared set instead of ending the chain. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…inish guard A throw from the not-ready path (storage, isAwake, stop, schedule) escaped drainStep and silently ended the chain; it now falls into the same fallback as any other drainStep error. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… flight isReady is false while a stop is in flight but isAwake is still true, so the give-up fired a second stop and a misleading error point for a box that was already going down. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ling map read A failing list fell back to admitting keys by the caps, so a replay with a working list could resolve different frames and push a second, different copy. Within the 6 hour retry window the object is now deferred; older keys keep the cap fallback. Documents the residual per-call version-cap risk. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… contract row Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 24fc828. Configure here.
… is full Once the per-wake deferred set hit its cap, an all-deferred batch recorded no new key and ended the chain without the visitor recheck, leaving it dead until the hard cap. Both all-deferred exits now share one helper. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
3 of 8 tasks
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.

Context
Follow-ups from DEV-3144 (PR #371 / ADR-0041) for the sleeping Grafana box and the R2 inbox drain. This PR closes the items that were verifiable and safe to batch; the Loki
index_schema change is in its own PR because it has a deploy-date constraint. Per item:/telemetry/collectnow has the same 10 s ingest deadline as the other ingest routes (503 +Retry-After), and a commit that lands after the deadline still writes its accepted items' analytics points once.GrafanaBoxguards the constructor-time outbound interception failure that the SDK leaves as an unhandled rejection, reports fail-open through a newo11y.ae_degradedmetric and anae-outbound-degradedalert rule (2 h window), resumes draining after a spend-cap pause while a visitor holds the box, and backs off and gives up when the box stays not-ready. Symbolication lists existing source maps once perservice.versionbefore reading, so forged paths no longer spend an inbox object's map budget, and a transient map read or listing failure defers only that key (bounded to keys under 6 h old) instead of pushing it unsymbolicated. Deferred keys are excluded for the rest of the wake so they no longer starve later keys. Local dev checks the clean-shutdown marker in MinIO over a SigV4 HEAD, so the ledger commits locally. A "dropped by reason" panel is added to Observability self. A nightly workflow builds the box image and runsstop-roundtrip.mjs, and the o11y-local E2E API boot wait is configurable and fails fast.Not in this PR (decided during triage): the lost-reply
acceptedundercount (not fixable without changing dedupe semantics, stays documented in ADR-0041), ClickHouse empty header names (unproven, needs a run against plugin 3.5.0), splitting the InboxWriter per tenant, bounding the stop grace period, the@cloudflare/containers"Network connection lost" (upstream report only), and an idle-stop integration check.Types of changes
runner/) changeHow was this verified?
Every behavior change was written test-first and revert-checked (the new test fails against the previous code, passes with the change). Full pipeline suite from
runner/pipeline:node --experimental-strip-types --test *.test.mjsis 2669 tests, 2669 pass, 0 fail, 0 skipped.npx tsc --noEmitinrunner/workers/o11yis clean andpnpm typecheckinrunnerpasses for all packages.actionlintis clean on the new workflow, and its commands (docker compose build box+stop-roundtrip.mjs) ran locally with all checks passing. Each piece had a low-effort review, then a high-effort review of the whole branch whose findings (a workflow that would have failed atsetup-node, drain starvation by deferred keys, lost late-commit analytics points, plus smaller items) are fixed here. Not verified: the new workflow on GitHub, the o11y-local E2E end to end, the dashboard panel rendered in Grafana, and the MinIO SigV4 HEAD against a real MinIO (the signer matches the AWS test vector; please run oneo11y-devwake). The new alert and metric go live only after the o11y worker deploys.Checklist
runner/config/frameworks.json(see CONTRIBUTING.md); otherwise it won't appear on demos.handsontable.compnpm build(andpnpm dev) in the affected example/server-example locallyNone of the example checklist items apply; this PR changes only
runner/and CI.Related issue(s):
Note
Medium Risk
Touches core o11y ingest deadlines, GrafanaBox lifecycle, drain ledger ordering, and telemetry accounting; behavior is heavily covered by pipeline tests but production box/stop and MinIO marker paths are not fully exercised in CI.
Overview
Hardens the sleeping Grafana box, inbox drain, and browser ingest paths after DEV-3144, with broad test and doc updates.
Ingest:
/telemetry/collectnow uses the same 10 sInboxWriter.ingestdeadline as other routes (503 +Retry-After). Analytics points for inbox-backed items are written only foracceptedoutcomes; late commits after a timeout are finished viactx.waitUntilso metrics are counted once when the client retries asduplicate.GrafanaBox / drain: Fail-open or failed reload of the
ae.internaloutbound interception emitso11y.ae_degradedand feeds a newae-outbound-degradedalert (2 h window). Constructor-time interception rejections are handled so they are not unhandled rejections. While spend-cap pauses drains, an awake box with a visitor keeps a ~60 s recheck chain and resumes draining when the pause clears. Boxes that stay not-ready for 10 minutes back off, emit a drain error, and stop when quiet (visitors keep polling). Failed inbox or source-map reads defer keys for the rest of the wake (nextWrittenKeysskips them) so they do not starve the batch; only-deferred backlog can slow-recheck or stop like an empty backlog. Symbolication lists maps perservice.versionbefore GETs (caps forged paths), retries transient map/list failures, and defers young keys (<6 h) instead of pushing unsymbolicated bodies. Local wake resolution checks clean-shutdown markers via SigV4 HEAD to MinIO (marker.ts).CI / observability: New
o11y-box-nightly.ymlbuilds the box image and runsstop-roundtrip.mjs. Observability self gains ano11y.ingestdropped-by-reason panel. E2E o11y-local shareswait-for-serverwith configurable API boot timeout and fast-fail when the child exits.Reviewed by Cursor Bugbot for commit 47bb325. Bugbot is set up for automated code reviews on this repo. Configure here.