Skip to content

Commit 0f24ad5

Browse files
authored
♻️ ref(metric alerts): refactor slack metric alert logic (#87261)
this pr doesn't do anything new, it decomposes the slack metric alert action handler util so it is ready for me to make the open period refactors it might be easier to review by cloning locally and taking a look pr to add open period stuff to follow
1 parent 35af521 commit 0f24ad5

File tree

5 files changed

+227
-135
lines changed

5 files changed

+227
-135
lines changed

src/sentry/incidents/typings/metric_detector.py

+18-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from sentry.incidents.models.incident import Incident, IncidentStatus
1515
from sentry.issues.issue_occurrence import IssueOccurrence
1616
from sentry.models.group import Group, GroupStatus
17-
from sentry.snuba.models import SnubaQuery
17+
from sentry.snuba.models import QuerySubscription, SnubaQuery
1818
from sentry.types.group import PriorityLevel
1919
from sentry.workflow_engine.models import Action, Detector
2020

@@ -60,6 +60,7 @@ class NotificationContext:
6060
NotificationContext is a dataclass that represents the context required send a notification.
6161
"""
6262

63+
id: int
6364
integration_id: int | None = None
6465
target_identifier: str | None = None
6566
target_display: str | None = None
@@ -69,6 +70,7 @@ class NotificationContext:
6970
@classmethod
7071
def from_alert_rule_trigger_action(cls, action: AlertRuleTriggerAction) -> NotificationContext:
7172
return cls(
73+
id=action.id,
7274
integration_id=action.integration_id,
7375
target_identifier=action.target_identifier,
7476
target_display=action.target_display,
@@ -79,6 +81,7 @@ def from_alert_rule_trigger_action(cls, action: AlertRuleTriggerAction) -> Notif
7981
def from_action_model(cls, action: Action) -> NotificationContext:
8082
if action.type == Action.Type.SENTRY_APP:
8183
return cls(
84+
id=action.id,
8285
integration_id=action.integration_id,
8386
target_display=action.config.get("target_display"),
8487
sentry_app_config=action.data.get("settings"),
@@ -87,6 +90,7 @@ def from_action_model(cls, action: Action) -> NotificationContext:
8790
)
8891
elif action.type == Action.Type.OPSGENIE or action.type == Action.Type.PAGERDUTY:
8992
return cls(
93+
id=action.id,
9094
integration_id=action.integration_id,
9195
target_identifier=action.config.get("target_identifier"),
9296
target_display=action.config.get("target_display"),
@@ -95,6 +99,7 @@ def from_action_model(cls, action: Action) -> NotificationContext:
9599
# TODO(iamrajjoshi): Add support for email here
96100

97101
return cls(
102+
id=action.id,
98103
integration_id=action.integration_id,
99104
target_identifier=action.config.get("target_identifier"),
100105
target_display=action.config.get("target_display"),
@@ -103,9 +108,11 @@ def from_action_model(cls, action: Action) -> NotificationContext:
103108

104109
@dataclass
105110
class MetricIssueContext:
106-
open_period_identifier: int
111+
id: int
112+
open_period_identifier: int # Used for link building
107113
snuba_query: SnubaQuery
108114
new_status: IncidentStatus
115+
subscription: QuerySubscription | None
109116
metric_value: float | None
110117

111118
@classmethod
@@ -128,6 +135,10 @@ def _get_snuba_query(cls, occurrence: IssueOccurrence) -> SnubaQuery:
128135
raise ValueError("Snuba query does not exist") from e
129136
return query
130137

138+
@classmethod
139+
def _get_subscription(cls, occurrence: IssueOccurrence) -> QuerySubscription | None:
140+
return occurrence.evidence_data.get("subscription_id")
141+
131142
@classmethod
132143
def _get_metric_value(cls, occurrence: IssueOccurrence) -> float:
133144
if (metric_value := occurrence.evidence_data.get("metric_value")) is None:
@@ -144,8 +155,11 @@ def from_group_event(cls, group_event: GroupEvent) -> MetricIssueContext:
144155
# TODO(iamrajjoshi): Replace with something once we know how we want to build the link
145156
# If we store open periods in the database, we can use the id from that
146157
# Otherwise, we can use the issue id
158+
id=group.id,
159+
# TODO(iamrajjoshi): This should probably be the id of the latest open period
147160
open_period_identifier=group.id,
148161
snuba_query=cls._get_snuba_query(occurrence),
162+
subscription=cls._get_subscription(occurrence),
149163
new_status=cls._get_new_status(group, occurrence),
150164
metric_value=cls._get_metric_value(occurrence),
151165
)
@@ -158,8 +172,10 @@ def from_legacy_models(
158172
metric_value: float | None = None,
159173
) -> MetricIssueContext:
160174
return cls(
175+
id=incident.id,
161176
open_period_identifier=incident.identifier,
162177
snuba_query=incident.alert_rule.snuba_query,
178+
subscription=incident.subscription,
163179
new_status=new_status,
164180
metric_value=metric_value,
165181
)

src/sentry/integrations/slack/message_builder/incidents.py

+6-11
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
from datetime import datetime
22

3-
from sentry.incidents.models.incident import IncidentStatus
4-
from sentry.incidents.typings.metric_detector import AlertContext
3+
from sentry.incidents.typings.metric_detector import AlertContext, MetricIssueContext
54
from sentry.integrations.metric_alerts import incident_attachment_info
65
from sentry.integrations.slack.message_builder.base.block import BlockSlackMessageBuilder
76
from sentry.integrations.slack.message_builder.types import (
@@ -11,7 +10,6 @@
1110
)
1211
from sentry.integrations.slack.utils.escape import escape_slack_text
1312
from sentry.models.organization import Organization
14-
from sentry.snuba.models import SnubaQuery
1513

1614

1715
def get_started_at(timestamp: datetime | None) -> str:
@@ -26,12 +24,9 @@ class SlackIncidentsMessageBuilder(BlockSlackMessageBuilder):
2624
def __init__(
2725
self,
2826
alert_context: AlertContext,
29-
open_period_identifier: int,
27+
metric_issue_context: MetricIssueContext,
3028
organization: Organization,
31-
snuba_query: SnubaQuery,
32-
new_status: IncidentStatus,
3329
date_started: datetime,
34-
metric_value: float | None = None,
3530
chart_url: str | None = None,
3631
notification_uuid: str | None = None,
3732
) -> None:
@@ -45,11 +40,11 @@ def __init__(
4540
"""
4641
super().__init__()
4742
self.alert_context = alert_context
48-
self.open_period_identifier = open_period_identifier
43+
self.open_period_identifier = metric_issue_context.open_period_identifier
4944
self.organization = organization
50-
self.snuba_query = snuba_query
51-
self.metric_value = metric_value
52-
self.new_status = new_status
45+
self.snuba_query = metric_issue_context.snuba_query
46+
self.metric_value = metric_issue_context.metric_value
47+
self.new_status = metric_issue_context.new_status
5348
self.date_started = date_started
5449
self.chart_url = chart_url
5550
self.notification_uuid = notification_uuid

0 commit comments

Comments
 (0)