commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simon Kitching <si...@ecnetwork.co.nz>
Subject Re: [digester] plugins module ready for evaluation
Date Tue, 21 Oct 2003 05:47:41 GMT
On Tue, 2003-10-21 at 17:36, Craig R. McClanahan wrote:
> Simon Kitching wrote:
> >I'm looking at this from the point-of-view of a contributor to Digester,
> >not a user (hence the dev list address). The problem is that in order to
> >integrate with this solution, every class written for the Digester which
> >needs to log output *must* do:
> >
> >  digester.getLog().warn("oops")
> >
> >rather than
> >
> >  Logger log = LogFactory.getLogger(ThisClass.class)
> >  ..
> >  log.warn("oops")
> >
> >  
> >
> 
> Why?  The first approach is only required if you want your log messages 
> to go to the same Log instance that Digester is using.  If you don't 
> care, don't bother.

Hmm .. we don't seem to be connecting here. 

The original cause of this thread was that I wrote a new set of classes
for Digester (plugins) and submitted them for inclusion in the base
Digester distribution. In these classes I use a Log category created via
LogFactory. Robert Donkin told me off (nicely) for not using
digester.getLog().

The easiest path for me is to accept Robert's solution and change the
code (which has been committed by Robert to the digester cvs repository
with the controversial code currently intact). 

However it seems to me that the current logging approach is broken (or
at least very clumsy), for the reasons I have put forward in the
previous emails [and are summarized at the end of this mail].

Rather than change my code to comply with what I think is an incorrect
API, I would like to discuss this and maybe come up with a better
solution. Or maybe I will just learn why the current solution is
necessary. So far my original questions have not been answered, though.

It may well be that my proposed solution (just use commons-logging's
LogFactory functionality) is broken too (does not support the needs of
users like Tomcat and Avalon).

> Why?  The first approach is only required if you want your log messages 
> to go to the same Log instance that Digester is using.  If you don't 
> care, don't bother.
> 

It doesn't make much sense to me to provide a framework with a way to
control *part* of the digester library's logging output. Someone who
uses the Plugins functionality would be bound to be confused if the
setLog() call redirected output generated by CallMethodRule instances,
but not output generated by PluginCreateRule instances...

Surely either *all* code that is part of the digester library (ie all
that stuff in cvs) should use digester.getLog().warn(...), or none of it
should.


> And, without a getLog() call, it would not even be *possible* to share 
> the same Log instance, which is what the Avalon folks were complaining 
> about.

Sharing a Log instance is a solution. 
What exactly is the problem or requirement that the Avalon folk had?

Did they want to be able to redirect log output emitted by all instances
of classes within the org.apache.commons.digester package? 

If so, isn't that achievable with a custom LogFactory class? Or even
with a standard LogFactory class, by retrieving the appropriate Log
object and configuring it? [this would require downcasting to the actual
Log implementation class as commons-logging does not provide Log object
configuration methods].

> 
> >The first implication is that with this pattern, *every* object which
> >wants to log output *requires* a reference to whatever object is the
> >"holder" of the master Log object (a Digester instance in this case). 
> >
> >  
> >
> Even if this were a *must* requirement, how many objects n the Digester 
> API don't have a Digester instance already?  That's right, basically 
> none of them :-).

Well, yes. However isn't this an issue that all commons components face?
Commons components are almost by definition intended to be called by
frameworks. And they *should* be using logging.

It is possible to say: hey, let's solve *our* problem by crafting a
solution that works for Digester alone. I feel that this is what the
current setLog() method does.

It does seem nicer, if possible, to come up with a solution to this
problem that can be applied to all commons components. Or better yet
steal a solution from some commons component which has "got it right"
with an approach that works for all libraries, not just one with certain
object relationship topologies (woo - big words!)

It means, for a start, that users of commons components have a single
consistent mechanism for controlling the logging of a commons component,
rather than "oh, Digester has a setLog methog, but Net has this
approach, and HiveMind has that approach".

I did look at collections, and found that they resolve the issue by not
having any logging at all :-). I've not had time to look into the other
components.

Have the Avalon team complained about logging in commons-net? Presumably
not. But if they do, and find that they can't put a setLog() method
anywhere in Net because there is no centrally-accessable object to store
the master Log object on, then what? A solution for Net which solves the
same problem, but is implemented in a totally different manner? ecch...

> Centralized logging for Digester is used by the standard Digester 
> classes to minimize the number of log names you have to administer.  
> Whether you utilize it in your own code (or whether your own code is 
> even aware that the possibility exists) is totally up to you.

"Minimize the number of log names you have to administer".

Isn't that what Catalog hierarchies are for? Configure the
"org.apache.commons.digester" hierarchy and you have set defaults for
all Categories below it. One config entry qualifies as "minimize" to me
:-)

And by "configure", I mean either configure one of the standard Log
implementations (eg log4j's), or replace it with a custom one (such as
an AvalonLog object).


Quick summary of the issues I have with the current solution:
[1] all logging goes through one category so it is not possible to
    isolate stuff like "log debug messages from RulesBase, info for
    everything else".
[2] objects can't log anything until after setDigester is called.
[3] objects which don't have an explicit reference to a digester
    (for example a custom org.xml.sax.ErrorHandler object) can't
    log.
[4] the solution isn't consistent with solutions to the same 
    problem in other commons projects - and can't be, because
    many of those projects won't have a place to store a central
    Log instance.
[5] in applications which use many commons components, custom logging
    will need to be configured for each one in turn, rather than
    doing it once.

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