I am about to check in a small bit of code that helps provide better TRACE details that are critical to diagnosing client Token validation errors because the information is lost in the various layers otherwise. However, I wanted to be diligent about making a good contribution, so I was reading through some of the contribution guidelines.
I was surprised to discover that the JBoss process guidelines specifically advise using a private static Logger declaration because that is not the best strategy for Logger instantiation. I admit that it is not common knowledge for some reason. I myself was not aware of this until only recently when I started searching for advice on the topic after having a discussion with a colleague about how often static declarations are abused in the name of performance, but can actually have a negative performance impact in multi-threaded code when synchronized upon, etc.
After that discussion, it struck me that most logging code I have seen follows this exact pattern, and probably should not be for those exact reasons. So I went searching for articles that might be relevant to the conversation in the context of Logger, and found that there are even more concerns that that. Apparently, it causes so many problems, that some logging libraries use special strategies (ThreadLocal variables, etc) in attempt to workaround this very common style of Logger declaration.
A much better strategy is simply to use a private transient Logger for most cases and use a local variable declaration for any static methods that require logging. Of course, there are more advanced strategies as well, but that simple strategy is the most effective. For those of you who are interested in the details directly from the source, please refer to the following reference: http://wiki.apache.org/commons/Logging/StaticLog
With all that said, I have two questions:
- How do we get this information to the right people so someone can update the JBoss process guidelines to use this better strategy, or at least allow alternate strategies, and provide the aforementioned reference materials to consult for more details.
- Are any of the key contributors going to have a major problem if I commit a change that uses this strategy for obtaining a Logger? In this particular case, I'm adding it to a class that does not already have it, so I'm not too worried about stepping on someones work.
- Does anyone agree that there should be an effort to change this throughout? After reading the reference I provided, I believe there should be an effort to make this change systematically because portions of PicketLink and PicketBox are intended to be used in other environments (not only JBoss) so the problems documented in that reference can (and most likely will) occur if not addressed.