Add metrics-server healthcheck - #871
Conversation
📝 WalkthroughWalkthroughAdds a ChangesHealth endpoint for metrics-server
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/metrics-server/src/lib.rs`:
- Around line 141-145: The test function health_endpoint_reports_ok currently
only validates the HTTP status code but does not verify the response body
structure. Add an assertion to extract the response body and validate that it
contains the expected JSON payload with the status field set to "ok". This will
ensure the endpoint contract is fully tested and will catch any future changes
to the response body structure, not just the status code.
🪄 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: CHILL
Plan: Pro Plus
Run ID: aa0fd27c-321f-4d0f-af57-477a44960d03
📒 Files selected for processing (4)
crates/metrics-server/README.mdcrates/metrics-server/src/api.rscrates/metrics-server/src/lib.rscrates/metrics-server/src/server.rs
| #[tokio::test] | ||
| async fn health_endpoint_reports_ok() { | ||
| let response = health().await.into_response(); | ||
| assert_eq!(response.status(), StatusCode::OK); | ||
| } |
There was a problem hiding this comment.
Assert the health response body contract, not only status.
This test passes even if the payload stops returning {"status":"ok"}. Add a body assertion to lock the endpoint contract.
Suggested test hardening
#[tokio::test]
async fn health_endpoint_reports_ok() {
let response = health().await.into_response();
assert_eq!(response.status(), StatusCode::OK);
+ let body = axum::body::to_bytes(response.into_body(), usize::MAX)
+ .await
+ .unwrap();
+ assert_eq!(body.as_ref(), br#"{"status":"ok"}"#);
}🤖 Prompt for 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.
In `@crates/metrics-server/src/lib.rs` around lines 141 - 145, The test function
health_endpoint_reports_ok currently only validates the HTTP status code but
does not verify the response body structure. Add an assertion to extract the
response body and validate that it contains the expected JSON payload with the
status field set to "ok". This will ensure the endpoint contract is fully tested
and will catch any future changes to the response body structure, not just the
status code.
Summary
Adds a process-level HTTP healthcheck for
metrics-serveratGET /health, returning200 OKwith{"status":"ok"}. This gives deploy and orchestration tooling a simple URL to verify that the metrics-server HTTP listener is alive without touching run lifecycle or OTLP ingest APIs.Changes
healthHTTP handler.GET /healthinto the metrics-server Axum router.Validation
cargo fmt --all -- --checkcargo check -p metrics-servercargo test -p metrics-server --libcargo clippy -p metrics-server --all-targets -- -D warningscargo build -p metrics-serverGET http://127.0.0.1:18080/healthreturned{"status":"ok"}Summary by CodeRabbit
New Features
/health) that returns JSON status confirmation. Enables external monitoring systems and orchestration platforms to verify service availability and operational readiness.Documentation