qos-ch / slf4j

Simple Logging Facade for Java
http://www.slf4j.org
MIT License
2.32k stars 980 forks source link

DefaultLoggingEventBuilder/Log4jEventBuilder "addArgument" with supplier behaves same as constant value. #367

Open oravecm opened 9 months ago

oravecm commented 9 months ago
        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-api</artifactId>
            <version>2.0.7</version>
        </dependency>

        <dependency>
            <groupId>org.apache.logging.log4j</groupId>
            <artifactId>log4j-slf4j2-impl</artifactId>
            <version>3.0.0-alpha1</version>
        </dependency>

DefaultLoggingEventBuilder addArgument with supplier should be executed an after calling log() not directly during "addArgument" calling, else it is same as constant value.

    @Override
    public LoggingEventBuilder addArgument(Supplier<?> objectSupplier) {
        loggingEvent.addArgument(objectSupplier.get());
        return this;
    }

should be

    @Override
    public LoggingEventBuilder addArgument(Supplier<?> objectSupplier) {
        loggingEvent.addArgument(objectSupplier);
        return this;
    }

Same behaviour is in "Log4jEventBuilder"

ceki commented 8 months ago

@oravecm Are you aware that the default implementations of org.slf4j.Logger#atTrace(), org.slf4j.Logger#atDebug(), org.slf4j.Logger#atInfo() etc. return an instance of NOPLoggingEventBuilder when the relevant level is disabled for current logger?

Thus, when DefaultLoggingEventBuilder#addArgument(Supplier<?> objectSupplier) method calls objectSupplier.get(), the event is certain to be logged.