Skip to content

Commit

Permalink
ref(mute alerts): Don't send muted alerts to integrations (#49375)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ceorourke authored May 18, 2023
1 parent 91f54f8 commit 865836e
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 4 deletions.
3 changes: 2 additions & 1 deletion src/sentry/integrations/slack/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
):
Expand Down
8 changes: 6 additions & 2 deletions src/sentry/rules/processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
130 changes: 129 additions & 1 deletion tests/sentry/rules/test_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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())
Expand Down

0 comments on commit 865836e

Please sign in to comment.