Skip to content

Commit fbfd680

Browse files
authored
feat(integration): Allow passing of RuleFireHistory (#67214)
We need to be able to access the instance of RuleFireHistory in the Slack notifications, such that we can record it for the notification message, and then try to get a plausible slack thread in future fires. To do this without having to query for the row/instance again, we need to be able to first grab the instance of the record when it's created, and pass that down to the action, specially only for Slack. In a previous attempt, we caused an issue because we had to query by a non-index column. We can avoid that altogether by just passing down the object that we need. Previous problem: #66936
1 parent 4438320 commit fbfd680

File tree

13 files changed

+111
-10
lines changed

13 files changed

+111
-10
lines changed

src/sentry/rules/actions/base.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,24 @@
77

88
from sentry.eventstore.models import GroupEvent
99
from sentry.models.rule import Rule
10+
from sentry.models.rulefirehistory import RuleFireHistory
1011
from sentry.rules.base import CallbackFuture, EventState, RuleBase
1112

1213
logger = logging.getLogger("sentry.rules")
1314

1415

15-
def instantiate_action(rule: Rule, action):
16+
def instantiate_action(rule: Rule, action, rule_fire_history: RuleFireHistory | None = None):
1617
from sentry.rules import rules
1718

18-
action_cls = rules.get(action["id"])
19+
action_id = action["id"]
20+
action_cls = rules.get(action_id)
1921
if action_cls is None:
2022
logger.warning("Unregistered action %r", action["id"])
2123
return None
2224

23-
action_inst = action_cls(rule.project, data=action, rule=rule)
25+
action_inst = action_cls(
26+
rule.project, data=action, rule=rule, rule_fire_history=rule_fire_history
27+
)
2428
if not isinstance(action_inst, EventAction):
2529
logger.warning("Unregistered action %r", action["id"])
2630
return None

src/sentry/rules/base.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from sentry.eventstore.models import GroupEvent
1212
from sentry.models.project import Project
1313
from sentry.models.rule import Rule
14+
from sentry.models.rulefirehistory import RuleFireHistory
1415
from sentry.snuba.dataset import Dataset
1516
from sentry.types.condition_activity import ConditionActivity
1617
from sentry.types.rules import RuleFuture
@@ -61,11 +62,13 @@ def __init__(
6162
project: Project,
6263
data: dict[str, Any] | None = None,
6364
rule: Rule | None = None,
65+
rule_fire_history: RuleFireHistory | None = None,
6466
) -> None:
6567
self.project = project
6668
self.data = data or {}
6769
self.had_data = data is not None
6870
self.rule = rule
71+
self.rule_fire_history = rule_fire_history
6972

7073
id: ClassVar[str]
7174
label: ClassVar[str]

src/sentry/rules/history/backends/postgres.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ def record(
4747
group: Group,
4848
event_id: str | None = None,
4949
notification_uuid: str | None = None,
50-
) -> None:
51-
RuleFireHistory.objects.create(
50+
) -> RuleFireHistory | None:
51+
return RuleFireHistory.objects.create(
5252
project=rule.project,
5353
rule=rule,
5454
group=group,

src/sentry/rules/history/base.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
from sentry.utils.services import Service
99

1010
if TYPE_CHECKING:
11+
from typing import Any
12+
1113
from sentry.models.group import Group
1214
from sentry.models.rule import Rule
1315
from sentry.utils.cursors import Cursor, CursorResult
@@ -40,7 +42,7 @@ def record(
4042
group: Group,
4143
event_id: str | None = None,
4244
notification_uuid: str | None = None,
43-
) -> None:
45+
) -> Any | None:
4446
"""
4547
Records an instance of an issue alert being fired for a given group.
4648
"""

src/sentry/rules/processor.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from sentry.models.environment import Environment
1616
from sentry.models.grouprulestatus import GroupRuleStatus
1717
from sentry.models.rule import Rule
18+
from sentry.models.rulefirehistory import RuleFireHistory
1819
from sentry.models.rulesnooze import RuleSnooze
1920
from sentry.rules import EventState, history, rules
2021
from sentry.rules.actions.base import instantiate_action
@@ -262,13 +263,18 @@ def apply_rule(self, rule: Rule, status: GroupRuleStatus) -> None:
262263
)
263264

264265
notification_uuid = str(uuid.uuid4())
265-
history.record(rule, self.group, self.event.event_id, notification_uuid)
266-
self.activate_downstream_actions(rule, notification_uuid)
266+
rule_fire_history = history.record(rule, self.group, self.event.event_id, notification_uuid)
267+
self.activate_downstream_actions(rule, notification_uuid, rule_fire_history)
267268

