Bug 28477 - INTRANET and OPAC loggers should print to STDERR instead of to file
Summary: INTRANET and OPAC loggers should print to STDERR instead of to file
Status: RESOLVED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-28 00:25 UTC by David Cook
Modified: 2023-06-02 01:57 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 28477: Change INTRANET and OPAC logs to use STDERR rather than file (2.57 KB, patch)
2021-05-28 00:31 UTC, David Cook
Details | Diff | Splinter Review
Bug 28477: Add unit test to prove logging to STDERR works (2.49 KB, patch)
2021-05-28 01:27 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2021-05-28 00:25:25 UTC
Bug 25284 notes that Plack will fail to start up if the /var/log/koha/<site>/opac-error.log file isn't writeable by the specific Koha Linux user. It has a workaround for chowning all the log files, but that's really not the best way to go.

We shouldn't be messing around with the log files that belong to the Apache web server. 

Instead, we should be printing to STDERR using the Log4Perl logger. For CGI Koha using Apache, that text will wind up in the correct web server log file automatically. For Plack Koha, it would wind up the correct web server log file as well... or thanks to Bug 16357 they'll wind up special log files created and owned by Koha.

It's as easy as changing the Log4Perl configuration as follows:

FROM:
log4perl.appender.INTRANET=Log::Log4perl::Appender::File
log4perl.appender.INTRANET.filename=__LOG_DIR__/intranet-error.log
log4perl.appender.OPAC=Log::Log4perl::Appender::File
log4perl.appender.OPAC.filename=__LOG_DIR__/opac-error.log

TO:

log4perl.appender.INTRANET=Log::Log4perl::Appender::Screen
log4perl.appender.INTRANET.stderr  = 1
log4perl.appender.OPAC=Log::Log4perl::Appender::Screen
log4perl.appender.OPAC.stderr  = 1
Comment 1 David Cook 2021-05-28 00:29:06 UTC
The logger is a bit of a pain to test though, so not sure how much energy I actually have to devote to testing this change...
Comment 2 David Cook 2021-05-28 00:31:38 UTC
Created attachment 121466 [details] [review]
Bug 28477: Change INTRANET and OPAC logs to use STDERR rather than file

This patch changes the default Log4Perl config to use STDERR instead
of the Apache log files for the INTRANET and OPAC loggers.
Comment 3 David Cook 2021-05-28 01:27:05 UTC
Created attachment 121467 [details] [review]
Bug 28477: Add unit test to prove logging to STDERR works

This patch shows that using the Screen/Stderr=1 Log4perl configuration
with Koha::Logger will output to STDERR.
Comment 4 David Cook 2021-05-28 01:27:43 UTC
Note that the first test in t/Logger.t was already broken, and looks like a pain to test due to Log::Log4perl being a singleton...
Comment 5 David Cook 2021-05-28 01:45:44 UTC
If folk have tests for the actual intranet and opac scripts, that would be great. 

I can't be bothered trying to work those out right now. 

--

Although the more I think about it...

While this should work fine with Apache, it might not work as one might expect with Plack in regards to Bug 16357.

Bug 16357 overwrites SIGWARN, but Log::Log4perl isn't using SIGWARN. It's printing to STDERR. So the errors would wind up in plack-error.log (rather than plack-intranet-error.log or plack-opac-error.log). 

That's not great because the logs aren't separated...

--

Maybe we shouldn't have called the error files plack-opac-error.log and plack-intranet-error.log and instead just called them koha-opac-error.log and koha-intranet-error.log and set both appenders to logging to the same file...
Comment 6 Andrii Nugged 2021-11-26 12:06:17 UTC
Sidenote: we also have pack-error.log without timestamps, which makes backtrace and debugging of important dies very hard on productions. Do we have anything on that?
Comment 7 David Cook 2021-11-30 06:23:27 UTC
(In reply to Andrew Nugged from comment #6)
> Sidenote: we also have pack-error.log without timestamps, which makes
> backtrace and debugging of important dies very hard on productions. Do we
> have anything on that?

Bug 16357 should resolve that issue. Looks like it's available from 20.11+ and backported to 20.05.05.
Comment 8 David Cook 2023-06-02 01:57:13 UTC
Koha has changed its Plack logging since this was reported