Skip to content

Commit b449f65

Browse files
committed
feat(crons): Start dual writing DataSource and Detector rows for crons
We want to have these rows available so that we can start linking crons to workflows.
1 parent fdb4ecc commit b449f65

File tree

7 files changed

+169
-2
lines changed

7 files changed

+169
-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/models.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ def get_name(cls, value):
209209
return dict(cls.as_choices())[value]
210210

211211

212+
@data_source_type_registry.register(DATA_SOURCE_CRON_MONITOR)
212213
@region_silo_model
213214
class Monitor(Model):
214215
__relocation_scope__ = RelocationScope.Organization

src/sentry/monitors/utils.py

Lines changed: 27 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,24 @@ def update_issue_alert_rule(
383389
)
384390

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

0 commit comments

Comments
 (0)