Comply with grace condition when repeat alert notifications is enabled

graylog2-server/src/main/java/org/graylog2/alerts/AlertScanner.java

@@ -69,11 +69,12 @@ public boolean checkAlertCondition(Stream stream, AlertCondition alertCondition)
                     LOG.debug("Alert condition [{}] is triggered. Sending alerts.", alertCondition);
                     handleTriggeredAlert(result, stream, alertCondition);
                 } else {
+                    final Alert triggeredAlert = alert.get();
                     // There is already an alert for this condition and is unresolved
-                    if (alert.isPresent() && alertCondition.shouldRepeatNotifications()) {
+                    if (alertService.shouldRepeatNotifications(alertCondition, triggeredAlert)) {
                         // Repeat notifications because user wants to do that
                         LOG.debug("Alert condition [{}] is triggered and configured to repeat alert notifications. Sending alerts.", alertCondition);
-                        handleRepeatedAlert(stream, alertCondition, result, alert.get());
+                        handleRepeatedAlert(stream, alertCondition, result, triggeredAlert);
                     } else {
                         LOG.debug("Alert condition [{}] is triggered but alerts were already sent. Nothing to do.", alertCondition);
                     }

graylog2-server/src/main/java/org/graylog2/alerts/AlertService.java

@@ -47,6 +47,7 @@ public interface AlertService {
     AlertCondition updateFromRequest(AlertCondition alertCondition, CreateConditionRequest ccr) throws ConfigurationException;
 
     boolean inGracePeriod(AlertCondition alertCondition);
+    boolean shouldRepeatNotifications(AlertCondition alertCondition, Alert alert);
 
     List<Alert> listForStreamId(String streamId, int skip, int limit);
     List<Alert> listForStreamIds(List<String> streamIds, AlertState state, int skip, int limit);

graylog2-server/src/main/java/org/graylog2/alerts/AlertServiceImpl.java

@@ -21,6 +21,8 @@
 import com.google.common.collect.ImmutableMap;
 import com.mongodb.BasicDBObject;
 import com.mongodb.DBCollection;
+import org.graylog2.alarmcallbacks.AlarmCallbackHistory;
+import org.graylog2.alarmcallbacks.AlarmCallbackHistoryService;
 import org.graylog2.alerts.Alert.AlertState;
 import org.graylog2.bindings.providers.MongoJackObjectMapperProvider;
 import org.graylog2.database.CollectionName;
@@ -52,12 +54,15 @@
 public class AlertServiceImpl implements AlertService {
     private final JacksonDBCollection<AlertImpl, String> coll;
     private final AlertConditionFactory alertConditionFactory;
+    private final AlarmCallbackHistoryService alarmCallbackHistoryService;
 
     @Inject
     public AlertServiceImpl(MongoConnection mongoConnection,
                             MongoJackObjectMapperProvider mapperProvider,
-                            AlertConditionFactory alertConditionFactory) {
+                            AlertConditionFactory alertConditionFactory,
+                            AlarmCallbackHistoryService alarmCallbackHistoryService) {
         this.alertConditionFactory = alertConditionFactory;
+        this.alarmCallbackHistoryService = alarmCallbackHistoryService;
         final String collectionName = AlertImpl.class.getAnnotation(CollectionName.class).value();
         final DBCollection dbCollection = mongoConnection.getDatabase().getCollection(collectionName);
 
@@ -207,6 +212,35 @@ public boolean inGracePeriod(AlertCondition alertCondition) {
         return lastAlertSecondsAgo < alertCondition.getGrace() * 60;
     }
 
+    @Override
+    public boolean shouldRepeatNotifications(AlertCondition alertCondition, Alert alert) {
+        // Do not repeat notifications if alert has no state, is resolved or the option to repeat notifications is disabled
+        if (!alert.isInterval() || isResolved(alert) || !alertCondition.shouldRepeatNotifications()) {
+            return false;
+        }
+
+        // Repeat notifications if no grace period is set, avoiding looking through the notification history
+        if (alertCondition.getGrace() == 0) {
+            return true;
+        }
+
+        AlarmCallbackHistory lastTriggeredAlertHistory = null;
+        for (AlarmCallbackHistory history : alarmCallbackHistoryService.getForAlertId(alert.getId())) {
+            if (lastTriggeredAlertHistory == null || lastTriggeredAlertHistory.createdAt().isBefore(history.createdAt())) {
+                lastTriggeredAlertHistory = history;
+            }
+        }
+
+        // Repeat notifications if no alert was ever triggered for this condition
+        if (lastTriggeredAlertHistory == null) {
+            return true;
+        }
+
+        final int lastAlertSecondsAgo = Seconds.secondsBetween(lastTriggeredAlertHistory.createdAt(), Tools.nowUTC()).getSeconds();
+
+        return lastAlertSecondsAgo >= alertCondition.getGrace() * 60;
+    }
+
     @Override
     public List<Alert> listForStreamIds(List<String> streamIds, AlertState state, int skip, int limit) {
         if (streamIds == null || streamIds.isEmpty()) {

graylog2-server/src/test/java/org/graylog2/alerts/AlertScannerTest.java

@@ -158,6 +158,7 @@ public void testCheckRepeatedAlertNotifications() throws Exception {
 
         when(alertService.getLastTriggeredAlert(stream.getId(), alertCondition.getId())).thenReturn(Optional.of(alert));
         when(alertService.isResolved(alert)).thenReturn(false);
+        when(alertService.shouldRepeatNotifications(alertCondition, alert)).thenReturn(true);
         assertThat(this.alertScanner.checkAlertCondition(stream, alertCondition)).isTrue();
 
         verify(alertCondition, times(2)).runCheck();

graylog2-server/src/test/java/org/graylog2/alerts/AlertServiceImplTest.java

@@ -20,8 +20,11 @@
 import com.google.common.collect.ImmutableList;
 import com.lordofthejars.nosqlunit.annotation.UsingDataSet;
 import com.lordofthejars.nosqlunit.core.LoadStrategyEnum;
+import org.graylog2.alarmcallbacks.AlarmCallbackHistory;
+import org.graylog2.alarmcallbacks.AlarmCallbackHistoryService;
 import org.graylog2.database.MongoDBServiceTest;
 import org.graylog2.plugin.Tools;
+import org.graylog2.plugin.alarms.AlertCondition;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.Seconds;
@@ -32,6 +35,8 @@
 import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class AlertServiceImplTest extends MongoDBServiceTest {
     private final String ALERT_ID = "581b3bff8e4dc4270055dfca";
@@ -41,10 +46,13 @@ public class AlertServiceImplTest extends MongoDBServiceTest {
     private AlertServiceImpl alertService;
     @Mock
     private AlertConditionFactory alertConditionFactory;
+    @Mock
+    private AlarmCallbackHistoryService alarmCallbackHistoryService;
 
     @Before
     public void setUpService() throws Exception {
-        this.alertService = new AlertServiceImpl(mongoRule.getMongoConnection(), mapperProvider, alertConditionFactory);
+        this.alarmCallbackHistoryService = mock(AlarmCallbackHistoryService.class);
+        this.alertService = new AlertServiceImpl(mongoRule.getMongoConnection(), mapperProvider, alertConditionFactory, alarmCallbackHistoryService);
     }
 
     @Test
@@ -262,4 +270,52 @@ public void unresolvedAlertIsUnresolved() throws Exception {
         assertThat(alertService.isResolved(alert)).isFalse();
     }
 
+    @Test
+    @UsingDataSet(locations = "non-interval-alert.json")
+    public void nonIntervalAlertShouldNotRepeatNotifications() throws Exception {
+        final AlertCondition alertCondition = mock(AlertCondition.class);
+        when(alertCondition.shouldRepeatNotifications()).thenReturn(true);
+        final Alert alert = alertService.load(ALERT_ID, STREAM_ID);
+        assertThat(alertService.shouldRepeatNotifications(alertCondition, alert)).isFalse();
+    }
+
+    @Test
+    @UsingDataSet(locations = "unresolved-alert.json")
+    public void shouldNotRepeatNotificationsWhenOptionIsDisabled() throws Exception {
+        final AlertCondition alertCondition = mock(AlertCondition.class);
+        when(alertCondition.shouldRepeatNotifications()).thenReturn(false);
+        final Alert alert = alertService.load(ALERT_ID, STREAM_ID);
+        assertThat(alertService.shouldRepeatNotifications(alertCondition, alert)).isFalse();
+    }
+
+    @Test
+    @UsingDataSet(locations = "unresolved-alert.json")
+    public void repeatNotificationsOptionShouldComplyWithGracePeriod() throws Exception {
+        final AlertCondition alertCondition = mock(AlertCondition.class);
+        when(alertCondition.getGrace()).thenReturn(15);
+        when(alertCondition.shouldRepeatNotifications()).thenReturn(true);
+        final Alert alert = alertService.load(ALERT_ID, STREAM_ID);
+        // Should repeat notification when there was no previous alert
+        assertThat(alertService.shouldRepeatNotifications(alertCondition, alert)).isTrue();
+
+        final AlarmCallbackHistory alarmCallbackHistory = mock(AlarmCallbackHistory.class);
+        when(alarmCallbackHistory.createdAt()).thenReturn(DateTime.now(DateTimeZone.UTC).minusMinutes(14));
+        when(alarmCallbackHistoryService.getForAlertId(ALERT_ID)).thenReturn(ImmutableList.of(alarmCallbackHistory));
+        // Should not repeat notification if a notification was sent during grace period
+        assertThat(alertService.shouldRepeatNotifications(alertCondition, alert)).isFalse();
+
+        when(alarmCallbackHistory.createdAt()).thenReturn(DateTime.now(DateTimeZone.UTC).minusMinutes(15));
+        when(alarmCallbackHistoryService.getForAlertId(ALERT_ID)).thenReturn(ImmutableList.of(alarmCallbackHistory));
+        // Should repeat notification after grace period passed
+        assertThat(alertService.shouldRepeatNotifications(alertCondition, alert)).isTrue();
+    }
+
+    @Test
+    @UsingDataSet(locations = "unresolved-alert.json")
+    public void shouldRepeatNotificationsWhenOptionIsEnabled() throws Exception {
+        final AlertCondition alertCondition = mock(AlertCondition.class);
+        when(alertCondition.shouldRepeatNotifications()).thenReturn(true);
+        final Alert alert = alertService.load(ALERT_ID, STREAM_ID);
+        assertThat(alertService.shouldRepeatNotifications(alertCondition, alert)).isTrue();
+    }
 }