fix(telemetry): drop permanently-rejected outbox batches instead of retrying forever#444
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Thank you for following the naming conventions! 🙏 |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughTelemetry HTTP drains now remove successfully delivered and permanently rejected batches from the outbox, while retaining rate-limited, server-error, and network-failed batches for retry. Tests cover these outcomes and a patch changeset documents the release. ChangesTelemetry drain retry behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem
The CLI telemetry outbox buffers events on disk and resends the entire pending batch together on every run. If the ingest endpoint rejects that batch with a non-2xx response for any reason, the old drain treated it as "keep retrying" — with no way to ever recover if the rejection was permanent (oversized batch, schema drift on old buffered events, unknown tool, etc.). That poisoned batch would then be resent first on every future run, blocking all telemetry for the tool indefinitely.
This was caught live: a local
evlog-clioutbox had accumulated 89 events since testing began (including some missing a field added later), which combined to trip bothmaxBatchSizeand payload validation on the deployed ingest endpoint — every subsequent command kept re-sending the same broken batch and getting a 400.Fix
createHttpDrainnow distinguishes permanent rejection from transient failure:Testing
packages/telemetry/test/drain.test.ts(new) — covers 2xx/400/429/5xx/network-error/empty-batchpnpm turbo run lint typecheck test --filter=@evlog/telemetry --filter=evlog-telemetry— all green@evlog/telemetry)Summary by CodeRabbit
Bug Fixes
Tests