From 886568242c75cbd5806ba3d4852cd3f6dfd3cbc1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Mon, 11 Oct 2021 15:15:57 +0200
Subject: [PATCH 1/2] misc: add log mecanism for cron jobs (#57604)

---
 tests/test_publisher.py                | 58 +++++++++++++++++++++++++-
 wcs/qommon/cron.py                     | 30 ++++++++++++-
 wcs/qommon/management/commands/cron.py |  5 +++
 3 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/tests/test_publisher.py b/tests/test_publisher.py
index 093db039..d2b96dcd 100644
--- a/tests/test_publisher.py
+++ b/tests/test_publisher.py
@@ -1,6 +1,7 @@
 import datetime
 import io
 import json
+import logging
 import os
 import pickle
 import re
@@ -206,7 +207,9 @@ def test_import_config_zip():
     assert pub.cfg['sp'] == {'what': 'ever'}
 
 
-def test_cron_command():
+def test_cron_command(caplog, settings):
+    caplog.set_level(logging.INFO, logger="wcs.cron")
+    offset = ord(settings.SECRET_KEY[-1]) % 60
     pub = create_temporary_pub()
     with mock.patch('tempfile.gettempdir') as gettempdir:
         gettempdir.side_effect = lambda: pub.app_dir
@@ -221,11 +224,23 @@ def test_cron_command():
                 mock_tenants.return_value = [
                     Tenant(os.path.join(pub.app_dir, x)) for x in ('example.net', 'foo.bar', 'something.com')
                 ]
+                caplog.clear()
                 call_command('cron')
                 assert cron_worker.call_count == 3
+                assert caplog.record_tuples == [
+                    ('wcs.cron', logging.INFO, 'starting cron (minutes offset is %s)' % offset),
+                    ('wcs.cron', logging.INFO, '[tenant example.net] start'),
+                    ('wcs.cron', logging.INFO, '[tenant foo.bar] start'),
+                    ('wcs.cron', logging.INFO, '[tenant something.com] start'),
+                ]
                 cron_worker.reset_mock()
+                caplog.clear()
                 call_command('cron', domain='example.net')
                 assert cron_worker.call_count == 1
+                assert caplog.record_tuples == [
+                    ('wcs.cron', logging.INFO, 'starting cron (minutes offset is %s)' % offset),
+                    ('wcs.cron', logging.INFO, '[tenant example.net] start'),
+                ]
                 cron_worker.reset_mock()
 
                 # disable cron on something.com
@@ -238,8 +253,14 @@ def test_cron_command():
                         '''
                     )
 
+                caplog.clear()
                 call_command('cron')
                 assert cron_worker.call_count == 2
+                assert caplog.record_tuples == [
+                    ('wcs.cron', logging.INFO, 'starting cron (minutes offset is %s)' % offset),
+                    ('wcs.cron', logging.INFO, '[tenant example.net] start'),
+                    ('wcs.cron', logging.INFO, '[tenant foo.bar] start'),
+                ]
                 cron_worker.reset_mock()
                 os.unlink(site_options_path)
 
@@ -289,8 +310,11 @@ def test_cron_command():
         def job2(pub):
             jobs.append('job2')
 
-        def job3(pub):
+        def job3(pub, job):
             jobs.append('job3')
+            for key in ['foo', 'bar', 'blah']:
+                with job.log_long_job(key):
+                    pass
 
         @classmethod
         def register_test_cronjobs(cls):
@@ -303,8 +327,38 @@ def test_cron_command():
             call_command('cron', job_name='job1', domain='example.net')
             assert jobs == []
             get_publisher_class().cronjobs = []
+            caplog.clear()
             call_command('cron', job_name='job2', domain='example.net')
             assert jobs == ['job2']
+            assert caplog.record_tuples == [
+                ('wcs.cron', logging.INFO, 'starting cron (minutes offset is %s)' % offset),
+                ('wcs.cron', logging.INFO, '[tenant example.net] start'),
+            ]
+            get_publisher_class().cronjobs = []
+            jobs = []
+            caplog.clear()
+            with mock.patch('wcs.qommon.cron.CronJob.LONG_JOB_DURATION', 0):
+                call_command('cron', job_name='job2', domain='example.net')
+            assert caplog.record_tuples == [
+                ('wcs.cron', logging.INFO, 'starting cron (minutes offset is %s)' % offset),
+                ('wcs.cron', logging.INFO, '[tenant example.net] start'),
+                ('wcs.cron', logging.INFO, '[tenant example.net] long job: job2 (took 0 minutes)'),
+            ]
+            assert jobs == ['job2']
+            get_publisher_class().cronjobs = []
+            jobs = []
+            caplog.clear()
+            with mock.patch('wcs.qommon.cron.CronJob.LONG_JOB_DURATION', 0):
+                call_command('cron', job_name='job3', domain='example.net')
+            assert caplog.record_tuples == [
+                ('wcs.cron', logging.INFO, 'starting cron (minutes offset is %s)' % offset),
+                ('wcs.cron', logging.INFO, '[tenant example.net] start'),
+                ('wcs.cron', logging.INFO, '[tenant example.net] job3: running on "foo" took 0 minutes'),
+                ('wcs.cron', logging.INFO, '[tenant example.net] job3: running on "bar" took 0 minutes'),
+                ('wcs.cron', logging.INFO, '[tenant example.net] job3: running on "blah" took 0 minutes'),
+                ('wcs.cron', logging.INFO, '[tenant example.net] long job: job3 (took 0 minutes)'),
+            ]
+            assert jobs == ['job3']
 
 
 def test_clean_afterjobs():
diff --git a/wcs/qommon/cron.py b/wcs/qommon/cron.py
index e3a6eb5e..d2f46f3c 100644
--- a/wcs/qommon/cron.py
+++ b/wcs/qommon/cron.py
@@ -14,7 +14,13 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, see <http://www.gnu.org/licenses/>.
 
+import inspect
+import logging
+from contextlib import contextmanager
+from time import perf_counter
+
 from django.conf import settings
+from quixote import get_publisher
 
 
 class CronJob:
@@ -25,6 +31,9 @@ class CronJob:
     days = None
     function = None
 
+    logger = logging.getLogger('wcs.cron')
+    LONG_JOB_DURATION = 2 * 60  # 2 minutes
+
     def __init__(self, function, name=None, hours=None, minutes=None, weekdays=None, days=None):
         self.function = function
         self.name = name
@@ -33,6 +42,21 @@ class CronJob:
         self.weekdays = weekdays
         self.days = days
 
+    @contextmanager
+    def log_long_job(self, message=None):
+        start = perf_counter()
+        yield
+        duration = perf_counter() - start
+        if duration > self.LONG_JOB_DURATION:
+            minutes = int(duration / 60)
+            if message:
+                self.log('%s: running on "%s" took %d minutes' % (self.name, message, minutes))
+            else:
+                self.log('long job: %s (took %s minutes)' % (self.name, minutes))
+
+    def log(self, message):
+        self.logger.info('[tenant %s] %s', get_publisher().tenant.hostname, message)
+
 
 def cron_worker(publisher, now, job_name=None):
     # reindex user and formdata if needed (should only be run once)
@@ -66,6 +90,10 @@ def cron_worker(publisher, now, job_name=None):
         for extra_source in publisher.extra_sources:
             publisher.substitutions.feed(extra_source(publisher, None))
         try:
-            job.function(publisher)
+            with job.log_long_job():
+                if 'job' in inspect.getargspec(job.function).args:
+                    job.function(publisher, job=job)
+                else:
+                    job.function(publisher)
         except Exception as e:
             publisher.record_error(exception=e, context='[CRON]', notify=True)
diff --git a/wcs/qommon/management/commands/cron.py b/wcs/qommon/management/commands/cron.py
index a7f38bb8..f82afa46 100644
--- a/wcs/qommon/management/commands/cron.py
+++ b/wcs/qommon/management/commands/cron.py
@@ -14,6 +14,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, see <http://www.gnu.org/licenses/>.
 
+import logging
 import os
 import tempfile
 import time
@@ -29,6 +30,7 @@ from wcs.wf.jump import JUMP_TIMEOUT_INTERVAL
 
 class Command(BaseCommand):
     help = 'Execute cronjobs'
+    logger = logging.getLogger('wcs.cron')
 
     def add_arguments(self, parser):
         parser.set_defaults(verbosity=0)
@@ -46,6 +48,8 @@ class Command(BaseCommand):
             if verbosity > 1:
                 print('Command is ignored because DISABLE_CRON_JOBS is set in settings')
             return
+        offset = ord(settings.SECRET_KEY[-1]) % 60
+        self.logger.info('starting cron (minutes offset is %s)', offset)
         lockfile = os.path.join(tempfile.gettempdir(), 'wcs-cron-in-progress.lock')
         if options.get('domain'):
             domains = [options.get('domain')]
@@ -67,6 +71,7 @@ class Command(BaseCommand):
                         continue
                     if verbosity > 1:
                         print('cron work on %s' % hostname)
+                    self.logger.info('[tenant %s] start', hostname)
                     cron_worker(publisher, now, job_name=options.get('job_name'))
             if verbosity > 2:
                 print('cron end (release lock %s)' % lockfile)
-- 
2.33.0

