You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by co...@covalent.net on 2002/02/02 17:33:46 UTC
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.
- 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.
- 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.
- 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.
- 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 )
- 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: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: Problems with commons-logging
Posted by Paulo Gaspar <pa...@krankikom.de>.
I already explained that one but the issue was solved and I should
behave and not restart that flame war.
=;o)
Besides, I had code and not a patch. I still must make things match
a bit.
Have fun,
Paulo Gaspar
> -----Original Message-----
> From: Geir Magnusson Jr. [mailto:geirm@optonline.net]
> Sent: Saturday, February 02, 2002 11:58 PM
> To: Jakarta Commons Developers List
> Subject: Re: Problems with commons-logging
>
>
> On 2/2/02 6:12 PM, "Paulo Gaspar" <pa...@krankikom.de> wrote:
>
> > Some of this issues are addressed in the code I have. Lets see if I
> > have time next week to take a look at both the commons and my code
> > base and try to merge them.
>
> :)
>
>
> Why didn't you submit a patch so many weeks ago?
>
>
> :)
>
> --
> Geir Magnusson Jr. geirm@optonline.net
> System and Software Consulting
> "Now what do we do?"
>
>
> --
> To unsubscribe, e-mail:
> <ma...@jakarta.apache.org>
> For additional commands, e-mail:
> <ma...@jakarta.apache.org>
>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Problems with commons-logging
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 2/2/02 6:12 PM, "Paulo Gaspar" <pa...@krankikom.de> wrote:
> Some of this issues are addressed in the code I have. Lets see if I
> have time next week to take a look at both the commons and my code
> base and try to merge them.
:)
Why didn't you submit a patch so many weeks ago?
:)
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
"Now what do we do?"
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: Problems with commons-logging
Posted by Paulo Gaspar <pa...@krankikom.de>.
Some of this issues are addressed in the code I have. Lets see if I
have time next week to take a look at both the commons and my code
base and try to merge them.
Have fun,
Paulo Gaspar
> -----Original Message-----
> From: Remy Maucherat [mailto:remm@apache.org]
> Sent: Saturday, February 02, 2002 8:42 PM
> To: Jakarta Commons Developers List
> Subject: Re: 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.
>
> You can always hide it in the core of the container. And the SM
> will protect
> you :)
> I agree it may be a problem in some cases, but it won't be for Tomcat.
>
> I don't see a problem with getInstance, though. What's the problem ?
> Using getInstance, or just constructing the object with a new seems
> equivalent to me.
>
> > - 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 ).
>
> Static everywhere, actually ;-)
> There's some syncing needed in there, also (the loggers are in a HashMap).
>
> > - 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.
>
> That sounds like a good idea.
>
> > - 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.
>
> Yes, that could be useful for more flexibility.
>
> > - 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.
>
> I don't see very well how to configure the logger either. In Catalina and
> Slide, we just instantiate the logger directly, so there's no
> such problem.
>
> > - 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.
>
> Vey good comment also.
>
> > - 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 )
>
> That can be added later IMO.
>
> > - 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.
>
> That could be a good idea also (stop having good ideas, you
> create too much
> trouble ;-)).
>
> > - 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.
>
> +1 for Hashtable, since it would also fix the possible syncing issues.
>
> > - 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 ).
>
> You mean have the logger handle the message bundle for you, and
> just give it
> the message key (+ params) ?
>
> Remy
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
For additional commands, e-mail:
<ma...@jakarta.apache.org>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: Problems with commons-logging
Posted by Paulo Gaspar <pa...@krankikom.de>.
Answer inline:
> -----Original Message-----
> From: costinm@covalent.net [mailto:costinm@covalent.net]
> Sent: Sunday, February 03, 2002 11:24 AM
>
>
> On Sat, 2 Feb 2002, Remy Maucherat wrote:
>
> ...
>
> > I don't see a problem with getInstance, though. What's the problem ?
> > Using getInstance, or just constructing the object with a new seems
> > equivalent to me.
>
> Creating a new object may be different - getInstance may give you the
> logger that is already used by a different application.
Maybe you should only be able to go down the current subtree of loggers.
(Or the current subtree of the Hierarchy or whatever vocabulary applies
in your favorite logger.)
> > > - 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 ).
> >
> > Static everywhere, actually ;-)
> > There's some syncing needed in there, also (the loggers are in
> a HashMap).
>
> Yes. I don't mind 1-2 static methods ( preferably in Log ), for
> convenience, but they should just be a shortcut for calling a virtual
> method in the log manager.
I do not mind a static method if they do not mind a singleton.
IMO, if you want to use something as a part of a big app or framework,
singletons are to be avoided at all costs.
> > > - 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.
> >
> > I don't see very well how to configure the logger either. In
> Catalina and
> > Slide, we just instantiate the logger directly, so there's no
> such problem.
>
> I just hate the config mechanism for log4j - a system property is
> a nightmare in a container env ( with multiple apps, etc ), and a
> properties file in classes/ is not very nice ( by my taste ). I would
> prefer a file in conf/ or etc/ or constructed from the component
> 'natural' config.
>
> Passing at least a basic piece of config to the logger is essential,
> it doesn't have to be supported by all loggers.
IMHO, there should be several configuration layers, each one building
on top of the previous one, and it should be easy to use a layer as
"low" as you want without having to pay the price of the more
sophisticated layers above.
Layers I am thinking on, from lower to higher level:
(0) You just configure your favorite logger yourself, wrap it with
the right wrapper and pass it to the rest of your app;
(1) There is a registry of loggers <=> wrappers to which you pass
your favorite logger already configured, getting the matching
wrapper ready to use. This simple registry is configured trough
an API and has an optional "default" configuration for the 3
"major APIs";
(2) You have a logger factory interface that will be used by the
several possible discovery implementations;
(3) There is a class-loader based implementation for the logger
factory (2) that tries to find out which API is available. This
is not good for some applications but is quite practical for
others and IMHO should be available.
- You can configure via API what are your most favorite and
less favorite logging APIs and the discovery process will
follow that order;
- You can set a "minimum common denominator" configuration
that works for any of the APIs available (e.g.: all the big
3 accept logging to a file with rotating file names).
(4) Configuring (3) via properties, directly passed to the factory
or loaded from a given source. Optionally you can set the names
and/or a common prefix for each property (e.g. for prefix:
saying that all the property names should be prefixed with
"velocity.logger.");
(5) Any other crazy thing that implements (2).
What do you think?
> > > - 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 ).
> >
> > You mean have the logger handle the message bundle for you, and
> just give it
> > the message key (+ params) ?
>
> That would be a good idea. One possible optimization for the logger would
> be to store the unprocessed key/params, and have them processed off line
> ( or just have an efficient way to generate the message ). StringBundle
> has some issues.
The "official" (SUN) i18n APIs all have several issues and that is why
several alternative solutions are popping across Jakarta. Avalon alone
as more than one.
I would suggest dropping this one from the logging API. Let separate
packages extend this one with their favorite i18n flavor without
favoring a specific solution that might prove not being the best in the
near future.
I also was adding i18n to my logging package and dropped it.
> Costin
Have fun,
Paulo Gaspar
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Problems with commons-logging
Posted by co...@covalent.net.
On Sat, 2 Feb 2002, Remy Maucherat wrote:
> > 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.
>
> You can always hide it in the core of the container. And the SM will protect
> you :)
> I agree it may be a problem in some cases, but it won't be for Tomcat.
Don't overestimate the SM. If the logger is successfull and a lot of
components will depend on it, it's very likely it'll end up in the common
class loader ( or even the system loader ), at least in some
configurations. Even if tomcat's class loader may protect a bit ( and
I doubt that), many other applications don't have such a xxxxx class
loader.
Remember all the problems and mess with jaxp in tc - the code is extremely
tricky if you don't want jaxp in the common loader. Not too many people
can understand how it works and when it works.
> I don't see a problem with getInstance, though. What's the problem ?
> Using getInstance, or just constructing the object with a new seems
> equivalent to me.
Creating a new object may be different - getInstance may give you the
logger that is already used by a different application.
> > - 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 ).
>
> Static everywhere, actually ;-)
> There's some syncing needed in there, also (the loggers are in a HashMap).
Yes. I don't mind 1-2 static methods ( preferably in Log ), for
convenience, but they should just be a shortcut for calling a virtual
method in the log manager.
> > - 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.
>
> I don't see very well how to configure the logger either. In Catalina and
> Slide, we just instantiate the logger directly, so there's no such problem.
I just hate the config mechanism for log4j - a system property is
a nightmare in a container env ( with multiple apps, etc ), and a
properties file in classes/ is not very nice ( by my taste ). I would
prefer a file in conf/ or etc/ or constructed from the component
'natural' config.
Passing at least a basic piece of config to the logger is essential,
it doesn't have to be supported by all loggers.
> > - 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 ).
>
> You mean have the logger handle the message bundle for you, and just give it
> the message key (+ params) ?
That would be a good idea. One possible optimization for the logger would
be to store the unprocessed key/params, and have them processed off line
( or just have an efficient way to generate the message ). StringBundle
has some issues.
Costin
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Problems with commons-logging
Posted by Remy Maucherat <re...@apache.org>.
> 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.
You can always hide it in the core of the container. And the SM will protect
you :)
I agree it may be a problem in some cases, but it won't be for Tomcat.
I don't see a problem with getInstance, though. What's the problem ?
Using getInstance, or just constructing the object with a new seems
equivalent to me.
> - 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 ).
Static everywhere, actually ;-)
There's some syncing needed in there, also (the loggers are in a HashMap).
> - 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.
That sounds like a good idea.
> - 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.
Yes, that could be useful for more flexibility.
> - 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.
I don't see very well how to configure the logger either. In Catalina and
Slide, we just instantiate the logger directly, so there's no such problem.
> - 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.
Vey good comment also.
> - 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 )
That can be added later IMO.
> - 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.
That could be a good idea also (stop having good ideas, you create too much
trouble ;-)).
> - 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.
+1 for Hashtable, since it would also fix the possible syncing issues.
> - 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 ).
You mean have the logger handle the message bundle for you, and just give it
the message key (+ params) ?
Remy
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Problems with commons-logging
Posted by Berin Loritsch <bl...@apache.org>.
Donnie Hale wrote:
> Paulo,
>
> I've seen you mention a couple of times that you consider singletons
> dangerous. Would you care to elaborate? Is it because you're concerned that
> people can't write thread-safe code correctly? Or because correct
> thread-safe code affects concurrency? Or something else?
>
I will jump in here with a few comments.
From the Avalon perspective, where we truly value Inversion of Control (i.e.
all resources are managed by their parent), singletons that are created by
a static method on a utility class are a violation of IOC. This is especially
true when you want to hide the manager from the implementation. Many times,
all you need is a little more thought, and some minor modifications, the
parent and the child can be decoupled.
There are a number of methods of creating singletons that don't require static
methods. One of the ways we accomplish this in the Avalon world, is by
componentizing a safe interface that other components can access, and passing
the interface to the children. Most books on Components agree that there are
two essential elements: the interface is purely an interface, and the
implementation is managed by a container of some sort. Anything else is not
a Component by textbook definitions. The Interface and the Implementation also
must be completely separate entities (that is why a URL will never be concidered
a Component).
As an example, Cocoon *used* to pass a reference to itself in this manner:
interface Processor {
void process( Environment env, String uri );
}
The parent container (servlet in this case) would instantiate the Container, and
pass a reference to Cocoon as a Processor. Now, that idea has been rethought,
and it is no longer necessary to pass the Cocoon object to the rest of the system.
In either case, there is only one Cocoon object in the entire system.
If the Container can guarantee only one instance will be made, you have a singleton.
Many times that is enough. By forcing access through static methods on a class,
you not only break IOC, but you open yourselves to a broader attack. Systems that
dynamically load objects and components are susceptible to evil components that do
bad things. If you have Singletons with global access, the bad things the evil
components can do are now expanded to the whole system. In an IOC based system,
it can only affect the parts that it can get a hold of. A properly designed system
will limit the number of things that a Component can attack.
So globally accessible Singletons can be a dangerous thing--not just for concurrency
reasons, but also for security reasons. If your system is completely closed, it
doesn't really make a whole lot of difference (other than lack of flexibility in
scaling your application). However, since logging is used in a wide variety of
contexts, it should not have a globally accessible singleton if it can avoid it.
--
"They that give up essential liberty to obtain a little temporary safety
deserve neither liberty nor safety."
- Benjamin Franklin
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: Problems with commons-logging
Posted by Paulo Gaspar <pa...@krankikom.de>.
Donnie,
Singletons are often great on the application level but tend to
be tricky on the component level.
On an application you have a clear understanding of what are the
advantages and drawbacks of your singletons - you implement a
singleton on your application because you do NOT want that
component to have more than one instance.
However, you are not always able to foresee what will be all the
use models of your component and a singleton will limit some of
such use models.
Considering the logging example, some APIs implement the logger
hierarchy as a singleton and that prevents the applications
using the logger from using multiple hierarchies (*). However,
many applications NEED or are simpler to implement using multiple
logger hierarchies - as it is the case with a server that embeds
several subsystems (**).
(*) At least without playing tricky class loader games that
are not always possible.
(**) This does not always mean having separate class loaders.
Have fun,
Paulo Gaspar
> -----Original Message-----
> From: Donnie Hale [mailto:donnie@haleonline.net]
> Sent: Monday, February 04, 2002 4:47 AM
> To: Jakarta Commons Developers List; paulo.gaspar@krankikom.de
> Subject: RE: Problems with commons-logging
>
>
> Paulo,
>
> I've seen you mention a couple of times that you consider singletons
> dangerous. Would you care to elaborate? Is it because you're
> concerned that
> people can't write thread-safe code correctly? Or because correct
> thread-safe code affects concurrency? Or something else?
>
> Thanks,
>
> Donnie
>
>
> > -----Original Message-----
> > From: Paulo Gaspar [mailto:paulo.gaspar@krankikom.de]
> > Sent: Sunday, February 03, 2002 8:19 PM
> > To: Jakarta Commons Developers List; sanders@apache.org
> > Subject: RE: Problems with commons-logging
> >
> >
> > Answer inline:
> >
> > > -----Original Message-----
> > > From: Scott Sanders [mailto:sanders@apache.org]
> > > Sent: Sunday, February 03, 2002 2:18 AM
> > >
> > > On Sat, Feb 02, 2002 at 08:33:46AM -0800, costinm@covalent.net wrote:
> > >
> > > > - 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.
> > > >
> > >
> > > I can see how getLogNames() could be considered evil, and I do
> > > not see what it gains us. I am +1 on remving getLogNames().
> >
> > +1
> >
> > > Are you saying that with getInstance(), we should remove it and
> > > just use newLogInstance()? I am also fine with this, albeit a +0.
> >
> > I am still not sure I understand this one.
> > (So, I probably don't.)
> >
> >
> > > > - 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 ).
> > >
> > > Why don't we rename LogSource to LogFactory, and change
> > > makeNewLogInstance() to newInstance()? +1 on this as well.
> > > I believe this would be consistent with things like JAXP, correct?
> >
> > +1
> >
> > Also, as I mentioned 2 posts ago on this thread, any Singleton should
> > be avoided. (Some static methods might make sense but Singletons are
> > usually dangerous.)
> >
> >
> > > > - 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.
> > > >
> > >
> > > I don't see the problem with having a factory take care of this,
> > > so I am -0. I like that Log is an interface.
> >
> > I am -1. I really like it being an interface.
> >
> > > > - 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 I do not see how to port the extensibility across APIs. For the
> > time being I am -1 on extensibility and +1 on having level constants.
> >
> > > I will code this up. Thanks for the comment. +1. I am assuming
> > > void log(int level, Object message) and the proper Throwable
> > > sister method?.
> >
> > Scott, I have this coded but no CPU time (mind frame) to adapt it.
> > Would it be ok if I send you my code and you take a look at it?
> >
> >
> > > > - 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.
> > > >
> > >
> > > Why would this end up in the top level loader? I do not
> > > understand this. Could you explain more please?
> > >
> > > > - 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.
> > > >
> > >
> > > I am -1 on walking the config line. No config. None. This API
> > > intends to mask all of this and allow a component to just log.
> > > The container using the component will be required to configure
> > > logging. We are not trying to replace LogKit/Log4J, we are only
> > > trying to replace System.out.println(), IMHO. Besides, that is
> > > orhtogonal IMHO, and if logging does this, it can do this in a
> > > later release, on a separate interface.
> >
> > I +1 on config, although on a separate/separable package. Again,
> > take a look on my previous post on this thread for details (not
> > my last one but the previous).
> >
> >
> > > > - 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.
> > >
> > > Fine. Done. +1
> > >
> > > > - 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 )
> > > >
> > >
> > > It IS pluggable. Just set the org.apache.commons.logging.log
> > > property with your Log impl. Could you code up the jar services
> > > manifest code to augment this? Thanks.
> >
> > I also rumble about this one on that previous post a mentioned above.
> >
> > > > - 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.
> > >
> > > -0. I don't really see the need to break this up. I agree that
> > > only 2 classes really 'matter', but I do not see that as a reason
> > > to move them. If someone else wants to, I wont stop them. Are
> > > you suggesting something like an o.a.c.l.impl package?
> >
> > +1
> >
> > I would call it o.a.c.l.wrappers since other implementation bits
> > might be added (like that o.a.c.l.config I am defending).
> >
> >
> > > > - 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.
> > >
> > > Consider this done.
> >
> > +1
> >
> > > > - makeNewLogInstance comment and impl are completely of sync.
> > >
> > > I will look into this.
> > >
> > > > - 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 ).
> > > >
> > >
> > > -0. I personally have no intention of supporting this. I think
> > > that commons-logging is just a simple conduit to a logging
> > > implementation, i18n is the concern of both sides, but not the
> > > middle. If we keep the Object as the log parameter, I don't see
> > > how we are 'missing out'.
> >
> > -1
> >
> > The official (SUN) i18n APIs are insufficient for many apps and
> > that is why alternative i18n APIs are popping up like mushrooms
> > all around Jakarta. It would not be nice to favor any of them.
> >
> > IMO, a i18n logger could be the subject of another component (or
> > several - one per i18n API) based on this one.
> >
> > I also mentioned this one on my previous post.
> >
> > I have an AbstractLoggable class that supports i18n but I will
> > NOT contribute it to this package.
> >
> > ...
> >
> >
> > Have fun,
> > Paulo Gaspar
> >
> > http://www.krankikom.de
> > http://www.ruhronline.de
> >
> >
> >
> > --
> > To unsubscribe, e-mail:
> > <ma...@jakarta.apache.org>
> > For additional commands, e-mail:
> > <ma...@jakarta.apache.org>
> >
> >
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
For additional commands, e-mail:
<ma...@jakarta.apache.org>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: Problems with commons-logging
Posted by Donnie Hale <do...@haleonline.net>.
Paulo,
I've seen you mention a couple of times that you consider singletons
dangerous. Would you care to elaborate? Is it because you're concerned that
people can't write thread-safe code correctly? Or because correct
thread-safe code affects concurrency? Or something else?
Thanks,
Donnie
> -----Original Message-----
> From: Paulo Gaspar [mailto:paulo.gaspar@krankikom.de]
> Sent: Sunday, February 03, 2002 8:19 PM
> To: Jakarta Commons Developers List; sanders@apache.org
> Subject: RE: Problems with commons-logging
>
>
> Answer inline:
>
> > -----Original Message-----
> > From: Scott Sanders [mailto:sanders@apache.org]
> > Sent: Sunday, February 03, 2002 2:18 AM
> >
> > On Sat, Feb 02, 2002 at 08:33:46AM -0800, costinm@covalent.net wrote:
> >
> > > - 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.
> > >
> >
> > I can see how getLogNames() could be considered evil, and I do
> > not see what it gains us. I am +1 on remving getLogNames().
>
> +1
>
> > Are you saying that with getInstance(), we should remove it and
> > just use newLogInstance()? I am also fine with this, albeit a +0.
>
> I am still not sure I understand this one.
> (So, I probably don't.)
>
>
> > > - 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 ).
> >
> > Why don't we rename LogSource to LogFactory, and change
> > makeNewLogInstance() to newInstance()? +1 on this as well.
> > I believe this would be consistent with things like JAXP, correct?
>
> +1
>
> Also, as I mentioned 2 posts ago on this thread, any Singleton should
> be avoided. (Some static methods might make sense but Singletons are
> usually dangerous.)
>
>
> > > - 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.
> > >
> >
> > I don't see the problem with having a factory take care of this,
> > so I am -0. I like that Log is an interface.
>
> I am -1. I really like it being an interface.
>
> > > - 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 I do not see how to port the extensibility across APIs. For the
> time being I am -1 on extensibility and +1 on having level constants.
>
> > I will code this up. Thanks for the comment. +1. I am assuming
> > void log(int level, Object message) and the proper Throwable
> > sister method?.
>
> Scott, I have this coded but no CPU time (mind frame) to adapt it.
> Would it be ok if I send you my code and you take a look at it?
>
>
> > > - 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.
> > >
> >
> > Why would this end up in the top level loader? I do not
> > understand this. Could you explain more please?
> >
> > > - 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.
> > >
> >
> > I am -1 on walking the config line. No config. None. This API
> > intends to mask all of this and allow a component to just log.
> > The container using the component will be required to configure
> > logging. We are not trying to replace LogKit/Log4J, we are only
> > trying to replace System.out.println(), IMHO. Besides, that is
> > orhtogonal IMHO, and if logging does this, it can do this in a
> > later release, on a separate interface.
>
> I +1 on config, although on a separate/separable package. Again,
> take a look on my previous post on this thread for details (not
> my last one but the previous).
>
>
> > > - 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.
> >
> > Fine. Done. +1
> >
> > > - 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 )
> > >
> >
> > It IS pluggable. Just set the org.apache.commons.logging.log
> > property with your Log impl. Could you code up the jar services
> > manifest code to augment this? Thanks.
>
> I also rumble about this one on that previous post a mentioned above.
>
> > > - 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.
> >
> > -0. I don't really see the need to break this up. I agree that
> > only 2 classes really 'matter', but I do not see that as a reason
> > to move them. If someone else wants to, I wont stop them. Are
> > you suggesting something like an o.a.c.l.impl package?
>
> +1
>
> I would call it o.a.c.l.wrappers since other implementation bits
> might be added (like that o.a.c.l.config I am defending).
>
>
> > > - 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.
> >
> > Consider this done.
>
> +1
>
> > > - makeNewLogInstance comment and impl are completely of sync.
> >
> > I will look into this.
> >
> > > - 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 ).
> > >
> >
> > -0. I personally have no intention of supporting this. I think
> > that commons-logging is just a simple conduit to a logging
> > implementation, i18n is the concern of both sides, but not the
> > middle. If we keep the Object as the log parameter, I don't see
> > how we are 'missing out'.
>
> -1
>
> The official (SUN) i18n APIs are insufficient for many apps and
> that is why alternative i18n APIs are popping up like mushrooms
> all around Jakarta. It would not be nice to favor any of them.
>
> IMO, a i18n logger could be the subject of another component (or
> several - one per i18n API) based on this one.
>
> I also mentioned this one on my previous post.
>
> I have an AbstractLoggable class that supports i18n but I will
> NOT contribute it to this package.
>
> ...
>
>
> Have fun,
> Paulo Gaspar
>
> http://www.krankikom.de
> http://www.ruhronline.de
>
>
>
> --
> To unsubscribe, e-mail:
> <ma...@jakarta.apache.org>
> For additional commands, e-mail:
> <ma...@jakarta.apache.org>
>
>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: Problems with commons-logging
Posted by Paulo Gaspar <pa...@krankikom.de>.
Answer inline:
> -----Original Message-----
> From: Scott Sanders [mailto:sanders@apache.org]
> Sent: Sunday, February 03, 2002 2:18 AM
>
> On Sat, Feb 02, 2002 at 08:33:46AM -0800, costinm@covalent.net wrote:
>
> > - 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.
> >
>
> I can see how getLogNames() could be considered evil, and I do
> not see what it gains us. I am +1 on remving getLogNames().
+1
> Are you saying that with getInstance(), we should remove it and
> just use newLogInstance()? I am also fine with this, albeit a +0.
I am still not sure I understand this one.
(So, I probably don't.)
> > - 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 ).
>
> Why don't we rename LogSource to LogFactory, and change
> makeNewLogInstance() to newInstance()? +1 on this as well.
> I believe this would be consistent with things like JAXP, correct?
+1
Also, as I mentioned 2 posts ago on this thread, any Singleton should
be avoided. (Some static methods might make sense but Singletons are
usually dangerous.)
> > - 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.
> >
>
> I don't see the problem with having a factory take care of this,
> so I am -0. I like that Log is an interface.
I am -1. I really like it being an interface.
> > - 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 I do not see how to port the extensibility across APIs. For the
time being I am -1 on extensibility and +1 on having level constants.
> I will code this up. Thanks for the comment. +1. I am assuming
> void log(int level, Object message) and the proper Throwable
> sister method?.
Scott, I have this coded but no CPU time (mind frame) to adapt it.
Would it be ok if I send you my code and you take a look at it?
> > - 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.
> >
>
> Why would this end up in the top level loader? I do not
> understand this. Could you explain more please?
>
> > - 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.
> >
>
> I am -1 on walking the config line. No config. None. This API
> intends to mask all of this and allow a component to just log.
> The container using the component will be required to configure
> logging. We are not trying to replace LogKit/Log4J, we are only
> trying to replace System.out.println(), IMHO. Besides, that is
> orhtogonal IMHO, and if logging does this, it can do this in a
> later release, on a separate interface.
I +1 on config, although on a separate/separable package. Again,
take a look on my previous post on this thread for details (not
my last one but the previous).
> > - 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.
>
> Fine. Done. +1
>
> > - 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 )
> >
>
> It IS pluggable. Just set the org.apache.commons.logging.log
> property with your Log impl. Could you code up the jar services
> manifest code to augment this? Thanks.
I also rumble about this one on that previous post a mentioned above.
> > - 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.
>
> -0. I don't really see the need to break this up. I agree that
> only 2 classes really 'matter', but I do not see that as a reason
> to move them. If someone else wants to, I wont stop them. Are
> you suggesting something like an o.a.c.l.impl package?
+1
I would call it o.a.c.l.wrappers since other implementation bits
might be added (like that o.a.c.l.config I am defending).
> > - 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.
>
> Consider this done.
+1
> > - makeNewLogInstance comment and impl are completely of sync.
>
> I will look into this.
>
> > - 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 ).
> >
>
> -0. I personally have no intention of supporting this. I think
> that commons-logging is just a simple conduit to a logging
> implementation, i18n is the concern of both sides, but not the
> middle. If we keep the Object as the log parameter, I don't see
> how we are 'missing out'.
-1
The official (SUN) i18n APIs are insufficient for many apps and
that is why alternative i18n APIs are popping up like mushrooms
all around Jakarta. It would not be nice to favor any of them.
IMO, a i18n logger could be the subject of another component (or
several - one per i18n API) based on this one.
I also mentioned this one on my previous post.
I have an AbstractLoggable class that supports i18n but I will
NOT contribute it to this package.
...
Have fun,
Paulo Gaspar
http://www.krankikom.de
http://www.ruhronline.de
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Problems with commons-logging
Posted by cm...@yahoo.com.
On Sat, 2 Feb 2002, Scott Sanders wrote:
> Are you saying that with getInstance(), we should remove it and just
> use newLogInstance()? I am also fine with this, albeit a +0.
It's not a naming issue, but a behavior issue.
The case: you have 2 apps you want to keep isolated. Allowing one to
log into the other's log is unacceptable. Classloader tricks are not
allways possible and are extremely error prone ( and I would say -
ineffective, can be tricked ). And the security manager is not
something most people understand.
We should at minimum require that getInstance( name ) would get
you a logger _local_ to your app, if in a container env.
( impl: use thread class loader, or the class loader as a
namespace qualifier ).
> > - 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 ).
> >
>
> Why don't we rename LogSource to LogFactory, and change
> makeNewLogInstance() to newInstance()? +1 on this as well.
> I believe this would be consistent with things like JAXP, correct?
My naming preference would be LogManager ( as it'll not be only
a Factory ).
But the problem is not the name here, but the use of static
methods. If we are to have a static method, it should just
be a shortcut for finding a manager/factory instance and
calling a virtual method.
> > - 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.
> >
>
> I don't see the problem with having a factory take care of this, so I
> am -0. I like that Log is an interface.
What does it gives you ? Most cases will use delegation anyway.
Having it as an abstract method, with a static getLog() method
will make the user interact with a single class - Log - instead
of 2. The LogManager/LogFactory will do the work 'behind the scene'.
Nothing else change in the code ( except replacing implements with
extends ). Except maybe simplifying some adapters by keeping
common stuff in the base class.
> > - 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.
> >
>
> I will code this up. Thanks for the comment. +1. I am assuming void
> log(int level, Object message) and the proper Throwable sister method?.
It's not a big priority - it can be done after the first release.
But combined with the previous point - it can make the adapter as simple
as overriding the log() method and supporting the base levels.
> > - 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.
> >
>
> Why would this end up in the top level loader? I do not understand
> this. Could you explain more please?
If commons is used everywhere, it may be used for example by the
Main class of tomcat ( or ant, etc ). It'll then be loaded by
the parent class loader - and the static fields will be set there.
You can have a CL that brakes delegation ( with all the secondary
effects ), but that's not allwasy possible or desirable and it
can create a huge maintainance problem.
> > - 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.
> >
>
> I am -1 on walking the config line. No config. None. This API intends
> to mask all of this and allow a component to just log. The container
> using the component will be required to configure logging. We are not
> trying to replace LogKit/Log4J, we are only trying to replace
> System.out.println(), IMHO. Besides, that is orhtogonal IMHO, and if
> logging does this, it can do this in a later release, on a separate
> interface.
Yes, but we should at least allow the user to pass a simple 'config file'
or base information to the real logger. Like attributes in servlet, jaxp,
etc.
Otherwise we'll still have to code against Log4j APIs ( to set the
config file for example ) - and what's the point of using commons logging
if we already require and hardcode log4j ?
The default config mechanism is not allways desirable ( at least for
log4j). The user will still use a log-specific config file, but at least
the location of the file could be passed ( I hate system properties
and props loaded from CLASSPATH ).
> > - 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 )
> >
>
> It IS pluggable. Just set the org.apache.commons.logging.log property
> with your Log impl. Could you code up the jar services manifest code to
> augment this? Thanks.
What jaxp does is to 'guess' what logger is _available_, without
any user configuration. Same is done by the Class.forName() you
use ( so part of the behavior is there ), but that's hard to extend
without changing the code and adding more ifs.
> > - 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.
> >
>
> -0. I don't really see the need to break this up. I agree that only 2
> classes really 'matter', but I do not see that as a reason to move them.
> If someone else wants to, I wont stop them. Are you suggesting
> something like an o.a.c.l.impl package?
1. It's harder to read ( for people who don't know the code ). A project
has 100s of files, it's better to keep things organized.
2. It'll grow messy, as more adapters are added.
> > - 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.
> >
>
> Consider this done.
Note that JDK1.1 is not an absolute requirement - if we end up
using the Thread.currentClassLoader() we'll loose it. But giving it
up by using HashMap when Hashtable can be used is not good.
> > - 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 ).
> >
>
> -0. I personally have no intention of supporting this. I think that
> commons-logging is just a simple conduit to a logging implementation,
> i18n is the concern of both sides, but not the middle. If we keep the
> Object as the log parameter, I don't see how we are 'missing out'.
Again, for the first release we probably don't need it.
It is a good idea because it makes a lot of optimizations possible and
simplifies the user code and allows a more centralized management
of resource bundles.
I18N requires a lot of string operations - all this overhead could
be eliminated by a smart logger.
BTW, this would also encourage people to pass 'primary' information
( the log key and params ), which can be more valuable than a simple
cooked string.
Costin
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Problems with commons-logging
Posted by Scott Sanders <sa...@apache.org>.
On Sat, Feb 02, 2002 at 08:33:46AM -0800, costinm@covalent.net wrote:
> 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.
>
Thanks for takeing the time to look at this Costin. I appreciate it.
> - 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.
>
I can see how getLogNames() could be considered evil, and I do not see what it gains us. I am +1 on remving getLogNames().
Are you saying that with getInstance(), we should remove it and just use newLogInstance()? I am also fine with this, albeit a +0.
> - 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 ).
>
Why don't we rename LogSource to LogFactory, and change makeNewLogInstance() to newInstance()? +1 on this as well.
I believe this would be consistent with things like JAXP, correct?
> - 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.
>
I don't see the problem with having a factory take care of this, so I am -0. I like that Log is an interface.
> - 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.
>
I will code this up. Thanks for the comment. +1. I am assuming void log(int level, Object message) and the proper Throwable sister method?.
> - 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.
>
Why would this end up in the top level loader? I do not understand this. Could you explain more please?
> - 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.
>
I am -1 on walking the config line. No config. None. This API intends to mask all of this and allow a component to just log. The container using the component will be required to configure logging. We are not trying to replace LogKit/Log4J, we are only trying to replace System.out.println(), IMHO. Besides, that is orhtogonal IMHO, and if logging does this, it can do this in a later release, on a separate interface.
> - 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.
>
Fine. Done. +1
> - 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 )
>
It IS pluggable. Just set the org.apache.commons.logging.log property with your Log impl. Could you code up the jar services manifest code to augment this? Thanks.
> - 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.
>
-0. I don't really see the need to break this up. I agree that only 2 classes really 'matter', but I do not see that as a reason to move them. If someone else wants to, I wont stop them. Are you suggesting something like an o.a.c.l.impl package?
> - 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.
>
Consider this done.
> - makeNewLogInstance comment and impl are completely of sync.
>
I will look into this.
> - 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 ).
>
-0. I personally have no intention of supporting this. I think that commons-logging is just a simple conduit to a logging implementation, i18n is the concern of both sides, but not the middle. If we keep the Object as the log parameter, I don't see how we are 'missing out'.
Thanks again for your thoughtful analysis. I will look into the things that I listed.
>
> Costin
>
--
Scott Sanders - sanders@apache.org
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>