Check for backsync updates before triggering connected webhooks (#4149)

Avoid triggering a webhook if it is from a connected integration and the
triggering event was caused by a backsync update.

Related to https://github.com/grafana/oncall-private/issues/2615

engine/apps/webhooks/listeners.py

@@ -7,10 +7,17 @@ logger.setLevel(logging.DEBUG)
 
 
 def on_alert_group_created(**kwargs):
-    alert_group_created.apply_async((kwargs["alert_group"].id,))
+    alert_group = kwargs["alert_group"]
+
+    # if we have an external_id, this alert_group was just created from a backsync update
+    external_id = alert_group.external_ids.filter(source_alert_receive_channel=alert_group.channel).first()
+    is_backsync = external_id is not None
+
+    alert_group_created.apply_async((kwargs["alert_group"].id,), kwargs={"is_backsync": is_backsync})
 
 
 def on_action_triggered(**kwargs):
+    from apps.alerts.constants import ActionSource
     from apps.alerts.models import AlertGroupLogRecord
 
     log_record = kwargs["log_record"]
@@ -20,4 +27,9 @@ def on_action_triggered(**kwargs):
         except AlertGroupLogRecord.DoesNotExist as e:
             logger.warning(f"Webhook action triggered: log record {log_record} never created or has been deleted")
             raise e
-    alert_group_status_change.apply_async((log_record.type, log_record.alert_group_id, log_record.author_id))
+
+    # keep track if this status change was triggered by a backsync event
+    is_backsync = log_record.action_source == ActionSource.BACKSYNC
+    alert_group_status_change.apply_async(
+        (log_record.type, log_record.alert_group_id, log_record.author_id), kwargs={"is_backsync": is_backsync}
+    )

engine/apps/webhooks/tasks/alert_group_status.py

@@ -28,7 +28,7 @@ ACTION_TO_TRIGGER_TYPE = {
 @shared_dedicated_queue_retry_task(
     bind=True, autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else MAX_RETRIES
 )
-def alert_group_created(self, alert_group_id):
+def alert_group_created(self, alert_group_id, is_backsync=False):
     try:
         alert_group = AlertGroup.objects.get(pk=alert_group_id)
     except AlertGroup.DoesNotExist:
@@ -38,17 +38,24 @@ def alert_group_created(self, alert_group_id):
     organization_id = alert_group.channel.organization_id
     webhooks = Webhook.objects.filter(trigger_type=trigger_type, organization_id=organization_id)
 
+    if is_backsync:
+        # only consider non-connected integration webhooks for backsync events
+        webhooks = webhooks.filter(is_from_connected_integration=False)
+
     # check if there are any webhooks before going on
     if not webhooks:
         return
 
-    send_webhook_event.apply_async((trigger_type, alert_group_id), kwargs={"organization_id": organization_id})
+    send_webhook_event.apply_async(
+        (trigger_type, alert_group_id),
+        kwargs={"organization_id": organization_id, "is_backsync": is_backsync},
+    )
 
 
 @shared_dedicated_queue_retry_task(
     bind=True, autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else MAX_RETRIES
 )
-def alert_group_status_change(self, action_type, alert_group_id, user_id):
+def alert_group_status_change(self, action_type, alert_group_id, user_id, is_backsync=False):
     try:
         alert_group = AlertGroup.objects.get(pk=alert_group_id)
     except AlertGroup.DoesNotExist:
@@ -63,11 +70,15 @@ def alert_group_status_change(self, action_type, alert_group_id, user_id):
         trigger_type=trigger_type, organization_id=organization_id
     ) | Webhook.objects.filter(trigger_type=Webhook.TRIGGER_STATUS_CHANGE, organization_id=organization_id)
 
+    if is_backsync:
+        # only consider non-connected integration webhooks for backsync events
+        webhooks = webhooks.filter(is_from_connected_integration=False)
+
     # check if there are any webhooks before going on
     if not webhooks:
         return
 
     send_webhook_event.apply_async(
         (trigger_type, alert_group_id),
-        kwargs={"organization_id": organization_id, "user_id": user_id},
+        kwargs={"organization_id": organization_id, "user_id": user_id, "is_backsync": is_backsync},
     )

engine/apps/webhooks/tasks/trigger_webhook.py

@@ -62,7 +62,7 @@ class WebhookRequestStatus(typing.TypedDict):
 @shared_dedicated_queue_retry_task(
     autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else None
 )
-def send_webhook_event(trigger_type, alert_group_id, organization_id=None, user_id=None):
+def send_webhook_event(trigger_type, alert_group_id, organization_id=None, user_id=None, is_backsync=False):
     from apps.webhooks.models import Webhook
 
     webhooks_qs = Webhook.objects.filter(
@@ -76,6 +76,9 @@ def send_webhook_event(trigger_type, alert_group_id, organization_id=None, user_
             organization_id=organization_id,
         ).exclude(is_webhook_enabled=False)
 
+    if is_backsync:
+        webhooks_qs = webhooks_qs.filter(is_from_connected_integration=False)
+
     for webhook in webhooks_qs:
         execute_webhook.apply_async((webhook.pk, alert_group_id, user_id, None), kwargs={"trigger_type": trigger_type})
 

engine/apps/webhooks/tests/test_alert_group_status_change.py

@@ -9,7 +9,10 @@ from apps.webhooks.tasks import alert_group_created, alert_group_status_change
 
 
 @pytest.mark.django_db
-def test_alert_group_created(make_organization, make_alert_receive_channel, make_alert_group, make_custom_webhook):
+@pytest.mark.parametrize("is_backsync", [True, False, None])
+def test_alert_group_created(
+    make_organization, make_alert_receive_channel, make_alert_group, make_custom_webhook, is_backsync
+):
     organization = make_organization()
     alert_receive_channel = make_alert_receive_channel(organization)
     alert_group = make_alert_group(alert_receive_channel)
@@ -17,12 +20,13 @@ def test_alert_group_created(make_organization, make_alert_receive_channel, make
     make_custom_webhook(organization=organization, trigger_type=Webhook.TRIGGER_ALERT_GROUP_CREATED)
 
     with patch("apps.webhooks.tasks.trigger_webhook.send_webhook_event.apply_async") as mock_send_event:
-        alert_group_created(alert_group.pk)
+        kwargs = {} if is_backsync is None else {"is_backsync": is_backsync}
+        alert_group_created(alert_group.pk, **kwargs)
 
     assert mock_send_event.called
     assert mock_send_event.call_args == call(
         (Webhook.TRIGGER_ALERT_GROUP_CREATED, alert_group.pk),
-        kwargs={"organization_id": organization.pk},
+        kwargs={"organization_id": organization.pk, "is_backsync": bool(is_backsync)},
     )
 
 
@@ -43,7 +47,7 @@ def test_alert_group_created_for_team(
     assert mock_send_event.called
     assert mock_send_event.call_args == call(
         (Webhook.TRIGGER_ALERT_GROUP_CREATED, alert_group.pk),
-        kwargs={"organization_id": organization.pk},
+        kwargs={"organization_id": organization.pk, "is_backsync": False},
     )
 
 
@@ -72,6 +76,7 @@ def test_alert_group_created_does_not_exist(make_organization, make_custom_webho
         (AlertGroupLogRecord.TYPE_UN_ACK, Webhook.TRIGGER_UNACKNOWLEDGE),
     ],
 )
+@pytest.mark.parametrize("is_backsync", [True, False, None])
 def test_alert_group_status_change(
     make_organization,
     make_user_for_organization,
@@ -80,6 +85,7 @@ def test_alert_group_status_change(
     make_custom_webhook,
     action_type,
     webhook_type,
+    is_backsync,
 ):
     organization = make_organization()
     user = make_user_for_organization(organization)
@@ -89,10 +95,12 @@ def test_alert_group_status_change(
     make_custom_webhook(organization=organization, trigger_type=webhook_type)
 
     with patch("apps.webhooks.tasks.trigger_webhook.send_webhook_event.apply_async") as mock_send_event:
-        alert_group_status_change(action_type, alert_group.pk, user.pk)
+        kwargs = {} if is_backsync is None else {"is_backsync": is_backsync}
+        alert_group_status_change(action_type, alert_group.pk, user.pk, **kwargs)
 
     assert mock_send_event.call_args == call(
-        (webhook_type, alert_group.pk), kwargs={"organization_id": organization.pk, "user_id": user.pk}
+        (webhook_type, alert_group.pk),
+        kwargs={"organization_id": organization.pk, "user_id": user.pk, "is_backsync": bool(is_backsync)},
     )
 
 
@@ -125,5 +133,5 @@ def test_alert_group_status_change_for_team(
 
     assert mock_send_event.call_args == call(
         (Webhook.TRIGGER_RESOLVE, alert_group.pk),
-        kwargs={"organization_id": organization.pk, "user_id": None},
+        kwargs={"organization_id": organization.pk, "user_id": None, "is_backsync": False},
     )

engine/apps/webhooks/tests/test_trigger_webhook.py

@@ -42,7 +42,10 @@ def test_send_webhook_event_filters(
     webhooks = {}
     for trigger_type in trigger_types:
         webhooks[trigger_type] = make_custom_webhook(
-            organization=organization, trigger_type=trigger_type, team=make_team(organization)
+            organization=organization,
+            trigger_type=trigger_type,
+            team=make_team(organization),
+            is_from_connected_integration=(trigger_type != Webhook.TRIGGER_ACKNOWLEDGE),
         )
 
     for trigger_type in trigger_types:
@@ -52,6 +55,18 @@ def test_send_webhook_event_filters(
             (webhooks[trigger_type].pk, alert_group.pk, None, None), kwargs={"trigger_type": trigger_type}
         )
 
+    # backsync event exclude connected integration webhooks
+    for trigger_type in trigger_types:
+        with patch("apps.webhooks.tasks.trigger_webhook.execute_webhook.apply_async") as mock_execute:
+            send_webhook_event(trigger_type, alert_group.pk, organization_id=organization.pk, is_backsync=True)
+            if trigger_type == Webhook.TRIGGER_ACKNOWLEDGE:
+                assert mock_execute.call_args == call(
+                    (webhooks[trigger_type].pk, alert_group.pk, None, None), kwargs={"trigger_type": trigger_type}
+                )
+            else:
+                # except for the acknowledge webhook (not connected integration set), the webhook is not triggered
+                mock_execute.assert_not_called()
+
     # other org
     other_org_webhook = make_custom_webhook(
         organization=other_organization, trigger_type=Webhook.TRIGGER_ALERT_GROUP_CREATED