Repository: jamesoff/simplemonitor

Issue:
Timezones in Alerters
Since #489 loggers now have a option `tz` for specifying a timezone (thanks for that!). Alerters don't have one though. So, for example, I can't change (or haven't found out how to) the timezone of the `{failed_at}` variable in the `execute` alerter. Maybe adding a `tz` option for alerters just like the one for loggers would be nice.

Discussion:

Patch:
diff --git a/docs/alerting.md b/docs/alerting.md
index 5448953..dc9af21 100644
--- a/docs/alerting.md
+++ b/docs/alerting.md
@@ -50,6 +50,7 @@ All alerters accept time period configuration. By default, an alerter is active
 | day | Which days an alerter can operate on. This is a comma-separated list of integers. 0 is Monday and 6 is Sunday. | no | (all days)|
 | times_type | Set to one of always, only, or not. “Only” means that the limits define the period that an alerter can operate. “Not” means that the limits define the period during which it will not operate. | no | always |
 | time_lower and time_upper| If *times_type* is only or not, these two settings define the time limits. time_lower must always be the lower time. The time format is hh:mm using 24-hour clock. Both are required if times_type is anything other than always. | when *times_type* is not `always` | |
+| times_tz | the timezone you want time_lower and time_upper interpreted as | no | local |
 | delay | If any kind of time/day restriction applies, the alerter will notify you of any monitors that failed while they were unable to alert you and are still failed. If a monitor fails and recovers during the restricted period, no catch-up alert is generated. Set to 1 to enable. | no | 0 |
 
 Here’s a quick example of setting time periods (some other configuration values omitted):
diff --git a/simplemonitor/Alerters/alerter.py b/simplemonitor/Alerters/alerter.py
index 4d0a3b7..35ec7f3 100644
--- a/simplemonitor/Alerters/alerter.py
+++ b/simplemonitor/Alerters/alerter.py
@@ -52,7 +52,7 @@ class Alerter:
     """BaseClass for Alerters"""
 
     alerter_type = "unknown"
-    _dependencies = None  # type: List[str]
+    _dependencies = None  # type: Optional[List[str]]
     hostname = gethostname()
     available = False
 
@@ -159,6 +159,7 @@ class Alerter:
             "only_failures", required_type=bool, default=False
         )
         self._tz = cast(str, self.get_config_option("tz", default="UTC"))
+        self._times_tz = cast(str, self.get_config_option("times_tz", default="local"))
 
         if self._ooh_failures is None:
             self._ooh_failures = []
@@ -175,7 +176,9 @@ class Alerter:
 
         If a monitor we depend on fails, it means we can't reach the database,
         so we shouldn't bother trying to write to it."""
-        return self._dependencies
+        if self._dependencies is not None:
+            return self._dependencies
+        return []
 
     @dependencies.setter
     def dependencies(self, dependency_list: List[str]) -> None:
@@ -197,6 +200,8 @@ class Alerter:
 
     def check_dependencies(self, failed_list: List[str]) -> bool:
         """Check if anything we depend on has failed."""
+        if self._dependencies is None or len(self._dependencies) == 0:
+            return True
         for dependency in failed_list:
             if dependency in self._dependencies:
                 self.available = False
@@ -275,7 +280,7 @@ class Alerter:
 
     def _allowed_today(self) -> bool:
         """Check if today is an allowed day for an alert."""
-        if arrow.now().weekday() not in self._days:
+        if arrow.now(self._times_tz).weekday() not in self._days:
             self.alerter_logger.debug("not allowed to alert today")
             return False
         return True
@@ -285,12 +290,12 @@ class Alerter:
         if self._times_type == AlertTimeFilter.ALWAYS:
             return True
         if self._time_info[0] is not None and self._time_info[1] is not None:
-            now = arrow.now().time()
-            in_time_range = (now > self._time_info[0]) and (now < self._time_info[1])
+            now = arrow.now(self._times_tz).time()
+            in_time_range = self._time_info[0] <= now < self._time_info[1]
             if self._times_type == AlertTimeFilter.ONLY:
                 self.alerter_logger.debug("in_time_range: %s", in_time_range)
                 return in_time_range
-            elif self._times_type == AlertTimeFilter.NOT:
+            if self._times_type == AlertTimeFilter.NOT:
                 self.alerter_logger.debug(
                     "in_time_range: %s (inverting due to AlertTimeFilter.NOT)",
                     in_time_range,
@@ -305,12 +310,11 @@ class Alerter:
     def _get_verb(alert_type: AlertType) -> str:
         if alert_type == AlertType.CATCHUP:
             return "failed earlier"
-        elif alert_type == AlertType.FAILURE:
+        if alert_type == AlertType.FAILURE:
             return "failed"
-        elif alert_type == AlertType.SUCCESS:
+        if alert_type == AlertType.SUCCESS:
             return "succeeded"
-        else:
-            return "unknowned"
+        return "unknowned"
 
     def build_message(
         self, length: AlertLength, alert_type: AlertType, monitor: Monitor


Test patch:
diff --git a/tests/test_alerter.py b/tests/test_alerter.py
index f568b9d..1d8d025 100644
--- a/tests/test_alerter.py
+++ b/tests/test_alerter.py
@@ -152,11 +152,25 @@ class TestAlerter(unittest.TestCase):
         a = alerter.Alerter({"days": "0,2,3,4,5,6"})
         self.assertFalse(a._allowed_today())
 
+    @freeze_time(
+        "2020-03-09 22:00:00+00:00"
+    )  # a Monday, but the TZ will push to Tuesday
+    def test_not_allowed_today_tz(self):
+        a = alerter.Alerter({"days": "0,2,3,4,5,6", "times_tz": "+05:00"})
+        self.assertFalse(a._allowed_today())
+
     @freeze_time("2020-03-10")
     def test_allowed_today(self):
         a = alerter.Alerter({"days": "1"})
         self.assertTrue(a._allowed_today())
 
+    @freeze_time(
+        "2020-03-09 22:00:00+00:00"
+    )  # a Monday, but the TZ will push to Tuesday
+    def test_allowed_today_tz(self):
+        a = alerter.Alerter({"days": "1", "times_tz": "+05:00"})
+        self.assertTrue(a._allowed_today())
+
     @freeze_time("2020-03-10")
     def test_allowed_default(self):
         a = alerter.Alerter({})
@@ -178,6 +192,22 @@ class TestAlerter(unittest.TestCase):
         with freeze_time("12:00", tz_offset=-self.utcoffset):
             self.assertFalse(a._allowed_time())
 
+    def test_allowed_only_tz(self):
+        a = alerter.Alerter(
+            {
+                "times_type": "only",
+                "time_lower": "15:00",
+                "time_upper": "16:00",
+                "times_tz": "+05:00",
+            }
+        )
+        with freeze_time("09:00"):
+            self.assertFalse(a._allowed_time())
+        with freeze_time("10:30"):
+            self.assertTrue(a._allowed_time())
+        with freeze_time("12:00"):
+            self.assertFalse(a._allowed_time())
+
     def test_allowed_not(self):
         a = alerter.Alerter(
             {"times_type": "not", "time_lower": "10:00", "time_upper": "11:00"}
@@ -189,6 +219,22 @@ class TestAlerter(unittest.TestCase):
         with freeze_time("12:00", tz_offset=-self.utcoffset):
             self.assertTrue(a._allowed_time())
 
+    def test_allowed_not_tz(self):
+        a = alerter.Alerter(
+            {
+                "times_type": "not",
+                "time_lower": "15:00",
+                "time_upper": "16:00",
+                "times_tz": "+05:00",
+            }
+        )
+        with freeze_time("09:00"):
+            self.assertTrue(a._allowed_time())
+        with freeze_time("10:30"):
+            self.assertFalse(a._allowed_time())
+        with freeze_time("12:00"):
+            self.assertTrue(a._allowed_time())
+
     def test_should_not_alert_ooh(self):
         config = {"times_type": "only", "time_lower": "10:00", "time_upper": "11:00"}
         a = alerter.Alerter(config)