From 8db285e4e88810024e80e8aac73b8a050e395101 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 7 May 2024 13:58:49 -0600 Subject: [PATCH] Fix escalation chain webhooks executing when disabled (#4319) # What this PR does Fixes issue where custom webhooks that are part of an escalation chain were still being executed even though they were disabled. Now the attempt will be logged in the escalation log and noted that the webhook was disabled. ## Which issue(s) this PR closes ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- .../escalation_policy_snapshot.py | 26 +++++++++++------- .../alerts/models/alert_group_log_record.py | 5 +++- .../tests/test_escalation_policy_snapshot.py | 27 +++++++++++++++++++ 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/engine/apps/alerts/escalation_snapshot/snapshot_classes/escalation_policy_snapshot.py b/engine/apps/alerts/escalation_snapshot/snapshot_classes/escalation_policy_snapshot.py index 98d894a4..541b5ffb 100644 --- a/engine/apps/alerts/escalation_snapshot/snapshot_classes/escalation_policy_snapshot.py +++ b/engine/apps/alerts/escalation_snapshot/snapshot_classes/escalation_policy_snapshot.py @@ -472,24 +472,32 @@ class EscalationPolicySnapshot: def _escalation_step_trigger_custom_webhook(self, alert_group: "AlertGroup", _reason: str) -> None: tasks = [] webhook = self.custom_webhook + failure_reason = None if webhook is not None: - custom_webhook_task = custom_webhook_result.signature( - (webhook.pk, alert_group.pk), - { - "escalation_policy_pk": self.id, - }, - immutable=True, - ) - tasks.append(custom_webhook_task) + if webhook.is_webhook_enabled: + custom_webhook_task = custom_webhook_result.signature( + (webhook.pk, alert_group.pk), + { + "escalation_policy_pk": self.id, + }, + immutable=True, + ) + tasks.append(custom_webhook_task) + else: + failure_reason = AlertGroupLogRecord.ERROR_ESCALATION_TRIGGER_WEBHOOK_IS_DISABLED else: + failure_reason = AlertGroupLogRecord.ERROR_ESCALATION_TRIGGER_WEBHOOK_STEP_IS_NOT_CONFIGURED + + if failure_reason: log_record = AlertGroupLogRecord( type=AlertGroupLogRecord.TYPE_ESCALATION_FAILED, alert_group=alert_group, escalation_policy=self.escalation_policy, - escalation_error_code=AlertGroupLogRecord.ERROR_ESCALATION_TRIGGER_WEBHOOK_STEP_IS_NOT_CONFIGURED, + escalation_error_code=failure_reason, escalation_policy_step=self.step, ) log_record.save() + self._execute_tasks(tasks) def _escalation_step_repeat_escalation_n_times( diff --git a/engine/apps/alerts/models/alert_group_log_record.py b/engine/apps/alerts/models/alert_group_log_record.py index e76e41f5..7da4f247 100644 --- a/engine/apps/alerts/models/alert_group_log_record.py +++ b/engine/apps/alerts/models/alert_group_log_record.py @@ -160,7 +160,8 @@ class AlertGroupLogRecord(models.Model): ERROR_ESCALATION_NOTIFY_IF_NUM_ALERTS_IN_WINDOW_STEP_IS_NOT_CONFIGURED, ERROR_ESCALATION_TRIGGER_CUSTOM_WEBHOOK_ERROR, ERROR_ESCALATION_NOTIFY_TEAM_MEMBERS_STEP_IS_NOT_CONFIGURED, - ) = range(19) + ERROR_ESCALATION_TRIGGER_WEBHOOK_IS_DISABLED, + ) = range(20) type = models.IntegerField(choices=TYPE_CHOICES) @@ -590,6 +591,8 @@ class AlertGroupLogRecord(models.Model): usergroup_handle = self.escalation_policy.notify_to_group.handle usergroup_handle_text = f" @{usergroup_handle}" if usergroup_handle else "" result += f"failed to notify User Group{usergroup_handle_text} in Slack" + elif self.escalation_error_code == AlertGroupLogRecord.ERROR_ESCALATION_TRIGGER_WEBHOOK_IS_DISABLED: + result += 'skipped escalation step "Trigger Outgoing Webhook" because it is disabled' return result def get_step_specific_info(self): diff --git a/engine/apps/alerts/tests/test_escalation_policy_snapshot.py b/engine/apps/alerts/tests/test_escalation_policy_snapshot.py index b959fe73..8a3eef60 100644 --- a/engine/apps/alerts/tests/test_escalation_policy_snapshot.py +++ b/engine/apps/alerts/tests/test_escalation_policy_snapshot.py @@ -486,6 +486,33 @@ def test_escalation_step_trigger_custom_webhook( mock_webhook_escalation_step.assert_called_once_with(alert_group, reason) +@patch("apps.alerts.escalation_snapshot.snapshot_classes.EscalationPolicySnapshot._execute_tasks", return_value=None) +@pytest.mark.django_db +def test_escalation_step_trigger_disabled_custom_webhook( + mocked_execute_tasks, + escalation_step_test_setup, + make_custom_webhook, + make_escalation_policy, +): + organization, _, _, channel_filter, alert_group, reason = escalation_step_test_setup + + custom_webhook = make_custom_webhook(organization=organization, is_webhook_enabled=False) + + trigger_custom_webhook_step = make_escalation_policy( + escalation_chain=channel_filter.escalation_chain, + escalation_policy_step=EscalationPolicy.STEP_TRIGGER_CUSTOM_WEBHOOK, + custom_webhook=custom_webhook, + ) + escalation_policy_snapshot = get_escalation_policy_snapshot_from_model(trigger_custom_webhook_step) + escalation_policy_snapshot.execute(alert_group, reason) + assert call([]) in mocked_execute_tasks.call_args_list + + log_record = AlertGroupLogRecord.objects.get( + alert_group_id=alert_group.id, escalation_policy=trigger_custom_webhook_step + ) + assert log_record.escalation_error_code == AlertGroupLogRecord.ERROR_ESCALATION_TRIGGER_WEBHOOK_IS_DISABLED + + @patch("apps.alerts.escalation_snapshot.snapshot_classes.EscalationPolicySnapshot._execute_tasks", return_value=None) @pytest.mark.django_db def test_escalation_step_repeat_escalation_n_times(