feat(observability): log octokit rate-limit headers via hook.after#183
Conversation
The two App constructors were bare: no octokit.hook.after listener installed GitHub's per-installation rate-limit headers (x-ratelimit-*, Retry-After), so the bot had zero log-visible signal on quota state or secondary-rate-limit backoff. - Add src/utils/octokit-observability.ts: installRateLimitHooks(octokit) wires hook.after (logs rate-limit context) + hook.error (warns on 429 / 403 secondary-limit, rethrows), a pure rateLimitFields() builder, a strict Zod GithubApiLogFieldsSchema, and ObservableOctokit = Octokit.plugin(...). - Wire Octokit: ObservableOctokit into both App constructors (app.ts, connection-handler.ts). Per octokit v5 the Octokit option enriches app.octokit AND every getInstallationOctokit(), so all call sites inherit the hooks from one shared subclass. - test/utils/octokit-observability.test.ts: parser cases + strict-schema drift + two hook-contract tests (after-hook preserves the response, error-hook rethrows a 429) via a stubbed request.fetch. - Docs: observability.md gains a GitHub API rate-limit fields section. Volume policy reuses LOG_LEVEL instead of a new sampling env var: the per-request line is debug (silent at default info on a fleet issuing thousands of calls/hour); a warn fires only when remaining drops below RATE_LIMIT_LOW_WATER (500) or on a rate-limit error. Closes #170 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 9 minutes and 20 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
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 |
There was a problem hiding this comment.
Pull request overview
Adds GitHub API rate-limit observability by introducing an Octokit plugin that logs rate-limit headers and rate-limit errors, then wiring it into App-created Octokit clients.
Changes:
- Adds
src/utils/octokit-observability.tswith rate-limit field parsing, strict Zod schema, hooks, andObservableOctokit. - Wires
ObservableOctokitinto the main and orchestratorAppconstructors. - Adds unit/contract tests and updates observability docs for the new log events.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/octokit-observability.ts |
Defines rate-limit log fields, hooks, and ObservableOctokit subclass. |
src/app.ts |
Uses ObservableOctokit for the main App constructor. |
src/orchestrator/connection-handler.ts |
Uses ObservableOctokit for the orchestrator App constructor. |
test/utils/octokit-observability.test.ts |
Covers parsing, schema strictness, and hook behavior. |
docs/operate/observability.md |
Documents GitHub API rate-limit log fields and volume policy. |
| /** | ||
| * Octokit subclass with the rate-limit hooks pre-installed. Pass as the `App`'s | ||
| * `Octokit` option so `app.octokit` and every `getInstallationOctokit()` inherit | ||
| * the observability hooks from a single shared class. | ||
| */ | ||
| export const ObservableOctokit = Octokit.plugin((octokit) => { |
| | `event` | Level | Fields | | ||
| | ------------------------------- | ----- | ------------------------------------------------------------------------------------------------------------- | | ||
| | `github.api.request` | debug | `route`, `status`, `rate_limit_limit`, `rate_limit_remaining`, `rate_limit_reset_in_s`, `rate_limit_resource` | | ||
| | `github.api.rate_limit_low` | warn | Same fields; emitted once `rate_limit_remaining` drops below `RATE_LIMIT_LOW_WATER` (500). | |
… mocks CI Lint & Test failed: `Octokit.plugin is not a function` in connection-handler.test.ts. The module-level `Octokit.plugin(...)` ran at import time against the test's mocked `octokit` (whose MockOctokit had no static `plugin`), crashing any test that imports a consumer of this module. - Replace the eager `export const ObservableOctokit` with a memoised `observableOctokit()` builder, so importing the module has no side effect; the plugin subclass is created on first call inside App construction. - Make connection-handler.test.ts's MockOctokit faithful: add a static `plugin()` (the real octokit Octokit exposes one) for the getOrCreateApp path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
What
Resolves #170. GitHub returns per-installation rate-limit context on every response (
x-ratelimit-limit/remaining/reset/used/resource, plusRetry-Afteron 429 / 403 secondary-limit). BothAppconstructors were bare — nooctokit.hook.afterlistener — so quota state and rate-limit backoff were invisible (grep -ri x-ratelimit src/returned nothing).Changes
src/utils/octokit-observability.ts:installRateLimitHooks(octokit)wireshook.after(rate-limit context) +hook.error(warn on 429 / 403-with-Retry-After, rethrows to preserve error propagation); a purerateLimitFields(...)builder; a strict ZodGithubApiLogFieldsSchema; andObservableOctokit = Octokit.plugin(installRateLimitHooks).Octokit: ObservableOctokitinto both App constructors (app.ts,connection-handler.ts). Per octokit v5 theOctokitoption enrichesapp.octokitand everygetInstallationOctokit()(confirmed via docs + source), so all call sites inherit the hooks from one shared subclass — no per-site install, no double-registration.test/utils/octokit-observability.test.ts: parser cases (healthy/low/boundary/absent/optional-omission), strict-schema drift, and two hook-contract tests via a stubbedrequest.fetch— one asserting the after-hook preserves the response (status/data intact), one asserting the error-hook rethrows a 429.observability.mdgains a "GitHub API rate-limit fields" section.Judgement calls
LOG_GITHUB_API_REQUESTSsampling env var (deviation from the proposal). Volume is controlled by reusingLOG_LEVEL: the per-request line isdebug(silent at defaultinfoon a fleet issuing thousands of calls/hour); awarnfires only whenremainingdrops belowRATE_LIMIT_LOW_WATER(500) or on a rate-limit error. Same volume control, zero new config surface.installation_idnot logged (not cleanly available in the octokit hook);route(method + url) gives per-repo context and the headers are themselves per-installation.Verification
typecheck / eslint (0 errors) / prettier / em-dash / docs-citations clean.
test/utils/octokit-observability.test.ts9 pass (incl. hook contracts; no network, no backoff). Senior-review gate: 2 independent passes, both clean — the two critical octokit contracts (after-hook ignores handler return value; error-hook must rethrow) were verified againstbefore-after-hooksource + a live probe. CodeRabbit unavailable this run (org out of credits), so senior-review is the quality gate.🤖 Generated with Claude Code