diff --git a/engine/apps/alerts/tasks/check_escalation_finished.py b/engine/apps/alerts/tasks/check_escalation_finished.py index a2c936cbff..c382aa5f4a 100644 --- 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 index 92c45eaa5c..ad05710241 100644 --- a/engine/apps/alerts/tests/test_check_escalation_finished_task.py +++ b/engine/apps/alerts/tests/test_check_escalation_finished_task.py @@ -1,4 +1,3 @@ -import random from unittest.mock import Mock, PropertyMock, patch import pytest @@ -6,7 +5,7 @@ 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