mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Fix alert re-notification rate limiting
Apply max alerts/hour to cooldown and critical re-notification paths so active alerts cannot bypass the UI delivery cap. Persist LastNotified on active alert records when dispatching, including clone dispatches after reload. Refs #1444
This commit is contained in:
@@ -189,6 +189,13 @@ forbidden.
|
||||
Per-platform defaults, per-resource overrides, global disables, active-alert
|
||||
reevaluation, history persistence, and notification delivery must use the same
|
||||
alert configuration shape rather than a platform-specific sidecar.
|
||||
Notification cadence is part of that runtime contract. `Schedule.Cooldown` and
|
||||
`Schedule.MaxAlertsHour` apply to already-active alert re-notifications as well
|
||||
as first-fire creation, including canonical metric alerts, legacy metric paths,
|
||||
and severity-change re-notifications. Accepted alert dispatch must record
|
||||
`LastNotified` back onto the live active-alert state before persistence, even
|
||||
when a restored or replayed alert is dispatched through a clone, so reloads do
|
||||
not reopen the same alert's notification window.
|
||||
The browser thresholds surface is also platform-shaped: Proxmox, Docker,
|
||||
Kubernetes, TrueNAS, vSphere, PBS, PMG, and Systems. It must use the shared
|
||||
FilterBar chip and "+ Filter" pattern for resource filtering, and alert tables
|
||||
|
||||
@@ -17281,6 +17281,47 @@ func TestDispatchAlert(t *testing.T) {
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("records last notified on active alert when dispatched clone succeeds", func(t *testing.T) {
|
||||
m := newTestManager(t)
|
||||
|
||||
m.SetAlertCallback(func(a *Alert) {})
|
||||
|
||||
m.mu.Lock()
|
||||
m.config.ActivationState = ActivationActive
|
||||
active := &Alert{
|
||||
ID: "test-alert",
|
||||
Type: "cpu",
|
||||
ResourceName: "testvm",
|
||||
}
|
||||
m.setActiveAlertNoLock(active.ID, active)
|
||||
m.mu.Unlock()
|
||||
|
||||
dispatched := active.Clone()
|
||||
if dispatched == nil {
|
||||
t.Fatal("expected alert clone")
|
||||
}
|
||||
|
||||
if !m.dispatchAlert(dispatched, false) {
|
||||
t.Fatal("expected dispatch to succeed")
|
||||
}
|
||||
if dispatched.LastNotified == nil {
|
||||
t.Fatal("expected dispatched clone to record LastNotified")
|
||||
}
|
||||
|
||||
m.mu.RLock()
|
||||
stored, exists := m.getActiveAlertNoLock(active.ID)
|
||||
m.mu.RUnlock()
|
||||
if !exists || stored == nil {
|
||||
t.Fatalf("expected active alert %s to remain stored", active.ID)
|
||||
}
|
||||
if stored.LastNotified == nil {
|
||||
t.Fatal("expected active alert to record LastNotified")
|
||||
}
|
||||
if !stored.LastNotified.Equal(*dispatched.LastNotified) {
|
||||
t.Fatalf("active LastNotified = %v, want %v", stored.LastNotified, dispatched.LastNotified)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("dispatches asynchronously when async is true", func(t *testing.T) {
|
||||
// t.Parallel()
|
||||
m := newTestManager(t)
|
||||
|
||||
@@ -266,9 +266,9 @@ func (m *Manager) evaluateCanonicalMetricAlert(spec alertspecs.ResourceAlertSpec
|
||||
shouldRenotify := false
|
||||
if existingAlert.Acknowledged {
|
||||
} else if m.shouldNotifyAfterCooldown(existingAlert) {
|
||||
shouldRenotify = true
|
||||
shouldRenotify = m.allowNotificationByRateLimit(trackingKey, existingAlert, "cooldown")
|
||||
} else if oldLevel != existingAlert.Level && existingAlert.Level == AlertLevelCritical {
|
||||
shouldRenotify = true
|
||||
shouldRenotify = m.allowNotificationByRateLimit(trackingKey, existingAlert, "critical-escalation")
|
||||
}
|
||||
|
||||
if shouldRenotify && m.getAlertCallback() != nil {
|
||||
|
||||
@@ -445,17 +445,21 @@ func (m *Manager) checkMetric(resourceID, resourceName, node, instance, resource
|
||||
Str("alertID", alertID).
|
||||
Msg("Alert is acknowledged, skipping re-notification")
|
||||
} else if m.shouldNotifyAfterCooldown(existingAlert) {
|
||||
shouldRenotify = true
|
||||
log.Debug().
|
||||
Str("alertID", alertID).
|
||||
Dur("cooldown", time.Duration(m.config.Schedule.Cooldown)*time.Minute).
|
||||
Msg("Cooldown period has passed, will re-notify")
|
||||
shouldRenotify = m.allowNotificationByRateLimit(trackingKey, existingAlert, "cooldown")
|
||||
if shouldRenotify {
|
||||
log.Debug().
|
||||
Str("alertID", alertID).
|
||||
Dur("cooldown", time.Duration(m.config.Schedule.Cooldown)*time.Minute).
|
||||
Msg("Cooldown period has passed, will re-notify")
|
||||
}
|
||||
} else if oldLevel != existingAlert.Level && existingAlert.Level == AlertLevelCritical {
|
||||
// Always re-notify if alert escalated to critical
|
||||
shouldRenotify = true
|
||||
log.Debug().
|
||||
Str("alertID", alertID).
|
||||
Msg("Alert escalated to critical, will re-notify despite cooldown")
|
||||
shouldRenotify = m.allowNotificationByRateLimit(trackingKey, existingAlert, "critical-escalation")
|
||||
if shouldRenotify {
|
||||
log.Debug().
|
||||
Str("alertID", alertID).
|
||||
Msg("Alert escalated to critical, will re-notify despite cooldown")
|
||||
}
|
||||
}
|
||||
|
||||
// Send re-notification if appropriate (may be suppressed by quiet hours)
|
||||
|
||||
@@ -174,7 +174,8 @@ func (m *Manager) dispatchAlert(alert *Alert, async bool) bool {
|
||||
}
|
||||
|
||||
notifiedAt := time.Now()
|
||||
alert.LastNotified = ¬ifiedAt
|
||||
m.recordAlertNotifiedNoLock(alert, notifiedAt)
|
||||
m.saveActiveAlertsAsync("alert-dispatch")
|
||||
|
||||
alertCopy := cloneAlertForOutput(alert)
|
||||
if async {
|
||||
@@ -212,6 +213,34 @@ func (m *Manager) dispatchAlert(alert *Alert, async bool) bool {
|
||||
return true
|
||||
}
|
||||
|
||||
func (m *Manager) recordAlertNotifiedNoLock(alert *Alert, notifiedAt time.Time) {
|
||||
if alert == nil {
|
||||
return
|
||||
}
|
||||
|
||||
setAlertLastNotified(alert, notifiedAt)
|
||||
|
||||
if trackingKey := canonicalTrackingKeyForAlert(alert); trackingKey != "" {
|
||||
if active, exists := m.getActiveAlertNoLock(trackingKey); exists && active != nil {
|
||||
setAlertLastNotified(active, notifiedAt)
|
||||
return
|
||||
}
|
||||
}
|
||||
if alert.ID != "" {
|
||||
if active, exists := m.getActiveAlertNoLock(alert.ID); exists && active != nil {
|
||||
setAlertLastNotified(active, notifiedAt)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func setAlertLastNotified(alert *Alert, notifiedAt time.Time) {
|
||||
if alert == nil {
|
||||
return
|
||||
}
|
||||
t := notifiedAt
|
||||
alert.LastNotified = &t
|
||||
}
|
||||
|
||||
func isMonitorOnlyAlert(alert *Alert) bool {
|
||||
if alert == nil || alert.Metadata == nil {
|
||||
return false
|
||||
@@ -539,6 +568,33 @@ func (m *Manager) shouldNotifyAfterCooldown(alert *Alert) bool {
|
||||
return timeSinceLastNotification >= cooldownDuration
|
||||
}
|
||||
|
||||
func (m *Manager) allowNotificationByRateLimit(trackingKey string, alert *Alert, reason string) bool {
|
||||
if trackingKey == "" && alert != nil {
|
||||
trackingKey = canonicalTrackingKeyForAlert(alert)
|
||||
}
|
||||
if trackingKey == "" && alert != nil {
|
||||
trackingKey = alert.ID
|
||||
}
|
||||
if m.checkRateLimit(trackingKey) {
|
||||
return true
|
||||
}
|
||||
|
||||
log.Debug().
|
||||
Str("alertID", alertIDForLog(alert)).
|
||||
Str("trackingKey", trackingKey).
|
||||
Str("reason", reason).
|
||||
Int("maxPerHour", m.config.Schedule.MaxAlertsHour).
|
||||
Msg("Alert notification suppressed due to rate limit")
|
||||
return false
|
||||
}
|
||||
|
||||
func alertIDForLog(alert *Alert) string {
|
||||
if alert == nil {
|
||||
return ""
|
||||
}
|
||||
return alert.ID
|
||||
}
|
||||
|
||||
// checkRateLimit checks if an alert has exceeded rate limit
|
||||
func (m *Manager) checkRateLimit(alertID string) bool {
|
||||
if m.config.Schedule.MaxAlertsHour <= 0 {
|
||||
|
||||
@@ -126,6 +126,63 @@ func assertAlertMissing(t *testing.T, m *Manager, alertID string) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestUnifiedGuestDiskRenotificationRespectsMaxAlertsHour(t *testing.T) {
|
||||
m := newTestManager(t)
|
||||
cfg := unifiedEvalBaseConfig()
|
||||
cfg.GuestDefaults = ThresholdConfig{
|
||||
Disk: &HysteresisThreshold{Trigger: 80, Clear: 70},
|
||||
}
|
||||
cfg.Schedule.Cooldown = 30
|
||||
cfg.Schedule.MaxAlertsHour = 1
|
||||
configureUnifiedEvalManager(t, m, cfg)
|
||||
|
||||
dispatched := make(chan string, 4)
|
||||
m.SetAlertCallback(func(alert *Alert) {
|
||||
dispatched <- alert.ID
|
||||
})
|
||||
|
||||
container := models.Container{
|
||||
ID: "ct101",
|
||||
Name: "smr",
|
||||
Node: "ryzen5800x",
|
||||
Status: "running",
|
||||
Disks: []models.Disk{{
|
||||
Mountpoint: "/",
|
||||
Usage: 90.5,
|
||||
Total: 100,
|
||||
Used: 90,
|
||||
Free: 10,
|
||||
}},
|
||||
}
|
||||
|
||||
m.CheckGuest(container, "pve1")
|
||||
|
||||
select {
|
||||
case <-dispatched:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("expected initial guest disk alert notification")
|
||||
}
|
||||
|
||||
alertID := canonicalMetricStateID("ct101-disk-root", "disk")
|
||||
lastNotified := time.Now().Add(-31 * time.Minute)
|
||||
m.mu.Lock()
|
||||
alert, exists := m.getActiveAlertNoLock(alertID)
|
||||
if !exists {
|
||||
m.mu.Unlock()
|
||||
t.Fatalf("expected active guest disk alert %s", alertID)
|
||||
}
|
||||
alert.LastNotified = &lastNotified
|
||||
m.mu.Unlock()
|
||||
|
||||
m.CheckGuest(container, "pve1")
|
||||
|
||||
select {
|
||||
case id := <-dispatched:
|
||||
t.Fatalf("expected max-alerts/hour to suppress repeated disk notification, got %s", id)
|
||||
case <-time.After(250 * time.Millisecond):
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckUnifiedResourceMajorFamilies(t *testing.T) {
|
||||
m := newTestManager(t)
|
||||
configureUnifiedEvalManager(t, m, unifiedEvalBaseConfig())
|
||||
|
||||
Reference in New Issue
Block a user