Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppress logging for user-defined functions in loggers #49

Open
fredrikekre opened this issue May 14, 2021 · 2 comments
Open

Suppress logging for user-defined functions in loggers #49

fredrikekre opened this issue May 14, 2021 · 2 comments

Comments

@fredrikekre
Copy link
Member

fredrikekre commented May 14, 2021

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 fredrikekre changed the title Supress logging for user-defined functions in loggers Suppress logging for user-defined functions in loggers May 14, 2021
@oxinabox
Copy link
Member

Yeah this came up in JuliaLogging/TensorBoardLogger.jl#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.

@fredrikekre
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants