Skip to content

Commit 433839b

Browse files
ceorourkesnigdhas
andauthored
feat(aci): Write to IncidentGroupOpenPeriod (#97621)
Redo of #96221 after [adding an index](#97534) to the `GroupOpenPeriod`'s `data` column on `pending_incident_detector_id` which should hopefully not result in [the error](https://sentry.sentry.io/issues/6799376702/) anymore. There are no changes to this PR from the original one. --------- Co-authored-by: Snigdha Sharma <[email protected]>
1 parent cea58ec commit 433839b

File tree

5 files changed

+560
-0
lines changed

5 files changed

+560
-0
lines changed

src/sentry/incidents/logic.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@
101101
from sentry.utils.not_set import NOT_SET, NotSet
102102
from sentry.utils.snuba import is_measurement
103103
from sentry.workflow_engine.endpoints.validators.utils import toggle_detector
104+
from sentry.workflow_engine.models import IncidentGroupOpenPeriod
104105
from sentry.workflow_engine.models.detector import Detector
105106

106107
# We can return an incident as "windowed" which returns a range of points around the start of the incident
@@ -181,6 +182,10 @@ def create_incident(
181182
incident_type=incident_type.value,
182183
)
183184

185+
# If this is a metric alert incident, check for pending group relationships
186+
if alert_rule and incident_type == IncidentType.ALERT_TRIGGERED:
187+
IncidentGroupOpenPeriod.create_pending_relationships_for_incident(incident, alert_rule)
188+
184189
return incident
185190

186191

src/sentry/issues/ingest.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
save_grouphash_and_group,
2323
)
2424
from sentry.eventstore.models import Event, GroupEvent, augment_message_with_occurrence
25+
from sentry.incidents.grouptype import MetricIssue
2526
from sentry.issues.grouptype import FeedbackGroup, should_create_group
2627
from sentry.issues.issue_occurrence import IssueOccurrence, IssueOccurrenceData
2728
from sentry.issues.priority import PriorityChangeReason, update_priority
@@ -34,6 +35,7 @@
3435
from sentry.utils import json, metrics, redis
3536
from sentry.utils.strings import truncatechars
3637
from sentry.utils.tag_normalization import normalized_sdk_tag_from_event
38+
from sentry.workflow_engine.models import IncidentGroupOpenPeriod
3739
from sentry.workflow_engine.models.detector_group import DetectorGroup
3840

3941
issue_rate_limiter = RedisSlidingWindowRateLimiter(
@@ -70,6 +72,23 @@ def save_issue_occurrence(
7072
group_info.group.project, environment, release, [group_info]
7173
)
7274
_get_or_create_group_release(environment, release, event, [group_info])
75+
76+
# Create IncidentGroupOpenPeriod relationship for metric issues
77+
if occurrence.type == MetricIssue:
78+
open_period = get_latest_open_period(group_info.group)
79+
if open_period:
80+
IncidentGroupOpenPeriod.create_from_occurrence(
81+
occurrence, group_info.group, open_period
82+
)
83+
else:
84+
logger.error(
85+
"save_issue_occurrence.no_open_period",
86+
extra={
87+
"group_id": group_info.group.id,
88+
"occurrence_id": occurrence.id,
89+
},
90+
)
91+
7392
send_issue_occurrence_to_eventstream(event, occurrence, group_info)
7493
return occurrence, group_info
7594

src/sentry/workflow_engine/models/incident_groupopenperiod.py

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import logging
2+
13
from django.db import models
24
from django.db.models import Q
35

@@ -8,6 +10,12 @@
810
FlexibleForeignKey,
911
region_silo_model,
1012
)
13+
from sentry.incidents.models.alert_rule import AlertRule
14+
from sentry.incidents.models.incident import Incident
15+
from sentry.models.groupopenperiod import GroupOpenPeriod
16+
from sentry.workflow_engine.models.alertrule_detector import AlertRuleDetector
17+
18+
logger = logging.getLogger(__name__)
1119

1220

