Skip to content

Commit 450bde5

Browse files
wedamijaandrewshie-sentry
authored andcommitted
feat(crons): Start dual writing DataSource and Detector rows for crons (#97542)
We want to have these rows available so that we can start linking crons to workflows. <!-- Describe your PR here. -->
1 parent c13409d commit 450bde5

File tree

6 files changed

+171
-2
lines changed

6 files changed

+171
-2
lines changed

src/sentry/monitors/consumers/monitor_consumer.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
from sentry.monitors.system_incidents import update_check_in_volume
7171
from sentry.monitors.types import CheckinItem
7272
from sentry.monitors.utils import (
73+
ensure_cron_detector,
7374
get_new_timeout_at,
7475
get_timeout_at,
7576
signal_first_checkin,
@@ -165,6 +166,7 @@ def _ensure_monitor_with_config(
165166
"is_upserting": True,
166167
},
167168
)
169+
ensure_cron_detector(monitor)
168170
if created:
169171
signal_monitor_created(project, None, True, monitor, None)
170172

src/sentry/monitors/endpoints/organization_monitor_index.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,11 @@
4747
MonitorSerializer,
4848
MonitorSerializerResponse,
4949
)
50-
from sentry.monitors.utils import create_issue_alert_rule, signal_monitor_created
50+
from sentry.monitors.utils import (
51+
create_issue_alert_rule,
52+
ensure_cron_detector,
53+
signal_monitor_created,
54+
)
5155
from sentry.monitors.validators import MonitorBulkEditValidator, MonitorValidator
5256
from sentry.search.utils import tokenize_query
5357
from sentry.types.actor import Actor
@@ -291,6 +295,8 @@ def post(self, request: AuthenticatedHttpRequest, organization) -> Response:
291295
except MonitorLimitsExceeded as e:
292296
return self.respond({type(e).__name__: str(e)}, status=403)
293297

298+
ensure_cron_detector(monitor)
299+
294300
# Attempt to assign a seat for this monitor
295301
seat_outcome = quotas.backend.assign_monitor_seat(monitor)
296302
if seat_outcome != Outcome.ACCEPTED:

src/sentry/monitors/utils.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import logging
12
from collections import defaultdict
23
from datetime import datetime, timedelta
34

@@ -13,6 +14,7 @@
1314
from sentry.models.rule import Rule, RuleActivity, RuleActivityType, RuleSource
1415
from sentry.monitors.constants import DEFAULT_CHECKIN_MARGIN, MAX_TIMEOUT, TIMEOUT
1516
from sentry.monitors.models import CheckInStatus, Monitor, MonitorCheckIn
17+
from sentry.monitors.types import DATA_SOURCE_CRON_MONITOR
1618
from sentry.projects.project_rules.creator import ProjectRuleCreator
1719
from sentry.projects.project_rules.updater import ProjectRuleUpdater
1820
from sentry.signals import (
@@ -23,7 +25,11 @@
2325
from sentry.users.models.user import User
2426
from sentry.utils.audit import create_audit_entry, create_system_audit_entry
2527
from sentry.utils.auth import AuthenticatedHttpRequest
28+
from sentry.utils.db import atomic_transaction
2629
from sentry.utils.projectflags import set_project_flag_and_signal
30+
from sentry.workflow_engine.models import DataSource, DataSourceDetector, Detector
31+
32+
logger = logging.getLogger(__name__)
2733

2834

2935
def signal_first_checkin(project: Project, monitor: Monitor):
@@ -383,3 +389,26 @@ def update_issue_alert_rule(
383389
)
384390

385391
return issue_alert_rule.id
392+
393+
394+
def ensure_cron_detector(monitor: Monitor):
395+
from sentry.issues.grouptype import MonitorCheckInFailure
396+
397+
try:
398+
with atomic_transaction(using=router.db_for_write(DataSource)):
399+
data_source, created = DataSource.objects.get_or_create(
400+
type=DATA_SOURCE_CRON_MONITOR,
401+
organization_id=monitor.organization_id,
402+
source_id=str(monitor.id),
403+
)
404+
if created:
405+
detector = Detector.objects.create(
406+
type=MonitorCheckInFailure.slug,
407+
project_id=monitor.project_id,
408+
name=monitor.name,
409+
owner_user_id=monitor.owner_user_id,
410+
owner_team_id=monitor.owner_team_id,
411+
)
412+
DataSourceDetector.objects.create(data_source=data_source, detector=detector)
413+
except Exception:
414+
logger.exception("Error creating cron detector")

tests/sentry/monitors/consumers/test_monitor_consumer.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,14 @@
2929
ScheduleType,
3030
)
3131
from sentry.monitors.processing_errors.errors import ProcessingErrorsException, ProcessingErrorType
32-
from sentry.monitors.types import CheckinItem
32+
from sentry.monitors.types import DATA_SOURCE_CRON_MONITOR, CheckinItem
3333
from sentry.testutils.asserts import assert_org_audit_log_exists
3434
from sentry.testutils.cases import TestCase
3535
from sentry.testutils.helpers.options import override_options
3636
from sentry.testutils.outbox import outbox_runner
3737
from sentry.utils import json
3838
from sentry.utils.outcomes import Outcome
39+
from sentry.workflow_engine.models import Detector
3940

