Skip to content

Expose Codex reset credits and add Prometheus monitoring examples - #623

Open
le-shi wants to merge 1 commit into
nesszer:mainfrom
le-shi:feat/reset-credit-observability
Open

le-shi wants to merge 1 commit into
nesszer:mainfrom
le-shi:feat/reset-credit-observability

Conversation

@le-shi

@le-shi le-shi commented Sep 28, 2026 •

Copy link
Copy Markdown

Behavior

Codex now exports reset-credit count and the earliest future expiry alongside the existing fixed Prometheus quota metrics. A count of 0 means exhausted; -1 means the reset-credit endpoint is unavailable or the provider is using PAT. Credit IDs and account details never become metric labels. OAuth reset-credit observations, including unavailable responses, are cached for ten minutes by account, credential and API base URL. Suspicious weekly-reset confirmation still gets independent inventory observations.

Adds an importable Chinese Grafana dashboard (weekly quota by default, single/all-node selection, collapsed auxiliary row), an authenticated scrape example, and alerts for collection health, quota, reset-credit exhaustion/unavailability and approaching expiry.

Closes #622.

Validation

  • cargo fmt --all -- --check
  • cargo test --manifest-path rust/Cargo.toml providers::codex --lib (51 passed)
  • cargo test --manifest-path rust/Cargo.toml cli::serve::metrics --lib (12 passed)
  • cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings -A clippy::manual_range_contains -A clippy::nonminimal_bool
  • cargo clippy --manifest-path apps/desktop-tauri/src-tauri/Cargo.toml --all-targets -- -D warnings -A clippy::manual_range_contains -A clippy::nonminimal_bool
  • Parsed dashboard JSON and alert/scrape YAML, and checked that PromQL metric references match the exported names.

Strict Clippy without the two targeted allowances reports pre-existing lints in Alibaba Token Plan, Kiro, and OpenAI modules. promtool and a live Grafana instance were not available locally, so the YAML and dashboard were validated structurally rather than imported into a running server.

Summary by CodeRabbit

  • New Features
    • Prometheus metrics now report Codex reset-credit availability and the next expiry when available.
    • Added a Chinese Grafana dashboard with quota, reset-credit, cost, and exporter-status panels, plus alert rules for collection issues and quota thresholds.
  • Documentation
    • Added Chinese setup guidance and Prometheus scrape examples, including Bearer Token authentication.
    • Documented reset-credit metric values and caching behavior.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: c209a7e3-0c78-4b2d-be71-dc8d1d215668

📥 Commits

Reviewing files that changed from the base of the PR and between b585d48 and b057c01.

📒 Files selected for processing (12)
  • .gitignore
  • docs/CLI.md
  • docs/grafana/codexbar-codex-zh.json
  • docs/prometheus/README.zh-CN.md
  • docs/prometheus/codexbar-alerts.yml
  • docs/prometheus/codexbar-scrape-example.yml
  • rust/src/cli/serve/metrics/definitions.rs
  • rust/src/cli/serve/metrics/rendering.rs
  • rust/src/cli/serve/metrics/snapshot.rs
  • rust/src/cli/serve/metrics/tests.rs
  • rust/src/providers/codex/api.rs
  • rust/src/providers/codex/mod.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The Codex OAuth flow now caches reset-credit observations and includes reset-credit availability and expiry in provider metrics. The change also adds Prometheus scrape and alert examples, a Grafana dashboard, and documentation.

Changes

Codex reset-credit monitoring

