Should logger be private static or not

Question

Should logger be declared static or not? Usually I've seen two types of declaration for a logger :

    protected Log log = new Log4JLogger(aClass.class);

or

    private static Log log = new Log4JLogger(aClass.class);

Which one should be used? what are the pro's and con's of both?

Logging is a cross-cutting concern. Use Aspects and the question is moot. — Dave Jarvis, Oct 03 '10 at 05:26
`static` is one reference per class. non-static is **one reference per instance** (+ initialization). So in some cases, the latter comes at a significant memory impact if you have tons of instances. Never use the non-static in a *frequent* object. I always use the static version. (which should be *uppercased* `LOG`) — Has QUIT--Anony-Mousse, Sep 05 '12 at 08:43
you can get rid of this variable at all if you use [jcabi-log](http://www.jcabi.com/jcabi-log/), a static wrapper around slf4j — yegor256, Oct 05 '12 at 07:19
as suggested already, use AOP and annotations, for example: http://www.jcabi.com/jcabi-aspects/annotation-loggable.html — yegor256, Feb 05 '13 at 11:26
@Anony-Mousse only constants like strings, numbers and other immutable objects should be uppercased (see SonarQube squid:S00115). — Robert Hume, Aug 27 '17 at 07:09
RobertHume the static version *is* using a constant. That is exactly why it should be uppercased. — Has QUIT--Anony-Mousse, Aug 28 '17 at 06:01
@ Anony-Mousse i up-ticked your comment that the logger should be static and uppercased, but (minor comment) I think it should be LOGGER and not LOG. — inor, Mar 04 '18 at 09:33
Google says it should not be upper case because loggers are generally mutable objects that have methods with side effects. https://google.github.io/styleguide/javaguide.html#s5.1-identifier-names — mrog, Mar 28 '18 at 20:58
@Anony-Mousse: quite likely, @RobertHume was pointing out that this is simply `static` and not `static final`. — haylem, Apr 09 '18 at 14:08
No it should be `private static final Log log` that is lowercase. The logger is not a constant, the logger is a static final object (that can be mutated). Personally I always use `logger`. — osundblad, Jul 04 '18 at 14:09

BalusC · Accepted Answer · 2010-10-01T20:37:05.323

112

The advantage of the non-static form is that you can declare it in an (abstract) base class like follows without worrying that the right classname will be used:

protected Log log = new Log4JLogger(getClass());

However its disadvantage is obviously that a whole new logger instance will be created for every instance of the class. This may not per se be expensive, but it adds a significant overhead. If you'd like to avoid this, you'd like to use the static form instead. But its disadvantage is in turn that you have to declare it in every individual class and take care in every class that the right classname is been used during logger's construction because getClass() cannot be used in static context. However, in the average IDE you can create an autocomplete template for this. E.g. logger + ctrl+space.

On the other hand, if you obtain the logger by a factory which in turn may cache the already-instantiated loggers, then using the non-static form won't add that much overhead. Log4j for example has a LogManager for this purpose.

protected Log log = LogManager.getLogger(getClass());

edited Oct 01 '10 at 20:37

answered Oct 01 '10 at 20:28

BalusC

1,082,665
372
3,610
3,555

6

Declare `abstract Log getLogger();` in the abstract class. Implement this method, returning the static logger for the particular instance. Add `private final static Log LOG = LogManager.getLogger(Clazz.class);` to your IDE class template. – Has QUIT--Anony-Mousse Sep 05 '12 at 08:45
3

For slf4j: `protected Logger log = LoggerFactory.getLogger(getClass());` – Markus Pscheidt Dec 18 '14 at 11:18
4

@BalusC the problem with passing getClass() to the getLogger method is that it returns the class of the current instance. Normally it's more desired the logging to be associated with the class where the code is. For example if the logging code is in class Parent then we want the logging to be associated with Parent even though the executing instance is and instance of class Child which is sub-class of Parent. With getClass() it will be associated with the Child, incorrectly – inor Feb 26 '18 at 11:31
1

@inor: "incorrectly"? If you don't want to abstract the class, then you should just not use the inherited getClass() in first place. There are developers which do find it correct and useful as it reveals information in which subclass exactly the logic has been performed. – BalusC Feb 26 '18 at 11:43
2

@ BalusC getLogger(getClass()) results in always logging the name of the sub-class incorrectly. Logging classes should always do getLogger(Clazz.class) to associate the logging made by the code in class Clazz. Developers that want to know which of the sub-classes is executing (E.g. SubClazz extends Clazz) should do in SubClazz: getLogger(SubClazz.class) and something like: log.info("calling "); – inor Mar 04 '18 at 09:27

score 50 · Answer 2 · edited Aug 14 '19 at 16:22

50

I used to think that all loggers should be static; however, this article at wiki.apache.org brings up some important memory concerns, regarding classloader leaks. Declaring a logger as static prevents the declaring class (and associated classloaders) from being garbage collected in J2EE containers that use a shared classloader. This will result in PermGen errors if you redeploy your application enough times.

I don't really see any way to work around this classloader leak issue, other than declaring loggers as non-static.

edited Aug 14 '19 at 16:22

jaco0646

15,303
7
59
83

answered Mar 02 '11 at 21:37

piepera

2,033
1
20
21

4

I suspected static field would have memory leak problem as well. Non-static may have performance issue as others said. What's the ideal way then? – liang Oct 08 '13 at 07:26
@piepera the main problem described in the article you referenced is the ability to control logging level in each application when "consider the case when a class using "private static Log log = " is deployed via a ClassLoader that is in the ancestry of multiple supposedly independent "applications"". I don't see that as a problem because in this particular situation the applications have a "common ground" and in that "common ground" the logging level for the class is decided and yeah, it holds for all applicaitons... but keep in mind that this class is [loaded] outside of these applications – inor Feb 20 '18 at 16:21

score 20 · Answer 3 · answered Oct 02 '10 at 20:44

The most important difference is how it affects your log files: in which category do logs go?

In your first choice, the logs of a subclass end up in the category of the superclass. That seem very counter-intuitive to me.
There is a variant of your first case:

protected Log log = new Log4JLogger(getClass());

In that case, your log category says which object the code that logged was working on.
In your second choice (private static), the log category is the class that contains the logging code. So normally the class that is doing the thing that is being logged.

I would strongly recommend that last option. It has these advantages, compared to the other solutions:

There is a direct relation between the log and the code. It is easy to find back where a log message came from.
If someone has to tune logging levels (which is done per category), it is usually because they are interested (or not) in some particular messages, written by a particular class. If the category is not the class that is writing the messages, it is harder to tune the levels.
You can log in static methods
Loggers only need to be initialized (or looked up) once per class, so at startup, instead of for every instance created.

It also has disadvantages:

It needs to be declared in every class where you log messages (no reuse of superclass loggers).
You need to take care to put the right classname when initializing the logger. (But good IDE's take care of that for you).

score 5 · Answer 4 · answered Oct 01 '10 at 20:31

5

Use inversion of control and pass the logger into the constructor. If you create the logger inside the class you are going to have a devil of a time with your unit tests. You are writing unit tests aren't you?

answered Oct 01 '10 at 20:31

Wayne Allen

842
4
12

7

Unit tests which examine the logging you're producing sound both useless and incredibly brittle. – Michael Nov 10 '17 at 11:52
3

Usefulness depending on the system under test. Sometimes the logging is all you have access to. – Wayne Allen Nov 15 '17 at 22:45
@Wayne Allen when you are doing unit tests, by definition, you also have test results. Are you suggesting a situation in which one does unit test but does not have tests results? only have the logs? – inor Feb 20 '18 at 15:54
creating the logger inside the class does not create problems. Can you show a simple example that is hard to UT because the class creates its own logger? – inor Feb 20 '18 at 15:56
3

Sure. How about a logger that sends emails. Don't want to do that every time you run your tests. Plus how do you assert the side effect? – Wayne Allen Feb 21 '18 at 04:44
All the logging frameworks I've used let you create separate configurations for test and production. Each environment can have its own logging levels and destinations. The logging framework essentially provides its own specialized form of dependency injection. – mrog Mar 28 '18 at 21:05
1

Changing your code injection strategy to adapt to your tests is never a good idea. Tests should adapt to your production code, not the other way around. There are plenty of ways to test log output without ever meddling with the actual logger instances - in the tests just add another (i.e. custom made) capturing appender to the log manager and examine what was passed to it. – Pavel Lechev Apr 27 '20 at 10:15
@Michael ever heard of business requirements? You might be under compliancy to alert specific events and you want to make sure your code indeed does that. So in that case having tests that check logs make very much sense and are very useful – Turkhan Badalov Mar 25 '21 at 07:53

Should logger be private static or not

4 Answers4

Linked