Browse Source

ref(mute alerts): Don't send muted alerts to integrations (#49375)

We shouldn't send a notification to an integration if the rule has been
muted for everyone - this specifically covers the case where the rule
action says to notify a channel, rather than the case where you notify a
user or team and we follow the notification settings.

Fixes #49338
Colleen O'Rourke 1 year ago
parent
commit
865836e851

+ 2 - 1
src/sentry/integrations/slack/notifications.py

@@ -104,7 +104,8 @@ def send_notification_as_slack(
     shared_context: Mapping[str, Any],
     extra_context_by_actor: Mapping[RpcActor, Mapping[str, Any]] | None,
 ) -> None:
-    """Send an "activity" or "alert rule" notification to a Slack user or team."""
+    """Send an "activity" or "alert rule" notification to a Slack user or team, but NOT to a channel directly.
+    Sending Slack notifications to a channel is in integrations/slack/actions/notification.py"""
     with sentry_sdk.start_span(
         op="notification.send_slack", description="gen_channel_integration_map"
     ):

+ 6 - 2
src/sentry/rules/processor.py

@@ -10,7 +10,7 @@ from django.utils import timezone
 
 from sentry import analytics
 from sentry.eventstore.models import GroupEvent
-from sentry.models import Environment, GroupRuleStatus, Rule
+from sentry.models import Environment, GroupRuleStatus, Rule, RuleSnooze
 from sentry.rules import EventState, history, rules
 from sentry.types.rules import RuleFuture
 from sentry.utils.hashlib import hash_values
@@ -264,8 +264,12 @@ class RuleProcessor:
 
         self.grouped_futures.clear()
         rules = self.get_rules()
+        snoozed_rules = RuleSnooze.objects.filter(rule__in=rules, user_id=None).values_list(
+            "rule", flat=True
+        )
         rule_statuses = self.bulk_get_rule_status(rules)
         for rule in rules:
-            self.apply_rule(rule, rule_statuses[rule.id])
+            if rule.id not in snoozed_rules:
+                self.apply_rule(rule, rule_statuses[rule.id])
 
         return self.grouped_futures.values()

+ 129 - 1
tests/sentry/rules/test_processor.py

@@ -7,13 +7,21 @@ from django.db import DEFAULT_DB_ALIAS, connections
 from django.test.utils import CaptureQueriesContext
 from django.utils import timezone
 
-from sentry.models import GroupRuleStatus, GroupStatus, ProjectOwnership, Rule, RuleFireHistory
+from sentry.models import (
+    GroupRuleStatus,
+    GroupStatus,
+    ProjectOwnership,
+    Rule,
+    RuleFireHistory,
+    RuleSnooze,
+)
 from sentry.notifications.types import ActionTargetType
 from sentry.rules import init_registry
 from sentry.rules.conditions import EventCondition
 from sentry.rules.filters.base import EventFilter
 from sentry.rules.processor import RuleProcessor
 from sentry.testutils import TestCase
+from sentry.testutils.helpers import install_slack
 from sentry.testutils.silo import region_silo_test
 
 EMAIL_ACTION_DATA = {
@@ -115,6 +123,126 @@ class RuleProcessorTest(TestCase):
         results = list(rp.apply())
         assert len(results) == 0
 
+    def test_muted_slack_rule(self):
+        """Test that we don't sent a notification for a muted Slack rule"""
+        integration = install_slack(self.organization)
+        action_data = [
+            {
+                "channel": "#my-channel",
+                "id": "sentry.integrations.slack.notify_action.SlackNotifyServiceAction",
+                "workspace": integration.id,
+            },
+        ]
+        slack_rule = self.create_project_rule(self.project, action_data)
+        action_data[0].update({"channel": "#my-other-channel"})
+        muted_slack_rule = self.create_project_rule(self.project, action_data)
+        RuleSnooze.objects.create(
+            user_id=None,
+            owner_id=self.user.id,
+            rule=muted_slack_rule,
+            until=None,
+        )
+        rp = RuleProcessor(
+            self.group_event,
+            is_new=True,
+            is_regression=True,
+            is_new_group_environment=True,
+            has_reappeared=True,
+        )
+        results = list(rp.apply())
+        # this indicates that both email and slack notifs were sent, though there could be more than one of each type
+        assert len(results) == 2
+        # this checks that there was only 1 slack notification sent
+        slack_notifs = results[1][1]
+        assert len(slack_notifs) == 1
+        assert slack_notifs[0].rule == slack_rule
+
+        email_notifs = results[0][1]
+        # this checks that there was only 1 email notification sent
+        assert len(email_notifs) == 1
+        assert results[0][1][0].rule == self.rule
+        assert (
+            RuleFireHistory.objects.filter(
+                rule=muted_slack_rule, group=self.group_event.group
+            ).count()
+            == 0
+        )
+        assert (
+            RuleFireHistory.objects.filter(rule=slack_rule, group=self.group_event.group).count()
+            == 1
+        )
+        assert (
+            RuleFireHistory.objects.filter(rule=self.rule, group=self.group_event.group).count()
+            == 1
+        )
+
+    def test_muted_msteams_rule(self):
+        """Test that we don't sent a notification for a muted MSTeams rule"""
+        tenant_id = "50cccd00-7c9c-4b32-8cda-58a084f9334a"
+        integration = self.create_integration(
+            self.organization,
+            tenant_id,
+            metadata={
+                "access_token": "xoxb-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx",
+                "service_url": "https://testserviceurl.com/testendpoint/",
+                "installation_type": "tenant",
+                "expires_at": 1234567890,
+                "tenant_id": tenant_id,
+            },
+            name="Personal Installation",
+            provider="msteams",
+        )
+
+        action_data = [
+            {
+                "channel": "secrets",
+                "id": "sentry.integrations.msteams.notify_action.MsTeamsNotifyServiceAction",
+                "team": integration.id,
+            },
+        ]
+        msteams_rule = self.create_project_rule(self.project, action_data, [])
+        action_data[0].update({"channel": "#secreter-secrets"})
+        muted_msteams_rule = self.create_project_rule(self.project, action_data, [])
+        RuleSnooze.objects.create(
+            user_id=None,
+            owner_id=self.user.id,
+            rule=muted_msteams_rule,
+            until=None,
+        )
+        rp = RuleProcessor(
+            self.group_event,
+            is_new=True,
+            is_regression=True,
+            is_new_group_environment=True,
+            has_reappeared=True,
+        )
+        results = list(rp.apply())
+        # this indicates that both email and msteams notifs were sent, though there could be more than one of each type
+        assert len(results) == 2
+        slack_notifs = results[1][1]
+        # this checks that there was only 1 msteams notification sent
+        assert len(slack_notifs) == 1
+        assert slack_notifs[0].rule == msteams_rule
+
+        email_notifs = results[0][1]
+        # this checks that there was only 1 email notification sent
+        assert len(email_notifs) == 1
+        assert results[0][1][0].rule == self.rule
+        assert (
+            RuleFireHistory.objects.filter(
+                rule=muted_msteams_rule, group=self.group_event.group
+            ).count()
+            == 0
+        )
+        assert (
+            RuleFireHistory.objects.filter(rule=msteams_rule, group=self.group_event.group).count()
+            == 1
+        )
+        assert (
+            RuleFireHistory.objects.filter(rule=self.rule, group=self.group_event.group).count()
+            == 1
+        )
+
     def run_query_test(self, rp, expected_queries):
         with CaptureQueriesContext(connections[DEFAULT_DB_ALIAS]) as queries:
             results = list(rp.apply())