Because of the way Koha::Logger has been used to log to different categories based on the interface and caller, it can be extremely hard to log all of a particular log statement to one place. For custom report runs, the category is plack-intranet.C4::Reports::Guided when run from the web interface, cron.C4::Reports::Guided when run from runreport.pl, and plack-intranet.C4::Auth when run from svc/report. We should add a more standardized report run log, both with and without the full query, so that administrators can log all report runs to a centralized location. If an administrator were to need the "point of entry" for reports, it is easy to include via parameters in PatternLayout.
Created attachment 161427 [details] [review] Bug 35907: Add ability to log all custom report runs with or without query Because of the way Koha::Logger has been used to log to different categories based on the interface and caller, it can be extremely hard to log all of a particular log statement to one place. For custom report runs, the category is plack-intranet.C4::Reports::Guided when run from the web interface, cron.C4::Reports::Guided when run from runreport.pl, and plack-intranet.C4::Auth when run from svc/report. We should add a more standardized report run log, both with and without the full query, so that administrators can log all report runs to a centralized location. If an administrator were to need the "point of entry" for reports, it is easy to include via parameters in PatternLayout. Test Plan: 1) Apply this patch 2) Modify your log4perl file, add the following: log4perl.logger.reports.execute.time = INFO, REPORTTIME log4perl.appender.REPORTTIME=Log::Log4perl::Appender::File log4perl.appender.REPORTTIME.filename=/tmp/report-time.log log4perl.appender.REPORTTIME.mode=append log4perl.appender.REPORTTIME.layout=PatternLayout log4perl.appender.REPORTTIME.layout.ConversionPattern=[%d] [%p] [%P] %m%n log4perl.appender.REPORTTIME.utf8=1 log4perl.logger.reports.execute.query = INFO, REPORTQUERY log4perl.appender.REPORTQUERY=Log::Log4perl::Appender::File log4perl.appender.REPORTQUERY.filename=/tmp/report-query.log log4perl.appender.REPORTQUERY.mode=append log4perl.appender.REPORTQUERY.layout=PatternLayout log4perl.appender.REPORTQUERY.layout.ConversionPattern=[%d] [%p] [%P] %m%n log4perl.appender.REPORTQUERY.utf8=1 3) Restart all the things! 4) Run a report somehow: CLI: ./misc/cronjobs/runreport.pl 1 API: /cgi-bin/koha/svc/report?id=1 Web: /cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Run this report 5) Note the report runs are logged to /tmp/report-time.log and /tmp/report-query.log
Created attachment 161429 [details] [review] Bug 35907: Tidy execute_query
Created attachment 161452 [details] [review] Bug 35907: Add ability to log all custom report runs with or without query Because of the way Koha::Logger has been used to log to different categories based on the interface and caller, it can be extremely hard to log all of a particular log statement to one place. For custom report runs, the category is plack-intranet.C4::Reports::Guided when run from the web interface, cron.C4::Reports::Guided when run from runreport.pl, and plack-intranet.C4::Auth when run from svc/report. We should add a more standardized report run log, both with and without the full query, so that administrators can log all report runs to a centralized location. If an administrator were to need the "point of entry" for reports, it is easy to include via parameters in PatternLayout. Test Plan: 1) Apply this patch 2) Modify your log4perl file, add the following: log4perl.logger.reports.execute.time = INFO, REPORTTIME log4perl.appender.REPORTTIME=Log::Log4perl::Appender::File log4perl.appender.REPORTTIME.filename=/tmp/report-time.log log4perl.appender.REPORTTIME.mode=append log4perl.appender.REPORTTIME.layout=PatternLayout log4perl.appender.REPORTTIME.layout.ConversionPattern=[%d] [%p] [%P] %m%n log4perl.appender.REPORTTIME.utf8=1 log4perl.logger.reports.execute.query = INFO, REPORTQUERY log4perl.appender.REPORTQUERY=Log::Log4perl::Appender::File log4perl.appender.REPORTQUERY.filename=/tmp/report-query.log log4perl.appender.REPORTQUERY.mode=append log4perl.appender.REPORTQUERY.layout=PatternLayout log4perl.appender.REPORTQUERY.layout.ConversionPattern=[%d] [%p] [%P] %m%n log4perl.appender.REPORTQUERY.utf8=1 3) Restart all the things! 4) Run a report somehow: CLI: ./misc/cronjobs/runreport.pl 1 API: /cgi-bin/koha/svc/report?id=1 Web: /cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Run this report 5) Note the report runs are logged to /tmp/report-time.log and /tmp/report-query.log
Created attachment 161453 [details] [review] Bug 35907: Tidy execute_query
Created attachment 161458 [details] [review] Bug 35907: Add ability to log all custom report runs with or without query Because of the way Koha::Logger has been used to log to different categories based on the interface and caller, it can be extremely hard to log all of a particular log statement to one place. For custom report runs, the category is plack-intranet.C4::Reports::Guided when run from the web interface, cron.C4::Reports::Guided when run from runreport.pl, and plack-intranet.C4::Auth when run from svc/report. We should add a more standardized report run log, both with and without the full query, so that administrators can log all report runs to a centralized location. If an administrator were to need the "point of entry" for reports, it is easy to include via parameters in PatternLayout. Test Plan: 1) Apply this patch 2) Modify your log4perl file, add the following: log4perl.logger.reports.execute.time = INFO, REPORTTIME log4perl.appender.REPORTTIME=Log::Log4perl::Appender::File log4perl.appender.REPORTTIME.filename=/tmp/report-time.log log4perl.appender.REPORTTIME.mode=append log4perl.appender.REPORTTIME.layout=PatternLayout log4perl.appender.REPORTTIME.layout.ConversionPattern=[%d] [%p] [%P] %m%n log4perl.appender.REPORTTIME.utf8=1 log4perl.logger.reports.execute.query = INFO, REPORTQUERY log4perl.appender.REPORTQUERY=Log::Log4perl::Appender::File log4perl.appender.REPORTQUERY.filename=/tmp/report-query.log log4perl.appender.REPORTQUERY.mode=append log4perl.appender.REPORTQUERY.layout=PatternLayout log4perl.appender.REPORTQUERY.layout.ConversionPattern=[%d] [%p] [%P] %m%n log4perl.appender.REPORTQUERY.utf8=1 3) Restart all the things! 4) Run a report somehow: CLI: ./misc/cronjobs/runreport.pl 1 API: /cgi-bin/koha/svc/report?id=1 Web: /cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Run this report 5) Note the report runs are logged to /tmp/report-time.log and /tmp/report-query.log
Created attachment 161459 [details] [review] Bug 35907: Tidy execute_query
I tried testing this but couldn't find the log files /tmp/report-time.log and /tmp/report-query.log I may be missing a step. 1. Applied the patch 2. Added the provided config to /koha/etc/log4perl.conf 3. restart_all 4. Set all Logging system prefs to 'Log' 5. Ran some reports 6. Check /tmp for report-time.log and report-query.log There were files in /tmp but none name like that.
(In reply to Brendan Lawlor from comment #7) > I tried testing this but couldn't find the log files /tmp/report-time.log > and /tmp/report-query.log > > I may be missing a step. > 1. Applied the patch > 2. Added the provided config to /koha/etc/log4perl.conf > 3. restart_all > 4. Set all Logging system prefs to 'Log' > 5. Ran some reports > 6. Check /tmp for report-time.log and report-query.log > > There were files in /tmp but none name like that. I just ran through the test plan and I can confirm it's still working as intended! Here is a video demonstration using runreport.pl: https://monosnap.com/file/CcQHcG8kTlJhvOpXxOITdWc5wj5eea
Created attachment 161806 [details] [review] Bug 35907: Add ability to log all custom report runs with or without query Because of the way Koha::Logger has been used to log to different categories based on the interface and caller, it can be extremely hard to log all of a particular log statement to one place. For custom report runs, the category is plack-intranet.C4::Reports::Guided when run from the web interface, cron.C4::Reports::Guided when run from runreport.pl, and plack-intranet.C4::Auth when run from svc/report. We should add a more standardized report run log, both with and without the full query, so that administrators can log all report runs to a centralized location. If an administrator were to need the "point of entry" for reports, it is easy to include via parameters in PatternLayout. Test Plan: 1) Apply this patch 2) Modify your log4perl file, add the following: log4perl.logger.reports.execute.time = INFO, REPORTTIME log4perl.appender.REPORTTIME=Log::Log4perl::Appender::File log4perl.appender.REPORTTIME.filename=/tmp/report-time.log log4perl.appender.REPORTTIME.mode=append log4perl.appender.REPORTTIME.layout=PatternLayout log4perl.appender.REPORTTIME.layout.ConversionPattern=[%d] [%p] [%P] %m%n log4perl.appender.REPORTTIME.utf8=1 log4perl.logger.reports.execute.query = INFO, REPORTQUERY log4perl.appender.REPORTQUERY=Log::Log4perl::Appender::File log4perl.appender.REPORTQUERY.filename=/tmp/report-query.log log4perl.appender.REPORTQUERY.mode=append log4perl.appender.REPORTQUERY.layout=PatternLayout log4perl.appender.REPORTQUERY.layout.ConversionPattern=[%d] [%p] [%P] %m%n log4perl.appender.REPORTQUERY.utf8=1 3) Restart all the things! 4) Run a report somehow: CLI: ./misc/cronjobs/runreport.pl 1 API: /cgi-bin/koha/svc/report?id=1 Web: /cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Run this report 5) Note the report runs are logged to /tmp/report-time.log and /tmp/report-query.log Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Created attachment 161807 [details] [review] Bug 35907: Tidy execute_query Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
It turned out I didn't edit the correct config file in my first test. I edited /kohadevbox/koha/etc/log4perl.conf I guess maybe that is just an example config file. Thanks for showing me where the real config file is /etc/koha/sites/kohadev/log4perl.conf
(In reply to Brendan Lawlor from comment #11) > It turned out I didn't edit the correct config file in my first test. > > I edited /kohadevbox/koha/etc/log4perl.conf > I guess maybe that is just an example config file. > > Thanks for showing me where the real config file is > /etc/koha/sites/kohadev/log4perl.conf Ack! Thanks for the signoff Brendan!
Created attachment 166501 [details] [review] Bug 35907: Add ability to log all custom report runs with or without query Because of the way Koha::Logger has been used to log to different categories based on the interface and caller, it can be extremely hard to log all of a particular log statement to one place. For custom report runs, the category is plack-intranet.C4::Reports::Guided when run from the web interface, cron.C4::Reports::Guided when run from runreport.pl, and plack-intranet.C4::Auth when run from svc/report. We should add a more standardized report run log, both with and without the full query, so that administrators can log all report runs to a centralized location. If an administrator were to need the "point of entry" for reports, it is easy to include via parameters in PatternLayout. Test Plan: 1) Apply this patch 2) Modify your log4perl file, add the following: log4perl.logger.reports.execute.time = INFO, REPORTTIME log4perl.appender.REPORTTIME=Log::Log4perl::Appender::File log4perl.appender.REPORTTIME.filename=/tmp/report-time.log log4perl.appender.REPORTTIME.mode=append log4perl.appender.REPORTTIME.layout=PatternLayout log4perl.appender.REPORTTIME.layout.ConversionPattern=[%d] [%p] [%P] %m%n log4perl.appender.REPORTTIME.utf8=1 log4perl.logger.reports.execute.query = INFO, REPORTQUERY log4perl.appender.REPORTQUERY=Log::Log4perl::Appender::File log4perl.appender.REPORTQUERY.filename=/tmp/report-query.log log4perl.appender.REPORTQUERY.mode=append log4perl.appender.REPORTQUERY.layout=PatternLayout log4perl.appender.REPORTQUERY.layout.ConversionPattern=[%d] [%p] [%P] %m%n log4perl.appender.REPORTQUERY.utf8=1 3) Restart all the things! 4) Run a report somehow: CLI: ./misc/cronjobs/runreport.pl 1 API: /cgi-bin/koha/svc/report?id=1 Web: /cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Run this report 5) Note the report runs are logged to /tmp/report-time.log and /tmp/report-query.log Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 166502 [details] [review] Bug 35907: Tidy execute_query Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Kyle M Hall from comment #0) > Because of the way Koha::Logger has been used to log to different categories > based on the interface and caller, it can be extremely hard to log all of a > particular log statement to one place. Just change your log4perl.conf configuration? And a sysadmin could always concatenate all logs and grep on whatever he wants to find? This works as expected but feels a bit like a workaround. + my $prefix = $params->{prefix} // 1; I would rather rename this one personally. What you want here is to suppress the plack prefix, not add a custom prefix. no_plack => 1 or something like that ? If the log4perl config for reports.execute.query is not found, it seems that Logger just silently ignores logging. This costed me about 15ms per call. Probably acceptable. Passing QA since it is just a few lines. But hesitantly since our code doesnt get cleaner.
Pushed for 24.05! Well done everyone, thank you!
Not backported to 23.11.x
This looks like backend architecture modification, nothing to change/edit in the manual. Please reopen with more details if needed.