From dc9dc9a57f881082f3132eb20815a6cb2bc98912 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Fri, 5 Apr 2024 13:04:13 -0300 Subject: [PATCH] Update backsync method to take source channel as param (#4159) Update by backsync will now expect the source alert receive channel triggering the transition (and update the log record using this information). Related to https://github.com/grafana/oncall-private/issues/2615 --- engine/apps/alerts/models/alert_group.py | 78 +++++++++++++------ engine/apps/alerts/tests/test_alert_group.py | 45 +++++++---- .../apps/slack/scenarios/distribute_alerts.py | 4 +- 3 files changed, 85 insertions(+), 42 deletions(-) diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py index 5b3d016b..769cac3f 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/engine/apps/alerts/models/alert_group.py @@ -612,26 +612,33 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. """Update metrics cache for response time and state as needed.""" update_metrics_for_alert_group.apply_async((self.id, organization_id, previous_state, state)) - def update_state_by_backsync(self, new_state: AlertGroupState) -> None: + def update_state_by_backsync(self, new_state: AlertGroupState, source_channel: "AlertReceiveChannel") -> None: if self.state == new_state: return logger.debug(f"Update state {self.state} -> {new_state} for alert_group {self.pk}") + kwargs = { + "source_channel": source_channel, + "action_source": ActionSource.BACKSYNC, + } if new_state == AlertGroupState.FIRING: if self.state == AlertGroupState.ACKNOWLEDGED: - self.un_acknowledge_by_user_or_backsync(action_source=ActionSource.BACKSYNC) + self.un_acknowledge_by_user_or_backsync(**kwargs) elif self.state == AlertGroupState.RESOLVED: - self.un_resolve_by_user_or_backsync(action_source=ActionSource.BACKSYNC) + self.un_resolve_by_user_or_backsync(**kwargs) elif self.state == AlertGroupState.SILENCED: - self.un_silence_by_user_or_backsync(action_source=ActionSource.BACKSYNC) + self.un_silence_by_user_or_backsync(**kwargs) elif new_state == AlertGroupState.ACKNOWLEDGED: - self.acknowledge_by_user_or_backsync(action_source=ActionSource.BACKSYNC) + self.acknowledge_by_user_or_backsync(**kwargs) elif new_state == AlertGroupState.RESOLVED: - self.resolve_by_user_or_backsync(action_source=ActionSource.BACKSYNC) + self.resolve_by_user_or_backsync(**kwargs) elif new_state == AlertGroupState.SILENCED: - self.silence_by_user_or_backsync(action_source=ActionSource.BACKSYNC) + self.silence_by_user_or_backsync(**kwargs) def acknowledge_by_user_or_backsync( - self, user: typing.Optional[User] = None, action_source: typing.Optional[ActionSource] = None + self, + user: typing.Optional[User] = None, + source_channel: typing.Optional["AlertReceiveChannel"] = None, + action_source: typing.Optional[ActionSource] = None, ) -> None: from apps.alerts.models import AlertGroupLogRecord @@ -639,7 +646,7 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. reason = "Acknowledge button" if user else "Backsync signal" acknowledged_by = AlertGroup.USER if user else AlertGroup.SOURCE step_specific_info = ( - {"source_integration_name": self.channel.verbal_name} if action_source == ActionSource.BACKSYNC else None + {"source_integration_name": source_channel.verbal_name} if action_source == ActionSource.BACKSYNC else None ) organization_id = user.organization_id if user else self.channel.organization_id logger.debug(f"Started acknowledge_by_user_or_backsync for alert_group {self.pk}") @@ -689,7 +696,9 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. transaction.on_commit(partial(send_alert_group_signal.delay, log_record.pk)) for dependent_alert_group in self.dependent_alert_groups.all(): - dependent_alert_group.acknowledge_by_user_or_backsync(user, action_source=action_source) + dependent_alert_group.acknowledge_by_user_or_backsync( + user, source_channel=source_channel, action_source=action_source + ) logger.debug(f"Finished acknowledge_by_user_or_backsync for alert_group {self.pk}") @@ -727,13 +736,16 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. dependent_alert_group.acknowledge_by_source() def un_acknowledge_by_user_or_backsync( - self, user: typing.Optional[User] = None, action_source: typing.Optional[ActionSource] = None + self, + user: typing.Optional[User] = None, + source_channel: typing.Optional["AlertReceiveChannel"] = None, + action_source: typing.Optional[ActionSource] = None, ) -> None: from apps.alerts.models import AlertGroupLogRecord initial_state = self.state step_specific_info = ( - {"source_integration_name": self.channel.verbal_name} if action_source == ActionSource.BACKSYNC else None + {"source_integration_name": source_channel.verbal_name} if action_source == ActionSource.BACKSYNC else None ) organization_id = user.organization_id if user else self.channel.organization_id logger.debug(f"Started un_acknowledge_by_user_or_backsync for alert_group {self.pk}") @@ -760,11 +772,16 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. transaction.on_commit(partial(send_alert_group_signal.delay, log_record.pk)) for dependent_alert_group in self.dependent_alert_groups.all(): - dependent_alert_group.un_acknowledge_by_user_or_backsync(user, action_source=action_source) + dependent_alert_group.un_acknowledge_by_user_or_backsync( + user, source_channel=source_channel, action_source=action_source + ) logger.debug(f"Finished un_acknowledge_by_user_or_backsync for alert_group {self.pk}") def resolve_by_user_or_backsync( - self, user: typing.Optional[User] = None, action_source: typing.Optional[ActionSource] = None + self, + user: typing.Optional[User] = None, + source_channel: typing.Optional["AlertReceiveChannel"] = None, + action_source: typing.Optional[ActionSource] = None, ) -> None: from apps.alerts.models import AlertGroupLogRecord @@ -772,7 +789,7 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. reason = "Resolve button" if user else "Backsync signal" resolved_by = AlertGroup.USER if user else AlertGroup.SOURCE step_specific_info = ( - {"source_integration_name": self.channel.verbal_name} if action_source == ActionSource.BACKSYNC else None + {"source_integration_name": source_channel.verbal_name} if action_source == ActionSource.BACKSYNC else None ) organization_id = user.organization_id if user else self.channel.organization_id # if incident was silenced, unsilence it without starting escalation @@ -807,7 +824,9 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. transaction.on_commit(partial(send_alert_group_signal.delay, log_record.pk)) for dependent_alert_group in self.dependent_alert_groups.all(): - dependent_alert_group.resolve_by_user_or_backsync(user, action_source=action_source) + dependent_alert_group.resolve_by_user_or_backsync( + user, source_channel=source_channel, action_source=action_source + ) def resolve_by_source(self): from apps.alerts.models import AlertGroupLogRecord @@ -888,14 +907,17 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. dependent_alert_group.resolve_by_disable_maintenance() def un_resolve_by_user_or_backsync( - self, user: typing.Optional[User] = None, action_source: typing.Optional[ActionSource] = None + self, + user: typing.Optional[User] = None, + source_channel: typing.Optional["AlertReceiveChannel"] = None, + action_source: typing.Optional[ActionSource] = None, ) -> None: from apps.alerts.models import AlertGroupLogRecord if self.wiped_at is None: initial_state = self.state step_specific_info = ( - {"source_integration_name": self.channel.verbal_name} + {"source_integration_name": source_channel.verbal_name} if action_source == ActionSource.BACKSYNC else None ) @@ -924,7 +946,9 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. transaction.on_commit(partial(send_alert_group_signal.delay, log_record.pk)) for dependent_alert_group in self.dependent_alert_groups.all(): - dependent_alert_group.un_resolve_by_user_or_backsync(user, action_source=action_source) + dependent_alert_group.un_resolve_by_user_or_backsync( + user, source_channel=source_channel, action_source=action_source + ) def attach_by_user( self, user: User, root_alert_group: "AlertGroup", action_source: typing.Optional[ActionSource] = None @@ -1063,6 +1087,7 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. def silence_by_user_or_backsync( self, user: typing.Optional[User] = None, + source_channel: typing.Optional["AlertReceiveChannel"] = None, silence_delay: typing.Optional[int] = None, action_source: typing.Optional[ActionSource] = None, ) -> None: @@ -1071,7 +1096,7 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. initial_state = self.state reason = "Silence button" if user else "Backsync signal" step_specific_info = ( - {"source_integration_name": self.channel.verbal_name} if action_source == ActionSource.BACKSYNC else None + {"source_integration_name": source_channel.verbal_name} if action_source == ActionSource.BACKSYNC else None ) organization_id = user.organization_id if user else self.channel.organization_id @@ -1146,17 +1171,20 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. transaction.on_commit(partial(send_alert_group_signal.delay, log_record.pk)) for dependent_alert_group in self.dependent_alert_groups.all(): - dependent_alert_group.silence_by_user_or_backsync(user, silence_delay, action_source) + dependent_alert_group.silence_by_user_or_backsync(user, source_channel, silence_delay, action_source) def un_silence_by_user_or_backsync( - self, user: typing.Optional[User] = None, action_source: typing.Optional[ActionSource] = None + self, + user: typing.Optional[User] = None, + source_channel: typing.Optional["AlertReceiveChannel"] = None, + action_source: typing.Optional[ActionSource] = None, ) -> None: from apps.alerts.models import AlertGroupLogRecord initial_state = self.state reason = "Unsilence button" if user else "Backsync signal" step_specific_info = ( - {"source_integration_name": self.channel.verbal_name} if action_source == ActionSource.BACKSYNC else None + {"source_integration_name": source_channel.verbal_name} if action_source == ActionSource.BACKSYNC else None ) organization_id = user.organization_id if user else self.channel.organization_id self.un_silence() @@ -1186,7 +1214,9 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. transaction.on_commit(partial(send_alert_group_signal.delay, log_record.pk)) for dependent_alert_group in self.dependent_alert_groups.all(): - dependent_alert_group.un_silence_by_user_or_backsync(user, action_source=action_source) + dependent_alert_group.un_silence_by_user_or_backsync( + user, source_channel=source_channel, action_source=action_source + ) def wipe_by_user(self, user: User) -> None: from apps.alerts.models import AlertGroupLogRecord diff --git a/engine/apps/alerts/tests/test_alert_group.py b/engine/apps/alerts/tests/test_alert_group.py index 566ecc48..aece4fc3 100644 --- a/engine/apps/alerts/tests/test_alert_group.py +++ b/engine/apps/alerts/tests/test_alert_group.py @@ -475,45 +475,55 @@ def test_alert_group_log_record_action_source( alert_group = make_alert_group(alert_receive_channel) root_alert_group = make_alert_group(alert_receive_channel) + if action_source == ActionSource.BACKSYNC: + base_kwargs = { + "source_channel": alert_receive_channel, + } + else: + base_kwargs = { + "user": user, + } + # Silence alert group - alert_group.silence_by_user_or_backsync(user, 42, action_source=action_source) + alert_group.silence_by_user_or_backsync(**base_kwargs, silence_delay=42, action_source=action_source) log_record = alert_group.log_records.last() assert (log_record.type, log_record.action_source) == (AlertGroupLogRecord.TYPE_SILENCE, action_source) # Unsilence alert group - alert_group.un_silence_by_user_or_backsync(user, action_source=action_source) + alert_group.un_silence_by_user_or_backsync(**base_kwargs, action_source=action_source) log_record = alert_group.log_records.last() assert (log_record.type, log_record.action_source) == (AlertGroupLogRecord.TYPE_UN_SILENCE, action_source) # Acknowledge alert group - alert_group.acknowledge_by_user_or_backsync(user, action_source=action_source) + alert_group.acknowledge_by_user_or_backsync(**base_kwargs, action_source=action_source) log_record = alert_group.log_records.last() assert (log_record.type, log_record.action_source) == (AlertGroupLogRecord.TYPE_ACK, action_source) # Unacknowledge alert group - alert_group.un_acknowledge_by_user_or_backsync(user, action_source=action_source) + alert_group.un_acknowledge_by_user_or_backsync(**base_kwargs, action_source=action_source) log_record = alert_group.log_records.last() assert (log_record.type, log_record.action_source) == (AlertGroupLogRecord.TYPE_UN_ACK, action_source) # Resolve alert group - alert_group.resolve_by_user_or_backsync(user, action_source=action_source) + alert_group.resolve_by_user_or_backsync(**base_kwargs, action_source=action_source) log_record = alert_group.log_records.last() assert (log_record.type, log_record.action_source) == (AlertGroupLogRecord.TYPE_RESOLVED, action_source) # Unresolve alert group - alert_group.un_resolve_by_user_or_backsync(user, action_source=action_source) + alert_group.un_resolve_by_user_or_backsync(**base_kwargs, action_source=action_source) log_record = alert_group.log_records.last() assert (log_record.type, log_record.action_source) == (AlertGroupLogRecord.TYPE_UN_RESOLVED, action_source) - # Attach alert group - alert_group.attach_by_user(user, root_alert_group, action_source=action_source) - log_record = alert_group.log_records.last() - assert (log_record.type, log_record.action_source) == (AlertGroupLogRecord.TYPE_ATTACHED, action_source) + if action_source != ActionSource.BACKSYNC: + # Attach alert group + alert_group.attach_by_user(user, root_alert_group, action_source=action_source) + log_record = alert_group.log_records.last() + assert (log_record.type, log_record.action_source) == (AlertGroupLogRecord.TYPE_ATTACHED, action_source) - # Unattach alert group - alert_group.un_attach_by_user(user, action_source=action_source) - log_record = alert_group.log_records.last() - assert (log_record.type, log_record.action_source) == (AlertGroupLogRecord.TYPE_UNATTACHED, action_source) + # Unattach alert group + alert_group.un_attach_by_user(user, action_source=action_source) + log_record = alert_group.log_records.last() + assert (log_record.type, log_record.action_source) == (AlertGroupLogRecord.TYPE_UNATTACHED, action_source) @pytest.mark.django_db @@ -710,19 +720,20 @@ def test_update_state_by_backsync( make_alert_group, ): organization = make_organization() + source_channel = make_alert_receive_channel(organization) alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) - expected_log_data = (ActionSource.BACKSYNC, None, {"source_integration_name": alert_receive_channel.verbal_name}) + expected_log_data = (ActionSource.BACKSYNC, None, {"source_integration_name": source_channel.verbal_name}) assert alert_group.state == AlertGroupState.FIRING # set to new_state - alert_group.update_state_by_backsync(new_state) + alert_group.update_state_by_backsync(new_state, source_channel=source_channel) alert_group.refresh_from_db() assert alert_group.state == new_state last_log = alert_group.log_records.last() assert (last_log.action_source, last_log.author, last_log.step_specific_info) == expected_log_data assert last_log.type == log_type # set back to firing - alert_group.update_state_by_backsync(AlertGroupState.FIRING) + alert_group.update_state_by_backsync(AlertGroupState.FIRING, source_channel=source_channel) alert_group.refresh_from_db() assert alert_group.state == AlertGroupState.FIRING last_log = alert_group.log_records.last() diff --git a/engine/apps/slack/scenarios/distribute_alerts.py b/engine/apps/slack/scenarios/distribute_alerts.py index 3ba16351..c125b1ba 100644 --- a/engine/apps/slack/scenarios/distribute_alerts.py +++ b/engine/apps/slack/scenarios/distribute_alerts.py @@ -287,7 +287,9 @@ class SilenceGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): # Deprecated handler kept for backward compatibility (so older Slack messages can still be processed) silence_delay = int(value) - alert_group.silence_by_user_or_backsync(self.user, silence_delay, action_source=ActionSource.SLACK) + alert_group.silence_by_user_or_backsync( + self.user, silence_delay=silence_delay, action_source=ActionSource.SLACK + ) def process_signal(self, log_record: AlertGroupLogRecord) -> None: self.alert_group_slack_service.update_alert_group_slack_message(log_record.alert_group)