From 99810fce9b9729421625cb72cee8ba182f396038 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/settings.py                      |  1 +
 tests/test_publisher.py                | 68 +++++++++++++++++++++++++-
 wcs/qommon/cron.py                     | 35 ++++++++++++-
 wcs/qommon/management/commands/cron.py |  5 +-
 wcs/settings.py                        |  1 +
 5 files changed, 106 insertions(+), 4 deletions(-)

diff --git a/tests/settings.py b/tests/settings.py
index 11c18924..08f8ada8 100644
--- a/tests/settings.py
+++ b/tests/settings.py
@@ -1,3 +1,4 @@
 TIME_ZONE = 'Europe/Paris'
 LANGUAGE_CODE = 'en-us'
 ALLOWED_HOSTS = ['*']
+CRON_LOG_FILENAME = '/var/lib/wcs/cron-test.log'
diff --git a/tests/test_publisher.py b/tests/test_publisher.py
index 093db039..18197033 100644
--- a/tests/test_publisher.py
+++ b/tests/test_publisher.py
@@ -206,7 +206,20 @@ def test_import_config_zip():
     assert pub.cfg['sp'] == {'what': 'ever'}
 
 
-def test_cron_command():
+def test_cron_command(settings):
+    def clear_log_file():
+        utcnow = datetime.datetime.utcnow()
+        with open('%s-%s' % (settings.CRON_LOG_FILENAME, utcnow.strftime('%Y%m%d')), 'w'):
+            pass
+
+    def get_logs():
+        utcnow = datetime.datetime.utcnow()
+        with open('%s-%s' % (settings.CRON_LOG_FILENAME, utcnow.strftime('%Y%m%d'))) as fd:
+            lines = fd.readlines()
+        lines = [line[27:].strip() for line in lines]  # 27 chars for date & time
+        return lines
+
+    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 +234,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')
                 ]
+                clear_log_file()
                 call_command('cron')
                 assert cron_worker.call_count == 3
+                assert get_logs() == [
+                    'starting cron (minutes offset is %s)' % offset,
+                    '[tenant example.net] start',
+                    '[tenant foo.bar] start',
+                    '[tenant something.com] start',
+                ]
                 cron_worker.reset_mock()
+                clear_log_file()
                 call_command('cron', domain='example.net')
                 assert cron_worker.call_count == 1
+                assert get_logs() == [
+                    'starting cron (minutes offset is %s)' % offset,
+                    '[tenant example.net] start',
+                ]
                 cron_worker.reset_mock()
 
                 # disable cron on something.com
