Enhance lsst.log by having a Log object and Python interface
Description
is triggering
relates to
Checklist
Lucidchart Diagrams
Issue Matrix
hideActivity
Merged to master.
Thanks, I opened https://rubinobs.atlassian.net/browse/DM-6978#icft=DM-6978 for myself to take care of qserv and remove those two static methods from Log class.
I see. Thanks a lot for helping me understand.
I added non-static log()
and logMsg()
and now everything uses the non-static ones inside this package and it can build without the static ones. But they will be kept for now so the qserv code can be updated later.
I also renamed defaultLogger()
to getDefaultLogger()
. I will run Jenkins of lsst_distrib and qserv_distrib before merging.
Regarding static log()
and logMsg()
- I'd like to see them non-static too if only for consistency with other methods. We can update qserv to use non-static methods but this needs some coordination between releasing log
and qserv
. We could probably start by adding non-static methods to Log
class and keeping static for now and when log
is released I can work on updating qserv code and removing static methods once qserv is ready.
Public static class members are problematic for the same reason as global objects - undefined initialization order of globals in C++. If defaultLogger
is a static member and some other global (or static) object tries to use it it may happen that defaultLogger
is not initialized yet. Making it a function-local static avoids this issue because that static is guaranteed to be initialized on the first call to that function.
Thank you very much @Andy Salnikov for your review and testing with qserv.
I've made corrections based on your comments on PR, and further cleaned up the interface. The static methods setLevel
, getLevel
, and isEnabledFor
in c++ are removed and replaced by the non-static ones. The c++ macros API are not changed. I didn't change logMsg
and log
as they seem used directly in qserv (e.g. https://github.com/lsst/qserv/blob/4dd49beb5ecd74430631d1a9eff22446ddc1b7f2/core/modules/proxy/czarProxy.cc#L116 )
About making defaultLogger
a static method instead of a static variable, I'm not sure I follow the idea and find a static variable a bit cleaner (In that case this patch only replaces static log4cxx::LoggerPtr
by static Log
). May you talk more about why a defaultLogger()
method is preferred? (The current ticket branch has changed it to a method already, though)
Based on branch u/ktlim/getLogger in
log
and requests from https://rubinobs.atlassian.net/browse/DM-3532#icft=DM-3532, implement a lsst::log Python interface through Log objects, and allow controllability of logger names and levels in Python.