diff --git a/core/archipelago/src/monitoring/mod.rs b/core/archipelago/src/monitoring/mod.rs index d02c8dbb..44ddb064 100644 --- a/core/archipelago/src/monitoring/mod.rs +++ b/core/archipelago/src/monitoring/mod.rs @@ -40,10 +40,12 @@ pub fn spawn_metrics_collector( store.push(snapshot).await; debug!("Metrics snapshot collected"); + // Reconcile even when all thresholds have recovered, so + // the previous disk/RAM/CPU warning disappears promptly. + if let Some(ref state_mgr) = state { + notifications::push_alert_notifications(state_mgr, &alerts).await; + } if !alerts.is_empty() { - if let Some(ref state_mgr) = state { - notifications::push_alert_notifications(state_mgr, &alerts).await; - } if let Some(ref dir) = data_dir { notifications::deliver_alert_webhooks(dir, &alerts).await; } diff --git a/core/archipelago/src/monitoring/notifications.rs b/core/archipelago/src/monitoring/notifications.rs index 6fdf478b..003b64b0 100644 --- a/core/archipelago/src/monitoring/notifications.rs +++ b/core/archipelago/src/monitoring/notifications.rs @@ -14,7 +14,11 @@ pub(crate) async fn push_alert_notifications( alerts: &[FiredAlert], ) { let (mut data, _rev) = state_mgr.get_snapshot().await; + let previous_count = data.notifications.len(); prune_stale_alert_notifications(&mut data.notifications, alerts); + if alerts.is_empty() && data.notifications.len() == previous_count { + return; + } for alert in alerts { let level = match alert.kind { AlertRuleKind::DiskUsage | AlertRuleKind::RamUsage => { @@ -42,7 +46,17 @@ pub(crate) async fn push_alert_notifications( data.notifications.remove(0); } state_mgr.update_data(data).await; - info!("Fired {} alert(s)", alerts.len()); + if !alerts.is_empty() { + info!("Fired {} alert(s)", alerts.len()); + } +} + +fn is_metrics_alert_id(id: &str) -> bool { + id.split_once('-').is_some_and(|(kind, timestamp)| { + matches!(kind, "disk" | "ram" | "cpu" | "latency") + && !timestamp.is_empty() + && timestamp.bytes().all(|byte| byte.is_ascii_digit()) + }) } fn prune_stale_alert_notifications( @@ -55,6 +69,11 @@ fn prune_stale_alert_notifications( if active_ids.contains(notification.id.as_str()) { return false; } + // A fresh timestamp is not evidence that an earlier threshold is + // still exceeded. Remove resolved monitoring alerts immediately. + if is_metrics_alert_id(¬ification.id) { + return false; + } if notification.app_id.is_some() || notification.id.starts_with("health-") { return true; } @@ -127,6 +146,8 @@ mod tests { notification("alert-active", fresh_timestamp.clone(), None), notification("alert-old", old_timestamp, None), notification("alert-fresh", fresh_timestamp.clone(), None), + notification("disk-123", fresh_timestamp.clone(), None), + notification("ram-123", fresh_timestamp.clone(), None), notification("health-indeedhub-1", fresh_timestamp, Some("indeedhub")), ]; @@ -135,4 +156,22 @@ mod tests { let ids: Vec<&str> = notifications.iter().map(|n| n.id.as_str()).collect(); assert_eq!(ids, vec!["alert-fresh", "health-indeedhub-1"]); } + + #[test] + fn clears_resolved_metrics_alerts_without_touching_other_notifications() { + let fresh = Utc::now().to_rfc3339(); + let mut notifications = vec![ + notification("disk-123", fresh.clone(), None), + notification("latency-456", fresh.clone(), None), + notification("disk-not-a-timestamp", fresh.clone(), None), + notification("health-indeedhub-1", fresh.clone(), Some("indeedhub")), + notification("app-notice", fresh, Some("indeedhub")), + ]; + prune_stale_alert_notifications(&mut notifications, &[]); + let ids: Vec<&str> = notifications.iter().map(|n| n.id.as_str()).collect(); + assert_eq!( + ids, + vec!["disk-not-a-timestamp", "health-indeedhub-1", "app-notice"] + ); + } }