@@ -238,8 +263,14 @@ def test_cron_command():
                         '''
                     )
 
+                clear_log_file()
                 call_command('cron')
                 assert cron_worker.call_count == 2
+                assert get_logs() == [
+                    'starting cron (minutes offset is %s)' % offset,
+                    '[tenant example.net] start',
+                    '[tenant foo.bar] start',
+                ]
                 cron_worker.reset_mock()
                 os.unlink(site_options_path)
 
@@ -289,8 +320,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 +337,38 @@ def test_cron_command():
             call_command('cron', job_name='job1', domain='example.net')
             assert jobs == []
             get_publisher_class().cronjobs = []
+            clear_log_file()
             call_command('cron', job_name='job2', domain='example.net')
             assert jobs == ['job2']
+            assert get_logs() == [
+                'starting cron (minutes offset is %s)' % offset,
+                '[tenant example.net] start',
+            ]
+            get_publisher_class().cronjobs = []
+            jobs = []
+            clear_log_file()
+            with mock.patch('wcs.qommon.cron.CronJob.LONG_JOB_DURATION', 0):
+                call_command('cron', job_name='job2', domain='example.net')
+            assert get_logs() == [
+                'starting cron (minutes offset is %s)' % offset,
+                '[tenant example.net] start',
+                '[tenant example.net] long job: job2 (took 0 minutes)',
+            ]
+            assert jobs == ['job2']
+            get_publisher_class().cronjobs = []
+            jobs = []
+            clear_log_file()
+            with mock.patch('wcs.qommon.cron.CronJob.LONG_JOB_DURATION', 0):
+                call_command('cron', job_name='job3', domain='example.net')
+            assert get_logs() == [
+                'starting cron (minutes offset is %s)' % offset,
+                '[tenant example.net] start',
+                '[tenant example.net] job3: running on "foo" took 0 minutes',
+                '[tenant example.net] job3: running on "bar" took 0 minutes',
+                '[tenant example.net] job3: running on "blah" took 0 minutes',
+                '[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..6476722c 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 datetime
+import inspect
+from contextlib import contextmanager
+from time import perf_counter
+
 from django.conf import settings
+from quixote import get_publisher
 
 
 class CronJob:
@@ -25,6 +31,8 @@ class CronJob:
     days = None
     function = None
 
+    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 +41,27 @@ class CronJob:
         self.weekdays = weekdays
         self.days = days
 
+    @contextmanager
+    def log_long_job(self, obj_description=None):
+        start = perf_counter()
+        yield
+        duration = perf_counter() - start
+        if duration > self.LONG_JOB_DURATION:
+            minutes = int(duration / 60)
+            if obj_description:
+                self.log('%s: running on "%s" took %d minutes' % (self.name, obj_description, minutes))
+            else:
+                self.log('long job: %s (took %s minutes)' % (self.name, minutes))
+
+    @staticmethod
+    def log(message, with_tenant=True):
+        utcnow = datetime.datetime.utcnow()
+        tenant_prefix = ''
+        if with_tenant:
+            tenant_prefix = '[tenant %s] ' % get_publisher().tenant.hostname
+        with open('%s-%s' % (settings.CRON_LOG_FILENAME, utcnow.strftime('%Y%m%d')), 'a+') as fd:
+            fd.write('%s %s%s\n' % (utcnow.isoformat(), tenant_prefix, message))
+
 
 def cron_worker(publisher, now, job_name=None):
     # reindex user and formdata if needed (should only be run once)
@@ -66,6 +95,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.getfullargspec(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..61354514 100644
--- a/wcs/qommon/management/commands/cron.py
+++ b/wcs/qommon/management/commands/cron.py
@@ -21,7 +21,7 @@ import time
 from django.conf import settings
 from django.core.management.base import BaseCommand, CommandError
 
-from wcs.qommon.cron import cron_worker
+from wcs.qommon.cron import CronJob, cron_worker
 from wcs.qommon.publisher import get_publisher_class
 from wcs.qommon.vendor import locket
 from wcs.wf.jump import JUMP_TIMEOUT_INTERVAL
@@ -46,6 +46,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
+        CronJob.log('starting cron (minutes offset is %s)' % offset, with_tenant=False)
         lockfile = os.path.join(tempfile.gettempdir(), 'wcs-cron-in-progress.lock')
         if options.get('domain'):
             domains = [options.get('domain')]
@@ -67,6 +69,7 @@ class Command(BaseCommand):
                         continue
                     if verbosity > 1:
                         print('cron work on %s' % hostname)
+                    CronJob.log('start')
                     cron_worker(publisher, now, job_name=options.get('job_name'))
             if verbosity > 2:
                 print('cron end (release lock %s)' % lockfile)
diff --git a/wcs/settings.py b/wcs/settings.py
index 8c206484..c083ec4a 100644
--- a/wcs/settings.py
+++ b/wcs/settings.py
@@ -191,6 +191,7 @@ REQUESTS_TIMEOUT = 28
 # components, one should disable cron jobs execution on secondary servers;
 # set the following variable True disables "cron" management command
 DISABLE_CRON_JOBS = False
+CRON_LOG_FILENAME = '/var/lib/wcs/cron.log'
 
 # w.c.s. can have very large forms, in backoffice and frontoffice
 DATA_UPLOAD_MAX_NUMBER_FIELDS = 2000  # Django default is 1000
-- 
2.33.0

