Read timezone for Crons monitors from celery_schedule if existing (#2497)

Author: antonpirker

sentry_sdk/integrations/celery.py

@@ -444,7 +444,15 @@ def _get_monitor_config(celery_schedule, app, monitor_name):
     if schedule_unit is not None:
         monitor_config["schedule"]["unit"] = schedule_unit
 
-    monitor_config["timezone"] = app.conf.timezone or "UTC"
+    monitor_config["timezone"] = (
+        (
+            hasattr(celery_schedule, "tz")
+            and celery_schedule.tz is not None
+            and str(celery_schedule.tz)
+        )
+        or app.timezone
+        or "UTC"
+    )
 
     return monitor_config
 

tests/integrations/celery/test_celery_beat_crons.py

@@ -1,3 +1,6 @@
+import datetime
+import sys
+
 import pytest
 
 from sentry_sdk.integrations.celery import (
@@ -207,25 +210,65 @@ def test_crons_task_retry():
 
 def test_get_monitor_config_crontab():
     app = MagicMock()
-    app.conf = MagicMock()
-    app.conf.timezone = "Europe/Vienna"
+    app.timezone = "Europe/Vienna"
 
+    # schedule with the default timezone
     celery_schedule = crontab(day_of_month="3", hour="12", minute="*/10")
+
     monitor_config = _get_monitor_config(celery_schedule, app, "foo")
     assert monitor_config == {
         "schedule": {
             "type": "crontab",
             "value": "*/10 12 3 * *",
         },
-        "timezone": "Europe/Vienna",
+        "timezone": "UTC",  # the default because `crontab` does not know about the app
     }
     assert "unit" not in monitor_config["schedule"]
 
+    # schedule with the timezone from the app
+    celery_schedule = crontab(day_of_month="3", hour="12", minute="*/10", app=app)
+
+    monitor_config = _get_monitor_config(celery_schedule, app, "foo")
+    assert monitor_config == {
+        "schedule": {
+            "type": "crontab",
+            "value": "*/10 12 3 * *",
+        },
+        "timezone": "Europe/Vienna",  # the timezone from the app
+    }
+
+    # schedule without a timezone, the celery integration will read the config from the app
+    celery_schedule = crontab(day_of_month="3", hour="12", minute="*/10")
+    celery_schedule.tz = None
+
+    monitor_config = _get_monitor_config(celery_schedule, app, "foo")
+    assert monitor_config == {
+        "schedule": {
+            "type": "crontab",
+            "value": "*/10 12 3 * *",
+        },
+        "timezone": "Europe/Vienna",  # the timezone from the app
+    }
+
+    # schedule without a timezone, and an app without timezone, the celery integration will fall back to UTC
+    app = MagicMock()
+    app.timezone = None
+
+    celery_schedule = crontab(day_of_month="3", hour="12", minute="*/10")
+    celery_schedule.tz = None
+    monitor_config = _get_monitor_config(celery_schedule, app, "foo")
+    assert monitor_config == {
+        "schedule": {
+            "type": "crontab",
+            "value": "*/10 12 3 * *",
+        },
+        "timezone": "UTC",  # default timezone from celery integration
+    }
+
 
 def test_get_monitor_config_seconds():
     app = MagicMock()
-    app.conf = MagicMock()
-    app.conf.timezone = "Europe/Vienna"
+    app.timezone = "Europe/Vienna"
 
     celery_schedule = schedule(run_every=3)  # seconds
 
@@ -243,25 +286,69 @@ def test_get_monitor_config_seconds():
 
 def test_get_monitor_config_minutes():
     app = MagicMock()
-    app.conf = MagicMock()
-    app.conf.timezone = "Europe/Vienna"
+    app.timezone = "Europe/Vienna"
+
+    # schedule with the default timezone
+    celery_schedule = schedule(run_every=60)  # seconds
+
+    monitor_config = _get_monitor_config(celery_schedule, app, "foo")
+    assert monitor_config == {
+        "schedule": {
+            "type": "interval",
+            "value": 1,
+            "unit": "minute",
+        },
+        "timezone": "UTC",
+    }
+
+    # schedule with the timezone from the app
+    celery_schedule = schedule(run_every=60, app=app)  # seconds
+
+    monitor_config = _get_monitor_config(celery_schedule, app, "foo")
+    assert monitor_config == {
+        "schedule": {
+            "type": "interval",
+            "value": 1,
+            "unit": "minute",
+        },
+        "timezone": "Europe/Vienna",  # the timezone from the app
+    }
+
+    # schedule without a timezone, the celery integration will read the config from the app
+    celery_schedule = schedule(run_every=60)  # seconds
+    celery_schedule.tz = None
+
+    monitor_config = _get_monitor_config(celery_schedule, app, "foo")
+    assert monitor_config == {
+        "schedule": {
+            "type": "interval",
+            "value": 1,
+            "unit": "minute",
+        },
+        "timezone": "Europe/Vienna",  # the timezone from the app
+    }
+
+    # schedule without a timezone, and an app without timezone, the celery integration will fall back to UTC
+    app = MagicMock()
+    app.timezone = None
 
     celery_schedule = schedule(run_every=60)  # seconds
+    celery_schedule.tz = None
+
     monitor_config = _get_monitor_config(celery_schedule, app, "foo")
     assert monitor_config == {
         "schedule": {
             "type": "interval",
             "value": 1,
             "unit": "minute",
         },
-        "timezone": "Europe/Vienna",
+        "timezone": "UTC",  # default timezone from celery integration
     }
 
 
 def test_get_monitor_config_unknown():
     app = MagicMock()
-    app.conf = MagicMock()
-    app.conf.timezone = "Europe/Vienna"
+    app.timezone = "Europe/Vienna"
 
     unknown_celery_schedule = MagicMock()
     monitor_config = _get_monitor_config(unknown_celery_schedule, app, "foo")
@@ -270,16 +357,45 @@ def test_get_monitor_config_unknown():
 
 def test_get_monitor_config_default_timezone():
     app = MagicMock()
-    app.conf = MagicMock()
-    app.conf.timezone = None
+    app.timezone = None
 
     celery_schedule = crontab(day_of_month="3", hour="12", minute="*/10")
 
-    monitor_config = _get_monitor_config(celery_schedule, app, "foo")
+    monitor_config = _get_monitor_config(celery_schedule, app, "dummy_monitor_name")
 
     assert monitor_config["timezone"] == "UTC"
 
 
+def test_get_monitor_config_timezone_in_app_conf():
+    app = MagicMock()
+    app.timezone = "Asia/Karachi"
+
+    celery_schedule = crontab(day_of_month="3", hour="12", minute="*/10")
+    celery_schedule.tz = None
+
+    monitor_config = _get_monitor_config(celery_schedule, app, "dummy_monitor_name")
+
+    assert monitor_config["timezone"] == "Asia/Karachi"
+
+
+@pytest.mark.skipif(
+    sys.version_info < (3, 0),
+    reason="no datetime.timezone for Python 2, so skipping this test.",
+)
+def test_get_monitor_config_timezone_in_celery_schedule():
+    app = MagicMock()
+    app.timezone = "Asia/Karachi"
+
+    panama_tz = datetime.timezone(datetime.timedelta(hours=-5), name="America/Panama")
+
+    celery_schedule = crontab(day_of_month="3", hour="12", minute="*/10")
+    celery_schedule.tz = panama_tz
+
+    monitor_config = _get_monitor_config(celery_schedule, app, "dummy_monitor_name")
+
+    assert monitor_config["timezone"] == str(panama_tz)
+
+
 @pytest.mark.parametrize(
     "task_name,exclude_beat_tasks,task_in_excluded_beat_tasks",
     [