fix(alerts): reduce swarm alert noise and preserve notification state (#1096)

(cherry picked from commit 6909264a02)

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 {

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 = &notifiedAt
+
+	// 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()