4041

4142
class ExpectNoProcessingError:
@@ -572,6 +573,10 @@ def test_monitor_create(self) -> None:
572573
monitor_environment.next_checkin_latest
573574
== monitor_environment.monitor.get_next_expected_checkin_latest(checkin.date_added)
574575
)
576+
assert Detector.objects.filter(
577+
datasource__type=DATA_SOURCE_CRON_MONITOR,
578+
datasource__source_id=str(monitor_environment.monitor_id),
579+
).exists()
575580

576581
def test_monitor_create_owner(self) -> None:
577582
self.send_checkin(

tests/sentry/monitors/endpoints/test_organization_monitor_index.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,14 @@
1212
from sentry.constants import ObjectStatus
1313
from sentry.models.rule import Rule, RuleSource
1414
from sentry.monitors.models import Monitor, MonitorStatus, ScheduleType
15+
from sentry.monitors.types import DATA_SOURCE_CRON_MONITOR
1516
from sentry.quotas.base import SeatAssignmentResult
1617
from sentry.slug.errors import DEFAULT_SLUG_ERROR_MESSAGE
1718
from sentry.testutils.asserts import assert_org_audit_log_exists
1819
from sentry.testutils.cases import MonitorTestCase
1920
from sentry.testutils.outbox import outbox_runner
2021
from sentry.utils.outcomes import Outcome
22+
from sentry.workflow_engine.models import Detector
2123

2224

2325
class ListOrganizationMonitorsTest(MonitorTestCase):
@@ -402,6 +404,11 @@ def test_simple(self, mock_record: MagicMock) -> None:
402404
data={"upsert": False, **monitor.get_audit_log_data()},
403405
)
404406

407+
assert Detector.objects.filter(
408+
datasource__type=DATA_SOURCE_CRON_MONITOR,
409+
datasource__source_id=str(monitor.id),
410+
).exists()
411+
405412
self.project.refresh_from_db()
406413
assert self.project.flags.has_cron_monitors
407414

