Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-801

Use lsst prefix in logger names

    XMLWordPrintable

Details

    • RFC
    • Status: Implemented
    • Resolution: Done
    • DM
    • None

    Description

      Current guidance for naming a loggerCurrent guidance for naming a logger is the following:

      Logger names should generally start with the fully qualified name of the module/file containing the logger, without the leading lsst.. Some example logger names are afw.image.MaskedImage and meas.algorithms.starSelector.

      Specifically we recommend that logger names do not include a fixed prefix. This goes against the advice for Python logging or using:

      logging.getLogger(__name__)
      

      which would include the prefix.

      Dropping the prefix has the advantage that the name is 5 characters shorter and so takes up less space when the log message is emitted and is less typing if manually constructed or an override is given on the command line.

      Dropping the prefix does have a number of disadvantages though:

      • There is a remote possibility of namespace clashes with other packages.
      • It's impossible to set the logger level globally for all our packages whilst leaving third party loggers untouched.

      In particular if anyone has done --log-level=DEBUG with pipetask they will see lots of fun output from matplotlib (and that message from conda changing standard output always turns up as INFO). In particular we could change how pipetask works such that it implicitly converts a general log-level request to setting the lsst. logger and not the root logger (But that can clearly not happen immediately).

      I propose we change our default recommendation to include the prefix in all new loggers (and over time switch over all loggers to using the recommended Python scheme where appropriate) including those constructed for Tasks.

      Attachments

        Issue Links

          Activity

            I'm in favor of this RFC.  Our packages typically have a lot of logging information at the INFO level, so being able to set that for all pipelines packages, but not have to set it for the root logger seems like a big win even if the log lines are slightly longer.

            krughoff Simon Krughoff (Inactive) added a comment - I'm in favor of this RFC.  Our packages typically have a lot of logging information at the INFO level, so being able to set that for all pipelines packages, but not have to set it for the root logger seems like a big win even if the log lines are slightly longer.
            tjenness Tim Jenness added a comment -

            Also, I should add that when we came up with the initial policy the only code sending log messages to lsst.log was our code and also at that time we were not forwarding python log messages to lsst.log. This meant that global configuration of lsst.log log levels had no effect on output from python loggers. This all changed once we started forwarding python to lsst.log a few years back.

            tjenness Tim Jenness added a comment - Also, I should add that when we came up with the initial policy the only code sending log messages to lsst.log was our code and also at that time we were not forwarding python log messages to lsst.log. This meant that global configuration of lsst.log log levels had no effect on output from python loggers. This all changed once we started forwarding python to lsst.log a few years back.
            ktl Kian-Tat Lim added a comment - - edited

            I'm in favor of this also, except for the transition plan. I think a shorter transition than "wait for people to modify code" would be desirable unless there is a list of (LSST) loggers (and associated modules) somewhere.

            ktl Kian-Tat Lim added a comment - - edited I'm in favor of this also, except for the transition plan. I think a shorter transition than "wait for people to modify code" would be desirable unless there is a list of (LSST) loggers (and associated modules) somewhere.
            tjenness Tim Jenness added a comment -

            Changing most of the Task loggers over seems straightforward since they already use the special logger from pipe_base (soon to be utils) and that getLogger can add the lsst prefix (it can have a prefix='lsst' option added to it or a prefix=None default that will look at the caller root module (thinking of SPHEREx defaulting here). It would only add the prefix if that prefix wasn't already there.

            tjenness Tim Jenness added a comment - Changing most of the Task loggers over seems straightforward since they already use the special logger from pipe_base (soon to be utils) and that getLogger can add the lsst prefix (it can have a prefix='lsst' option added to it or a prefix=None default that will look at the caller root module (thinking of SPHEREx defaulting here). It would only add the prefix if that prefix wasn't already there.
            rowen Russell Owen added a comment - - edited

            I am in favor of adding the lsst. prefix (and updating pipe_base/utils to include it) because it is likely to reduce confusion.

            rowen Russell Owen added a comment - - edited I am in favor of adding the lsst. prefix (and updating pipe_base/utils to include it) because it is likely to reduce confusion.

            +1 for adopting it. I find it aesthetically pleasing to have the logger start as `lsst.afw.` instead of `afw.`. It also makes it explicit that a component is in the LSST code, in case that component is not too familiar to some developers.

            kannawad Arun Kannawadi added a comment - +1 for adopting it. I find it aesthetically pleasing to have the logger start as `lsst.afw.` instead of `afw.`. It also makes it explicit that a component is in the LSST code, in case that component is not too familiar to some developers.

            Agree with everything Arun says.

            mrawls Meredith Rawls added a comment - Agree with everything Arun says.
            tjenness Tim Jenness added a comment -

            No objections so I'm adopting this. Task loggers should be able to be changed in bulk. Explicitly named loggers will need to be changed but it might be best to do that as part of the reorganization moving the special task logger class into the utils package and then using a getLogger function that can automatically add the prefix if it's missing.

            tjenness Tim Jenness added a comment - No objections so I'm adopting this. Task loggers should be able to be changed in bulk. Explicitly named loggers will need to be changed but it might be best to do that as part of the reorganization moving the special task logger class into the utils package and then using a getLogger function that can automatically add the prefix if it's missing.

            People

              tjenness Tim Jenness
              tjenness Tim Jenness
              Arun Kannawadi, Jim Bosch, John Parejko, Kian-Tat Lim, Meredith Rawls, Russell Owen, Simon Krughoff (Inactive), Tim Jenness, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                Jenkins

                  No builds found.