Layer / File(s) Summary
Fetch and integrate reset-credit observations
rust/src/providers/codex/api.rs, rust/src/providers/codex/mod.rs
Codex API reset-credit observations use a ten-minute cache scoped by API base URL, account/auth path, and token. Weekly-reset confirmation uses separate observations. OAuth provider results include reset-credit inventory data.
Build and render reset-credit metrics
rust/src/cli/serve/metrics/*
Provider metrics include reset-credit availability and future expiry. Rendering exports availability when present and expiry only when it is in the future. Tests cover counts, expiry, and missing inventory.
Add monitoring examples and documentation
.gitignore, docs/CLI.md, docs/grafana/*, docs/prometheus/*
Adds authenticated scrape configuration, alert rules, a Chinese Grafana dashboard, and metric documentation. The dashboard includes quota, reset-credit, collection, cost, data-age, and exporter panels.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant OAuth as Codex OAuth provider
  participant API as CodexApi
  participant Cache as Reset-credit cache
  participant Metrics as Metrics endpoint
  participant Prometheus
  participant Grafana

  OAuth->>API: Fetch usage and reset-credit data
  API->>Cache: Read or update scoped observation
  Cache-->>API: Return cached or fetched observation
  API-->>OAuth: Return usage and reset-credit data
  OAuth->>Metrics: Provide reset-credit inventory
  Prometheus->>Metrics: Scrape /metrics
  Prometheus-->>Grafana: Provide metric series
Loading

Suggested reviewers: finesssee

Merge Risk: ⚪ Minimal · up to b057c

The reset-credit metrics and monitoring examples are ready to merge after normal checks. Cache-slot cleanup can be considered separately.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to b057c

The new metrics remain authenticated and do not expose account or credit identifiers. However, the monitoring guide recommends sending a shared bearer token over plaintext LAN connections; if deployed as written, interception could grant access to protected data across nodes. Cache and alert behavior also merit review, though the guide does not change a running deployment automatically.

Retained concerns

  • Medium · security · inferred: The new multi-node example opts into plaintext non-loopback HTTP and recommends sharing one bearer token. An on-path party that captures a scrape credential could use it against reachable nodes' protected metrics, usage, cost, and dashboard-snapshot routes. The guide limits intended access to the Prometheus host and a trusted LAN, but those restrictions do not encrypt the token.
  • Low · reliability · inferred: The new process-wide cache creates a slot for each base-URL, account, and token scope but never evicts slots. Repeated credential or endpoint rotation can retain obsolete entries for the life of the process even though their observations expire, weakening long-running collection availability. The required local credential or configuration churn limits independently attackable scope.
  • Low · reliability · inferred: When a fresh weekly-reset confirmation cannot obtain credits, the producer can retain and export an earlier positive count. During that interval the example's exhausted and unavailable alerts need not fire, despite the failed confirmation. Ordinary fetch failures instead report unavailable, and the cached observation is subject to a ten-minute TTL.
Security review details

Security Blast Radius

  • inferred — If the example is deployed, the principal independent attack path is interception of a bearer token on the Prometheus-to-node LAN connection. A shared token can extend the resulting access to other reachable example nodes; neither deployment nor network reachability was verified.

Security Findings and Attack Paths

  • inferred — The example explicitly accepts plaintext bearer transmission rather than bypassing an authentication check. Capturing that token would authorize reads of protected usage, cost, metrics, and dashboard-snapshot data; the added metrics themselves do not contain credential identifiers.

Trust Boundaries and Controls

  • observed — Non-loopback startup requires a configured token and explicit plaintext opt-in. Request authorization checks a token digest, while the guide calls for a long random token, access limited to the Prometheus host, and a trusted LAN. These controls limit exposure but do not provide transport confidentiality.

Resilience and Maintainability Implications

  • inferred — Credential rotation isolates cache observations but leaves old slots resident; confirmation failure can temporarily preserve an older positive value while the example's unavailable alert checks only for -1. These are failure-containment and monitoring-integrity limits, not evidence of an unauthenticated cache or metrics path.

Hardening Proposals

  • proposed — Prefer a TLS-terminating, access-restricted scrape path and per-node credentials over the shared plaintext example. Bound cache slot residency without evicting still-fresh alternate-account observations, and distinguish a failed confirmation from a newly verified positive credit count in monitoring output.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.93% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 6 files. (6 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: exposing Codex reset credits and adding Prometheus monitoring examples.
Linked Issues check ✅ Passed The PR implements the coding requirements in [#622]. It adds both fixed-label reset-credit metrics, uses 0 for exhausted credits and -1 for unavailable data, and emits only a future expiry. Codex …
Out of Scope Changes check ✅ Passed The changed files support [#622]. The API changes provide reset-credit caching and confirmation behavior. The metrics changes expose the requested data. The tests verify the requested behavior. The do…
Full details: Docstring Coverage

Explanation

Docstring coverage is 37.93% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 6 files. (6 skipped: 6 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Finesssee

Copy link
Copy Markdown
Collaborator

Thermo-nuclear code-quality review

Verdict: FINDINGS (blocking). Reviewed head b057c0119. Exposing reset credits is useful. The findings are about the contract and about how much new state this adds to an already very large file.

P1: The -1 sentinel contradicts the metrics contract in the same doc

The new docs/CLI.md:104 says codexbar_reset_credits_available is -1 when credits are unavailable or the source is PAT. Two lines further down, the same file (docs/CLI.md:106) says: "Unknown, informational, non-finite, and dynamic additional-limit values are omitted instead of being inferred or replaced with sentinels." The expiry gauge in this PR follows that rule and is omitted when unknown; the count gauge doesn't.

A sentinel in a gauge also breaks aggregation. sum(), min() and < 1 style alerts all treat -1 as a real value, which is why the shipped alert needs its own == -1 rule (docs/prometheus/codexbar-alerts.yml:83). Omit the series when the count is unknown, and alert with absent(codexbar_reset_credits_available{provider="codex"}) together with codexbar_provider_up == 1.

P1: codex/api.rs grows from 2,128 to 2,442 lines with a hand-rolled cache state machine

The file was already over 1k lines. This PR adds about 310 more: a process-global RESET_CREDITS_CACHE: OnceLock<Mutex<HashMap<String, Arc<AsyncMutex<..>>>>> (api.rs:30), a three-field cache record, and three accessors (api.rs:346, :368, :400). Two of those accessors, fresh_reset_credits_for_confirmation and fetch_rate_limit_reset_credits_fresh, share the same fetch / write-back / failure-stamp body word for word. Keys include a hash of the access token and nothing ever evicts entries, so a long-running codexbar serve grows the map on every token rotation.

Suggested restructure: first ask whether the provider needs a 10-minute cache at all. Refresh cadence already belongs to the refresh engine (shell semaphore and provider_cache) and to serve's own snapshot interval. If one is still needed, move it to providers/codex/reset_credits.rs, the same way subscription.rs is split out, with one entry point: get(freshness: Freshness::{Cached, NewerThan(Instant), Fresh}). The three accessors collapse to one, the duplicated body disappears, and the provider module shrinks instead of growing.

P2: The reset-credits window is applied twice, then de-duplicated

apply_reset_credits_window runs in fetch_usage_once (api.rs:322) and again at the end of fetch_usage_with_reset_credits (api.rs:253). The retain(|w| w.id != "reset-credits") inside it exists only to undo the first application. Apply it once, at the end.

P2: Codex-specific inventory logic in the shared serve metrics path

metrics/snapshot.rs:90 checks id == ProviderId::Codex, then searches result.inventory twice for the magic string "reset-credits". Grok also emits a "reset-credits" inventory item (providers/grok/mod.rs:454) and is silently left out. The inventory type is already provider-agnostic, so export it that way, for example codexbar_inventory_available{provider, item} and codexbar_inventory_next_expiry_timestamp_seconds{provider, item}. That removes the provider check and the string search, and Grok gets coverage without any extra code. If the item id is kept, make it a shared constant rather than a literal repeated in four places.

P3: Name the tuple

fetch_usage_with_reset_credits returns a four-element tuple, and fetch_usage exists only to drop the fourth element. A CodexUsageFetch { usage, cost, account, reset_credits } struct makes the call sites self-describing.

P3: Docs language

docs/ is English. The Grafana dashboard titles and the only README (README.zh-CN.md) are Chinese-only. Add an English README, keep zh-CN alongside it if you like, and have docs/CLI.md link to the English one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose Codex reset-credit metrics and provide monitoring examples

2 participants