Skip to content

Commit

Permalink
Do not support sub-minute cron intervals (#2172)
Browse files Browse the repository at this point in the history
* Do not support sub-minute cron intervals
* Do not send checkins for unsupported schedule types

---------

Co-authored-by: Ivana Kellyerova <ivana.kellyerova@sentry.io>
  • Loading branch information
antonpirker and Ivana Kellyerova committed Jun 16, 2023
1 parent fe7e501 commit 4f0ab40
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 24 deletions.
45 changes: 27 additions & 18 deletions sentry_sdk/integrations/celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ def _get_humanized_interval(seconds):
interval = int(seconds / divider)
return (interval, unit)

return (1, "minute")
return (int(seconds), "second")


def _get_monitor_config(celery_schedule, app):
Expand All @@ -400,6 +400,12 @@ def _get_monitor_config(celery_schedule, app):
celery_schedule.seconds
)

if schedule_unit == "second":
logger.warning(
"Intervals shorter than one minute are not supported by Sentry Crons."
)
return {}

else:
logger.warning(
"Celery schedule type '%s' not supported by Sentry Crons.",
Expand Down Expand Up @@ -441,24 +447,27 @@ def sentry_apply_entry(*args, **kwargs):

monitor_config = _get_monitor_config(celery_schedule, app)

headers = schedule_entry.options.pop("headers", {})
headers.update(
{
"sentry-monitor-slug": monitor_name,
"sentry-monitor-config": monitor_config,
}
)

check_in_id = capture_checkin(
monitor_slug=monitor_name,
monitor_config=monitor_config,
status=MonitorStatus.IN_PROGRESS,
)
headers.update({"sentry-monitor-check-in-id": check_in_id})
is_supported_schedule = bool(monitor_config)
if is_supported_schedule:
headers = schedule_entry.options.pop("headers", {})
headers.update(
{
"sentry-monitor-slug": monitor_name,
"sentry-monitor-config": monitor_config,
}
)

check_in_id = capture_checkin(
monitor_slug=monitor_name,
monitor_config=monitor_config,
status=MonitorStatus.IN_PROGRESS,
)
headers.update({"sentry-monitor-check-in-id": check_in_id})

# Set the Sentry configuration in the options of the ScheduleEntry.
# Those will be picked up in `apply_async` and added to the headers.
schedule_entry.options["headers"] = headers

# Set the Sentry configuration in the options of the ScheduleEntry.
# Those will be picked up in `apply_async` and added to the headers.
schedule_entry.options["headers"] = headers
return original_apply_entry(*args, **kwargs)

Scheduler.apply_entry = sentry_apply_entry
Expand Down
34 changes: 28 additions & 6 deletions tests/integrations/celery/test_celery_beat_crons.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,11 @@ def test_get_headers():
@pytest.mark.parametrize(
"seconds, expected_tuple",
[
(0, (1, "minute")),
(0.00001, (1, "minute")),
(1, (1, "minute")),
(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")),
Expand Down Expand Up @@ -205,13 +207,12 @@ def test_crons_task_retry():
)


def test_get_monitor_config():
def test_get_monitor_config_crontab():
app = MagicMock()
app.conf = MagicMock()
app.conf.timezone = "Europe/Vienna"

celery_schedule = crontab(day_of_month="3", hour="12", minute="*/10")

monitor_config = _get_monitor_config(celery_schedule, app)
assert monitor_config == {
"schedule": {
Expand All @@ -222,8 +223,23 @@ def test_get_monitor_config():
}
assert "unit" not in monitor_config["schedule"]

celery_schedule = schedule(run_every=3)

def test_get_monitor_config_seconds():
app = MagicMock()
app.conf = MagicMock()
app.conf.timezone = "Europe/Vienna"

celery_schedule = schedule(run_every=3) # seconds
monitor_config = _get_monitor_config(celery_schedule, app)
assert monitor_config == {}


def test_get_monitor_config_minutes():
app = MagicMock()
app.conf = MagicMock()
app.conf.timezone = "Europe/Vienna"

celery_schedule = schedule(run_every=60) # seconds
monitor_config = _get_monitor_config(celery_schedule, app)
assert monitor_config == {
"schedule": {
Expand All @@ -234,6 +250,12 @@ def test_get_monitor_config():
"timezone": "Europe/Vienna",
}


def test_get_monitor_config_unknown():
app = MagicMock()
app.conf = MagicMock()
app.conf.timezone = "Europe/Vienna"

unknown_celery_schedule = MagicMock()
monitor_config = _get_monitor_config(unknown_celery_schedule, app)
assert monitor_config == {}
Expand Down

0 comments on commit 4f0ab40

Please sign in to comment.