commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simon Kitching <skitch...@apache.org>
Subject Re: [logging] log4j 1.3 support
Date Sat, 01 Oct 2005 02:42:51 GMT
Well, rain has unexpectedly started this afternoon, so I've got some
time to talk about this now...

On Sat, 2005-10-01 at 12:49 +1200, Simon Kitching wrote:
> On Fri, 2005-09-30 at 13:30 +0200, Boris Unckel wrote:
> > Hello,
> > 
> > > > Level.Trace was introduced in 1.2.12
> > > >
> > > http://logging.apache.org/log4j/docs/api/org/apache/log4j/Level.html#TRACE
> > > You are right. Thanks!
> > > And 1.2.12 is released. As soon as it is in the ibiblio, I will update the
> > > version in project.xml
> > > Only a little confusing that Log4J13Logger is for Log4j 1.2.12+
> > > I will also add this as comment to the Log4J13Logger.
> > 
> > Since these are internal classes I would recommend to detect the log4j
> > Feature, not it's version, and rename the Log4J??Logger to
> > feature dependend name (i.E. Log4JWithTraceLogger and Log4JLegacyLogger).
> > The older one could be depraceted (first step/version) and afterwards
> > made be final to make users who extended it, are forced to migrate
> > in a definded timeframe.

I suggest you look at the current code again. Log4J12Logger has:
<code>
 // Releases of log4j1.2 >= 1.2.12 have 
 // Priority.TRACE available, earlier
 // versions do not. If TRACE is not available, then we have to map
 // calls to Log.trace(...) onto the DEBUG level.
        
 try {
   traceLevel = (Priority) Priority.class.getDeclaredField("TRACE").get(null);
 } catch(Exception ex) {
   // ok, trace not available
   traceLevel = Priority.DEBUG;
 }
</code>

So TRACE is supported in log4j12Logger as long as there is a TRACE member on the 
Priority class, ie it is already implemented as you request above: detecting
the feature, not the version.

The Log4J13Logger class can (and does) simply assume TRACE is available.

> Basically, Log4j 1.2 and log4j 1.3 are not binary compatible. There is
> no way for us to write a single class that will run against both these
> log4j versions. This is due to the way that the log4j team decided to
> phase out the Category/Priority classes. This was discussed at length on
> the log4j lists, and they were basically committed to that approach. So
> we *must* have separate logger implementations for these two lib
> versions. And we must compile each logger class against the appropriate
> lib version, which basically makes using Maven to do the builds
> impossible; Maven assumes one classpath is used to compile all the code.
> So builds are ant-only.

This binary incompatibility is the reason the two logger classes exist,
not to support the TRACE feature. The two different classes need to be
compiled against different versions of log4j.

> Whether log4j12logger should be called Log4JLogger is debatable. There
> are drawbacks to this. Yes, renaming the class will break setups where
> someone has a config file that specifies Log4JLogger. However this setup
> will fail anyway if they try to use log4j 1.3 in the classpath instead
> of log4j 1.2. Given log4j's binary incompatibility between the two
> versions we *can't* guarantee smooth upgrades for all combinations, and
> I think the rename approach currently implemented will give the most
> obvious error message, and have the most obvious fix for the users. This
> is up for debate though.
> 

To expand on this: assume for the moment that we provide a Log4JLogger
class compiled against the 1.2 log4j library, and a Log4J13Logger
compiled against the 1.3 log4j library.

An app which has a commons-logging.properties file that specifies
Log4JLogger as the logger to use will continue to work fine when
commons-logging is upgraded. But when they upgrade log4j, they will
start getting strange "incompatible class definition" or "no such
method" exceptions, because the code is compiled to expect to call
Category.log(Priority p, ...) but that method no longer exists [the
details might not be entirely correct here; I'm working from memory].
That's a rather confusing message to deal with, and no hint that the
fix is to change their commons-logging configuration.

Now if we instead provide only Log4J12Logger/Log4J13Logger then when
they upgrade their commons-logging implementation they get the message
"Log4JLogger not found" which seems reasonably clear. They will
(hopefully) immediately refer to the commons-logging release notes which
will tell them how to resolve this by changing their
commons-logging.properties file. Yes, this is a nuisance, but it's one
imposed by the fact that the log4j releases are binary incompatible.

It's also confusing to provide "Log4JLogger" when it doesn't work
against the current Log4J release; I think the
Log4J12Logger/Log4J13Logger names are clearer. Of course this will get
interesting when/if Log4J 1.4 is released...

It would be possible to provide a Log4JLogger class that auto-detected
which version of log4j is in the classpath, and delegated to the
relevant implementation, but that would have to be implemented by
delegating calls at runtime as far as I can tell, which would have a
performance impact. If someone could come up with an auto-detect
solution that *doesn't* have a runtime impact I would be very happy to
see that included. Note that here we're *not* talking about the generic
commons-logging auto-detection, but where an app has explicitly provided
a logger class name that happens to be o.a.c.l.impl.Log4JLogger.

Regards,

Simon


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message