tests/sentry/monitors/test_utils.py

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
from unittest.mock import patch
2+
3+
from django.db import IntegrityError
4+
5+
from sentry.issues.grouptype import MonitorIncidentType
6+
from sentry.monitors.types import DATA_SOURCE_CRON_MONITOR
7+
from sentry.monitors.utils import ensure_cron_detector
8+
from sentry.testutils.cases import TestCase
9+
from sentry.workflow_engine.models import DataSource, DataSourceDetector, Detector
10+
11+
12+
class EnsureCronDetectorTest(TestCase):
13+
def setUp(self):
14+
super().setUp()
15+
self.monitor = self.create_monitor(owner_user_id=None)
16+
17+
def test_creates_data_source_and_detector_for_new_monitor(self):
18+
assert not DataSource.objects.filter(
19+
type=DATA_SOURCE_CRON_MONITOR,
20+
organization_id=self.monitor.organization_id,
21+
source_id=str(self.monitor.id),
22+
).exists()
23+
24+
ensure_cron_detector(self.monitor)
25+
data_source = DataSource.objects.get(
26+
type=DATA_SOURCE_CRON_MONITOR,
27+
organization_id=self.monitor.organization_id,
28+
source_id=str(self.monitor.id),
29+
)
30+
assert data_source is not None
31+
detector = Detector.objects.get(
32+
type=MonitorIncidentType.slug,
33+
project_id=self.monitor.project_id,
34+
name=self.monitor.name,
35+
)
36+
assert detector is not None
37+
assert detector.owner_user_id == self.monitor.owner_user_id
38+
assert detector.owner_team_id == self.monitor.owner_team_id
39+
assert DataSourceDetector.objects.filter(
40+
data_source=data_source,
41+
detector=detector,
42+
).exists()
43+
44+
def test_idempotent_for_existing_data_source(self):
45+
ensure_cron_detector(self.monitor)
46+
data_source = DataSource.objects.get(
47+
type=DATA_SOURCE_CRON_MONITOR,
48+
organization_id=self.monitor.organization_id,
49+
source_id=str(self.monitor.id),
50+
)
51+
detector = Detector.objects.get(
52+
type=MonitorIncidentType.slug,
53+
project_id=self.monitor.project_id,
54+
name=self.monitor.name,
55+
)
56+
link = DataSourceDetector.objects.get(
57+
data_source=data_source,
58+
detector=detector,
59+
)
60+
ensure_cron_detector(self.monitor)
61+
data_source_after = DataSource.objects.get(
62+
type=DATA_SOURCE_CRON_MONITOR,
63+
organization_id=self.monitor.organization_id,
64+
source_id=str(self.monitor.id),
65+
)
66+
detector_after = Detector.objects.get(
67+
type=MonitorIncidentType.slug,
68+
project_id=self.monitor.project_id,
69+
name=self.monitor.name,
70+
)
71+
link_after = DataSourceDetector.objects.get(
72+
data_source=data_source,
73+
detector=detector,
74+
)
75+
assert data_source.id == data_source_after.id
76+
assert detector.id == detector_after.id
77+
assert link.id == link_after.id
78+
79+
def test_with_owner_user(self):
80+
self.monitor.owner_user_id = self.user.id
81+
self.monitor.save()
82+
ensure_cron_detector(self.monitor)
83+
detector = Detector.objects.get(
84+
type=MonitorIncidentType.slug,
85+
project_id=self.monitor.project_id,
86+
)
87+
assert detector.owner_user_id == self.user.id
88+
assert detector.owner_team_id is None
89+
90+
def test_with_no_owner(self):
91+
ensure_cron_detector(self.monitor)
92+
93+
detector = Detector.objects.get(
94+
type=MonitorIncidentType.slug,
95+
project_id=self.monitor.project_id,
96+
)
97+
assert detector.owner_user_id is None
98+
assert detector.owner_team_id is None
99+
100+
def test_handles_database_errors_gracefully(self):
101+
with (
102+
patch("sentry.monitors.utils.logger") as mock_logger,
103+
patch("sentry.monitors.utils.DataSource.objects.get_or_create") as mock_get_or_create,
104+
):
105+
mock_get_or_create.side_effect = IntegrityError("Database error")
106+
107+
ensure_cron_detector(self.monitor)
108+
mock_logger.exception.assert_called_once_with("Error creating cron detector")
109+
assert not DataSource.objects.filter(
110+
type=DATA_SOURCE_CRON_MONITOR, source_id=str(self.monitor.id)
111+
).exists()
112+
113+
def test_atomic_transaction_rollback(self):
114+
with patch("sentry.monitors.utils.Detector.objects.create") as mock_create:
115+
mock_create.side_effect = IntegrityError("Cannot create detector")
116+
117+
ensure_cron_detector(self.monitor)
118+
assert not DataSource.objects.filter(
119+
type=DATA_SOURCE_CRON_MONITOR, source_id=str(self.monitor.id)
120+
).exists()

0 commit comments

Comments
 (0)