From 4f0ab408e5a2288de1485aebef6e3e609ede89e3 Mon Sep 17 00:00:00 2001
From: Anton Pirker <anton.pirker@sentry.io>
Date: Fri, 16 Jun 2023 10:18:23 +0200
Subject: [PATCH] Do not support sub-minute cron intervals (#2172)

* Do not support sub-minute cron intervals
* Do not send checkins for unsupported schedule types

---------

Co-authored-by: Ivana Kellyerova <ivana.kellyerova@sentry.io>
---
 sentry_sdk/integrations/celery.py             | 45 +++++++++++--------
 .../celery/test_celery_beat_crons.py          | 34 +++++++++++---
 2 files changed, 55 insertions(+), 24 deletions(-)

diff --git a/sentry_sdk/integrations/celery.py b/sentry_sdk/integrations/celery.py
index ef629ea167..741a2c8bb7 100644
--- a/sentry_sdk/integrations/celery.py
+++ b/sentry_sdk/integrations/celery.py
@@ -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):
@@ -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.",
@@ -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
diff --git a/tests/integrations/celery/test_celery_beat_crons.py b/tests/integrations/celery/test_celery_beat_crons.py
index 1b0c82ba8d..636bcb545c 100644
--- a/tests/integrations/celery/test_celery_beat_crons.py
+++ b/tests/integrations/celery/test_celery_beat_crons.py
@@ -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")),
@@ -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": {
@@ -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": {
@@ -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 == {}