[Koha-bugs] [Bug 14167] Add Koha::Logger based on Log4perl

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14167

--- Comment #28 from Kyle M Hall <kyle at bywatersolutions.com> ---
(In reply to Marcel de Rooy from comment #27)
> QA Comment:
> Nice feature. Economic implementation of logging.
> Although the code looks good to me, I still have some concerns:
> 
> [1] For logging level and category handling, I made some tiny adjustments
> via amending or follow-up.

Excellent! 
> 
> [2] A bigger problem could be permissions on the logfile. 
> If you do not have sufficient permissions, Log4perl will make a crash. So
> this is a very fundamental issue. In Debian the koha instance user should
> have permissions on the file, but I noticed that after a forced logrotate,
> the file permissions for the new log file were changed to root and Koha
> crashed on that.
> What about other distros btw?

Is logrotate creating the files? I don't see a create command in
/etc/logrotate.d/koha-common.

It's possible that this could be fixed with an addition like:

create 660 root koha

where the owner can be root as long as we were to create a koha group that each
koha-* user belonged to. However, I don't have any idea how to write the
upgrade patch for that.

http://www.thecave.info/logrotate-set-file-permissions/

> [3] The BEGIN section of Koha::Logger assumes that it will have either a
> conf file via $ENV{LOG4PERL_CONF} or the koha-conf.xml.
> If you do not have either one, Koha will again crash on you (Configuration
> not defined).
> It is safer imo to go always via the koha-conf. If there is nothing in
> koha-conf, set some flag, warn once and do nothing or perhaps fall back to
> warn. (Note that you should trap all calls to warn, trace etc. in that
> case.) 
> Note that a new installation would be fine. But an upgrade does not
> guarantee the presence of a config file.
> Also note that you can pass some default configuration string to init() too. 

That could be fixed with a followup. Basically we should check for the
existance of the log4perl config file, and if it doesn't exist pass it a
default config that can be hard coded into the module ( and would be the same
as the config we have here ).

> By conclusion, we could argue that a Koha administrator should just take
> care of the perl module and the log4perl config file and push this as-is.
> But we also know that this will result in problems at various locations when
> upgrading. What would be the best way to go here?

I would say we should do what is in our power and is reasonable to make the
upgrade path as smooth as possible. It may be that we can't automate
everything, and we'll have to provide upgrade instructions, but none of this
seems too onerous.

> I will be happy to follow through on this report, but not in the next two
> weeks. (But another QAer is welcome too.) 
> 
> Changing status to reflect need for changes or clarification..

-- 
You are receiving this mail because:
You are watching all bug changes.