Add extra metrics#175
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds admin role-based access control via a new ChangesLogging, Monitoring, and Admin Authorization Expansion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hub_adapter/auth.py`:
- Around line 270-288: The RBAC check currently returns verified_token when
role_claim_name is set but admin_role is empty, allowing unintended access;
update the check in the block that handles role_claim_name/parsed_claim (the
code that reads role_claim_name, admin_role and returns verified_token) to fail
closed by raising an HTTPException when role_claim_name is truthy but admin_role
is falsy (e.g., raise HTTPException(status_code=500 or 400 with a clear message
about misconfiguration and service "Auth")). Place this validation before
parsing the claim keys so any partial RBAC config triggers an error instead of
bypassing admin role enforcement.
In `@hub_adapter/routers/logs.py`:
- Around line 389-439: The handler currently applies the incoming limit to the
raw _execute_raw_query call so only up to limit raw log rows are fetched before
aggregating into totals (logsql_query/params), which truncates
bytes_in/bytes_out when an analysis produces more rows; change the logic to page
through all raw rows before summing: use count_logs(base_query, params) to get
total, then loop calls to _execute_raw_query with offset/limit (or repeatedly
advance a start/offset parameter) until you've fetched total rows, accumulate
into totals (the totals dict / NetStatRun population) and only then compute meta
and return — alternatively, if you prefer pagination, remove limit from the raw
query and instead paginate the final grouped list (totals) after aggregation;
update usage around logsql_query, params, _execute_raw_query, count_logs, and
totals to implement one of these fixes.
- Around line 216-217: The async endpoint handlers (get_events,
get_analysis_logs, get_analysis_log_history, get_netstats, raw_log_query,
get_api_requests) are calling synchronous helpers (count_logs, query_logs,
_query_pod_logs, _get_analysis_container_names, _execute_raw_query) that use
httpx.Client and block the event loop; convert those helpers to async functions
that use httpx.AsyncClient and await the network calls, then update callers to
await the new async helpers (or alternatively make the handlers normal def so
FastAPI runs them in the threadpool). Specifically: change count_logs,
query_logs, _query_pod_logs, _get_analysis_container_names, _execute_raw_query
to async, replace httpx.Client() with httpx.AsyncClient() and await
.get/post/.stream as appropriate, update all handler calls (e.g., inside
get_events, get_analysis_logs, get_analysis_log_history, get_netstats,
raw_log_query, get_api_requests) to await the helpers, and ensure
require_victoria_logs compatibility and tests pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 03e89bf1-fa8f-42b9-911b-b7dd2eabdae7
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
hub_adapter/auth.pyhub_adapter/errors.pyhub_adapter/routers/hub.pyhub_adapter/routers/logs.pyhub_adapter/schemas/logs.pytests/router_tests/routes.pytests/router_tests/test_logs.pytests/test_auth.pytests/test_user_settings.py
1 participant
Summary by CodeRabbit
Release Notes
New Features
Improvements