From d0b41657a60b37f6f8a324497760943d371682ca Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 2 Oct 2026 16:03:57 -0700 Subject: [PATCH 1/2] fix(celery): Don't round down beat schedule intervals Interval schedules were truncated to the largest unit they exceed, so a 90-minute schedule was reported as 1 hour and a 36-hour one as 1 day. Use the largest unit that divides the interval evenly instead, and send no monitor config (with a warning) if it isn't a whole number of minutes. Co-Authored-By: Claude --- sentry_sdk/crons/utils.py | 29 +++++++ sentry_sdk/integrations/celery/beat.py | 37 +++++---- sentry_sdk/integrations/celery/utils.py | 22 ------ .../celery/test_celery_beat_crons.py | 75 ++++++++++++++----- tests/test_crons.py | 27 +++++++ 5 files changed, 133 insertions(+), 57 deletions(-) create mode 100644 sentry_sdk/crons/utils.py diff --git a/sentry_sdk/crons/utils.py b/sentry_sdk/crons/utils.py new file mode 100644 index 0000000000..d5fa55e500 --- /dev/null +++ b/sentry_sdk/crons/utils.py @@ -0,0 +1,29 @@ +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Optional, Tuple + + from sentry_sdk._types import MonitorConfigSchedule, MonitorConfigScheduleUnit + + +_INTERVAL_UNITS: "Tuple[Tuple[MonitorConfigScheduleUnit, int], ...]" = ( + ("day", 60 * 60 * 24), + ("hour", 60 * 60), + ("minute", 60), +) + + +def _get_interval_schedule(seconds: float) -> "Optional[MonitorConfigSchedule]": + """ + Express an interval in the largest unit that divides it evenly. + + Returns None if the interval isn't a whole number of minutes. + """ + for unit, unit_seconds in _INTERVAL_UNITS: + if seconds >= unit_seconds and seconds % unit_seconds == 0: + return { + "type": "interval", + "value": int(seconds // unit_seconds), + "unit": unit, + } + return None diff --git a/sentry_sdk/integrations/celery/beat.py b/sentry_sdk/integrations/celery/beat.py index b5027d212a..508b7b57a3 100644 --- a/sentry_sdk/integrations/celery/beat.py +++ b/sentry_sdk/integrations/celery/beat.py @@ -2,11 +2,9 @@ import sentry_sdk from sentry_sdk.crons import MonitorStatus, capture_checkin +from sentry_sdk.crons.utils import _get_interval_schedule from sentry_sdk.integrations import DidNotEnable -from sentry_sdk.integrations.celery.utils import ( - _get_humanized_interval, - _now_seconds_since_epoch, -) +from sentry_sdk.integrations.celery.utils import _now_seconds_since_epoch from sentry_sdk.utils import ( logger, match_regex_list, @@ -74,19 +72,28 @@ def _get_monitor_config( "{0._orig_day_of_week}".format(celery_schedule) ) elif isinstance(celery_schedule, schedule): - schedule_type = "interval" - (schedule_value, schedule_unit) = _get_humanized_interval( - celery_schedule.seconds - ) - - if schedule_unit == "second": - logger.warning( - "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.", - monitor_name, - schedule_value, - ) + seconds = celery_schedule.seconds + interval_schedule = _get_interval_schedule(seconds) + + if interval_schedule is None: + if seconds < 60: + logger.warning( + "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.", + monitor_name, + int(seconds), + ) + else: + logger.warning( + "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.", + monitor_name, + seconds, + ) return {} + schedule_type = "interval" + schedule_value = interval_schedule["value"] + schedule_unit = interval_schedule["unit"] + else: logger.warning( "Celery schedule type '%s' not supported by Sentry Crons.", diff --git a/sentry_sdk/integrations/celery/utils.py b/sentry_sdk/integrations/celery/utils.py index 8d181f1f24..a413011622 100644 --- a/sentry_sdk/integrations/celery/utils.py +++ b/sentry_sdk/integrations/celery/utils.py @@ -1,10 +1,4 @@ import time -from typing import TYPE_CHECKING, cast - -if TYPE_CHECKING: - from typing import Tuple - - from sentry_sdk._types import MonitorConfigScheduleUnit def _now_seconds_since_epoch() -> float: @@ -14,19 +8,3 @@ def _now_seconds_since_epoch() -> float: # Start happens in the Celery Beat process, # the end in a Celery Worker process. return time.time() - - -def _get_humanized_interval(seconds: float) -> "Tuple[int, MonitorConfigScheduleUnit]": - TIME_UNITS = ( # noqa: N806 - ("day", 60 * 60 * 24.0), - ("hour", 60 * 60.0), - ("minute", 60.0), - ) - - seconds = float(seconds) - for unit, divider in TIME_UNITS: - if seconds >= divider: - interval = int(seconds / divider) - return (interval, cast("MonitorConfigScheduleUnit", unit)) - - return (int(seconds), "second") diff --git a/tests/integrations/celery/test_celery_beat_crons.py b/tests/integrations/celery/test_celery_beat_crons.py index 17b4a5e73d..bec10644cb 100644 --- a/tests/integrations/celery/test_celery_beat_crons.py +++ b/tests/integrations/celery/test_celery_beat_crons.py @@ -15,7 +15,6 @@ crons_task_retry, crons_task_success, ) -from sentry_sdk.integrations.celery.utils import _get_humanized_interval def test_get_headers(): @@ -52,25 +51,6 @@ def test_get_headers(): assert _get_headers(fake_task) == {"bla": "blub", "tri": "blub", "bar": "baz"} -@pytest.mark.parametrize( - "seconds, expected_tuple", - [ - (0, (0, "second")), - (1, (1, "second")), - (0.00001, (0, "second")), - (59, (59, "second")), - (60, (1, "minute")), - (100, (1, "minute")), - (1000, (16, "minute")), - (10000, (2, "hour")), - (100000, (1, "day")), - (100000000, (1157, "day")), - ], -) -def test_get_humanized_interval(seconds, expected_tuple): - assert _get_humanized_interval(seconds) == expected_tuple - - def test_crons_task_success(): fake_task = MagicMock() fake_task.request = { @@ -339,6 +319,61 @@ def test_get_monitor_config_minutes(): } +@pytest.mark.parametrize( + "run_every, value, unit", + [ + (datetime.timedelta(minutes=90), 90, "minute"), + (datetime.timedelta(hours=2), 2, "hour"), + (datetime.timedelta(hours=36), 36, "hour"), + (datetime.timedelta(days=1), 1, "day"), + ], +) +def test_get_monitor_config_interval_not_rounded(run_every, value, unit): + app = MagicMock() + app.timezone = "Europe/Vienna" + + celery_schedule = schedule(run_every=run_every) + + monitor_config = _get_monitor_config(celery_schedule, app, "foo") + assert monitor_config["schedule"] == { + "type": "interval", + "value": value, + "unit": unit, + } + + +def test_get_monitor_config_sub_minute_interval(): + app = MagicMock() + app.timezone = "Europe/Vienna" + + celery_schedule = schedule(run_every=datetime.timedelta(seconds=30)) + + with mock.patch("sentry_sdk.integrations.logger.warning") as mock_logger_warning: + monitor_config = _get_monitor_config(celery_schedule, app, "foo") + mock_logger_warning.assert_called_with( + "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.", + "foo", + 30, + ) + assert monitor_config == {} + + +def test_get_monitor_config_interval_not_whole_minutes(): + app = MagicMock() + app.timezone = "Europe/Vienna" + + celery_schedule = schedule(run_every=datetime.timedelta(seconds=90)) + + with mock.patch("sentry_sdk.integrations.logger.warning") as mock_logger_warning: + monitor_config = _get_monitor_config(celery_schedule, app, "foo") + mock_logger_warning.assert_called_with( + "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.", + "foo", + 90, + ) + assert monitor_config == {} + + def test_get_monitor_config_unknown(): app = MagicMock() app.timezone = "Europe/Vienna" diff --git a/tests/test_crons.py b/tests/test_crons.py index 30031db6e1..ab1180036d 100644 --- a/tests/test_crons.py +++ b/tests/test_crons.py @@ -5,6 +5,7 @@ import sentry_sdk from sentry_sdk.crons import capture_checkin +from sentry_sdk.crons.utils import _get_interval_schedule @sentry_sdk.monitor(monitor_slug="abc123") @@ -482,3 +483,29 @@ async def test_contextmanager_error_async(sentry_init): assert fake_capture_checkin.call_args[1]["status"] == "error" assert fake_capture_checkin.call_args[1]["duration"] assert fake_capture_checkin.call_args[1]["check_in_id"] + + +@pytest.mark.parametrize( + "seconds, expected", + [ + (60, (1, "minute")), + (90 * 60, (90, "minute")), + (2 * 60 * 60, (2, "hour")), + (36 * 60 * 60, (36, "hour")), + (24 * 60 * 60, (1, "day")), + (7 * 24 * 60 * 60, (7, "day")), + (300.0, (5, "minute")), + ], +) +def test_get_interval_schedule(seconds, expected): + value, unit = expected + assert _get_interval_schedule(seconds) == { + "type": "interval", + "value": value, + "unit": unit, + } + + +@pytest.mark.parametrize("seconds", [0, 0.5, 30, 59, 90, 61.5, 3601]) +def test_get_interval_schedule_not_whole_minutes(seconds): + assert _get_interval_schedule(seconds) is None From fe764c59202e05ebde411979c16e61cba87e7179 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Mon, 5 Oct 2026 12:21:47 -0700 Subject: [PATCH 2/2] ref(celery): Log whole seconds in the interval warning Matches the sub-minute warning. Co-Authored-By: Claude --- sentry_sdk/integrations/celery/beat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/celery/beat.py b/sentry_sdk/integrations/celery/beat.py index 508b7b57a3..fbe3593c87 100644 --- a/sentry_sdk/integrations/celery/beat.py +++ b/sentry_sdk/integrations/celery/beat.py @@ -86,7 +86,7 @@ def _get_monitor_config( logger.warning( "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.", monitor_name, - seconds, + int(seconds), ) return {}