diff --git a/internal/alerts/active_cleanup.go b/internal/alerts/active_cleanup.go index c153fff87..b86e7b4b5 100644 --- a/internal/alerts/active_cleanup.go +++ b/internal/alerts/active_cleanup.go @@ -230,6 +230,13 @@ func shouldPreserveAlertOutsideNodeCleanup(alertID string, alert *Alert) bool { if alert == nil { return true } + // System alerts have no node, so the empty-node removal below would + // silently delete them every sweep. The delivery-health alert then + // re-raised as new on its next evaluation, notifying again each cycle + // with no recovery in between (#1721). + if IsSystemAlert(alert) { + return true + } if strings.HasPrefix(alertID, "docker-") || strings.HasPrefix(alert.ResourceID, "docker:") { return true } diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index 1f4c24f2c..d270935b1 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -20388,6 +20388,80 @@ func TestRaiseSystemAlertIsIdempotentForAnUnchangedCondition(t *testing.T) { } } +func TestRaiseSystemAlertFingerprintKeepsCountDriftSilent(t *testing.T) { + m := newTestManager(t) + + if raised := m.RaiseSystemAlert(SystemAlertInput{ + Type: NotificationDeliveryAlertType, + Level: AlertLevelWarning, + Message: "11 dead-lettered deliveries gave up.", + Fingerprint: "degraded|retained_dead_letter_deliveries", + }); !raised { + t.Fatal("expected the first raise to report a new system alert") + } + + // A moving counter in the message is not new information while the + // fingerprint holds: the standing alert must refresh silently. + if raised := m.RaiseSystemAlert(SystemAlertInput{ + Type: NotificationDeliveryAlertType, + Level: AlertLevelWarning, + Message: "14 dead-lettered deliveries gave up.", + Fingerprint: "degraded|retained_dead_letter_deliveries", + }); raised { + t.Error("expected a count-only drift under an unchanged fingerprint to stay silent") + } + + alertID := SystemAlertID(NotificationDeliveryAlertType) + var standing *Alert + for _, alert := range m.GetActiveAlerts() { + if alert.ID == alertID { + clone := alert + standing = &clone + } + } + if standing == nil { + t.Fatal("expected the system alert to be standing") + } + if standing.Message != "14 dead-lettered deliveries gave up." { + t.Errorf("Message = %q, want the refreshed counter text", standing.Message) + } + + // A fingerprint change is new information and must notify again. + if raised := m.RaiseSystemAlert(SystemAlertInput{ + Type: NotificationDeliveryAlertType, + Level: AlertLevelWarning, + Message: "Pulse cannot read the notification queue.", + Fingerprint: "unavailable|queue_stats_unavailable", + }); !raised { + t.Error("expected a fingerprint change to report a change") + } +} + +func TestCleanupAlertsForNodesPreservesSystemAlerts(t *testing.T) { + m := newTestManager(t) + + m.RaiseSystemAlert(SystemAlertInput{ + Type: NotificationDeliveryAlertType, + Level: AlertLevelWarning, + Message: "Notifications are not being delivered.", + }) + + // A system alert has no node, so the empty-node sweep used to delete it + // every cycle and the next evaluation re-raised and re-notified (#1721). + m.CleanupAlertsForNodes(map[string]bool{"pve1": true}) + + alertID := SystemAlertID(NotificationDeliveryAlertType) + found := false + for _, alert := range m.GetActiveAlerts() { + if alert.ID == alertID { + found = true + } + } + if !found { + t.Fatal("expected the system alert to survive node cleanup") + } +} + func TestRaiseSystemAlertReportsAChangedCondition(t *testing.T) { m := newTestManager(t) diff --git a/internal/alerts/system_alert.go b/internal/alerts/system_alert.go index 451cf936b..79d7654d7 100644 --- a/internal/alerts/system_alert.go +++ b/internal/alerts/system_alert.go @@ -32,12 +32,23 @@ const ( // identifies the condition; the same Type always maps to the same alert, so // repeated raises update one alert rather than accumulating. type SystemAlertInput struct { - Type string - Level AlertLevel - Message string - Metadata map[string]interface{} + Type string + Level AlertLevel + Message string + // Fingerprint, when set, identifies the notify-worthy state of the + // condition. A re-raise whose level and fingerprint both match the + // standing alert refreshes the message and metadata silently, so a + // message carrying a moving counter does not notify on every tick. + // Callers that leave it empty keep the message itself as the change + // signal. + Fingerprint string + Metadata map[string]interface{} } +// systemAlertFingerprintKey stores the raise fingerprint on the alert metadata +// so the next raise can compare against it. +const systemAlertFingerprintKey = "systemAlertFingerprint" + // SystemAlertID returns the stable alert ID for a system alert type. func SystemAlertID(alertType string) string { alertType = strings.TrimSpace(alertType) @@ -78,9 +89,21 @@ func (m *Manager) RaiseSystemAlert(input SystemAlertInput) bool { now := time.Now() existing, exists := m.getActiveAlertNoLock(alertID) if exists && existing != nil { - unchanged := existing.Level == level && existing.Message == input.Message + unchanged := existing.Level == level + if unchanged { + existingFingerprint, _ := existing.Metadata[systemAlertFingerprintKey].(string) + if input.Fingerprint != "" || existingFingerprint != "" { + unchanged = existingFingerprint == input.Fingerprint + } else { + unchanged = existing.Message == input.Message + } + } existing.LastSeen = now if unchanged { + // Same condition state: keep the presentation current (a message + // may carry counters) without treating it as new information. + existing.Message = input.Message + existing.Metadata = systemAlertMetadata(alertType, input.Fingerprint, input.Metadata) m.setActiveAlertNoLock(alertID, existing) m.mu.Unlock() m.saveActiveAlertsAsync("system-alert-refresh") @@ -89,7 +112,7 @@ func (m *Manager) RaiseSystemAlert(input SystemAlertInput) bool { existing.Level = level existing.Message = input.Message - existing.Metadata = systemAlertMetadata(alertType, input.Metadata) + existing.Metadata = systemAlertMetadata(alertType, input.Fingerprint, input.Metadata) m.setActiveAlertNoLock(alertID, existing) // dispatchAlert reads flapping and schedule state through the primary // lock, so it must run before the unlock rather than after. @@ -116,7 +139,7 @@ func (m *Manager) RaiseSystemAlert(input SystemAlertInput) bool { Message: input.Message, StartTime: now, LastSeen: now, - Metadata: systemAlertMetadata(alertType, input.Metadata), + Metadata: systemAlertMetadata(alertType, input.Fingerprint, input.Metadata), } m.setActiveAlertNoLock(alertID, alert) @@ -154,12 +177,15 @@ func (m *Manager) ClearSystemAlert(alertType string) bool { // systemAlertMetadata stamps the marker that surfaces use to tell a // system-scoped alert apart from a resource alert, without letting a caller // overwrite it. -func systemAlertMetadata(alertType string, extra map[string]interface{}) map[string]interface{} { - metadata := make(map[string]interface{}, len(extra)+2) +func systemAlertMetadata(alertType, fingerprint string, extra map[string]interface{}) map[string]interface{} { + metadata := make(map[string]interface{}, len(extra)+3) for key, value := range extra { metadata[key] = value } metadata["systemAlert"] = true metadata["systemAlertType"] = alertType + if fingerprint != "" { + metadata[systemAlertFingerprintKey] = fingerprint + } return metadata } diff --git a/internal/monitoring/system_alerts.go b/internal/monitoring/system_alerts.go index d2b079c87..54e61f3b2 100644 --- a/internal/monitoring/system_alerts.go +++ b/internal/monitoring/system_alerts.go @@ -2,6 +2,7 @@ package monitoring import ( "fmt" + "strings" "time" "github.com/rcourtman/pulse-go-rewrite/internal/alerts" @@ -54,6 +55,11 @@ func (m *Monitor) evaluateNotificationDelivery(now time.Time) { Type: alerts.NotificationDeliveryAlertType, Level: alerts.AlertLevelWarning, Message: notificationDeliveryAlertMessage(health), + // The message embeds delivery counts, which move as retained failures + // accumulate or expire. Fingerprinting on status and reason codes keeps + // the standing alert's text current without re-notifying on every + // count tick (#1721). + Fingerprint: deliveryHealthFingerprint(health), Metadata: map[string]interface{}{ "deliveryStatus": string(health.Status), "failedDeliveries": health.Failed, @@ -64,6 +70,13 @@ func (m *Monitor) evaluateNotificationDelivery(now time.Time) { }) } +// deliveryHealthFingerprint identifies the notify-worthy state of the delivery +// verdict: which coarse status holds and which failure classes are present. +// Counts deliberately stay out so their drift does not page. +func deliveryHealthFingerprint(health notifications.DeliveryHealth) string { + return string(health.Status) + "|" + strings.Join(health.ReasonCodes, ",") +} + // notificationDeliveryAlertMessage says what failed and where to fix it. The // operator reading this has, by definition, not received a notification about // it, so the message has to stand on its own. diff --git a/internal/monitoring/system_alerts_test.go b/internal/monitoring/system_alerts_test.go index b6266da32..435fd94c8 100644 --- a/internal/monitoring/system_alerts_test.go +++ b/internal/monitoring/system_alerts_test.go @@ -75,6 +75,31 @@ func TestNotificationDeliveryAlertMessageSingularises(t *testing.T) { } } +func TestDeliveryHealthFingerprintIgnoresCountDrift(t *testing.T) { + base := notifications.ClassifyQueueHealth(map[string]int{ + string(notifications.QueueStatusDLQ): 11, + }) + drifted := notifications.ClassifyQueueHealth(map[string]int{ + string(notifications.QueueStatusDLQ): 14, + }) + if deliveryHealthFingerprint(base) != deliveryHealthFingerprint(drifted) { + t.Error("expected count drift within one failure class to keep the fingerprint stable") + } + + withFailed := notifications.ClassifyQueueHealth(map[string]int{ + string(notifications.QueueStatusDLQ): 14, + string(notifications.QueueStatusFailed): 1, + }) + if deliveryHealthFingerprint(base) == deliveryHealthFingerprint(withFailed) { + t.Error("expected a new failure class to change the fingerprint") + } + + unavailable := notifications.UnavailableDeliveryHealth() + if deliveryHealthFingerprint(base) == deliveryHealthFingerprint(unavailable) { + t.Error("expected an unavailable queue to change the fingerprint") + } +} + func TestEvaluateNotificationDeliveryThrottles(t *testing.T) { // The poll ticker runs on the polling cadence, which can be seconds, and // reading queue health costs a SQLite query. A nil notification manager