Skip to content

Commit b95d638

Browse files
authored
feat(aci): Ensure DetectorGroup for recurring Groups (#103419)
We want DetectorGroups for all Groups that can associate with a Detector. We already have path to add it for all _new_ groups, but backfilling existing groups is a little tricky, and by doing it incrementally be recurrence here, we can make DetectorGroup reliable for Detector lookup for essentially any Group the workflow engine is asked to process. We may still need a backfill for completeness if we can't wait until the Group retention window for completeness.
1 parent 7839023 commit b95d638

File tree

5 files changed

+172
-2
lines changed

5 files changed

+172
-2
lines changed

src/sentry/event_manager.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,10 @@
146146
from sentry.utils.safe import get_path, safe_execute, setdefault_path, trim
147147
from sentry.utils.sdk import set_span_attribute
148148
from sentry.utils.tag_normalization import normalized_sdk_tag_from_event
149-
from sentry.workflow_engine.processors.detector import associate_new_group_with_detector
149+
from sentry.workflow_engine.processors.detector import (
150+
associate_new_group_with_detector,
151+
ensure_association_with_detector,
152+
)
150153

151154
from .utils.event_tracker import TransactionStageStatus, track_sampled_event
152155

@@ -1434,6 +1437,9 @@ def handle_existing_grouphash(
14341437
release=job["release"],
14351438
)
14361439

1440+
# Ensure the group has a DetectorGroup association for existing groups
1441+
ensure_association_with_detector(group)
1442+
14371443
return GroupInfo(group=group, is_new=False, is_regression=is_regression)
14381444

14391445

src/sentry/issues/ingest.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,10 @@
3636
from sentry.utils.strings import truncatechars
3737
from sentry.utils.tag_normalization import normalized_sdk_tag_from_event
3838
from sentry.workflow_engine.models import IncidentGroupOpenPeriod
39-
from sentry.workflow_engine.processors.detector import associate_new_group_with_detector
39+
from sentry.workflow_engine.processors.detector import (
40+
associate_new_group_with_detector,
41+
ensure_association_with_detector,
42+
)
4043

4144
issue_rate_limiter = RedisSlidingWindowRateLimiter(
4245
**settings.SENTRY_ISSUE_PLATFORM_RATE_LIMITER_OPTIONS
@@ -320,6 +323,11 @@ def save_issue_from_occurrence(
320323
is_regression = _process_existing_aggregate(group, group_event, issue_kwargs, release)
321324
group_info = GroupInfo(group=group, is_new=False, is_regression=is_regression)
322325

326+
detector_id = None
327+
if occurrence.evidence_data:
328+
detector_id = occurrence.evidence_data.get("detector_id")
329+
ensure_association_with_detector(group, detector_id)
330+
323331
# if it's a regression and the priority changed, we should update the existing GroupOpenPeriodActivity
324332
# row if applicable. Otherwise, we should record a new row if applicable.
325333
if (

src/sentry/options/defaults.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3177,6 +3177,13 @@
31773177
flags=FLAG_AUTOMATOR_MODIFIABLE,
31783178
)
31793179

3180+
register(
3181+
"workflow_engine.ensure_detector_association",
3182+
type=Bool,
3183+
default=True,
3184+
flags=FLAG_AUTOMATOR_MODIFIABLE,
3185+
)
3186+
31803187
register(
31813188
"grouping.grouphash_metadata.ingestion_writes_enabled",
31823189
type=Bool,

src/sentry/workflow_engine/processors/detector.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,3 +413,69 @@ def associate_new_group_with_detector(group: Group, detector_id: int | None = No
413413
tags={"group_type": group.type, "result": "success"},
414414
)
415415
return True
416+
417+
418+
def ensure_association_with_detector(group: Group, detector_id: int | None = None) -> bool:
419+
"""
420+
Ensure a Group has a DetectorGroup association, creating it if missing.
421+
Backdates date_added to group.first_seen for gradual backfill of existing groups.
422+
"""
423+
if not options.get("workflow_engine.ensure_detector_association"):
424+
return False
425+
426+
# Common case: it exists, we verify and move on.
427+
if DetectorGroup.objects.filter(group_id=group.id).exists():
428+
return True
429+
430+
# Association is missing, determine the detector_id if not provided
431+
if detector_id is None:
432+
# For error Groups, we know there is a Detector and we can find it by project.
433+
if group.type == ErrorGroupType.type_id:
434+
try:
435+
detector_id = Detector.get_error_detector_for_project(group.project.id).id
436+
except Detector.DoesNotExist:
437+
logger.warning(
438+
"ensure_association_with_detector_detector_not_found",
439+
extra={
440+
"group_id": group.id,
441+
"group_type": group.type,
442+
"project_id": group.project.id,
443+
},
444+
)
445+
return False
446+
else:
447+
return False
448+
else:
449+
# Check if the explicitly provided detector exists. If not, create DetectorGroup
450+
# with null detector_id to make it clear that we were associated with a detector
451+
# that no longer exists.
452+
if not Detector.objects.filter(id=detector_id).exists():
453+
detector_group, created = DetectorGroup.objects.get_or_create(
454+
group_id=group.id,
455+
defaults={"detector_id": None},
456+
)
457+
if created:
458+
# Backdate the date_added to match the group's first_seen
459+
DetectorGroup.objects.filter(id=detector_group.id).update(
460+
date_added=group.first_seen
461+
)
462+
metrics.incr(
463+
"workflow_engine.ensure_association_with_detector.created",
464+
tags={"group_type": group.type},
465+
)
466+
return True
467+
468+
detector_group, created = DetectorGroup.objects.get_or_create(
469+
group_id=group.id,
470+
defaults={"detector_id": detector_id},
471+
)
472+
473+
if created:
474+
# Backdate the date_added to match the group's first_seen
475+
DetectorGroup.objects.filter(id=detector_group.id).update(date_added=group.first_seen)
476+
metrics.incr(
477+
"workflow_engine.ensure_association_with_detector.created",
478+
tags={"group_type": group.type},
479+
)
480+
481+
return True

tests/sentry/workflow_engine/processors/test_detector.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
from sentry.workflow_engine.models.detector_group import DetectorGroup
2929
from sentry.workflow_engine.processors.detector import (
3030
associate_new_group_with_detector,
31+
ensure_association_with_detector,
3132
get_detector_by_event,
3233
get_detectors_by_groupevents_bulk,
3334
process_detectors,
@@ -1070,3 +1071,85 @@ def test_deleted_detector_creates_null_association(self) -> None:
10701071
detector_group = DetectorGroup.objects.get(group_id=group.id)
10711072
assert detector_group.detector_id is None
10721073
assert detector_group.group_id == group.id
1074+
1075+
1076+
class TestEnsureAssociationWithDetector(TestCase):
1077+
def setUp(self) -> None:
1078+
super().setUp()
1079+
self.metric_detector = self.create_detector(project=self.project, type="metric_issue")
1080+
self.error_detector = self.create_detector(project=self.project, type="error")
1081+
self.options_context = self.options({"workflow_engine.ensure_detector_association": True})
1082+
self.options_context.__enter__()
1083+
1084+
def tearDown(self) -> None:
1085+
self.options_context.__exit__(None, None, None)
1086+
super().tearDown()
1087+
1088+
def test_feature_disabled_returns_false(self) -> None:
1089+
group = self.create_group(project=self.project, type=ErrorGroupType.type_id)
1090+
1091+
with self.options({"workflow_engine.ensure_detector_association": False}):
1092+
assert not ensure_association_with_detector(group)
1093+
assert not DetectorGroup.objects.filter(group_id=group.id).exists()
1094+
1095+
def test_already_exists_returns_true(self) -> None:
1096+
group = self.create_group(project=self.project, type=ErrorGroupType.type_id)
1097+
DetectorGroup.objects.create(detector=self.error_detector, group=group)
1098+
1099+
assert ensure_association_with_detector(group)
1100+
assert DetectorGroup.objects.filter(group_id=group.id).count() == 1
1101+
1102+
def test_error_group_creates_association(self) -> None:
1103+
group = self.create_group(project=self.project, type=ErrorGroupType.type_id)
1104+
1105+
assert ensure_association_with_detector(group)
1106+
detector_group = DetectorGroup.objects.get(group_id=group.id)
1107+
assert detector_group.detector_id == self.error_detector.id
1108+
assert detector_group.group_id == group.id
1109+
1110+
def test_metric_group_with_detector_id(self) -> None:
1111+
group = self.create_group(project=self.project, type=MetricIssue.type_id)
1112+
1113+
assert ensure_association_with_detector(group, self.metric_detector.id)
1114+
detector_group = DetectorGroup.objects.get(group_id=group.id)
1115+
assert detector_group.detector_id == self.metric_detector.id
1116+
assert detector_group.group_id == group.id
1117+
1118+
def test_feedback_group_returns_false(self) -> None:
1119+
group = self.create_group(project=self.project, type=FeedbackGroup.type_id)
1120+
1121+
assert not ensure_association_with_detector(group)
1122+
assert not DetectorGroup.objects.filter(group_id=group.id).exists()
1123+
1124+
def test_deleted_detector_creates_null_association(self) -> None:
1125+
group = self.create_group(project=self.project, type=MetricIssue.type_id)
1126+
deleted_detector_id = self.metric_detector.id
1127+
1128+
self.metric_detector.delete()
1129+
1130+
assert ensure_association_with_detector(group, deleted_detector_id)
1131+
1132+
detector_group = DetectorGroup.objects.get(group_id=group.id)
1133+
assert detector_group.detector_id is None
1134+
assert detector_group.group_id == group.id
1135+
1136+
def test_backdates_date_added_to_group_first_seen(self) -> None:
1137+
group = self.create_group(project=self.project, type=ErrorGroupType.type_id)
1138+
1139+
assert ensure_association_with_detector(group)
1140+
detector_group = DetectorGroup.objects.get(group_id=group.id)
1141+
assert detector_group.date_added == group.first_seen
1142+
1143+
def test_race_condition_handled(self) -> None:
1144+
group = self.create_group(project=self.project, type=ErrorGroupType.type_id)
1145+
1146+
assert ensure_association_with_detector(group)
1147+
assert ensure_association_with_detector(group)
1148+
assert DetectorGroup.objects.filter(group_id=group.id).count() == 1
1149+
1150+
def test_detector_not_found(self) -> None:
1151+
group = self.create_group(project=self.project, type=ErrorGroupType.type_id)
1152+
self.error_detector.delete()
1153+
1154+
assert not ensure_association_with_detector(group)
1155+
assert not DetectorGroup.objects.filter(group_id=group.id).exists()

0 commit comments

Comments
 (0)