[logback-dev] RFC 5424 and Structured Data support.

[Ralph Goers]

In reading your response I'm not sure if you are just describing how you originally thought this should be done or critiquing what I've done. In your analysis below I really don't see any comments about what is wrong but rather, how you had anticipated it would be done. I've tried to answer the points in your email as best I can, but the main issue is that it seemed to me that your proposal was for a new version of SLF4J whereas I am trying to make the changes without the need for a major release or breaking any compatibility.

On Jan 25, 2010, at 3:40 AM, Joern Huxhorn wrote:

> Ok, I've had time to take a look now.
> 
> My plan was that the logback-classic Logger would simply extend org.slf4j.n.helper.AbstractLoggerBase.

I saw that in your original note. That class doesn't currently exist. I assume you would have created it. As I recall I thought about that but decided against it. Logback has a lot of logic in its Logger class and implementing AbstractLoggerBase in SLF4J doesn't save much as you might think.  In addition, all the current Loggers implement the interface. Creating a new base class that includes the Message API and then having Logback extend that creates serious problems 

> 
> That way, only the following 3 methods would need implementation:
> Threshold getThreshold()
> boolean isEnabled(Level level, Marker marker)
> void log(Level level, Marker marker, Message message, Throwable throwable)

That isn't really true. Again, look at the Logger class. While the public methods like info(), debug() etc, could be factored out there are a lot of other methods that can't be in an abstract class.

> 
> The log-method above would just call filterAndLog(String localFQCN, Marker marker, Level level, Message msg, Throwable t).
> 
> The implementation of old and new SLF4J-API implementations must be separated so it's possible to either call org.slf4j.n.LoggerFactory (for org.slf4j.n.Logger)  or org.slf4j.LoggerFactory (for org.slf4j.Logger) while deserializing a Logger, i.e. the Logback Logger implementation should *not* implement org.slf4j.Logger anymore.

There are some issues with this. LoggerFactory returns a Logger. To maintain compatibility this interface cannot be changed. Instead I created MessageLoggerFactory which returns a MessageLogger which extends LocationAwareLogger. Logback's Logger now implements MessageLogger instead of LocationAwareLogger. Other applications, such as Sling, which implement the Logger interface will not be impacted by this at all. I don't see how using org.slf4j.message.MessageLoggerFactory is much different than using org.slf4j.new.LoggerFactory, except I believe my name is a bit clearer.

Logback doesn't implement Logger directly in either Ceki's fork or mine. Currently, it implements LocationAwareLogger. In mine, MessageLogger. 

> 
> Instead, org.slf4j.impl.StaticLoggerBinder should return org.slf4j.n.helpers.UsingNewLoggerFactory which is implemented like this:
> Logger getLogger(String name) {
>    return org.slf4j.n.LoggerFactory.getLogger(name).getOldLogger();
>  }

Why? I left the existing getLogger alone. Instead the MessageLogger is a LoggerWrapper, so the existing API is untouched.

> 
> Does this help as an explanation so far?

Not really.  I'm working under the premise that a) SLF4J is not being upgraded to Java 5 b) no new SLF4J version is required because c) backward compatibility must be maintained. If I was not working with those constraints I certainly would have made a few different choices, but then I'm sure I would have had a snowballs chance in hell of getting it adopted since it would break so much stuff.  

Ralph