Repository navigation
fix(metrics): add path label to HTTP response metrics - #1328
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 44 minutes and 55 seconds. ⌛ 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughHTTP response metrics are enhanced to track request paths as a new label dimension. The metric recording API signature is updated to accept path information, and middleware responsibilities are refactored to move error code extraction from the logging middleware to the metrics middleware. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/getting-started/monitoring.md`:
- Around line 186-192: The markdown lint rule MD031 flags the fenced code block
after the heading "**`/v1/responses` Success Rate**" as not surrounded by blank
lines; fix by inserting a single blank line between that bold heading and the
opening triple-backtick fence so the PromQL block (the code starting with
sum(rate(smg_http_responses_total...))) is separated by an empty line from the
heading.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: da0f3d09-c5df-48b3-b93f-c6af50146000
📒 Files selected for processing (5)
docs/getting-started/monitoring.mddocs/reference/metrics.mdmodel_gateway/src/middleware/logging.rsmodel_gateway/src/middleware/metrics.rsmodel_gateway/src/observability/metrics.rs
Signed-off-by: Ziwen Zhao <zzw.mose@gmail.com>
Signed-off-by: Ziwen Zhao <zzw.mose@gmail.com>
ac69da0 to
991c7c5
Compare
Description
Problem
/v1/responsessuccess rate could not be computed from the existing HTTP metrics.The main issue was that
smg_http_requests_totalalready exposed a normalizedpathlabel, butsmg_http_responses_totalonly exposedstatus_codeanderror_code. That meant we could count requests for/v1/responses, and we could count responses globally, but we could not slice response outcomes on the same path dimension to build a correct numerator and denominator for a/v1/responsessuccess-rate query.Existing error-count metrics were not a reliable substitute:
smg_http_responses_total{status_code=~"5.."}only gave a global HTTP error count, not a per-path error count, so it could not isolate/v1/responsesfailures.smg_router_request_errors_total{endpoint="responses"}was also not a reliable source of truth for this use case. Responses API traffic is not uniformly represented by router-level error metrics across implementations, so pairing router-level error counts with edge HTTP request counts would produce an incomplete and potentially misleading success-rate calculation.Because of that, the dedicated error-count metrics could show that failures existed, but they still could not answer the actual question: what is the HTTP success rate for
/v1/responses?Solution
Add the normalized HTTP
pathlabel tosmg_http_responses_totaland emit the response metric fromHttpMetricsLayer, where both the normalized request path and the final HTTP response are available together.This keeps request, response, and latency metrics aligned on the same path dimension and makes
/v1/responsessuccess-rate queries possible directly from Layer 1 HTTP metrics.Changes
pathtosmg_http_responses_totalMetrics::record_http_response(...)to recordpath,status_code, anderror_codeHttpMetricsLayer/v1/responsessuccess-rate query example/v1/responsessuccess rateTest Plan
Before this change, the following query was not possible because
smg_http_responses_totaldid not have apathlabel:After this change:
POST /v1/responsessmg_http_responses_totalnow includespath="/v1/responses"Validation run for this PR:
cargo test -p smg test_normalize_path_with_prefixed_id --libpre-commit run --all-files(with only the branch-protection hook skipped onmain)Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspasses