[IMP] maintenance_service_http_monitoring : modify logic to avoid false-positive http alerts

maintenance_service_http_monitoring/.gitignore

@@ -0,0 +1,2 @@
+*.*~
+*pyc

maintenance_service_http_monitoring/__manifest__.py

@@ -5,7 +5,7 @@
     "license": "AGPL-3",
     "category": "Tools",
     "summary": "Monitor HTTP availability of services",
-    "depends": ["base", "maintenance", "hr_maintenance", "maintenance_server_data"],
+    "depends": ["base", "maintenance", "base_maintenance", "hr_maintenance", "maintenance_server_data"],
     "external_dependencies": {"python": ["requests"]},
     "data": [
         "data/ir_config_parameter.xml",

maintenance_service_http_monitoring/data/cron.xml

@@ -5,7 +5,7 @@
         <field name="model_id" ref="maintenance_server_data.model_service_instance" />
         <field name="state">code</field>
         <field name="code">model.cron_check_http_services()</field>
-        <field name="interval_number">15</field>
+        <field name="interval_number">10</field>
         <field name="interval_type">minutes</field>
     </record>
     <record id="ir_cron_maintenance_mode_expiry" model="ir.cron">

maintenance_service_http_monitoring/models/service_instance.py

@@ -1,5 +1,5 @@
 import logging
-import time
+from datetime import timedelta
 
 from odoo import api, fields, models
 
@@ -10,8 +10,8 @@ except ImportError:
 
 _logger = logging.getLogger(__name__)
 
-HTTP_CHECK_TIMEOUT = 10  # seconds
-HTTP_RETRY_DELAY = 2  # seconds between pass 1 and pass 2
+HTTP_CHECK_TIMEOUT = 20  # seconds
+HTTP_KO_CONFIRMATION_DELAY = timedelta(minutes=5)
 
 
 class ServiceInstance(models.Model):
@@ -36,14 +36,20 @@ class ServiceInstance(models.Model):
         string="HTTP Maintenance Request",
         readonly=True,
     )
+    http_first_ko_at = fields.Datetime(
+        string="First HTTP KO Check (current streak)",
+        readonly=True,
+    )
 
     def check_http_status(self):
         """
         Perform HTTP check for each record and return the KO recordset.
 
         Writes last_http_status_code, last_http_check_date and http_status_ok on every
-        checked record. Does NOT create maintenance.request — that decision belongs to
-        the caller (cron) after optional retry logic.
+        checked record. Does NOT create maintenance.request — the cron only opens one
+        once the service has been continuously KO for at least
+        HTTP_KO_CONFIRMATION_DELAY, to avoid flagging transient outages (e.g. a short
+        server overload) as real incidents.
         """
         ko_records = self.browse()
         for rec in self:
@@ -63,13 +69,16 @@ class ServiceInstance(models.Model):
                 status_ok = status_code == 200
             except requests.exceptions.RequestException as e:
                 _logger.warning("HTTP check failed for %s: %s", rec.service_url, e)
-            rec.write(
-                {
+            vals = {
                 "last_http_status_code": status_code,
                 "last_http_check_date": now,
                 "http_status_ok": status_ok,
             }
-            )
+            if status_ok:
+                vals["http_first_ko_at"] = False
+            elif not rec.http_first_ko_at:
+                vals["http_first_ko_at"] = now
+            rec.write(vals)
             if not status_ok:
                 ko_records |= rec
         return ko_records
@@ -106,13 +115,12 @@ class ServiceInstance(models.Model):
     @api.model
     def cron_check_http_services(self):
         """
-        Check all active services with a URL, with one retry on failure.
+        Check all active services with a URL.
 
-        Pass 1: test every eligible service.
-        - Services that had an open request and are now OK are auto-resolved.
-        - Services still KO after pass 1 are retested after HTTP_RETRY_DELAY seconds.
-        maintenance.request is created only for services that fail both passes,
-        reducing noise from transient HTTP errors.
+        A service must be continuously KO for at least HTTP_KO_CONFIRMATION_DELAY
+        before a maintenance.request is created — this tolerates transient outages
+        (e.g. a temporary server overload) regardless of how often this cron runs.
+        Services that had an open request and are now OK are auto-resolved.
         """
         domain = [
             ("active", "=", True),
@@ -123,25 +131,22 @@ class ServiceInstance(models.Model):
             lambda s: not s.equipment_id.maintenance_mode
         )
 
-        # Snapshot services that currently have an open request before pass 1
+        # Snapshot services that currently have an open request before the check
         services_with_open_request = services.filtered(
             lambda s: s.http_maintenance_request
             and not s.http_maintenance_request.stage_id.done
         )
 
-        ko_after_pass1 = services.check_http_status()
+        ko_services = services.check_http_status()
 
-        # Auto-resolve services that recovered during pass 1
+        # Auto-resolve services that recovered
         recovered = services_with_open_request.filtered(lambda s: s.http_status_ok)
         if recovered:
             recovered._close_http_maintenance_request()
 
-        if not ko_after_pass1:
-            return
-
-        time.sleep(HTTP_RETRY_DELAY)
-
-        ko_confirmed = ko_after_pass1.check_http_status()
-
-        for service in ko_confirmed:
+        confirmed_ko = ko_services.filtered(
+            lambda s: s.last_http_check_date - s.http_first_ko_at
+            >= HTTP_KO_CONFIRMATION_DELAY
+        )
+        for service in confirmed_ko:
             service.equipment_id.create_http_maintenance_request(service)

maintenance_service_http_monitoring/tests/test_http_monitoring.py

@@ -11,9 +11,9 @@ EQUIPMENT_HTTP_REQUESTS = (
     "odoo.addons.maintenance_service_http_monitoring"
     ".models.maintenance_equipment.http_requests"
 )
-SERVICE_INSTANCE_SLEEP = (
-    "odoo.addons.maintenance_service_http_monitoring.models.service_instance.time.sleep"
-)
+
+# Comfortably past HTTP_KO_CONFIRMATION_DELAY (currently 10 minutes).
+PAST_CONFIRMATION_DELAY = timedelta(minutes=20)
 
 
 def _mock_response(status_code):
@@ -41,6 +41,13 @@ class TestHttpMonitoring(TransactionCase):
             }
         )
 
+    def _backdate_first_ko(self, *service_instances):
+        """Simulate that the current KO streak started long enough ago to be confirmed."""
+        for service_instance in service_instances:
+            service_instance.write(
+                {"http_first_ko_at": fields.Datetime.now() - PAST_CONFIRMATION_DELAY}
+            )
+
     # ------------------------------------------------------------------
     # Test 1 -- HTTP 200 -> service marked OK
     # ------------------------------------------------------------------
@@ -55,19 +62,26 @@ class TestHttpMonitoring(TransactionCase):
         self.assertIsNotNone(self.service_instance.last_http_check_date)
 
     # ------------------------------------------------------------------
-    # Test 2 -- Two KO passes -> maintenance.request created on the service
+    # Test 2 -- KO confirmed only once continuously KO for HTTP_KO_CONFIRMATION_DELAY,
+    # not on the first observed failure
     # ------------------------------------------------------------------
-    def test_http_500_creates_maintenance_request(self):
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
+    def test_http_500_creates_request_after_confirmation_delay(self):
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(500)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
 
         self.assertFalse(self.service_instance.http_status_ok)
         self.assertEqual(self.service_instance.last_http_status_code, 500)
+        self.assertTrue(self.service_instance.http_first_ko_at)
+        self.assertFalse(self.service_instance.http_maintenance_request)
+
+        self._backdate_first_ko(self.service_instance)
+
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
 
         request = self.service_instance.http_maintenance_request
         self.assertTrue(request)
@@ -90,27 +104,29 @@ class TestHttpMonitoring(TransactionCase):
         self.assertEqual(self.service_instance.last_http_status_code, -1)
 
     # ------------------------------------------------------------------
-    # Test 4 -- Two consecutive cron runs KO -> no duplicate request
+    # Test 4 -- Repeated failure after confirmation -> a single request, no duplicate
     # ------------------------------------------------------------------
     def test_no_duplicate_request_on_repeated_failure(self):
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(500)
             mock_requests.exceptions.RequestException = Exception
-            self.env["service.instance"].cron_check_http_services()
+            self.env["service.instance"].cron_check_http_services()  # first_ko_at set
+
+        self.assertFalse(self.service_instance.http_maintenance_request)
+        self._backdate_first_ko(self.service_instance)
+
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()  # confirmed, request created
 
         request_1 = self.service_instance.http_maintenance_request
         self.assertTrue(request_1)
 
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(500)
             mock_requests.exceptions.RequestException = Exception
-            self.env["service.instance"].cron_check_http_services()
+            self.env["service.instance"].cron_check_http_services()  # still KO
 
         self.assertEqual(self.service_instance.http_maintenance_request, request_1)
         self.assertEqual(
@@ -181,7 +197,8 @@ class TestHttpMonitoring(TransactionCase):
         self.assertEqual(self.service_instance.last_http_status_code, 404)
 
     # ------------------------------------------------------------------
-    # Test 9 -- Webhook called when a new maintenance.request is created
+    # Test 9 -- Webhook called only once the request is actually created
+    # (after confirmation delay), not on the first observed failure
     # ------------------------------------------------------------------
     def test_webhook_called_on_new_request(self):
         self.env["ir.config_parameter"].sudo().set_param(
@@ -190,7 +207,17 @@ class TestHttpMonitoring(TransactionCase):
         )
         with (
             patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
+            patch(EQUIPMENT_HTTP_REQUESTS) as mock_http,
+        ):
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
+
+        mock_http.post.assert_not_called()
+        self._backdate_first_ko(self.service_instance)
+
+        with (
+            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
             patch(EQUIPMENT_HTTP_REQUESTS) as mock_http,
         ):
             mock_requests.get.return_value = _mock_response(500)
@@ -209,15 +236,22 @@ class TestHttpMonitoring(TransactionCase):
         self.env["ir.config_parameter"].sudo().set_param(
             "maintenance_service_http_monitoring.webhook_url", ""
         )
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
+
+        self._backdate_first_ko(self.service_instance)
+
         with (
             patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
             patch(EQUIPMENT_HTTP_REQUESTS) as mock_http,
         ):
             mock_requests.get.return_value = _mock_response(500)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
 
+        self.assertTrue(self.service_instance.http_maintenance_request)
         mock_http.post.assert_not_called()
 
     # ------------------------------------------------------------------
@@ -235,22 +269,25 @@ class TestHttpMonitoring(TransactionCase):
         self.assertFalse(self.service_instance.last_http_check_date)
 
     # ------------------------------------------------------------------
-    # Test 12 -- Transient failure (KO pass 1, OK pass 2) -> no request created
+    # Test 12 -- Transient failure (KO, then back OK before confirmation) ->
+    # no request created, and the KO streak is reset
     # ------------------------------------------------------------------
     def test_transient_failure_no_request_created(self):
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
-            mock_requests.get.side_effect = [
-                _mock_response(500),  # pass 1: KO
-                _mock_response(200),  # pass 2 (retry): OK
-            ]
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
+
+        self.assertTrue(self.service_instance.http_first_ko_at)
+
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(200)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
 
         self.assertTrue(self.service_instance.http_status_ok)
         self.assertEqual(self.service_instance.last_http_status_code, 200)
+        self.assertFalse(self.service_instance.http_first_ko_at)
         self.assertFalse(self.service_instance.http_maintenance_request)
         self.assertEqual(
             self.env["maintenance.request"].search_count(
@@ -260,25 +297,33 @@ class TestHttpMonitoring(TransactionCase):
         )
 
     # ------------------------------------------------------------------
-    # Test 13 -- Confirmed failure (KO pass 1 and 2) -> request created
+    # Test 13 -- Confirmed failure (KO continuously past the confirmation delay)
+    # -> request created
     # ------------------------------------------------------------------
     def test_confirmed_failure_creates_request(self):
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP) as mock_sleep,
-        ):
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(503)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
 
-        mock_sleep.assert_called_once_with(2)
-        self.assertEqual(mock_requests.get.call_count, 2)
+        self.assertEqual(mock_requests.get.call_count, 1)
+        self.assertFalse(self.service_instance.http_maintenance_request)
+
+        self._backdate_first_ko(self.service_instance)
+
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(503)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
+
+        self.assertEqual(mock_requests.get.call_count, 1)
         self.assertFalse(self.service_instance.http_status_ok)
         self.assertEqual(self.service_instance.last_http_status_code, 503)
         self.assertTrue(self.service_instance.http_maintenance_request)
 
     # ------------------------------------------------------------------
     # Test 14 -- 2 KO services on same equipment -> 2 distinct requests
+    # once both reach the confirmation delay
     # ------------------------------------------------------------------
     def test_two_ko_services_same_equipment_create_two_requests(self):
         service2 = self.env["service"].create({"name": "Test Service 2"})
@@ -290,10 +335,14 @@ class TestHttpMonitoring(TransactionCase):
             }
         )
 
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
+
+        self._backdate_first_ko(self.service_instance, service_instance2)
+
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(500)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
@@ -317,11 +366,16 @@ class TestHttpMonitoring(TransactionCase):
     # Test 15 -- Service recovery closes the open request and posts a note
     # ------------------------------------------------------------------
     def test_service_recovery_closes_request(self):
-        # First cron run: service is KO -> request created
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
+        # First cron run: service is KO, streak just started -> no request yet
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
+            mock_requests.get.return_value = _mock_response(500)
+            mock_requests.exceptions.RequestException = Exception
+            self.env["service.instance"].cron_check_http_services()
+
+        self._backdate_first_ko(self.service_instance)
+
+        # Second cron run: streak confirmed -> request created
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(500)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
@@ -330,11 +384,8 @@ class TestHttpMonitoring(TransactionCase):
         self.assertTrue(request)
         self.assertFalse(request.stage_id.done)
 
-        # Second cron run: service is back OK -> request auto-closed
-        with (
-            patch(SERVICE_INSTANCE_REQUESTS) as mock_requests,
-            patch(SERVICE_INSTANCE_SLEEP),
-        ):
+        # Next cron run: service is back OK -> request auto-closed
+        with patch(SERVICE_INSTANCE_REQUESTS) as mock_requests:
             mock_requests.get.return_value = _mock_response(200)
             mock_requests.exceptions.RequestException = Exception
             self.env["service.instance"].cron_check_http_services()
@@ -343,6 +394,8 @@ class TestHttpMonitoring(TransactionCase):
         self.assertTrue(request.stage_id.done)
         # http_maintenance_request must be cleared on the service instance
         self.assertFalse(self.service_instance.http_maintenance_request)
+        # KO streak must be reset
+        self.assertFalse(self.service_instance.http_first_ko_at)
         # A chatter note must have been posted mentioning the service URL
         notes = request.message_ids.filtered(
             lambda m: self.service_instance.service_url in (m.body or "")

maintenance_service_http_monitoring/views/service_instance_views.xml

@@ -18,6 +18,7 @@
                 <field name="last_http_status_code" />
                 <field name="http_status_ok" />
                 <field name="http_maintenance_request" optional="hide" />
+                <field name="http_first_ko_at" optional="hide" />
             </field>
         </field>
     </record>