mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
feat(health): granular /health liveness — a degraded component no longer 503s the container (#3312) (#3572)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
3d57bb2857
commit
4c79343a84
+31
-16
@@ -1056,35 +1056,50 @@ pub(super) fn with_cors_headers(mut response: Response, origin: Option<&str>) ->
|
||||
}
|
||||
|
||||
/// Handler for the health check endpoint.
|
||||
///
|
||||
/// Liveness is granular (#3312): a single degraded *background* component
|
||||
/// (scheduler, channels, update_checker, …) no longer 503s the whole container.
|
||||
/// `/health` returns 503 only when a *critical* component is unhealthy (see
|
||||
/// `health::CRITICAL_COMPONENTS`); otherwise it returns 200 — with a `degraded`
|
||||
/// flag and per-component buckets in the body so readiness probes and operators
|
||||
/// can still see partial failures.
|
||||
async fn health_handler() -> impl IntoResponse {
|
||||
let snapshot = crate::openhuman::health::snapshot();
|
||||
let unhealthy: Vec<&str> = snapshot
|
||||
.components
|
||||
.iter()
|
||||
.filter_map(|(name, c)| {
|
||||
if c.status == "ok" || c.status == "starting" {
|
||||
None
|
||||
} else {
|
||||
Some(name.as_str())
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
let is_ok = unhealthy.is_empty();
|
||||
let verdict = crate::openhuman::health::verdict(&snapshot);
|
||||
|
||||
let status = if is_ok {
|
||||
let status = if verdict.healthy {
|
||||
StatusCode::OK
|
||||
} else {
|
||||
StatusCode::SERVICE_UNAVAILABLE
|
||||
};
|
||||
|
||||
// Augment the snapshot body with the verdict so the components map stays
|
||||
// backward-compatible while exposing overall liveness/readiness.
|
||||
let mut body = serde_json::to_value(&snapshot).unwrap_or_else(|_| serde_json::json!({}));
|
||||
if let Some(obj) = body.as_object_mut() {
|
||||
obj.insert("healthy".to_string(), serde_json::json!(verdict.healthy));
|
||||
obj.insert("degraded".to_string(), serde_json::json!(verdict.degraded));
|
||||
obj.insert(
|
||||
"critical_unhealthy".to_string(),
|
||||
serde_json::json!(verdict.critical_unhealthy),
|
||||
);
|
||||
obj.insert(
|
||||
"degraded_components".to_string(),
|
||||
serde_json::json!(verdict.degraded_components),
|
||||
);
|
||||
}
|
||||
|
||||
tracing::debug!(
|
||||
"[health] status={} components={} unhealthy={:?}",
|
||||
"[health] status={} components={} healthy={} degraded={} critical_unhealthy={:?} degraded_components={:?}",
|
||||
status.as_u16(),
|
||||
snapshot.components.len(),
|
||||
unhealthy
|
||||
verdict.healthy,
|
||||
verdict.degraded,
|
||||
verdict.critical_unhealthy,
|
||||
verdict.degraded_components,
|
||||
);
|
||||
|
||||
(status, Json(snapshot))
|
||||
(status, Json(body))
|
||||
}
|
||||
|
||||
/// Handler for the schema discovery endpoint.
|
||||
|
||||
@@ -1195,20 +1195,43 @@ async fn test_http_health_handler_returns_correct_status() {
|
||||
.as_object()
|
||||
.expect("components should be an object");
|
||||
|
||||
// Derive the expected HTTP status solely from the response body so the
|
||||
// test asserts internal consistency of the handler rather than racing on
|
||||
// live component state.
|
||||
let body_says_ok = components.values().all(|c| {
|
||||
let s = c["status"].as_str().unwrap_or("");
|
||||
s == "ok" || s == "starting"
|
||||
});
|
||||
let expected_status = if body_says_ok {
|
||||
// Granular liveness (#3312): the HTTP status is driven by the `healthy`
|
||||
// verdict (no *critical* component unhealthy), not by all-components-ok.
|
||||
// Derive the expectation from the body so the test asserts the handler's
|
||||
// internal consistency rather than racing on live component state.
|
||||
let body_healthy = snapshot["healthy"]
|
||||
.as_bool()
|
||||
.expect("body exposes a `healthy` verdict flag");
|
||||
let expected_status = if body_healthy {
|
||||
StatusCode::OK
|
||||
} else {
|
||||
StatusCode::SERVICE_UNAVAILABLE
|
||||
};
|
||||
|
||||
assert_eq!(status, expected_status);
|
||||
|
||||
// `healthy` must mean "no critical component is unhealthy", and any
|
||||
// unhealthy component must be bucketed as either critical or degraded.
|
||||
let critical_unhealthy = snapshot["critical_unhealthy"]
|
||||
.as_array()
|
||||
.expect("body exposes critical_unhealthy");
|
||||
assert_eq!(body_healthy, critical_unhealthy.is_empty());
|
||||
|
||||
let unhealthy_count = components
|
||||
.values()
|
||||
.filter(|c| {
|
||||
let s = c["status"].as_str().unwrap_or("");
|
||||
s != "ok" && s != "starting"
|
||||
})
|
||||
.count();
|
||||
let degraded_count = snapshot["degraded_components"]
|
||||
.as_array()
|
||||
.expect("body exposes degraded_components")
|
||||
.len();
|
||||
assert_eq!(
|
||||
unhealthy_count,
|
||||
critical_unhealthy.len() + degraded_count,
|
||||
"every unhealthy component is bucketed as critical or degraded"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -7,6 +7,7 @@ In-process health registry for the OpenHuman core. Tracks per-component liveness
|
||||
- Maintain a process-global registry of named components, each with `status`, `updated_at`, `last_ok`, `last_error`, `restart_count`.
|
||||
- Provide mutators: `mark_component_ok`, `mark_component_error`, `bump_component_restart`.
|
||||
- Produce a point-in-time `HealthSnapshot` (PID, uptime seconds, components map) and its JSON form.
|
||||
- Classify a snapshot into a `HealthVerdict` (`verdict()`): a single degraded *non-critical* component no longer makes the container unhealthy — `/health` returns 503 only when a *critical* component (`CRITICAL_COMPONENTS` = `core`, `memory_tree_db`) is unhealthy; non-critical components (scheduler, channels, update_checker, …) return 200 + a `degraded` flag (#3312).
|
||||
- Drive component state automatically from `DomainEvent`s via an event-bus subscriber.
|
||||
- Expose `health.snapshot` and `health.system_info` RPC/CLI controllers.
|
||||
|
||||
@@ -23,8 +24,10 @@ In-process health registry for the OpenHuman core. Tracks per-component liveness
|
||||
## Public surface
|
||||
|
||||
From `core.rs` (re-exported via `pub use core::*`):
|
||||
- Types: `ComponentHealth`, `HealthSnapshot`.
|
||||
- Functions: `mark_component_ok(component)`, `mark_component_error(component, error)`, `bump_component_restart(component)`, `snapshot() -> HealthSnapshot`, `snapshot_json() -> serde_json::Value`.
|
||||
- Types: `ComponentHealth`, `HealthSnapshot`, `HealthVerdict`.
|
||||
- Functions: `mark_component_ok(component)`, `mark_component_error(component, error)`, `bump_component_restart(component)`, `snapshot() -> HealthSnapshot`, `snapshot_json() -> serde_json::Value`, `verdict(&HealthSnapshot) -> HealthVerdict`, `is_critical_component(name) -> bool`.
|
||||
|
||||
The HTTP `GET /health` handler (`core::jsonrpc::health_handler`) uses `verdict()` for its status code (200 unless a critical component is unhealthy) and adds `healthy` / `degraded` / `critical_unhealthy` / `degraded_components` fields alongside the `components` map in the body. The `components` map shape is unchanged — the new fields are additive.
|
||||
|
||||
From `ops.rs` (re-exported via `pub use ops::*`, also aliased `pub use ops as rpc`):
|
||||
- `health_snapshot() -> RpcOutcome<serde_json::Value>`, `system_info() -> RpcOutcome<SystemInfo>`, and the `SystemInfo` struct.
|
||||
|
||||
@@ -109,6 +109,77 @@ pub fn snapshot_json() -> serde_json::Value {
|
||||
})
|
||||
}
|
||||
|
||||
/// Components whose sustained failure means the whole container should be
|
||||
/// recycled — and the **only** ones whose unhealth makes `/health` return 503.
|
||||
///
|
||||
/// Everything else is a degradable background service whose failure must NOT
|
||||
/// flip the container `unhealthy` (#3312): a single cron-job timeout marked the
|
||||
/// `scheduler` component `error` and 503'd the container for 7h43m even though
|
||||
/// the core RPC was serving fine the whole time. `scheduler`, `channels`, and
|
||||
/// `update_checker` are therefore intentionally **non-critical**.
|
||||
///
|
||||
/// - `core` — the core process / RPC serving capability itself.
|
||||
/// - `memory_tree_db` — the memory database. Its health signal is a *debounced*
|
||||
/// circuit breaker that only trips after several consecutive schema-init
|
||||
/// failures (a genuine, restart-worthy data-layer fault), so unlike the
|
||||
/// scheduler case it does not false-trip on a transient blip.
|
||||
///
|
||||
/// New components default to **non-critical**: add a name here deliberately when
|
||||
/// its failure should recycle the container.
|
||||
const CRITICAL_COMPONENTS: &[&str] = &["core", "memory_tree_db"];
|
||||
|
||||
/// Whether `name` is a critical component (see [`CRITICAL_COMPONENTS`]).
|
||||
pub fn is_critical_component(name: &str) -> bool {
|
||||
CRITICAL_COMPONENTS.contains(&name)
|
||||
}
|
||||
|
||||
/// A component status counts as healthy for liveness purposes when it is `ok`
|
||||
/// or still `starting` (boot grace — a component that hasn't reported yet must
|
||||
/// not 503 the container).
|
||||
fn is_healthy_status(status: &str) -> bool {
|
||||
status == "ok" || status == "starting"
|
||||
}
|
||||
|
||||
/// Liveness/readiness verdict derived from a [`HealthSnapshot`]. Pure function
|
||||
/// of the snapshot so it is unit-testable without the global registry.
|
||||
#[derive(Debug, Clone, Serialize, PartialEq, Eq)]
|
||||
pub struct HealthVerdict {
|
||||
/// True when no *critical* component is unhealthy → `/health` returns 200.
|
||||
pub healthy: bool,
|
||||
/// True when at least one *non-critical* component is unhealthy. The
|
||||
/// container stays live (200) but is degraded — surfaced for readiness /
|
||||
/// observability, not for the liveness 503.
|
||||
pub degraded: bool,
|
||||
/// Names of unhealthy critical components — these drive the 503.
|
||||
pub critical_unhealthy: Vec<String>,
|
||||
/// Names of unhealthy non-critical components (informational).
|
||||
pub degraded_components: Vec<String>,
|
||||
}
|
||||
|
||||
/// Classify a snapshot into a [`HealthVerdict`]: a single degraded background
|
||||
/// component no longer makes the whole container unhealthy — only an unhealthy
|
||||
/// *critical* component does (#3312).
|
||||
pub fn verdict(snapshot: &HealthSnapshot) -> HealthVerdict {
|
||||
let mut critical_unhealthy = Vec::new();
|
||||
let mut degraded_components = Vec::new();
|
||||
for (name, component) in &snapshot.components {
|
||||
if is_healthy_status(&component.status) {
|
||||
continue;
|
||||
}
|
||||
if is_critical_component(name) {
|
||||
critical_unhealthy.push(name.clone());
|
||||
} else {
|
||||
degraded_components.push(name.clone());
|
||||
}
|
||||
}
|
||||
HealthVerdict {
|
||||
healthy: critical_unhealthy.is_empty(),
|
||||
degraded: !degraded_components.is_empty(),
|
||||
critical_unhealthy,
|
||||
degraded_components,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -188,4 +259,91 @@ mod tests {
|
||||
assert!(component_json["last_ok"].as_str().is_some());
|
||||
assert!(json["uptime_seconds"].as_u64().is_some());
|
||||
}
|
||||
|
||||
// ── Critical-component verdict (#3312) ────────────────────────────────
|
||||
|
||||
fn component(status: &str) -> ComponentHealth {
|
||||
ComponentHealth {
|
||||
status: status.to_string(),
|
||||
updated_at: "2026-06-10T00:00:00Z".to_string(),
|
||||
last_ok: None,
|
||||
last_error: None,
|
||||
restart_count: 0,
|
||||
}
|
||||
}
|
||||
|
||||
/// Build a synthetic snapshot from `(name, status)` pairs — lets the
|
||||
/// verdict be tested without mutating the process-global registry.
|
||||
fn snapshot_of(components: &[(&str, &str)]) -> HealthSnapshot {
|
||||
HealthSnapshot {
|
||||
pid: 1,
|
||||
updated_at: "2026-06-10T00:00:00Z".to_string(),
|
||||
uptime_seconds: 0,
|
||||
components: components
|
||||
.iter()
|
||||
.map(|(n, s)| ((*n).to_string(), component(s)))
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn critical_set_membership() {
|
||||
assert!(is_critical_component("core"));
|
||||
assert!(is_critical_component("memory_tree_db"));
|
||||
assert!(!is_critical_component("scheduler"));
|
||||
assert!(!is_critical_component("channels"));
|
||||
assert!(!is_critical_component("update_checker"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn all_ok_is_healthy_and_not_degraded() {
|
||||
let v = verdict(&snapshot_of(&[("core", "ok"), ("scheduler", "ok")]));
|
||||
assert!(v.healthy);
|
||||
assert!(!v.degraded);
|
||||
assert!(v.critical_unhealthy.is_empty());
|
||||
assert!(v.degraded_components.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn noncritical_failure_stays_healthy_but_degraded() {
|
||||
// The exact #3312 case: scheduler in error must NOT 503 the container.
|
||||
let v = verdict(&snapshot_of(&[("core", "ok"), ("scheduler", "error")]));
|
||||
assert!(v.healthy, "a degraded background service must not 503");
|
||||
assert!(v.degraded);
|
||||
assert_eq!(v.degraded_components, vec!["scheduler".to_string()]);
|
||||
assert!(v.critical_unhealthy.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn critical_failure_is_unhealthy() {
|
||||
let v = verdict(&snapshot_of(&[("memory_tree_db", "error")]));
|
||||
assert!(
|
||||
!v.healthy,
|
||||
"a critical component failure 503s the container"
|
||||
);
|
||||
assert_eq!(v.critical_unhealthy, vec!["memory_tree_db".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mixed_failures_report_both_buckets_and_503() {
|
||||
let v = verdict(&snapshot_of(&[
|
||||
("core", "error"),
|
||||
("scheduler", "error"),
|
||||
("channels", "ok"),
|
||||
]));
|
||||
assert!(!v.healthy);
|
||||
assert_eq!(v.critical_unhealthy, vec!["core".to_string()]);
|
||||
assert_eq!(v.degraded_components, vec!["scheduler".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn starting_status_is_treated_as_healthy() {
|
||||
// Boot grace: a not-yet-reported component must not 503 nor degrade.
|
||||
let v = verdict(&snapshot_of(&[
|
||||
("core", "starting"),
|
||||
("scheduler", "starting"),
|
||||
]));
|
||||
assert!(v.healthy);
|
||||
assert!(!v.degraded);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user