feat(crons): Add cronsim behind an option

requirements-base.txt

@@ -9,6 +9,7 @@ celery>=5
 click>=8.1
 confluent-kafka>=2.3.0
 croniter>=1.3.10
+cronsim>=2.6
 cssselect>=1.0.3
 datadog>=0.49
 django-crispy-forms>=1.14.0

requirements-dev-frozen.txt

@@ -32,6 +32,7 @@ confluent-kafka==2.3.0
 covdefaults==2.3.0
 coverage==7.6.4
 croniter==1.3.10
+cronsim==2.6
 cryptography==43.0.1
 cssselect==1.0.3
 cssutils==2.9.0

requirements-frozen.txt

@@ -27,6 +27,7 @@ click-plugins==1.1.1
 click-repl==0.3.0
 confluent-kafka==2.3.0
 croniter==1.3.10
+cronsim==2.6
 cryptography==43.0.1
 cssselect==1.0.3
 cssutils==2.9.0

src/sentry/monitors/schedule.py

@@ -1,8 +1,10 @@
 from datetime import datetime
 
 from croniter import croniter
+from cronsim import CronSim
 from dateutil import rrule
 
+from sentry import options
 from sentry.monitors.types import IntervalUnit, ScheduleConfig
 
 SCHEDULE_INTERVAL_MAP: dict[IntervalUnit, int] = {
@@ -38,12 +40,16 @@ def get_next_schedule(
     # of granularity we're able to support
 
     if schedule.type == "crontab":
-        iterator = croniter(
-            expr_format=schedule.crontab,
-            start_time=reference_ts,
-            ret_type=datetime,
-        )
-        return iterator.get_next().replace(second=0, microsecond=0)
+        if options.get("crons.use_cronsim"):
+            sim = CronSim(schedule.crontab, reference_ts)
+            return next(sim).replace(second=0, microsecond=0)
+        else:
+            iterator = croniter(
+                expr_format=schedule.crontab,
+                start_time=reference_ts,
+                ret_type=datetime,
+            )
+            return iterator.get_next().replace(second=0, microsecond=0)
 
     if schedule.type == "interval":
         rule = rrule.rrule(
@@ -81,12 +87,16 @@ def get_prev_schedule(
     >>> 05:30
     """
     if schedule.type == "crontab":
-        iterator = croniter(
-            expr_format=schedule.crontab,
-            start_time=reference_ts,
-            ret_type=datetime,
-        )
-        return iterator.get_prev().replace(second=0, microsecond=0)
+        if options.get("crons.use_cronsim"):
+            sim = CronSim(schedule.crontab, reference_ts, reverse=True)
+            return next(sim).replace(second=0, microsecond=0)
+        else:
+            iterator = croniter(
+                expr_format=schedule.crontab,
+                start_time=reference_ts,
+                ret_type=datetime,
+            )
+            return iterator.get_prev().replace(second=0, microsecond=0)
 
     if schedule.type == "interval":
         rule = rrule.rrule(

src/sentry/monitors/validators.py

@@ -2,7 +2,8 @@
 from typing import Literal
 
 import sentry_sdk
-from croniter import CroniterBadDateError, croniter
+from croniter import CroniterBadCronError, CroniterBadDateError
+from cronsim import CronSimError
 from django.core.exceptions import ValidationError
 from django.utils import timezone
 from drf_spectacular.types import OpenApiTypes
@@ -222,9 +223,6 @@ def validate(self, attrs):
                     schedule = NONSTANDARD_CRONTAB_SCHEDULES[schedule]
                 except KeyError:
                     raise ValidationError({"schedule": "Schedule was not parseable"})
-            # crontab schedule must be valid
-            if not croniter.is_valid(schedule):
-                raise ValidationError({"schedule": "Schedule was not parseable"})
 
             # XXX(epurkhiser): Make sure we can traverse forward and back in
             # the schedule. croniter is good, but there are some very edge case
@@ -233,7 +231,7 @@ def validate(self, attrs):
             try:
                 get_next_schedule(now, CrontabSchedule(schedule))
                 get_prev_schedule(now, now, CrontabSchedule(schedule))
-            except CroniterBadDateError:
+            except (CroniterBadCronError, CroniterBadDateError, CronSimError):
                 raise ValidationError({"schedule": "Schedule is invalid"})
 
             # Do not support 6 or 7 field crontabs

tests/sentry/monitors/test_schedule.py

@@ -1,18 +1,18 @@
 from datetime import datetime, timezone
 from zoneinfo import ZoneInfo
 
-import pytest
-
 from sentry.monitors.schedule import get_next_schedule, get_prev_schedule
 from sentry.monitors.types import CrontabSchedule, IntervalSchedule
+from sentry.testutils.helpers.options import override_options
+from sentry.testutils.pytest.fixtures import django_db_all
 
 
 def t(hour: int, minute: int):
     return datetime(2019, 1, 1, hour, minute, 0, tzinfo=timezone.utc)
 
 
+@django_db_all
 def test_get_next_schedule():
-
     # 00 * * * *: 5:30 -> 6:00
     assert get_next_schedule(t(5, 30), CrontabSchedule("0 * * * *")) == t(6, 00)
 
@@ -26,6 +26,7 @@ def test_get_next_schedule():
     assert get_next_schedule(t(5, 42), IntervalSchedule(interval=2, unit="hour")) == t(7, 42)
 
 
+@django_db_all
 def test_get_next_schedule_cron_dst():
     # Minute rollover during DST start
     assert get_next_schedule(
@@ -40,7 +41,8 @@ def test_get_next_schedule_cron_dst():
     ) == datetime(2024, 3, 10, 3, 0, 0, tzinfo=ZoneInfo("America/New_York"))
 
 
-@pytest.mark.skip(reason="croniter bug must be fixed")
+@django_db_all
+@override_options({"crons.use_cronsim": True})
 def test_get_next_schedule_cron_dst_bugs():
     """
     XXX(epurkhiser): We have a bug in our handling of DST transitions for some
@@ -50,6 +52,8 @@ def test_get_next_schedule_cron_dst_bugs():
 
     The following test cases document the incorrectly handled cases and sahould
     be correct once we've fixed the upstream issue in croniter.
+
+    This is fixed by using cronsim
     """
 
     # DST beginning with a daily schedule
@@ -69,6 +73,7 @@ def test_get_next_schedule_cron_dst_bugs():
     ) == datetime(2024, 3, 10, 12, 0, 0, tzinfo=ZoneInfo("America/New_York"))
 
 
+@django_db_all
 def test_get_prev_schedule():
     start_ts = datetime(2019, 1, 1, 1, 30, 0, tzinfo=timezone.utc)