invenia / Memento.jl

A flexible logging library for Julia
https://invenia.github.io/Memento.jl/latest
Other
88 stars 14 forks source link

Setting logging level for a particular logger #60

Closed vtjeng closed 4 years ago

vtjeng commented 6 years ago

I'm using Memento.jl for logging for my MIPVerify.jl package, and am wondering what the best approach is to allow users to select what types of log levels they want to see.

Right now, I have written a wrapper that sets the log level recursively all the way up to the root. Unfortunately, this means that I'm interefering with the root handler.

function getlogger()::Memento.Logger
    Memento.getlogger(current_module())
end

function setloglevel!(level::String)
    # Options correspond to Memento.jl's levels.
    # https://invenia.github.io/Memento.jl/latest/man/intro.html#Logging-levels-1
    logger = MIPVerify.getlogger()
    Memento.setlevel!(logger, level)
    while logger.name != "root"
        logger = Memento.getparent(logger.name)
        Memento.setlevel!(logger, level)
    end
end

Alternatively, I can add a handler to the logger for the current module, but that seems to lead to duplicated messages for anything that is a warn and above.

What other options do I have (and what would you recommend?)

rofinn commented 6 years ago

Hi @vtjeng,

I agree, this interface could definitely be improved. This sounds very similar to #58, is that about right? I'm thinking that we'll probably use a combination of:

  1. the root default level approach proposed by @omus and
  2. allow batch setting of logger levels

Ideally, I'd suggest leaving configuration of the loggers to the application code (e.g., your examples) and limit the Memento logger setup to the following in your package module:

module MyModule

using Memento

# Create our module level logger (this will get precompiled)
const LOGGER = get_logger(current_module())   # or `get_logger(@__MODULE__)` on 0.7

# Register the module level logger at runtime so that folks can access the logger via `get_logger(MyModule)`
# NOTE: If this line is not included then the precompiled `MyModule.LOGGER` won't be registered at runtime.
__init__() = Memento.register(LOGGER)

end

From there, the application code should be responsible for managing logging levels and attaching handlers. Given that you want the application code to set the logging level for all loggers in the hierarchy, I think keeping setloglevel! as an unexported method is probably your best bet for a simple one liner until we resolve issue #58.

I'll post back once I've fixed #58 to see if that addresses your concerns. Thanks for the feedback!

vtjeng commented 6 years ago

Thanks for your response. I decided to file this as a separate issue because #58 focuses on managing logging levels for multiple loggers, while I wanted to ask what the right thing to do is in my situation.

  1. 54 specifies that getting lower priority messages from child loggers now requires that you either 1) change the level for each logger along the chain or 2) add a specific handler to the child logger. Do you prefer one of these options over the other?

  2. What's the best way to test that users see log output in STDOUT at a specific logging level? My initial test approach was to add a DefaultHandler that wrote to a IOBuffer, but that did not capture the situation where the module level logger was set at the appropriate level (e.g. debug) but the root logger was at a higher level (e.g. warn). (After #54, many of my log messages did not get to STDOUT, leading to some confused users). Would you recommend capturing STDOUT?
rofinn commented 6 years ago

Sorry, for the delayed response, I haven't had a chance to think much about Memento lately.

  1. It depends on your use case:

    • I usually try changing each logger level in the chain first, but this can often increase the signal to noise ratio. For example, if I'm debugging some module Bar inside a package Foo.jl I might find that setting both the "Foo" and "Foo.Bar" loggers to debug will give me too many messages. We'll probably add some methods to make it easier to test this out, but sometimes this behaviour doesn't make sense.
    • Using the same situation above, if I want to just inspect the debug messages from "Foo.Bar" then I'll usually just add a handler to it. We'll probably add some methods which make this easier as well. For example, if we let Memento.config take an existing logger as input then you could configure that logger in the same way you setup the root logger.
  2. Hmmm, if you just want to test that some log output is going to STDOUT I'd probably suggestion using the @capture_out macro in Suppressor.jl. Memento.jl has some testing macros in Memento.Test (e.g., @test_log, @test_warn), but I'd still consider them to be a beta feature that we're using internally. They're also most specific to testing the behaviour of individual loggers at the package level. In the future, we may support passing in an IO type where an applications expect logs to go, but this isn't a huge priority for us at the moment.

I hope that helps.