Skip to content

Logger.none() is misnamed — writes ERROR to disk and detaches logback root appenders globally #32

Description

@jtnelson

Summary

Logger.none() is documented as "a Logger that doesn't log messages
to anywhere," but the current implementation has two surprising side
effects that make it unsuitable as a default no-op:

  1. It's configured at Level.ERROR, so ERROR-level events are written
    to an error.log file under a random-UUID temp directory. Not
    silent.
  2. It's built with enableConsoleLogging(false), which causes
    Logger#setup to call root.detachAndStopAllAppenders() on the
    logback root logger four times during construction (once per
    level). This is a process-wide side effect that tears down any
    appenders other code — or logback's default config — had attached
    to root.

The combination means a consumer using Logger.none() as a "default
until the embedder configures real logging" placeholder silently
destroys global logging in the host application the moment the field
initializes.

Reproduction

ch.qos.logback.classic.Logger root = (ch.qos.logback.classic.Logger)
        LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
ListAppender<ILoggingEvent> sentinel = new ListAppender<>();
sentinel.start();
root.addAppender(sentinel);

Logger.none();   // construction has the side effect

LoggerFactory.getLogger("anything").error("after");
assertEquals(0, sentinel.list.size());   // sentinel was detached

Root cause

In Logger.java lines 213–216, the per-level setup runs:

if(!enableConsoleLogging) {
    ch.qos.logback.classic.Logger root = (ch.qos.logback.classic.Logger)
            LoggerFactory.getLogger(ROOT_LOGGER_NAME);
    root.detachAndStopAllAppenders();
}

This fires four times (once per setup() for info/error/warn/debug).
The apparent intent is to prevent additivity from fanning events out
to root when console logging isn't desired, but the implementation
also discards every other consumer's appenders, including logback's
default ConsoleAppender.

Logger.none() then compounds this at line 87 by routing all four
levels through the same path with level(Level.ERROR) — so even
ignoring the root-appender issue, it's not a no-op.

Suggested fix

For Logger.none() specifically: build at Level.OFF with
enableConsoleLogging(true) so the logger has no behavioral output
and no destructive side effect. With OFF, no event ever fires
regardless of additivity, so the root-detach branch is unnecessary.

public static Logger none() {
    return builder().name("none")
            .directory(Files.tempDir(UUID.randomUUID().toString()))
            .enableConsoleLogging(true).level(Level.OFF).build();
}

For the broader enableConsoleLogging(false) path: the destructive
detachAndStopAllAppenders() is dangerous in any embedding context.
A safer alternative is setAdditive(false) on the named loggers so
events don't propagate to root, instead of clearing root globally.
That lets multiple consumers coexist.

Context

Hit while integrating Logger into Amalgam as a pluggable framework
logger. The intent was to use Logger.none() as the default until an
embedder calls setLogger(...). That tore down logging in the
relay-server host application the moment Amalgam's static initializer
ran.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions