final changes

grafana · Mar 17, 2023 · 6e8ebd9 · 6e8ebd9
1 parent 5e96d7d
commit 6e8ebd9
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 82 deletions.
diff --git a/engine/apps/alerts/tasks/check_escalation_finished.py b/engine/apps/alerts/tasks/check_escalation_finished.py
@@ -30,8 +30,6 @@ def send_alert_group_escalation_auditor_task_heartbeat() -> None:
 
 
 def audit_alert_group_escalation(alert_group: "AlertGroup") -> None:
-    AlertGroupLogRecord = apps.get_model("alerts", "AlertGroupLogRecord")
-
     escalation_snapshot = alert_group.escalation_snapshot
     alert_group_id = alert_group.id
     base_msg = f"Alert group {alert_group_id}"
@@ -66,33 +64,33 @@ def audit_alert_group_escalation(alert_group: "AlertGroup") -> None:
         task_logger.info(
             f"{base_msg}'s escalation snapshot does not have any executed escalation policies, skipping further validation"
         )
-        return
-    task_logger.info(
-        f"{base_msg}'s escalation snapshot has {num_of_executed_escalation_policy_snapshots} executed escalation policies"
-    )
+    else:
+        task_logger.info(
+            f"{base_msg}'s escalation snapshot has {num_of_executed_escalation_policy_snapshots} executed escalation policies"
+        )
 
+    # TODO: consider adding the below checks later on. This is it a bit trickier to properly audit as the
+    # number of log records can vary if there are any STEP_NOTIFY_IF_NUM_ALERTS_IN_TIME_WINDOW or
+    # STEP_REPEAT_ESCALATION_N_TIMES escalation policy steps in the escalation chain
+    # see conversations in the original PR (https://github.com/grafana/oncall/pull/1266) for more context on this
+    #
     # compare number of triggered/failed alert group log records to the number of executed
     # escalation policy snapshot steps
-    num_of_relevant_log_records = AlertGroupLogRecord.objects.filter(
-        alert_group_id=alert_group_id,
-        type__in=[AlertGroupLogRecord.TYPE_ESCALATION_TRIGGERED, AlertGroupLogRecord.TYPE_ESCALATION_FAILED],
-    ).count()
-
-    if num_of_relevant_log_records < num_of_executed_escalation_policy_snapshots:
-        raise AlertGroupEscalationPolicyExecutionAuditException(
-            f"{base_msg}'s number of triggered/failed alert group log records ({num_of_relevant_log_records}) is less "
-            f"than the number of executed escalation policy snapshot steps ({num_of_executed_escalation_policy_snapshots})"
-        )
-
-    task_logger.info(
-        f"{base_msg}'s number of triggered/failed alert group log records ({num_of_relevant_log_records}) is greater "
-        f"than or equal to the number of executed escalation policy snapshot steps ({num_of_executed_escalation_policy_snapshots})"
-    )
-
-    # TODO: check following steps:
-    # STEP_NOTIFY_IF_NUM_ALERTS_IN_TIME_WINDOW
-    # STEP_REPEAT_ESCALATION_N_TIMES
-    # not sure what to do here?
+    # num_of_relevant_log_records = AlertGroupLogRecord.objects.filter(
+    #     alert_group_id=alert_group_id,
+    #     type__in=[AlertGroupLogRecord.TYPE_ESCALATION_TRIGGERED, AlertGroupLogRecord.TYPE_ESCALATION_FAILED],
+    # ).count()
+
+    # if num_of_relevant_log_records < num_of_executed_escalation_policy_snapshots:
+    #     raise AlertGroupEscalationPolicyExecutionAuditException(
+    #         f"{base_msg}'s number of triggered/failed alert group log records ({num_of_relevant_log_records}) is less "
+    #         f"than the number of executed escalation policy snapshot steps ({num_of_executed_escalation_policy_snapshots})"
+    #     )
+
+    # task_logger.info(
+    #     f"{base_msg}'s number of triggered/failed alert group log records ({num_of_relevant_log_records}) is greater "
+    #     f"than or equal to the number of executed escalation policy snapshot steps ({num_of_executed_escalation_policy_snapshots})"
+    # )
 
     task_logger.info(f"{base_msg} passed the audit checks")
 

