SLF4j 311 - Enable swapping of NOPLogger in SubstituteLoggerFactory

[chetanmeh]

Changed the implementation of SubstituteLoggerFactory to return a SubstitutableLogger which can delegate to a different logger implementation. By default it uses NOPLogger but enables changing the delegate.

An application can later find out the list of SubstitutableLogger and replace the delegate with right implementation. An application must also clear the SubstitutableLogger map to prevent any possible memory leaks

Fix for http://bugzilla.slf4j.org/show_bug.cgi?id=311

[ceki]

Hi Chetan, looking at the code, I can see how the delegate is useful in switching implementations. However, how and when is the switch made? Is it done inside SLF4J or were you thinking of doing the switch in user code, i.e. in Sling?

[chetanmeh]

The switch can be done in either place.

User Code

For example in Sling we do wait till LoggerFactory.getILoggerFactory() returns an instance of LoggerContext. So post that we know that initialization phase is done and then logic can check back the list of SubstitutedLoggers from SubstituteLoggerFactory and swap out the delegates.

A proper fix would probably require a change in Logback where such a task is done in org.slf4j.impl.StaticLoggerBinder#StaticLoggerBinder constructor.

Slf4j Code

Here we can do such a change in org.slf4j.LoggerFactory#bind. So instead of emitting the substitute logger name list we fix the substituted loggers.

If that is fine with you I can update the patch to include that.

[ceki]

OK. I think it makes most sense to set the delegates of the SubstituteLoggers while in the LoggerFactory.bind method. The LoggerFactory.emitSubstituteLoggerWarning() seems like a good place (although the method would need to renamed). I'd keep the warning but change the message to indicate that a set of substitute loggers may have been accessed during the initialization phase and that logging calls during this phase were not honored but that calls in the future to those loggers will be honored.

The http://slf4j.org/codes.html#substituteLogger would need to be changed as well. The source is plain old XML and is located in codes.html under the slf4j-site/src/site/pages/ folder.

[korzha]

Since you use ConcurrentMap, there is no need to use synchronized block.
See https://github.com/qos-ch/slf4j/blob/master/slf4j-simple/src/main/java/org/slf4j/impl/SimpleLoggerFactory.java

[chetanmeh]

Makes sense. Refactored as per suggestion

[ceki]

See other LoggerFactory impls for a better way of using the concurrent map.