LoggingExtras.jl icon indicating copy to clipboard operation
LoggingExtras.jl copied to clipboard

Suppress logging for user-defined functions in loggers

Open fredrikekre opened this issue 4 years ago • 2 comments

I just ran into a case where my format function used functionality that got deprecated. Since the deprecation warning machinery uses the logging system this naturally lead to a stack overflow error. Can be replicated with:

julia> using LoggingExtras

julia> f(args) = (Base.depwarn("msg", :f; force=true); args.message);

julia> fmt(io, args) = println(io, f(args));

julia> global_logger(FormatLogger(fmt, stderr));

julia> @info "hello"

One way to mitigate this is to suppress logging from these functions, e.g. for FormatLogger:

diff --git a/src/formatlogger.jl b/src/formatlogger.jl
index b259034..5dbc94e 100644
--- a/src/formatlogger.jl
+++ b/src/formatlogger.jl
@@ -40,7 +40,9 @@ function handle_message(logger::FormatLogger, args...; kwargs...)
     # to make sure that everything writes to the logger io in one go.
     iob = IOBuffer()
     ioc = IOContext(iob, logger.stream)
-    logger.f(ioc, log_args)
+    with_logger(NullLogger()) do
+        logger.f(ioc, log_args)
+    end
     write(logger.stream, take!(iob))
     logger.always_flush && flush(logger.stream)
     return nothing

Of course there could be deprecations from the surrounding code too, but probably less likely (?).

Another way to fix would be to make sure all sinks (just sinks?) honor the maxlog keyword, like SimpleLogger and ConsoleLogger do.

Perhaps not worth fixing though, the solution to this issue could be to upgrade your dependencies I guess.

fredrikekre avatar May 14 '21 00:05 fredrikekre

Yeah this came up in https://github.com/PhilipVinc/TensorBoardLogger.jl/issues/96 too

One way to mitigate this is to suppress logging from these functions,

I think this is the correct thing to do. and document it for sinks as required We might also need to do it to the filter loggers and tranform logger? Since they also call functions?

Another way to fix would be to make sure all sinks (just sinks?) honor the maxlog keyword, like SimpleLogger and ConsoleLogger do.

This is against the philosophy of the package. maxlog should be implemented as a EarlyFilteredLogger and then composed with your logger of choice. I think it is one of our examples.

oxinabox avatar May 14 '21 20:05 oxinabox

One way to mitigate this is to suppress logging from these functions,

I think this is the correct thing to do.

Cool. I wonder if there is any noticable overhead from doing this? Unlikely to be noticable compared to the rest of the machinery involved I guess.

If you want to protect yourself from this you can always just wrap your own functionin a NullLogger.

fredrikekre avatar May 14 '21 20:05 fredrikekre