Why calling LoggerFactory.getLogger(...) every time is not recommended?

danirod picture danirod · Oct 13, 2010 · Viewed 70.5k times · Source

I've read tons of posts and documents (on this site and elsewhere) pointing that the recommended pattern for SFL4J logging is:

public class MyClass {
    final static Logger logger = LoggerFactory.getLogger(MyClass.class);

    public void myMethod() {
        //do some stuff
        logger.debug("blah blah blah");
    }
}

My boss prefers we just use a wrapper to intercept log calls and avoid boiler plate code for declaring the logger on every class:

public class MyLoggerWrapper {
    public static void debug(Class clazz, String msg){
        LoggerFactory.getLogger(clazz).debug(msg));
    }
}

and simply using it like this:

public class MyClass {

    public void myMethod() {
        //do some stuff
        MyLoggerWrapper.debug(this.getClass(), "blah blah blah");
    }
}

I presume instantiating a logger every time we log is somewhat expensive but I've been unable to find any document backing that assumption. Besides he says surely the framework (LogBack or Log4J we're still deciding) will "cache" the loggers and also that in any case the servers are running very much below their capacity so it is not an issue.

Any help pointing out potential problems with this approach?

Answer

matt b picture matt b · Oct 13, 2010

Here is one obvious problem with this approach: the String messages will be constructed on each call to debug(), there is no obvious way to use a guard clause with your wrapper.

The standard idiom with log4j/commons-logging/slf4j is to use a guard clause such as:

if (log.isDebugEnabled()) log.debug("blah blah blah");

With the purpose being that if the DEBUG level is not enabled for the logger, the compiler can avoid concatenating together any longer strings you may send it:

if (log.isDebugEnabled()) log.debug("the result of method foo is " + bar 
     + ", and the length is " + blah.length());

See "What is the fastest way of (not) logging?" in the SLF4J or log4j FAQ.

I would recommend against the "wrapper" your boss suggests. A library like slf4j or commons-logging is already a facade around the actual underlying logging implementation used. In addition, each invocation of the logger becomes much lengthier - compare the above with

 MyLoggerWrapper.debug(Foo.class, "some message");

This is the type of trivial and unimportant "wrapping" and obfuscation that serves no real purpose other than adding layers of indirection and ugly-fying your code. I think your boss can find more important issues to obsess over.