[Bug 42714] New: Koha::Script should automatically log start and end of scripts when they are run
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 Bug ID: 42714 Summary: Koha::Script should automatically log start and end of scripts when they are run Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Command-line Utilities Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: jake.deery@openfifth.co.uk, robin@catalyst.net.nz Currently, our scripts/cronjobs are expected to add cronlogaction calls at the start and end to ensure they are recorded in the action logs. This is tricky as we have to maintain these lines in all cron scripts An easier way to do this is to add INIT and END blocks to Koha::Script - these can log the run, and require only one line per file to add the logging. Adding to Koha::Script will mean that any command line script using it will be recorded, but I think this is actually a benefit and makes tracing actions easier -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #1 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 199662 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199662&action=edit Bug 42714: Add automatic cronlogaction logging to Koha::Script Add INIT and END blocks to Koha::Script that automatically call cronlogaction on script start (with command line options) and completion. This removes the need for scripts to manually handle cron log entries. This will mean that any script importing Koha::Script will be logged, but I think the addition is beneficial. To test: 1 - Ensure CronjobLog is enabled 2 - perl misc/cronjobs/fines.pl --test 3 - Confirm start and end are logged in action logs: http://localhost:8081/cgi-bin/koha/tools/viewlog.pl?src=&user=&modules=CRONJOBS&actions=&object=&info=&interfaces=&from=&to=&output=screen&basename=Export&report_name=&do_it=1 4 - Apply patch 5 - Repeat 2 6 - Reload the logs, confirm the new logs look the same 7777777 - Sign off Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #2 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 199663 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199663&action=edit Bug 42714: Remove redundant manual cronlogaction calls from scripts Remove start/end cronlogaction calls and unused C4::Log imports from scripts that use Koha::Script, as these are now handled automatically by the INIT/END blocks. Scripts with mid-execution informational logging (plugins_nightly, harvest_oai, fines, process_message_queue) retain their C4::Log import and non-start/end cronlogaction calls. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #3 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 199664 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199664&action=edit Bug 42714: Add missing use Koha::Script -cron to cron scripts Add Koha::Script -cron to restrict_patrons_with_failed_notices.pl and sftp_file.pl which used cronlogaction directly without importing Koha::Script. Remove their now-redundant manual calls. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #4 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 199665 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199665&action=edit Bug 42714: Adjust cronlogaction to use caller(1) Since we are calling from within another module we need more caller information to log the correct script -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199662|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199663|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199664|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199665|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 199690 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199690&action=edit Bug 42714: Add automatic cronlogaction logging to Koha::Script Add INIT and END blocks to Koha::Script that automatically call cronlogaction on script start (with command line options) and completion. This removes the need for scripts to manually handle cron log entries. This will mean that any script importing Koha::Script will be logged, but I think the addition is beneficial. To test: 1 - Ensure CronjobLog is enabled 2 - perl misc/cronjobs/fines.pl --test 3 - Confirm start and end are logged in action logs: http://localhost:8081/cgi-bin/koha/tools/viewlog.pl?src=&user=&modules=CRONJOBS&actions=&object=&info=&interfaces=&from=&to=&output=screen&basename=Export&report_name=&do_it=1 4 - Apply patch 5 - Repeat 2 6 - Reload the logs, confirm the new logs look the same 7777777 - Sign off Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 199691 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199691&action=edit Bug 42714: Remove redundant manual cronlogaction calls from scripts Remove start/end cronlogaction calls and unused C4::Log imports from scripts that use Koha::Script, as these are now handled automatically by the INIT/END blocks. Scripts with mid-execution informational logging (plugins_nightly, harvest_oai, fines, process_message_queue) retain their C4::Log import and non-start/end cronlogaction calls. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #7 from David Nind <david@davidnind.com> --- Created attachment 199692 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199692&action=edit Bug 42714: Add missing use Koha::Script -cron to cron scripts Add Koha::Script -cron to restrict_patrons_with_failed_notices.pl and sftp_file.pl which used cronlogaction directly without importing Koha::Script. Remove their now-redundant manual calls. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 199693 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199693&action=edit Bug 42714: Adjust cronlogaction to use caller(1) Since we are calling from within another module we need more caller information to log the correct script Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #9 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. The log entries are the same before and after the patch (numbers in the object column different): ... Cron jobs Run 5430 misc/cronjobs/fines.pl --test ... Cron jobs End 5430 misc/cronjobs/fines.pl COMPLETED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk CC| |martin.renvoize@openfifth.c | |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42766 Patch complexity|Trivial patch |Small patch Sponsorship status|--- |Unsponsored Status|Signed Off |Passed QA Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42766 [Bug 42766] Add ScriptLog syspref and auto-logging for non-cron Koha scripts -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199690|0 |1 is obsolete| | Attachment #199691|0 |1 is obsolete| | Attachment #199692|0 |1 is obsolete| | Attachment #199693|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199873 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199873&action=edit Bug 42714: Add automatic cronlogaction logging to Koha::Script Add INIT and END blocks to Koha::Script that automatically call cronlogaction on script start (with command line options) and completion. This removes the need for scripts to manually handle cron log entries. This will mean that any script importing Koha::Script will be logged, but I think the addition is beneficial. To test: 1 - Ensure CronjobLog is enabled 2 - perl misc/cronjobs/fines.pl --test 3 - Confirm start and end are logged in action logs: http://localhost:8081/cgi-bin/koha/tools/viewlog.pl?src=&user=&modules=CRONJOBS&actions=&object=&info=&interfaces=&from=&to=&output=screen&basename=Export&report_name=&do_it=1 4 - Apply patch 5 - Repeat 2 6 - Reload the logs, confirm the new logs look the same 7 - Sign off Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199874 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199874&action=edit Bug 42714: Remove redundant manual cronlogaction calls from scripts Remove start/end cronlogaction calls and unused C4::Log imports from scripts that use Koha::Script, as these are now handled automatically by the INIT/END blocks. Scripts with mid-execution informational logging (plugins_nightly, harvest_oai, fines, process_message_queue) retain their C4::Log import and non-start/end cronlogaction calls. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199875 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199875&action=edit Bug 42714: Add missing use Koha::Script -cron to cron scripts Add Koha::Script -cron to restrict_patrons_with_failed_notices.pl and sftp_file.pl which used cronlogaction directly without importing Koha::Script. Remove their now-redundant manual calls. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #13 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199876 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199876&action=edit Bug 42714: Adjust cronlogaction to use caller(1) Since we are calling from within another module we need more caller information to log the correct script Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #14 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199877 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199877&action=edit Bug 42714: (QA follow-up) Use \$0 instead of caller() in cronlogaction caller(1) was introduced to skip past Koha/Script.pm when cronlogaction is called from the INIT/END blocks, but it breaks any direct caller that invokes cronlogaction from within a Try::Tiny try/catch closure or a callback sub - in those contexts caller(1) resolves to Try/Tiny.pm (or the calling module) rather than the script itself. \$0 is always the path of the running script regardless of call depth, INIT/END phase, or enclosing closure, making it the correct choice here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #15 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199878 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199878&action=edit Bug 42714: (QA follow-up) Limit auto-logging to -cron scripts The INIT/END blocks in Koha::Script fire for every consumer of the module, including non-cron admin and migration tools. Logging those as CRONJOBS when CronjobLog is enabled would pollute the action log with entries for tools that are not scheduled jobs. Introduce a package-level $_cron flag that is set by import() when the -cron argument is passed (import() runs at compile time, before INIT blocks execute, so the flag is in the correct state when INIT fires). Guard both the INIT and END blocks behind this flag so only scripts that explicitly declare themselves as cron scripts receive automatic logging. Also update the import() POD to document the auto-logging behaviour. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #16 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199879 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199879&action=edit Bug 42714: (QA follow-up) Add tests for cronlogaction and Koha::Script logging t/db_dependent/Log.t - Add subtest verifying cronlogaction() prefixes loginfo with \$0 (the running script path), appends the info parameter, defaults the action to 'Run', and respects custom action values. t/Koha/Script.t - Assert that \$Koha::Script::_cron is false when the module is loaded without the -cron flag, confirming INIT/END blocks will not log. t/Koha/Script_cron.t - Assert that \$Koha::Script::_cron is true when the module is loaded with the -cron flag, confirming INIT/END blocks will log. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #17 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 199900 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199900&action=edit Bug 42714: (QA follow-up) Add comment with Perl doc for bash Patch from commit 3a07a6f -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |26.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #18 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Thanks everyone! Pushed to main for 26.11! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@openfifth.co.u | |k Keywords| |additional_work_needed --- Comment #19 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- This is causing the following tests to fail: prove t/00-load.t prove xt/author/pod_coverage.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 --- Comment #20 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Created attachment 203577 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203577&action=edit Bug 42714: RM follow-up: Fix tests prove t/00-load.t prove xt/author/pod_coverage.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42714 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting CC| |lucas@bywatersolutions.com --- Comment #21 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Enhancement, no 26.05.x backport. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org