From eb7b399a550db1b6cd11116060dd34ca66aaa0ae Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sat, 7 Feb 2026 14:18:39 +0000 Subject: [PATCH] fix(alerts): reduce swarm alert noise and preserve notification state (#1096) (cherry picked from commit 6909264a0262b13ebb556a93211994551275eb2f) --- internal/alerts/alerts.go | 46 +++++++++- internal/alerts/alerts_test.go | 157 +++++++++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 3 deletions(-) diff --git a/internal/alerts/alerts.go b/internal/alerts/alerts.go index 1d925712b..5ec38d4b5 100644 --- a/internal/alerts/alerts.go +++ b/internal/alerts/alerts.go @@ -4119,13 +4119,46 @@ func (m *Manager) evaluateDockerService(host models.DockerHost, service models.D m.mu.Lock() if existing, exists := m.activeAlerts[alertID]; exists && existing != nil { - alert.StartTime = existing.StartTime + escalatedToCritical := existing.Level != AlertLevelCritical && alert.Level == AlertLevelCritical + m.preserveAlertState(alertID, alert) + m.activeAlerts[alertID] = alert + m.recentAlerts[alertID] = alert + + if escalatedToCritical { + m.historyManager.AddAlert(*alert) + if m.checkRateLimit(alertID) { + m.dispatchAlert(alert, true) + log.Warn(). + Str("service", serviceName). + Str("host", host.DisplayName). + Float64("percentMissing", percentMissing). + Str("fromLevel", string(existing.Level)). + Str("toLevel", string(alert.Level)). + Msg("Docker service alert escalated") + } else { + log.Debug(). + Str("alertID", alertID). + Int("maxPerHour", m.config.Schedule.MaxAlertsHour). + Msg("Docker service escalation notification suppressed due to rate limit") + } + } + m.mu.Unlock() + return } + m.preserveAlertState(alertID, alert) m.activeAlerts[alertID] = alert m.recentAlerts[alertID] = alert m.historyManager.AddAlert(*alert) - m.dispatchAlert(alert, severity == AlertLevelCritical) + if !m.checkRateLimit(alertID) { + m.mu.Unlock() + log.Debug(). + Str("alertID", alertID). + Int("maxPerHour", m.config.Schedule.MaxAlertsHour). + Msg("Docker service alert notification suppressed due to rate limit") + return + } + m.dispatchAlert(alert, true) m.mu.Unlock() log.Warn(). @@ -6385,7 +6418,8 @@ func (m *Manager) checkMetric(resourceID, resourceName, node, instance, resource if shouldRenotify && m.onAlert != nil { now := time.Now() existingAlert.LastNotified = &now - if m.dispatchAlert(existingAlert, false) { + // Dispatch asynchronously so callback I/O cannot block alert evaluation. + if m.dispatchAlert(existingAlert, true) { log.Info(). Str("alertID", alertID). Str("level", string(existingAlert.Level)). @@ -6640,6 +6674,12 @@ func (m *Manager) preserveAlertState(alertID string, updated *Alert) { if exists && existing != nil { // Preserve the original start time so duration calculations are correct updated.StartTime = existing.StartTime + if existing.LastNotified != nil { + t := *existing.LastNotified + updated.LastNotified = &t + } else { + updated.LastNotified = nil + } updated.Acknowledged = existing.Acknowledged updated.AckUser = existing.AckUser if existing.AckTime != nil { diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index d87f673b2..c2bca4048 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -1623,6 +1623,163 @@ func TestDockerServiceReplicaAlerts(t *testing.T) { } } +func TestDockerServiceAlertDoesNotRenotifyWhenUnchanged(t *testing.T) { + m := newTestManager(t) + m.ClearActiveAlerts() + + cfg := m.GetConfig() + cfg.Enabled = true + cfg.ActivationState = ActivationActive + cfg.Schedule.MaxAlertsHour = 100 + m.UpdateConfig(cfg) + + dispatched := make(chan string, 4) + m.SetAlertCallback(func(alert *Alert) { + dispatched <- alert.ID + }) + + host := models.DockerHost{ + ID: "host-1", + DisplayName: "Prod Swarm", + Hostname: "swarm-prod", + Services: []models.DockerService{ + { + ID: "svc-1", + Name: "web", + DesiredTasks: 4, + RunningTasks: 2, + Mode: "replicated", + }, + }, + } + + m.CheckDockerHost(host) + + select { + case <-dispatched: + case <-time.After(1 * time.Second): + t.Fatal("expected initial docker service alert notification") + } + + // Same degraded state should update LastSeen/value but not re-notify every poll. + m.CheckDockerHost(host) + + select { + case id := <-dispatched: + t.Fatalf("expected no second notification for unchanged service alert, got %s", id) + case <-time.After(250 * time.Millisecond): + } +} + +func TestDockerServiceAlertPreservesLastNotifiedWhenUnchanged(t *testing.T) { + m := newTestManager(t) + m.ClearActiveAlerts() + + cfg := m.GetConfig() + cfg.Enabled = true + cfg.ActivationState = ActivationActive + cfg.Schedule.MaxAlertsHour = 100 + m.UpdateConfig(cfg) + + host := models.DockerHost{ + ID: "host-1", + DisplayName: "Prod Swarm", + Hostname: "swarm-prod", + Services: []models.DockerService{ + { + ID: "svc-1", + Name: "web", + DesiredTasks: 4, + RunningTasks: 2, + Mode: "replicated", + }, + }, + } + + m.CheckDockerHost(host) + + resourceID := dockerServiceResourceID(host.ID, "svc-1", "web") + alertID := fmt.Sprintf("docker-service-health-%s", resourceID) + alert, exists := m.activeAlerts[alertID] + if !exists { + t.Fatalf("expected service alert %s to be raised", alertID) + } + + notifiedAt := time.Now().Add(-2 * time.Minute).UTC() + alert.LastNotified = ¬ifiedAt + + // Same degraded state should keep LastNotified while refreshing state. + m.CheckDockerHost(host) + + updated, exists := m.activeAlerts[alertID] + if !exists { + t.Fatalf("expected service alert %s to remain active", alertID) + } + if updated.LastNotified == nil { + t.Fatal("expected LastNotified to be preserved, got nil") + } + if !updated.LastNotified.Equal(notifiedAt) { + t.Fatalf("expected LastNotified %s, got %s", notifiedAt, updated.LastNotified) + } +} + +func TestDockerServiceAlertRenotifiesOnEscalationToCritical(t *testing.T) { + m := newTestManager(t) + m.ClearActiveAlerts() + + cfg := m.GetConfig() + cfg.Enabled = true + cfg.ActivationState = ActivationActive + cfg.Schedule.MaxAlertsHour = 100 + cfg.DockerDefaults.ServiceWarnGapPct = 10 + cfg.DockerDefaults.ServiceCritGapPct = 50 + m.UpdateConfig(cfg) + + dispatched := make(chan AlertLevel, 4) + m.SetAlertCallback(func(alert *Alert) { + dispatched <- alert.Level + }) + + host := models.DockerHost{ + ID: "host-1", + DisplayName: "Prod Swarm", + Hostname: "swarm-prod", + Services: []models.DockerService{ + { + ID: "svc-1", + Name: "web", + DesiredTasks: 4, + RunningTasks: 3, // 25% missing -> warning + Mode: "replicated", + }, + }, + } + + m.CheckDockerHost(host) + + select { + case level := <-dispatched: + if level != AlertLevelWarning { + t.Fatalf("expected warning notification first, got %s", level) + } + case <-time.After(1 * time.Second): + t.Fatal("expected initial warning notification") + } + + // Escalate from warning to critical: should notify again. + host.Services[0].RunningTasks = 1 // 75% missing -> critical + m.CheckDockerHost(host) + + select { + case level := <-dispatched: + if level != AlertLevelCritical { + t.Fatalf("expected critical escalation notification, got %s", level) + } + case <-time.After(1 * time.Second): + t.Fatal("expected escalation notification") + } +} + func TestDockerServiceUpdateStateAlert(t *testing.T) { m := newTestManager(t) cfg := m.GetConfig()