Skip to content

Commit 3cb3cfc

Browse files
authored
feat(aci): use new detector chart for test org (#102513)
Set up flow for using detector/open period data to generate notification charts. Hide this behind a feature flag so we can test.
1 parent 1af5d93 commit 3cb3cfc

File tree

15 files changed

+193
-32
lines changed

15 files changed

+193
-32
lines changed

src/sentry/features/temporary.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,8 @@ def register_temporary_features(manager: FeatureManager) -> None:
215215
manager.add("organizations:more-slow-alerts", OrganizationFeature, FeatureHandlerStrategy.INTERNAL, api_expose=False)
216216
# Enable higher limit for workflows
217217
manager.add("organizations:more-workflows", OrganizationFeature, FeatureHandlerStrategy.INTERNAL, api_expose=False)
218+
# Generate charts using detector/open period payload
219+
manager.add("organizations:new-metric-issue-charts", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
218220
# Extract on demand metrics
219221
manager.add("organizations:on-demand-metrics-extraction", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
220222
# Extract on demand metrics (experimental features)

src/sentry/incidents/action_handlers.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@
6262
from sentry.users.services.user.service import user_service
6363
from sentry.users.services.user_option import RpcUserOption, user_option_service
6464
from sentry.utils.email import MessageBuilder, get_email_addresses
65+
from sentry.workflow_engine.endpoints.serializers.detector_serializer import (
66+
DetectorSerializerResponse,
67+
)
6568
from sentry.workflow_engine.models.incident_groupopenperiod import IncidentGroupOpenPeriod
6669

6770
EMAIL_STATUS_DISPLAY = {TriggerStatus.ACTIVE: "Fired", TriggerStatus.RESOLVED: "Resolved"}
@@ -509,6 +512,7 @@ def generate_incident_trigger_email_context(
509512
trigger_threshold: float | None,
510513
user: User | RpcUser | None = None,
511514
notification_uuid: str | None = None,
515+
detector_serialized_response: DetectorSerializerResponse | None = None,
512516
) -> dict[str, Any]:
513517
from sentry.notifications.notification_action.utils import should_fire_workflow_actions
514518
from sentry.seer.anomaly_detection.types import AnomalyDetectionThresholdType
@@ -562,6 +566,7 @@ def generate_incident_trigger_email_context(
562566
open_period_context=open_period_context,
563567
size=ChartSize({"width": 600, "height": 200}),
564568
subscription=subscription,
569+
detector_serialized_response=detector_serialized_response,
565570
)
566571
except Exception:
567572
logging.exception("Error while attempting to build_metric_alert_chart")
@@ -682,6 +687,7 @@ def email_users(
682687
targets: list[tuple[int, str]],
683688
project: Project,
684689
notification_uuid: str | None = None,
690+
detector_serialized_response: DetectorSerializerResponse | None = None,
685691
) -> list[int]:
686692
users = user_service.get_many_by_id(ids=[user_id for user_id, _ in targets])
687693
sent_to_users = []
@@ -704,6 +710,7 @@ def email_users(
704710
trigger_threshold=alert_context.alert_threshold,
705711
user=user,
706712
notification_uuid=notification_uuid,
713+
detector_serialized_response=detector_serialized_response,
707714
)
708715
build_message(email_context, trigger_status, user_id).send_async(to=[email])
709716
sent_to_users.append(user_id)

src/sentry/incidents/charts.py

Lines changed: 79 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424
from sentry.snuba.utils import build_query_strings
2525
from sentry.users.models.user import User
2626
from sentry.users.services.user import RpcUser
27+
from sentry.workflow_engine.endpoints.serializers.detector_serializer import (
28+
DetectorSerializerResponse,
29+
)
2730
from sentry.workflow_engine.models import AlertRuleDetector
2831

2932
CRASH_FREE_SESSIONS = "percentage(sessions_crashed, sessions) AS _crash_rate_alert_aggregate"
@@ -134,6 +137,7 @@ def fetch_metric_issue_open_periods(
134137
time_period: Mapping[str, str],
135138
user: User | RpcUser | None = None,
136139
) -> list[Any]:
140+
detector_id = open_period_identifier
137141
try:
138142
if features.has(
139143
"organizations:workflow-engine-single-process-metric-issues",
@@ -147,23 +151,36 @@ def fetch_metric_issue_open_periods(
147151
# open_period_identifier is a metric detector ID -> get the alert rule ID
148152
open_period_identifier = alert_rule_detector.alert_rule_id
149153

150-
resp = client.get(
151-
auth=ApiKey(organization_id=organization.id, scope_list=["org:read"]),
152-
user=user,
153-
path=f"/organizations/{organization.slug}/incidents/",
154-
# TODO(iamrajjoshi): Use the correct endpoint and update the params
155-
params={
156-
"alertRule": open_period_identifier,
157-
"expand": "activities",
158-
"includeSnapshots": True,
159-
"project": -1,
160-
**time_period,
161-
},
162-
)
154+
if features.has(
155+
"organizations:new-metric-issue-charts",
156+
organization,
157+
):
158+
resp = client.get(
159+
auth=ApiKey(organization_id=organization.id, scope_list=["org:read"]),
160+
user=user,
161+
path=f"/organizations/{organization.slug}/open-periods/",
162+
params={
163+
"detectorId": detector_id,
164+
**time_period,
165+
},
166+
)
167+
else:
168+
resp = client.get(
169+
auth=ApiKey(organization_id=organization.id, scope_list=["org:read"]),
170+
user=user,
171+
path=f"/organizations/{organization.slug}/incidents/",
172+
params={
173+
"alertRule": open_period_identifier,
174+
"expand": "activities",
175+
"includeSnapshots": True,
176+
"project": -1,
177+
**time_period,
178+
},
179+
)
163180
# TODO (mifu67): temporary log that I'm going to remove after debugging. Get the data for old and new
164-
if organization.slug == "sentry" or organization.slug == "demo":
181+
if organization.slug == "mf-test-n7":
165182
logger.info(
166-
"fetching metric issue incidents",
183+
"fetching metric issue open periods",
167184
extra={
168185
"organization_id": organization.id,
169186
"open_period_id": open_period_identifier,
@@ -193,18 +210,30 @@ def build_metric_alert_chart(
193210
user: User | RpcUser | None = None,
194211
size: ChartSize | None = None,
195212
subscription: QuerySubscription | None = None,
213+
detector_serialized_response: DetectorSerializerResponse | None = None,
196214
) -> str | None:
197215
"""
198216
Builds the dataset required for metric alert chart the same way the frontend would
199217
"""
200218
dataset = Dataset(snuba_query.dataset)
201219
query_type = SnubaQuery.Type(snuba_query.type)
202220
is_crash_free_alert = query_type == SnubaQuery.Type.CRASH_RATE
203-
style = (
204-
ChartType.SLACK_METRIC_ALERT_SESSIONS
205-
if is_crash_free_alert
206-
else ChartType.SLACK_METRIC_ALERT_EVENTS
221+
using_new_charts = features.has(
222+
"organizations:new-metric-issue-charts",
223+
organization,
207224
)
225+
if is_crash_free_alert:
226+
style = (
227+
ChartType.SLACK_METRIC_DETECTOR_SESSIONS
228+
if using_new_charts
229+
else ChartType.SLACK_METRIC_ALERT_SESSIONS
230+
)
231+
else:
232+
style = (
233+
ChartType.SLACK_METRIC_DETECTOR_EVENTS
234+
if using_new_charts
235+
else ChartType.SLACK_METRIC_ALERT_EVENTS
236+
)
208237

209238
if open_period_context:
210239
time_period = incident_date_range(
@@ -220,17 +249,32 @@ def build_metric_alert_chart(
220249
"start": period_start.strftime(TIME_FORMAT),
221250
"end": timezone.now().strftime(TIME_FORMAT),
222251
}
223-
224-
chart_data = {
225-
"rule": alert_rule_serialized_response,
226-
"selectedIncident": selected_incident_serialized,
227-
"incidents": fetch_metric_issue_open_periods(
228-
organization,
229-
alert_context.action_identifier_id,
230-
time_period,
231-
user,
232-
),
233-
}
252+
if features.has(
253+
"organizations:new-metric-issue-charts",
254+
organization,
255+
):
256+
# TODO(mifu67): create detailed serializer for open period, pass here.
257+
# But we don't need it to render the chart, so it's fine for now.
258+
chart_data_detector = {
259+
"detector": detector_serialized_response,
260+
"open_periods": fetch_metric_issue_open_periods(
261+
organization,
262+
alert_context.action_identifier_id,
263+
time_period,
264+
user,
265+
),
266+
}
267+
else:
268+
chart_data_alert_rule = {
269+
"rule": alert_rule_serialized_response,
270+
"selectedIncident": selected_incident_serialized,
271+
"incidents": fetch_metric_issue_open_periods(
272+
organization,
273+
alert_context.action_identifier_id,
274+
time_period,
275+
user,
276+
),
277+
}
234278

235279
allow_mri = features.has(
236280
"organizations:insights-alerts",
@@ -264,6 +308,7 @@ def build_metric_alert_chart(
264308
)
265309
)
266310

311+
chart_data = {}
267312
query_params = {
268313
**env_params,
269314
**time_period,
@@ -303,6 +348,10 @@ def build_metric_alert_chart(
303348
)
304349

305350
try:
351+
if using_new_charts:
352+
chart_data.update(chart_data_detector)
353+
else:
354+
chart_data.update(chart_data_alert_rule)
306355
return charts.generate_chart(style, chart_data, size=size)
307356
except RuntimeError as exc:
308357
logger.error(

src/sentry/integrations/discord/actions/metric_alert.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424
)
2525
from sentry.models.organization import Organization
2626
from sentry.shared_integrations.exceptions import ApiError
27+
from sentry.workflow_engine.endpoints.serializers.detector_serializer import (
28+
DetectorSerializerResponse,
29+
)
2730

2831
from ..utils import logger
2932

@@ -36,6 +39,7 @@ def send_incident_alert_notification(
3639
open_period_context: OpenPeriodContext,
3740
alert_rule_serialized_response: AlertRuleSerializerResponse | None,
3841
incident_serialized_response: DetailedIncidentSerializerResponse | None,
42+
detector_serialized_response: DetectorSerializerResponse | None = None,
3943
notification_uuid: str | None = None,
4044
) -> bool:
4145
chart_url = None
@@ -53,6 +57,7 @@ def send_incident_alert_notification(
5357
open_period_context=open_period_context,
5458
selected_incident_serialized=incident_serialized_response,
5559
subscription=metric_issue_context.subscription,
60+
detector_serialized_response=detector_serialized_response,
5661
)
5762
except Exception as e:
5863
sentry_sdk.capture_exception(e)

src/sentry/integrations/slack/utils/notifications.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@
5454
from sentry.models.organization import Organization
5555
from sentry.notifications.notification_action.utils import should_fire_workflow_actions
5656
from sentry.notifications.utils.open_period import open_period_start_for_group
57+
from sentry.workflow_engine.endpoints.serializers.detector_serializer import (
58+
DetectorSerializerResponse,
59+
)
5760
from sentry.workflow_engine.models.action import Action
5861

5962
_logger = logging.getLogger(__name__)
@@ -193,6 +196,7 @@ def _build_notification_payload(
193196
open_period_context: OpenPeriodContext,
194197
alert_rule_serialized_response: AlertRuleSerializerResponse | None,
195198
incident_serialized_response: DetailedIncidentSerializerResponse | None,
199+
detector_serialized_response: DetectorSerializerResponse | None,
196200
notification_uuid: str | None,
197201
) -> tuple[str, str]:
198202
chart_url = None
@@ -210,6 +214,7 @@ def _build_notification_payload(
210214
open_period_context=open_period_context,
211215
selected_incident_serialized=incident_serialized_response,
212216
subscription=metric_issue_context.subscription,
217+
detector_serialized_response=detector_serialized_response,
213218
)
214219
except Exception as e:
215220
sentry_sdk.capture_exception(e)
@@ -410,6 +415,7 @@ def send_incident_alert_notification(
410415
open_period_context: OpenPeriodContext,
411416
alert_rule_serialized_response: AlertRuleSerializerResponse | None,
412417
incident_serialized_response: DetailedIncidentSerializerResponse | None,
418+
detector_serialized_response: DetectorSerializerResponse | None = None,
413419
notification_uuid: str | None = None,
414420
) -> bool:
415421
# Make sure organization integration is still active:
@@ -435,6 +441,7 @@ def send_incident_alert_notification(
435441
alert_rule_serialized_response=alert_rule_serialized_response,
436442
incident_serialized_response=incident_serialized_response,
437443
notification_uuid=notification_uuid,
444+
detector_serialized_response=detector_serialized_response,
438445
)
439446

440447
# TODO(iamrajjoshi): This will need to be updated once we plan out Metric Alerts rollout

src/sentry/models/groupopenperiod.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from django.contrib.postgres.fields import DateTimeRangeField
77
from django.contrib.postgres.fields.ranges import RangeBoundary, RangeOperators
88
from django.db import models, router, transaction
9+
from django.db.models import Q
910
from django.utils import timezone
1011

1112
from sentry import options
@@ -155,7 +156,9 @@ def get_open_periods_for_group(
155156
date_started__gte=query_start,
156157
).order_by("-date_started")
157158
if query_end:
158-
group_open_periods = group_open_periods.filter(date_ended__lte=query_end)
159+
group_open_periods = group_open_periods.filter(
160+
Q(date_ended__lte=query_end) | Q(date_ended__isnull=True)
161+
)
159162

160163
return group_open_periods[:limit]
161164

src/sentry/notifications/notification_action/metric_alert_registry/handlers/discord_metric_alert_handler.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from sentry.notifications.notification_action.metric_alert_registry.handlers.utils import (
1414
get_alert_rule_serializer,
1515
get_detailed_incident_serializer,
16+
get_detector_serializer,
1617
)
1718
from sentry.notifications.notification_action.registry import metric_alert_handler_registry
1819
from sentry.notifications.notification_action.types import BaseMetricAlertHandler
@@ -49,6 +50,7 @@ def send_alert(
4950
raise ValueError("Open period not found")
5051

5152
alert_rule_serialized_response = get_alert_rule_serializer(detector)
53+
detector_serialized_response = get_detector_serializer(detector)
5254
incident_serialized_response = get_detailed_incident_serializer(open_period)
5355

5456
logger.info(
@@ -67,4 +69,5 @@ def send_alert(
6769
notification_uuid=notification_uuid,
6870
alert_rule_serialized_response=alert_rule_serialized_response,
6971
incident_serialized_response=incident_serialized_response,
72+
detector_serialized_response=detector_serialized_response,
7073
)

src/sentry/notifications/notification_action/metric_alert_registry/handlers/email_metric_alert_handler.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from sentry.notifications.notification_action.metric_alert_registry.handlers.utils import (
1919
get_alert_rule_serializer,
2020
get_detailed_incident_serializer,
21+
get_detector_serializer,
2122
)
2223
from sentry.notifications.notification_action.registry import metric_alert_handler_registry
2324
from sentry.notifications.notification_action.types import BaseMetricAlertHandler
@@ -54,6 +55,7 @@ def send_alert(
5455
raise ValueError("Open period not found")
5556

5657
alert_rule_serialized_response = get_alert_rule_serializer(detector)
58+
detector_serialized_response = get_detector_serializer(detector)
5759
incident_serialized_response = get_detailed_incident_serializer(open_period)
5860

5961
recipients = list(
@@ -77,6 +79,7 @@ def send_alert(
7779
alert_context=alert_context,
7880
alert_rule_serialized_response=alert_rule_serialized_response,
7981
incident_serialized_response=incident_serialized_response,
82+
detector_serialized_response=detector_serialized_response,
8083
trigger_status=trigger_status,
8184
targets=targets,
8285
project=project,

src/sentry/notifications/notification_action/metric_alert_registry/handlers/slack_metric_alert_handler.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from sentry.notifications.notification_action.metric_alert_registry.handlers.utils import (
1515
get_alert_rule_serializer,
1616
get_detailed_incident_serializer,
17+
get_detector_serializer,
1718
)
1819
from sentry.notifications.notification_action.registry import metric_alert_handler_registry
1920
from sentry.notifications.notification_action.types import BaseMetricAlertHandler
@@ -46,6 +47,7 @@ def send_alert(
4647
raise ValueError("Open period not found")
4748

4849
alert_rule_serialized_response = get_alert_rule_serializer(detector)
50+
detector_serialized_response = get_detector_serializer(detector)
4951
incident_serialized_response = get_detailed_incident_serializer(open_period)
5052

5153
logger.info(
@@ -66,4 +68,5 @@ def send_alert(
6668
notification_uuid=notification_uuid,
6769
alert_rule_serialized_response=alert_rule_serialized_response,
6870
incident_serialized_response=incident_serialized_response,
71+
detector_serialized_response=detector_serialized_response,
6972
)

src/sentry/notifications/notification_action/metric_alert_registry/handlers/utils.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,23 @@
1212
WorkflowEngineIncidentSerializer,
1313
)
1414
from sentry.models.groupopenperiod import GroupOpenPeriod
15+
from sentry.workflow_engine.endpoints.serializers.detector_serializer import (
16+
DetectorSerializer,
17+
DetectorSerializerResponse,
18+
)
1519
from sentry.workflow_engine.models import Detector
1620

1721

1822
def get_alert_rule_serializer(detector: Detector) -> AlertRuleSerializerResponse:
1923
return serialize(detector, None, WorkflowEngineDetectorSerializer())
2024

2125

26+
def get_detector_serializer(detector: Detector) -> DetectorSerializerResponse:
27+
return serialize(detector, None, DetectorSerializer())
28+
29+
30+
# TODO(mifu67): These take an open period, get the incident, and call the incident
31+
# serializer on the incident. They will need to be replaced.
2232
def get_detailed_incident_serializer(
2333
open_period: GroupOpenPeriod,
2434
) -> DetailedIncidentSerializerResponse:

0 commit comments

Comments
 (0)