Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/metrics-server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,15 @@ stage, and model identifiers for report export and post-run debugging.
metrics-server serve --db metrics.sqlite
metrics-server serve --db metrics.sqlite --debug-retain-raw-otlp
metrics-server emit-fixture --run-id fixture
curl http://127.0.0.1:18080/health
```

## Responsibilities

- ingest OTLP traces, metrics, and logs
- accept OTLP/gRPC on `--otlp-grpc-addr`
- accept OTLP/HTTP protobuf at `/v1/traces`, `/v1/metrics`, and `/v1/logs`
- expose `/health` for process-level HTTP health checks
- persist scalar OTLP gauge/sum data points in `metric_points`
- index data by run/request/session/stage IDs
- expose run lifecycle HTTP APIs
Expand Down
4 changes: 4 additions & 0 deletions crates/metrics-server/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ pub(crate) async fn create_run(
}))
}

pub(crate) async fn health() -> Json<Value> {
Json(serde_json::json!({ "status": "ok" }))
}

pub(crate) async fn run_status(
State(state): State<AppState>,
Path(run_id): Path<String>,
Expand Down
8 changes: 7 additions & 1 deletion crates/metrics-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub async fn run() -> Result<()> {
#[cfg(test)]
mod tests {
use super::{
api::otlp_http_traces,
api::{health, otlp_http_traces},
otlp_value::{kv_i64, kv_string},
server::AppState,
store::Store,
Expand Down Expand Up @@ -138,6 +138,12 @@ mod tests {
}
}

#[tokio::test]
async fn health_endpoint_reports_ok() {
let response = health().await.into_response();
assert_eq!(response.status(), StatusCode::OK);
}
Comment on lines +141 to +145

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.


#[test]
fn grpc_trace_ingest_populates_report_without_raw_by_default() {
let store = in_memory_store(false);
Expand Down
5 changes: 3 additions & 2 deletions crates/metrics-server/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use tokio::net::TcpListener;

use crate::{
api::{
artifacts, create_run, finalize_run, otlp_http_logs, otlp_http_metrics, otlp_http_traces,
report_json, run_status,
artifacts, create_run, finalize_run, health, otlp_http_logs, otlp_http_metrics,
otlp_http_traces, report_json, run_status,
},
cli::ServeArgs,
otlp::OtlpIngest,
Expand Down Expand Up @@ -73,6 +73,7 @@ pub(crate) async fn serve(args: ServeArgs) -> Result<()> {

pub(crate) async fn serve_http(state: AppState, addr: SocketAddr) -> Result<()> {
let app = Router::new()
.route("/health", get(health))
.route("/v1/runs", post(create_run))
.route("/v1/runs/{run_id}/status", get(run_status))
.route("/v1/runs/{run_id}/finalize", post(finalize_run))
Expand Down
Loading