268-
def activate_downstream_actions(self, rule: Rule, notification_uuid: str | None = None) -> None:
269+
def activate_downstream_actions(
270+
self,
271+
rule: Rule,
272+
notification_uuid: str | None = None,
273+
rule_fire_history: RuleFireHistory | None = None,
274+
) -> None:
269275
state = self.get_state()
270276
for action in rule.data.get("actions", ()):
271-
action_inst = instantiate_action(rule, action)
277+
action_inst = instantiate_action(rule, action, rule_fire_history)
272278
if not action_inst:
273279
continue
274280

tests/sentry/integrations/slack/actions/__init__.py

Whitespace-only changes.

tests/sentry/integrations/slack/actions/notification/__init__.py

Whitespace-only changes.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
from uuid import uuid4
2+
3+
from sentry.integrations.slack import SlackNotifyServiceAction
4+
from sentry.models.rulefirehistory import RuleFireHistory
5+
from sentry.testutils.cases import TestCase
6+
7+
8+
class TestInit(TestCase):
9+
def setUp(self) -> None:
10+
self.rule = self.create_project_rule(project=self.project)
11+
self.notification_uuid = str(uuid4())
12+
self.event_id = 456
13+
self.rule_fire_history = RuleFireHistory.objects.create(
14+
project=self.project,
15+
rule=self.rule,
16+
group=self.group,
17+
event_id=self.event_id,
18+
notification_uuid=self.notification_uuid,
19+
)
20+
21+
def test_when_rule_fire_history_is_passed_in(self) -> None:
22+
instance = SlackNotifyServiceAction(
23+
self.project, data={}, rule=self.rule, rule_fire_history=self.rule_fire_history
24+
)
25+
assert instance.rule_fire_history is not None
26+
27+
def test_when_rule_fire_history_is_not_passed_in(self) -> None:
28+
instance = SlackNotifyServiceAction(self.project, data={}, rule=self.rule)
29+
assert instance.rule_fire_history is None
30+
31+
def test_when_rule_fire_history_is_none(self) -> None:
32+
instance = SlackNotifyServiceAction(
33+
self.project, data={}, rule=self.rule, rule_fire_history=None
34+
)
35+
assert instance.rule_fire_history is None

tests/sentry/rules/actions/__init__.py

Whitespace-only changes.

tests/sentry/rules/actions/base/__init__.py

Whitespace-only changes.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
from sentry.rules.actions import EventAction
2+
from sentry.testutils.cases import TestCase
3+
4+
5+
class TestRuleType(TestCase):
6+
def test_default(self) -> None:
7+
assert EventAction.rule_type == "action/event"
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
from uuid import uuid4
2+
3+
from sentry.integrations.slack import SlackNotifyServiceAction
4+
from sentry.models.rulefirehistory import RuleFireHistory
5+
from sentry.rules.actions.base import instantiate_action
6+
from sentry.testutils.cases import TestCase
7+
8+
9+
class TestInstantiateAction(TestCase):
10+
def setUp(self) -> None:
11+
self.rule = self.create_project_rule(project=self.project)
12+
self.notification_uuid = str(uuid4())
13+
self.event_id = 456
14+
self.rule_fire_history = RuleFireHistory.objects.create(
15+
project=self.project,
16+
rule=self.rule,
17+
group=self.group,
18+
event_id=self.event_id,
19+
notification_uuid=self.notification_uuid,
20+
)
21+
22+
def test_passes_in_rule_fire_history(self) -> None:
23+
action = {
24+
"id": SlackNotifyServiceAction.id,
25+
}
26+
27+
instance = instantiate_action(
28+
rule=self.rule, action=action, rule_fire_history=self.rule_fire_history
29+
)
30+
assert instance.rule_fire_history is not None
31+
assert instance.rule_fire_history == self.rule_fire_history
32+
33+
def test_respects_empty_rule_fire_history(self) -> None:
34+
action = {
35+
"id": SlackNotifyServiceAction.id,
36+
}
37+
38+
instance = instantiate_action(rule=self.rule, action=action)
39+
assert instance.rule_fire_history is None

tests/sentry/rules/history/backends/test_postgres.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ def test(self):
2929
assert RuleFireHistory.objects.filter(rule=rule, group=group_2).count() == 1
3030
assert RuleFireHistory.objects.filter(rule=rule).count() == 3
3131

32+
def test_returns_new_instance(self) -> None:
33+
rule = Rule.objects.create(project=self.event.project)
34+
new_instance = self.backend.record(rule, self.group)
35+
assert new_instance is not None
36+
3237

3338
@freeze_time()
3439
class FetchRuleGroupsPaginatedTest(BasePostgresRuleHistoryBackendTest):

0 commit comments

Comments
 (0)