-
Notifications
You must be signed in to change notification settings - Fork 28
refactor(metrics): Add status_code for api_requests_by_status metric #135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds a third label "status_code" to the API request counter metric to enable more granular monitoring by HTTP status codes. The change allows tracking of specific HTTP response codes (200, 400, 404, 409, 500) alongside the existing endpoint and status labels.
- Updates the metric definition to include a "status_code" label
- Adds HTTP status codes to all API_REQUEST_COUNTER metric calls throughout the codebase
- Modifies one function to dynamically extract status codes from error responses
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/metrics.rs | Adds "status_code" as third label to API_REQUEST_COUNTER metric definition |
| src/routes/update_score.rs | Adds static status codes (200, 400) to metric calls and fixes import order |
| src/routes/update_gateway_score.rs | Adds static status codes (200, 400) to metric calls |
| src/routes/rule_configuration.rs | Adds status codes (200, 400, 404, 409, 500) to metric calls |
| src/routes/merchant_account_config.rs | Adds status codes (200, 404, 409, 500) to metric calls |
| src/routes/decision_gateway.rs | Adds status codes (200, 400) and dynamic status code extraction |
| src/routes/decide_gateway.rs | Adds static status codes (200, 400) to metric calls |
| src/euclid/handlers/routing_rules.rs | Adds status codes (200, 400, 404, 500) to metric calls |
| let status_code = e.status.clone(); | ||
| API_REQUEST_COUNTER | ||
| .with_label_values(&["decision_gateway", "failure"]) | ||
| .with_label_values(&["decision_gateway", "failure", &status_code]) |
Copilot
AI
Aug 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The clone() operation on e.status is unnecessary since it's being used immediately as a string reference. Consider using &e.status instead of cloning.
| logger::debug!(tag = "DecideGateway", "Error: {:?}", e); | ||
| metrics::API_REQUEST_COUNTER | ||
| .with_label_values(&["decide_gateway", "failure"]) | ||
| .with_label_values(&["decide_gateway", "failure", "400"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| .with_label_values(&["decide_gateway", "failure", "400"]) | |
| .with_label_values(&["decide_gateway", "failure", "500"]) |
isn't it 500?
| .with_label_values(&["update_gateway_score", "failure", "400"]) | ||
| .inc(); | ||
| timer.observe_duration(); | ||
| println!("Error: {:?}", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| println!("Error: {:?}", e); |
remove if not needed
Add status_code for api_requests_by_status metric