Keep the delivery-health system alert standing and quiet across count drift

The Proxmox node sweep removes any non-preserved alert whose Node is
empty, and system alerts have no node, so every sweep silently deleted
the notification-delivery alert. Its five-minute evaluation then
re-raised it as a brand-new alert, firing a fresh notification each
cycle with no recovery in between, which reads as an alert appearing,
vanishing without a recovery, and paging again minutes later. System
alerts are now preserved outside node cleanup.

Raises also carried the delivery counts inside the message, and a
message change re-notifies, so each newly retained failure re-paged
even while the condition was unchanged. System alerts now take an
optional fingerprint: while level and fingerprint hold, a re-raise
refreshes the message and metadata silently. The delivery alert
fingerprints on status and failure classes, keeping its counter text
current without paging on drift.

Refs #1721

Contract-Neutral: behavioral fix: delivery-health system alert survived node sweep and stops re-paging on count drift, no public contract delta (Refs #1721)
This commit is contained in:
rcourtman
2026-08-19 16:54:39 +01:00
parent 5bbee0da3d
commit 6f9a01a72a
5 changed files with 154 additions and 9 deletions
+7
View File
@@ -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
}
+74
View File
@@ -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)
+35 -9
View File
@@ -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
}
+13
View File
@@ -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.
+25
View File
@@ -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