From 865836e8514ee19efd91197568700fddb3093a02 Mon Sep 17 00:00:00 2001 From: Colleen O'Rourke Date: Thu, 18 May 2023 14:46:21 -0700 Subject: [PATCH] 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 --- .../integrations/slack/notifications.py | 3 +- src/sentry/rules/processor.py | 8 +- tests/sentry/rules/test_processor.py | 130 +++++++++++++++++- 3 files changed, 137 insertions(+), 4 deletions(-) diff --git a/src/sentry/integrations/slack/notifications.py b/src/sentry/integrations/slack/notifications.py index a93e6d7f006c65..ae5e2a296c5c23 100644 --- a/src/sentry/integrations/slack/notifications.py +++ b/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" ): diff --git a/src/sentry/rules/processor.py b/src/sentry/rules/processor.py index 9993b2f09a8f33..3480e5d695e937 100644 --- a/src/sentry/rules/processor.py +++ b/src/sentry/rules/processor.py @@ -10,7 +10,7 @@ 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 @@ def apply( 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() diff --git a/tests/sentry/rules/test_processor.py b/tests/sentry/rules/test_processor.py index 180a4a4ba3fb9d..3e5fba6caa654f 100644 --- a/tests/sentry/rules/test_processor.py +++ b/tests/sentry/rules/test_processor.py @@ -7,13 +7,21 @@ 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 @@ def test_resolved_issue(self): 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())