commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Steve Downey <steve.dow...@netfolio.com>
Subject RE: Problems with commons-logging
Date Sun, 03 Feb 2002 04:31:37 GMT
Inline:

> -----Original Message-----
> From: costinm@covalent.net [mailto:costinm@covalent.net]
> Sent: Saturday, February 02, 2002 11:34 AM
> To: Jakarta Commons Developers List
> Subject: Problems with commons-logging
> 
> 
> Here's a list of problems I see in the current logging API.
> 
> Given that a lot of packages will eventually depend and use this API,
> I believe at least the first issues should be resolved before 
> a release.
> 
> - security: getLogNames() and getInstance() are evil and unacceptable.
> Both log4j and logkit have solutions that allow safe use in a 
> container
> environment, i.e. support of isolation for the users of the API
> ( one app using the package can't mess with another app's logging ).
> I'm -1 on releasing with this whole in it.
> 
Could you elaborate on getInstance()? If the underlying logging packages
provide protection, how does a passthrough API bypass security? I can
understand the objection to getLogNames(), however. I don't see any client
needing to know about other log categories.

If it's the recycling of logs in getInstance(), I think I agree. That's a
matter of policy on the part of the logging package, not something that the
adapter should be doing.


> - static methods in LogSource. I suppose LogSource is a sort of
> factory - the pattern used here is completely unnatural ( or at least
> different from most APIs in use ).
> 
> - I would prefer Log to be an abstract class or even to be a
> normal class, with the minimal logger - and have LogSource return
> a particular impl. If static methods are used, it's cleaner to put
> them in Log, and let the LogSource ( I would rename it LogManager )
> be used behind the scene. I.e.
>      Log log=Log.getLog( name );
> and getLog() will find a LogManager, etc.
> 


> - It's missing a log() method that takes a level parameter.
> Having 5 fixed levels is fine for most apps, but not 
> extensible enough.
> Most loggers provide such a thing.
> 

But deciding how to map those different log level parameters to levels is a
matter of the logger policy. The commons log package is, to my
understanding, to be used by components and containers that are not in
charge of policy. While I agree that some applications may be interested in
using more than {trace, debug, info, warn, error, fatal}, that should be
sufficient for a component that is policy neutral and meant to be reused.

> - also in the 'container use' case, given that the Logger will
> probably be used by many components it's likely it'll end up at top
> level loader. It would be important for different apps to use 
> different
> logger adapters if they want to - the current solution allow only
> one.
> 
> - Given that it is a facade, it would need some way to pass at least
> config info to the underlying logger ( at least setProperty like ).
> Some logger may not need any, but if they do it'll require using
> internal APIs.
> 

The configuration should be done with the logging package API. A component
is not going to do configuration, the application, or the administrator, is
going to. The components need a uniform way of accessing the logging system
that the application is using.

Incidentally, this implies to me that if no logging system is configured,
then components should not log. It should be a no-op. Not even print to
stderr. [god forbid stdout]

> - I don't like the idea of constructors with a  param. All other APIs
> use a no-param constructor. You can easily call a setter if 
> you need to.
> 
Two things come to mind. What are the semantics of calling the setter after
the object has been created? More important, in a multithreaded environment,
setting during the constructor avoids messy coherency issues, as no other
thread has access until the construction is complete.

> - pluggable mechanism for finding the impl. Right now everything is
> hardcoded. Reading a properties from CLASSPATH or a similar mechanism
> is needed. ( jaxp style preferable - i.e. java services manifest )
> 
Finding classes on the classpath, the way it is now, is probably not a good
idea. I think something much more deliberate should be required. A property
file, or system property, seems like a good idea. I was going to say that
the application could take care of the configuration, but that's difficult,
and may be too late.

> - Separation of interface and impl - the 2 classes that 'matter'
> are Log and LogSource, everything else should be in a 
> different package.
> It'll get messy long term, and it's harder to read.
> 
> - I would prefer for the base impl to be JDK1.1 compatible. There is
> no valid reason to exclude JDK1.1 usage - Hashtable can be used
> without any problem, there is nothing performance critical here.
> 
> - makeNewLogInstance comment and impl are completely of sync.
> 
> - no support for i18n-style messages. Probably not a big deal
> for the first release, but it would be nice to think about it ( I know
> log4j can do that, I assume other as well ).
> 
> 
> Costin
> 
> 
> --
> To unsubscribe, e-mail:   
> <mailto:commons-dev-unsubscribe@jakarta.apache.org>
> For additional commands, e-mail: 
> <mailto:commons-dev-help@jakarta.apache.org>
> 

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


Mime
View raw message