1321
@region_silo_model
@@ -32,3 +40,157 @@ class Meta:
3240
name="inc_id_inc_identifier_together",
3341
)
3442
]
43+
44+
@classmethod
45+
def create_from_occurrence(self, occurrence, group, open_period):
46+
"""
47+
Creates an IncidentGroupOpenPeriod relationship from an issue occurrence.
48+
This method handles the case where the incident might not exist yet.
49+
50+
Args:
51+
occurrence: The IssueOccurrence that triggered the group creation
52+
group: The Group that was created
53+
open_period: The GroupOpenPeriod for the group
54+
"""
55+
try:
56+
# Extract alert_id from evidence_data using the detector_id
57+
detector_id = occurrence.evidence_data.get("detector_id")
58+
if detector_id:
59+
alert_id = AlertRuleDetector.objects.get(detector_id=detector_id).alert_rule_id
60+
else:
61+
raise Exception("No detector_id found in evidence_data for metric issue")
62+
63+
# Try to find the active incident for this alert rule and project
64+
try:
65+
alert_rule = AlertRule.objects.get(id=alert_id)
66+
incident = Incident.objects.get_active_incident(
67+
alert_rule=alert_rule,
68+
project=group.project,
69+
)
70+
except AlertRule.DoesNotExist:
71+
logger.warning(
72+
"AlertRule not found for alert_id",
73+
extra={
74+
"alert_id": alert_id,
75+
"group_id": group.id,
76+
},
77+
)
78+
incident = None
79+
80+
if incident:
81+
# Incident exists, create the relationship immediately
82+
return self.create_relationship(incident, open_period)
83+
else:
84+
# Incident doesn't exist yet, create a placeholder relationship
85+
# that will be updated when the incident is created
86+
return self.create_placeholder_relationship(detector_id, open_period, group.project)
87+
88+
except Exception as e:
89+
logger.exception(
90+
"Failed to create IncidentGroupOpenPeriod relationship",
91+
extra={
92+
"group_id": group.id,
93+
"occurrence_id": occurrence.id,
94+
"error": str(e),
95+
},
96+
)
97+
return None
98+
99+
@classmethod
100+
def create_relationship(self, incident, open_period):
101+
"""
102+
Creates IncidentGroupOpenPeriod relationship.
103+
104+
Args:
105+
incident: The Incident to link
106+
open_period: The GroupOpenPeriod to link
107+
"""
108+
try:
109+
incident_group_open_period, _ = self.objects.get_or_create(
110+
group_open_period=open_period,
111+
defaults={
112+
"incident_id": incident.id,
113+
"incident_identifier": incident.identifier,
114+
},
115+
)
116+
117+
return incident_group_open_period
118+
119+
except Exception as e:
120+
logger.exception(
121+
"Failed to create/update IncidentGroupOpenPeriod relationship",
122+
extra={
123+
"incident_id": incident.id,
124+
"open_period_id": open_period.id,
125+
"error": str(e),
126+
},
127+
)
128+
return None
129+
130+
@classmethod
131+
def create_placeholder_relationship(self, detector_id, open_period, project):
132+
"""
133+
Creates a placeholder relationship when the incident doesn't exist yet.
134+
This will be updated when the incident is created.
135+
136+
Args:
137+
detector_id: The detector ID
138+
open_period: The GroupOpenPeriod to link
139+
project: The project for the group
140+
"""
141+
try:
142+
# Store the alert_id in the open_period data for later lookup
143+
data = open_period.data or {}
144+
data["pending_incident_detector_id"] = detector_id
145+
open_period.update(data=data)
146+
147+
return None
148+
149+
except Exception as e:
150+
logger.exception(
151+
"Failed to create placeholder IncidentGroupOpenPeriod relationship",
152+
extra={
153+
"detector_id": detector_id,
154+
"open_period_id": open_period.id,
155+
"error": str(e),
156+
},
157+
)
158+
return None
159+
160+
@classmethod
161+
def create_pending_relationships_for_incident(self, incident, alert_rule):
162+
"""
163+
Creates IncidentGroupOpenPeriod relationships for any groups that were created
164+
before the incident. This handles the timing issue where groups might be created
165+
before incidents.
166+
167+
Args:
168+
incident: The Incident that was just created
169+
alert_rule: The AlertRule that triggered the incident
170+
"""
171+
try:
172+
# Find all open periods that have a pending incident detector_id for this alert rule
173+
detector_id = AlertRuleDetector.objects.get(alert_rule_id=alert_rule.id).detector_id
174+
pending_open_periods = GroupOpenPeriod.objects.filter(
175+
data__pending_incident_detector_id=detector_id,
176+
group__project__in=incident.projects.all(),
177+
)
178+
179+
for open_period in pending_open_periods:
180+
# Create the relationship
181+
relationship = self.create_relationship(incident, open_period)
182+
if relationship:
183+
# Remove the pending flag from the open_period data
184+
data = open_period.data or {}
185+
data.pop("pending_incident_detector_id", None)
186+
open_period.update(data=data)
187+
188+
except Exception as e:
189+
logger.exception(
190+
"Failed to create pending IncidentGroupOpenPeriod relationships",
191+
extra={
192+
"incident_id": incident.id,
193+
"alert_rule_id": alert_rule.id,
194+
"error": str(e),
195+
},
196+
)
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
from unittest.mock import patch
2+
3+
from django.utils import timezone
4+
5+
from sentry.event_manager import GroupInfo
6+
from sentry.incidents.grouptype import MetricIssue
7+
from sentry.issues.grouptype import FeedbackGroup
8+
from sentry.issues.ingest import save_issue_occurrence
9+
from sentry.issues.issue_occurrence import IssueOccurrence, IssueOccurrenceData
10+
from sentry.models.group import GroupStatus
11+
from sentry.models.groupopenperiod import GroupOpenPeriod
12+
from sentry.testutils.cases import TestCase
13+
from sentry.testutils.helpers import with_feature
14+
from sentry.workflow_engine.models import IncidentGroupOpenPeriod
15+
16+
17+
class IncidentGroupOpenPeriodIntegrationTest(TestCase):
18+
def setUp(self) -> None:
19+
super().setUp()
20+
self.organization = self.create_organization()
21+
self.project = self.create_project(organization=self.organization)
22+
self.alert_rule = self.create_alert_rule(
23+
organization=self.organization,
24+
projects=[self.project],
25+
name="Test Alert Rule",
26+
)
27+
self.detector = self.create_detector(
28+
project=self.project,
29+
name="Test Detector",
30+
)
31+
self.alert_rule_detector = self.create_alert_rule_detector(
32+
alert_rule_id=self.alert_rule.id,
33+
detector=self.detector,
34+
)
35+
36+
def save_issue_occurrence(
37+
self, group_type: int = MetricIssue.type_id
38+
) -> tuple[IssueOccurrence, GroupInfo]:
39+
event = self.store_event(
40+
data={"timestamp": timezone.now().isoformat()}, project_id=self.project.id
41+
)
42+
43+
occurrence_data: IssueOccurrenceData = {
44+
"id": "1",
45+
"project_id": self.project.id,
46+
"event_id": event.event_id,
47+
"fingerprint": ["test-fingerprint"],
48+
"issue_title": "Test Issue",
49+
"subtitle": "Test Subtitle",
50+
"resource_id": None,
51+
"evidence_data": {"detector_id": self.detector.id},
52+
"evidence_display": [
53+
{"name": "Test Evidence", "value": "Test Value", "important": True}
54+
],
55+
"type": group_type,
56+
"detection_time": timezone.now().timestamp(),
57+
"level": "error",
58+
"culprit": "test-culprit",
59+
}
60+
61+
with patch("sentry.issues.ingest.eventstream") as _:
62+
occurrence, group_info = save_issue_occurrence(occurrence_data, event)
63+
64+
assert group_info is not None
65+
assert group_info.group.type == group_type
66+
return occurrence, group_info
67+
68+
@with_feature("organizations:issue-open-periods")
69+
def test_save_issue_occurrence_creates_relationship_when_incident_exists(self) -> None:
70+
"""Test that save_issue_occurrence creates the relationship when incident exists"""
71+
incident = self.create_incident(
72+
organization=self.organization,
73+
title="Test Incident",
74+
date_started=timezone.now(),
75+
alert_rule=self.alert_rule,
76+
)
77+
78+
_, group_info = self.save_issue_occurrence()
79+
group = group_info.group
80+
assert group is not None
81+
82+
open_period = GroupOpenPeriod.objects.get(group=group)
83+
item = IncidentGroupOpenPeriod.objects.get(group_open_period=open_period)
84+
assert item.incident_id == incident.id
85+
assert item.incident_identifier == incident.identifier
86+
87+
@with_feature("organizations:issue-open-periods")
88+
def test_save_issue_occurrence_creates_placeholder_when_incident_doesnt_exist(self) -> None:
89+
"""Test that save_issue_occurrence creates placeholder when incident doesn't exist"""
90+
_, group_info = self.save_issue_occurrence()
91+
group = group_info.group
92+
assert group is not None
93+
94+
open_period = GroupOpenPeriod.objects.get(group=group)
95+
assert open_period.data["pending_incident_detector_id"] == self.detector.id
96+
97+
assert not IncidentGroupOpenPeriod.objects.filter(group_open_period=open_period).exists()
98+
99+
@with_feature("organizations:issue-open-periods")
100+
def test_save_issue_occurrence_creates_relationship_for_existing_group(self) -> None:
101+
"""Test that save_issue_occurrence creates relationship for existing groups"""
102+
incident = self.create_incident(
103+
organization=self.organization,
104+
title="Test Incident",
105+
date_started=timezone.now(),
106+
alert_rule=self.alert_rule,
107+
)
108+
109+
_, group_info = self.save_issue_occurrence()
110+
group = group_info.group
111+
assert group is not None
112+
113+
assert GroupOpenPeriod.objects.filter(group=group, project=self.project).exists()
114+
115+
group.update(status=GroupStatus.RESOLVED)
116+
open_period = GroupOpenPeriod.objects.get(group=group, project=self.project)
117+
open_period.update(date_ended=timezone.now())
118+
119+
_, group_info = self.save_issue_occurrence()
120+
group = group_info.group
121+
assert group is not None
122+
123+
item = IncidentGroupOpenPeriod.objects.get(group_open_period=open_period)
124+
assert item.incident_id == incident.id
125+
assert item.incident_identifier == incident.identifier
126+
127+
@with_feature("organizations:issue-open-periods")
128+
def test_save_issue_occurrence_no_relationship_for_non_metric_issues(self) -> None:
129+
# Test that save_issue_occurrence doesn't create relationships for non-metric issues
130+
_, group_info = self.save_issue_occurrence(group_type=FeedbackGroup.type_id)
131+
132+
group = group_info.group
133+
assert group is not None
134+
135+
open_period = GroupOpenPeriod.objects.get(group=group)
136+
assert not IncidentGroupOpenPeriod.objects.filter(group_open_period=open_period).exists()
137+
assert "pending_incident_alert_id" not in open_period.data

0 commit comments

Comments
 (0)