diff --git a/engine/apps/alerts/tests/test_check_escalation_finished_task.py b/engine/apps/alerts/tests/test_check_escalation_finished_task.py
@@ -1,12 +1,11 @@
-import random
 from unittest.mock import Mock, PropertyMock, patch
 
 import pytest
 import requests
 from django.test import override_settings
 from django.utils import timezone
 
-from apps.alerts.models import AlertGroup, AlertGroupLogRecord, EscalationPolicy
+from apps.alerts.models import AlertGroup
 from apps.alerts.tasks.check_escalation_finished import (
     AlertGroupEscalationPolicyExecutionAuditException,
     audit_alert_group_escalation,
@@ -17,8 +16,8 @@
 MOCKED_HEARTBEAT_URL = "https://hello.com/lsdjjkf"
 
 
-def _get_relevant_log_record_type() -> int:
-    return random.choice([AlertGroupLogRecord.TYPE_ESCALATION_TRIGGERED, AlertGroupLogRecord.TYPE_ESCALATION_FAILED])
+# def _get_relevant_log_record_type() -> int:
+#     return random.choice([AlertGroupLogRecord.TYPE_ESCALATION_TRIGGERED, AlertGroupLogRecord.TYPE_ESCALATION_FAILED])
 
 
 def test_send_alert_group_escalation_auditor_task_heartbeat_does_not_call_the_heartbeat_url_if_one_is_not_configured():
@@ -120,58 +119,59 @@ def test_audit_alert_group_escalation_no_executed_escalation_policy_snapshots(es
         mock_executed_escalation_policy_snapshots.assert_called_once_with()
 
 
-@pytest.mark.django_db
-def test_audit_alert_group_escalation_all_executed_escalation_policy_snapshots_have_triggered_log_records(
-    escalation_snapshot_test_setup,
-    make_organization_and_user,
-    make_alert_group_log_record,
-):
-    _, user = make_organization_and_user()
-    alert_group, _, _, _ = escalation_snapshot_test_setup
-    escalation_policies_snapshots = alert_group.escalation_snapshot.escalation_policies_snapshots
-
-    for escalation_policy_snapshot in escalation_policies_snapshots:
-        escalation_policy = EscalationPolicy.objects.get(id=escalation_policy_snapshot.id)
-        log_record_type = _get_relevant_log_record_type()
-
-        make_alert_group_log_record(alert_group, log_record_type, user, escalation_policy=escalation_policy)
-
-    with patch(
-        "apps.alerts.escalation_snapshot.snapshot_classes.escalation_snapshot.EscalationSnapshot.executed_escalation_policy_snapshots",
-        new_callable=PropertyMock,
-    ) as mock_executed_escalation_policy_snapshots:
-        mock_executed_escalation_policy_snapshots.return_value = escalation_policies_snapshots
-        audit_alert_group_escalation(alert_group)
-        mock_executed_escalation_policy_snapshots.assert_called_once_with()
-
-
-@pytest.mark.django_db
-def test_audit_alert_group_escalation_one_executed_escalation_policy_snapshot_does_not_have_a_triggered_log_record(
-    escalation_snapshot_test_setup,
-    make_organization_and_user,
-    make_alert_group_log_record,
-):
-    _, user = make_organization_and_user()
-    alert_group, _, _, _ = escalation_snapshot_test_setup
-    escalation_policies_snapshots = alert_group.escalation_snapshot.escalation_policies_snapshots
-
-    # let's skip creating a relevant alert group log record for the first executed escalation policy
-    for idx, escalation_policy_snapshot in enumerate(escalation_policies_snapshots):
-        if idx != 0:
-            escalation_policy = EscalationPolicy.objects.get(id=escalation_policy_snapshot.id)
-            make_alert_group_log_record(
-                alert_group, _get_relevant_log_record_type(), user, escalation_policy=escalation_policy
-            )
-
-    with patch(
-        "apps.alerts.escalation_snapshot.snapshot_classes.escalation_snapshot.EscalationSnapshot.executed_escalation_policy_snapshots",
-        new_callable=PropertyMock,
-    ) as mock_executed_escalation_policy_snapshots:
-        mock_executed_escalation_policy_snapshots.return_value = escalation_policies_snapshots
-
-        with pytest.raises(AlertGroupEscalationPolicyExecutionAuditException):
-            audit_alert_group_escalation(alert_group)
-            mock_executed_escalation_policy_snapshots.assert_called_once_with()
+# # see TODO: comment in engine/apps/alerts/tasks/check_escalation_finished.py
+# @pytest.mark.django_db
+# def test_audit_alert_group_escalation_all_executed_escalation_policy_snapshots_have_triggered_log_records(
+#     escalation_snapshot_test_setup,
+#     make_organization_and_user,
+#     make_alert_group_log_record,
+# ):
+#     _, user = make_organization_and_user()
+#     alert_group, _, _, _ = escalation_snapshot_test_setup
+#     escalation_policies_snapshots = alert_group.escalation_snapshot.escalation_policies_snapshots
+
+#     for escalation_policy_snapshot in escalation_policies_snapshots:
+#         escalation_policy = EscalationPolicy.objects.get(id=escalation_policy_snapshot.id)
+#         log_record_type = _get_relevant_log_record_type()
+
+#         make_alert_group_log_record(alert_group, log_record_type, user, escalation_policy=escalation_policy)
+
+#     with patch(
+#         "apps.alerts.escalation_snapshot.snapshot_classes.escalation_snapshot.EscalationSnapshot.executed_escalation_policy_snapshots",
+#         new_callable=PropertyMock,
+#     ) as mock_executed_escalation_policy_snapshots:
+#         mock_executed_escalation_policy_snapshots.return_value = escalation_policies_snapshots
+#         audit_alert_group_escalation(alert_group)
+#         mock_executed_escalation_policy_snapshots.assert_called_once_with()
+
+# see TODO: comment in engine/apps/alerts/tasks/check_escalation_finished.py
+# @pytest.mark.django_db
+# def test_audit_alert_group_escalation_one_executed_escalation_policy_snapshot_does_not_have_a_triggered_log_record(
+#     escalation_snapshot_test_setup,
+#     make_organization_and_user,
+#     make_alert_group_log_record,
+# ):
+#     _, user = make_organization_and_user()
+#     alert_group, _, _, _ = escalation_snapshot_test_setup
+#     escalation_policies_snapshots = alert_group.escalation_snapshot.escalation_policies_snapshots
+
+#     # let's skip creating a relevant alert group log record for the first executed escalation policy
+#     for idx, escalation_policy_snapshot in enumerate(escalation_policies_snapshots):
+#         if idx != 0:
+#             escalation_policy = EscalationPolicy.objects.get(id=escalation_policy_snapshot.id)
+#             make_alert_group_log_record(
+#                 alert_group, _get_relevant_log_record_type(), user, escalation_policy=escalation_policy
+#             )
+
+#     with patch(
+#         "apps.alerts.escalation_snapshot.snapshot_classes.escalation_snapshot.EscalationSnapshot.executed_escalation_policy_snapshots",
+#         new_callable=PropertyMock,
+#     ) as mock_executed_escalation_policy_snapshots:
+#         mock_executed_escalation_policy_snapshots.return_value = escalation_policies_snapshots
+
+#         with pytest.raises(AlertGroupEscalationPolicyExecutionAuditException):
+#             audit_alert_group_escalation(alert_group)
+#             mock_executed_escalation_policy_snapshots.assert_called_once_with()
 
 
 @pytest.mark.django_db