Skip to content

Commit 60116ac

Browse files
wedamijaclaude
andauthored
fix(celery): Don't round down beat schedule intervals (#7842)
### Description Fixes Celery beat interval schedules being rounded down. A 90-minute schedule was sent as "1 hour" and a 36-hour one as "1 day", so Sentry expected runs that never happened and raised false missed-check-in alerts. Intervals now use the largest unit that divides them evenly (90 minutes stays 90 minutes). Intervals that aren't a whole number of minutes, such as 90 seconds, send no monitor config and log a warning, like sub-minute intervals already do. Existing monitors with uneven intervals get the correct interval on their next check-in. For intervals that now send no config, new monitors aren't auto-created and existing ones keep their current schedule. ### Testing Unit tests for the interval conversion and the beat monitor config. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 68bc4dd commit 60116ac

4 files changed

Lines changed: 104 additions & 57 deletions

File tree

‎sentry_sdk/integrations/celery/beat.py‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@
22

33
import sentry_sdk
44
from sentry_sdk.crons import MonitorStatus, capture_checkin
5+
from sentry_sdk.crons.utils import _get_interval_schedule
56
from sentry_sdk.integrations import DidNotEnable
6-
from sentry_sdk.integrations.celery.utils import (
7-
_get_humanized_interval,
8-
_now_seconds_since_epoch,
9-
)
7+
from sentry_sdk.integrations.celery.utils import _now_seconds_since_epoch
108
from sentry_sdk.utils import (
119
logger,
1210
match_regex_list,
@@ -74,19 +72,28 @@ def _get_monitor_config(
7472
"{0._orig_day_of_week}".format(celery_schedule)
7573
)
7674
elif isinstance(celery_schedule, schedule):
77-
schedule_type = "interval"
78-
(schedule_value, schedule_unit) = _get_humanized_interval(
79-
celery_schedule.seconds
80-
)
81-
82-
if schedule_unit == "second":
83-
logger.warning(
84-
"Intervals shorter than one minute are not supported by Sentry Crons. Monitor '%s' has an interval of %s seconds. Use the `exclude_beat_tasks` option in the celery integration to exclude it.",
85-
monitor_name,
86-
schedule_value,
87-
)
75+
seconds = celery_schedule.seconds
76+
interval_schedule = _get_interval_schedule(seconds)
77+
78+
if interval_schedule is None:
79+
if seconds < 60:
80+
logger.warning(
81+
"Intervals shorter than one minute are not supported by Sentry Crons. Monitor '%s' has an interval of %s seconds. Use the `exclude_beat_tasks` option in the celery integration to exclude it.",
82+
monitor_name,
83+
int(seconds),
84+
)
85+
else:
86+
logger.warning(
87+
"Sentry Crons only supports intervals of whole minutes. Monitor '%s' has an interval of %s seconds. Use the `exclude_beat_tasks` option in the celery integration to exclude it.",
88+
monitor_name,
89+
int(seconds),
90+
)
8891
return {}
8992

93+
schedule_type = "interval"
94+
schedule_value = interval_schedule["value"]
95+
schedule_unit = interval_schedule["unit"]
96+
9097
else:
9198
logger.warning(
9299
"Celery schedule type '%s' not supported by Sentry Crons.",
Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
11
import time
2-
from typing import TYPE_CHECKING, cast
3-
4-
if TYPE_CHECKING:
5-
from typing import Tuple
6-
7-
from sentry_sdk._types import MonitorConfigScheduleUnit
82

93

104
def _now_seconds_since_epoch() -> float:
@@ -14,19 +8,3 @@ def _now_seconds_since_epoch() -> float:
148
# Start happens in the Celery Beat process,
159
# the end in a Celery Worker process.
1610
return time.time()
17-
18-
19-
def _get_humanized_interval(seconds: float) -> "Tuple[int, MonitorConfigScheduleUnit]":
20-
TIME_UNITS = ( # noqa: N806
21-
("day", 60 * 60 * 24.0),
22-
("hour", 60 * 60.0),
23-
("minute", 60.0),
24-
)
25-
26-
seconds = float(seconds)
27-
for unit, divider in TIME_UNITS:
28-
if seconds >= divider:
29-
interval = int(seconds / divider)
30-
return (interval, cast("MonitorConfigScheduleUnit", unit))
31-
32-
return (int(seconds), "second")

‎tests/integrations/celery/test_celery_beat_crons.py‎

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
crons_task_retry,
1616
crons_task_success,
1717
)
18-
from sentry_sdk.integrations.celery.utils import _get_humanized_interval
1918

2019

2120
def test_get_headers():
@@ -52,25 +51,6 @@ def test_get_headers():
5251
assert _get_headers(fake_task) == {"bla": "blub", "tri": "blub", "bar": "baz"}
5352

5453

55-
@pytest.mark.parametrize(
56-
"seconds, expected_tuple",
57-
[
58-
(0, (0, "second")),
59-
(1, (1, "second")),
60-
(0.00001, (0, "second")),
61-
(59, (59, "second")),
62-
(60, (1, "minute")),
63-
(100, (1, "minute")),
64-
(1000, (16, "minute")),
65-
(10000, (2, "hour")),
66-
(100000, (1, "day")),
67-
(100000000, (1157, "day")),
68-
],
69-
)
70-
def test_get_humanized_interval(seconds, expected_tuple):
71-
assert _get_humanized_interval(seconds) == expected_tuple
72-
73-
7454
def test_crons_task_success():
7555
fake_task = MagicMock()
7656
fake_task.request = {
@@ -339,6 +319,61 @@ def test_get_monitor_config_minutes():
339319
}
340320

341321

322+
@pytest.mark.parametrize(
323+
"run_every, value, unit",
324+
[
325+
(datetime.timedelta(minutes=90), 90, "minute"),
326+
(datetime.timedelta(hours=2), 2, "hour"),
327+
(datetime.timedelta(hours=36), 36, "hour"),
328+
(datetime.timedelta(days=1), 1, "day"),
329+
],
330+
)
331+
def test_get_monitor_config_interval_not_rounded(run_every, value, unit):
332+
app = MagicMock()
333+
app.timezone = "Europe/Vienna"
334+
335+
celery_schedule = schedule(run_every=run_every)
336+
337+
monitor_config = _get_monitor_config(celery_schedule, app, "foo")
338+
assert monitor_config["schedule"] == {
339+
"type": "interval",
340+
"value": value,
341+
"unit": unit,
342+
}
343+
344+
345+
def test_get_monitor_config_sub_minute_interval():
346+
app = MagicMock()
347+
app.timezone = "Europe/Vienna"
348+
349+
celery_schedule = schedule(run_every=datetime.timedelta(seconds=30))
350+
351+
with mock.patch("sentry_sdk.integrations.logger.warning") as mock_logger_warning:
352+
monitor_config = _get_monitor_config(celery_schedule, app, "foo")
353+
mock_logger_warning.assert_called_with(
354+
"Intervals shorter than one minute are not supported by Sentry Crons. Monitor '%s' has an interval of %s seconds. Use the `exclude_beat_tasks` option in the celery integration to exclude it.",
355+
"foo",
356+
30,
357+
)
358+
assert monitor_config == {}
359+
360+
361+
def test_get_monitor_config_interval_not_whole_minutes():
362+
app = MagicMock()
363+
app.timezone = "Europe/Vienna"
364+
365+
celery_schedule = schedule(run_every=datetime.timedelta(seconds=90))
366+
367+
with mock.patch("sentry_sdk.integrations.logger.warning") as mock_logger_warning:
368+
monitor_config = _get_monitor_config(celery_schedule, app, "foo")
369+
mock_logger_warning.assert_called_with(
370+
"Sentry Crons only supports intervals of whole minutes. Monitor '%s' has an interval of %s seconds. Use the `exclude_beat_tasks` option in the celery integration to exclude it.",
371+
"foo",
372+
90,
373+
)
374+
assert monitor_config == {}
375+
376+
342377
def test_get_monitor_config_unknown():
343378
app = MagicMock()
344379
app.timezone = "Europe/Vienna"

‎tests/test_crons.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import sentry_sdk
77
from sentry_sdk.crons import capture_checkin
8+
from sentry_sdk.crons.utils import _get_interval_schedule
89

910

1011
@sentry_sdk.monitor(monitor_slug="abc123")
@@ -517,3 +518,29 @@ async def test_contextmanager_error_async(sentry_init):
517518
assert fake_capture_checkin.call_args[1]["status"] == "error"
518519
assert fake_capture_checkin.call_args[1]["duration"]
519520
assert fake_capture_checkin.call_args[1]["check_in_id"]
521+
522+
523+
@pytest.mark.parametrize(
524+
"seconds, expected",
525+
[
526+
(60, (1, "minute")),
527+
(90 * 60, (90, "minute")),
528+
(2 * 60 * 60, (2, "hour")),
529+
(36 * 60 * 60, (36, "hour")),
530+
(24 * 60 * 60, (1, "day")),
531+
(7 * 24 * 60 * 60, (7, "day")),
532+
(300.0, (5, "minute")),
533+
],
534+
)
535+
def test_get_interval_schedule(seconds, expected):
536+
value, unit = expected
537+
assert _get_interval_schedule(seconds) == {
538+
"type": "interval",
539+
"value": value,
540+
"unit": unit,
541+
}
542+
543+
544+
@pytest.mark.parametrize("seconds", [0, 0.5, 30, 59, 90, 61.5, 3601])
545+
def test_get_interval_schedule_not_whole_minutes(seconds):
546+
assert _get_interval_schedule(seconds) is None

0 commit comments

Comments
 (0)