You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Scott Sanders <ss...@nextance.com> on 2002/04/22 18:48:54 UTC

[LOGGING] ClassLoader Problems

Hi all,

I am working on vindico, a replacement for Alexandria over in the
Alexandria CVS, and I am using commons-digester, which in turn uses
commons-logging.

When running Vindico as an Ant task,  it starts running and then says
that it cannot find the LogFactoryImpl class.

I have traced this down to the getClassLoader() function in LogFactory,
which returns the Thread.getContextClassLoader() if there is one.

The problem that I have is the LogFactoryImpl class is in the same class
loader as LogFactory, so I am suggesting a change to LogFactory, such
that the default class loader is used.  Patch is below.  Are there other
problems that this does not address?

Any questions, comments, suggestions?

If not, I will commit this.

Scott

Index: src/java/org/apache/commons/logging/LogFactory.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/logging/src/java/org/apache/commons/logging/Lo
gFactory.java,v
retrieving revision 1.6
diff -u -r1.6 LogFactory.java
--- src/java/org/apache/commons/logging/LogFactory.java 15 Mar 2002
22:57:36 -0000      1.6
+++ src/java/org/apache/commons/logging/LogFactory.java 22 Apr 2002
16:48:12 -0000
@@ -347,7 +347,7 @@

         // Fourth, try the fallback implementation class
         if (factory == null) {
-            factory = newFactory(FACTORY_DEFAULT, classLoader);
+            factory = newFactory(FACTORY_DEFAULT,
LogFactory.class.getClassLoader());
         }

         if( props!=null ) {

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [LOGGING] ClassLoader Problems

Posted by John McNally <jm...@collab.net>.
"Craig R. McClanahan" wrote:
> 
> On Mon, 22 Apr 2002, John McNally wrote:
> 
> > Date: Mon, 22 Apr 2002 10:58:16 -0700
> > From: John McNally <jm...@collab.net>
> > Reply-To: Jakarta Commons Developers List <co...@jakarta.apache.org>
> > To: Jakarta Commons Developers List <co...@jakarta.apache.org>
> > Subject: Re: [LOGGING] ClassLoader Problems
> >
> >   It does not seem that static factory methods like
> > > Log.getLogger(String) or Category.getInstance(String) are the best way
> > > to invoke logging within a component meant to be used within a larger
> > > system.
> > >
> >
> > I should say it might not be a bad way to invoke the logger, but
> > hardcoding the String key to be the classname where the logger is used
> > appears to be a bad choice.  A component needs to have a way to allow
> > the system using the component to override the region/category.
> >
> 
> "Good" versus "bad" is a pretty emotion-laden discussion starter :-)
> 

I just thought it might limit the usefulness of a component, or
potentially force the consumer to live with problems due to the use of
that design.  I'm glad to see it is possible to still use the commons
components within multiple webapp contexts even though commons-logging
is shared.  Thanks for the info.

I still wonder about the use case of a application that uses
commons-pool (assuming it would use commons-logging) in two packages and
would prefer to have the logging segregated along the package lines. 
E.g. myapp.foo and myapp.bar both using the pool code for different
reasons.  Assuming this is possible, just ignore me until I actually
need to do this and I look for a way to accomplish it.

Thanks, 
john mcnally

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [LOGGING] ClassLoader Problems

Posted by "Craig R. McClanahan" <cr...@apache.org>.

On Mon, 22 Apr 2002, John McNally wrote:

> Date: Mon, 22 Apr 2002 10:58:16 -0700
> From: John McNally <jm...@collab.net>
> Reply-To: Jakarta Commons Developers List <co...@jakarta.apache.org>
> To: Jakarta Commons Developers List <co...@jakarta.apache.org>
> Subject: Re: [LOGGING] ClassLoader Problems
>
>   It does not seem that static factory methods like
> > Log.getLogger(String) or Category.getInstance(String) are the best way
> > to invoke logging within a component meant to be used within a larger
> > system.
> >
>
> I should say it might not be a bad way to invoke the logger, but
> hardcoding the String key to be the classname where the logger is used
> appears to be a bad choice.  A component needs to have a way to allow
> the system using the component to override the region/category.
>

"Good" versus "bad" is a pretty emotion-laden discussion starter :-)

I would point out, though, that the existing commons-logging
implementation lets you accomplish precisely the goal you specified,
without requiring a setLog() or equivalent method on the component itself.

As a test of this, I wrote a trivial LogFactory implementation whose
getInstance() methods simply renamed the one requested by the component to
match an application-specified hierarchy.  This works even if the
component itself thinks it is asking for a logger named after the
component class itself, with:

  private static Log log = LogFactory.getLog("my.class.name");

This approach accomodates lots of other options -- from "make all the
components share a single Log implementation" up to very sophisticated
sharing approaches.

Personally, I find that this feature, coupled with the hierarchical
configuration supported by Log4J (I can set a default logging level for
"org.apache" to cover all my Apache-source components, and then override
it for just "org.apache.foo.bar")  makes a very flexible paradigm, without
imposing any requirements on the components themselves (except to document
what logger names they use :-).

> john mcnally

Craig


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [LOGGING] ClassLoader Problems

Posted by John McNally <jm...@collab.net>.
  It does not seem that static factory methods like
> Log.getLogger(String) or Category.getInstance(String) are the best way
> to invoke logging within a component meant to be used within a larger
> system.
> 

I should say it might not be a bad way to invoke the logger, but
hardcoding the String key to be the classname where the logger is used
appears to be a bad choice.  A component needs to have a way to allow
the system using the component to override the region/category.

john mcnally

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [LOGGING] ClassLoader Problems

Posted by John McNally <jm...@collab.net>.
I think this is a related issue, but I have not had time to investigate
fully, so sorry if this is OT.  I have an app, that using
commons-logging in some of its dependencies, which is configured to use
log4j.  As commons-logging is in <tomcat>/common/lib, I have to put
log4j in common/lib as well or I get ClassNotFoundException's.  Now if I
have multiple contexts with similar webapps, the log files get munged
together/corrupted.

This might be more an issue with tomcat in that it should not be putting
a library like logging into a classpath used by webapps.  But is there
some way of configuring/using commons logging so that a component that
is used in multiple places within an application can log to the location
appropriate for each of its uses without relying on classloader imposed
separation.  It does not seem that static factory methods like
Log.getLogger(String) or Category.getInstance(String) are the best way
to invoke logging within a component meant to be used within a larger
system.

john mcnally


"Craig R. McClanahan" wrote:
> 
> On Mon, 22 Apr 2002, Scott Sanders wrote:
> 
> > Date: Mon, 22 Apr 2002 09:48:54 -0700
> > From: Scott Sanders <ss...@nextance.com>
> > Reply-To: Jakarta Commons Developers List <co...@jakarta.apache.org>
> > To: Jakarta Commons Developers List <co...@jakarta.apache.org>
> > Subject: [LOGGING] ClassLoader Problems
> >
> > Hi all,
> >
> > I am working on vindico, a replacement for Alexandria over in the
> > Alexandria CVS, and I am using commons-digester, which in turn uses
> > commons-logging.
> >
> > When running Vindico as an Ant task,  it starts running and then says
> > that it cannot find the LogFactoryImpl class.
> >
> > I have traced this down to the getClassLoader() function in LogFactory,
> > which returns the Thread.getContextClassLoader() if there is one.
> >
> > The problem that I have is the LogFactoryImpl class is in the same class
> > loader as LogFactory, so I am suggesting a change to LogFactory, such
> > that the default class loader is used.  Patch is below.  Are there other
> > problems that this does not address?
> >
> 
> Haven't looked at the patch yet, but from your description it sounds like
> this would break the use of commons-logging in a servlet container where
> the commons-logging.jar file was in a shared directory and the desired
> custom LogFactory implementation class was in the webapp.  Please make
> sure that this use case works correctly.
> 
> > Any questions, comments, suggestions?
> >
> > If not, I will commit this.
> >
> > Scott
> >
> 
> Craig
> 
> > Index: src/java/org/apache/commons/logging/LogFactory.java
> > ===================================================================
> > RCS file:
> > /home/cvs/jakarta-commons/logging/src/java/org/apache/commons/logging/Lo
> > gFactory.java,v
> > retrieving revision 1.6
> > diff -u -r1.6 LogFactory.java
> > --- src/java/org/apache/commons/logging/LogFactory.java 15 Mar 2002
> > 22:57:36 -0000      1.6
> > +++ src/java/org/apache/commons/logging/LogFactory.java 22 Apr 2002
> > 16:48:12 -0000
> > @@ -347,7 +347,7 @@
> >
> >          // Fourth, try the fallback implementation class
> >          if (factory == null) {
> > -            factory = newFactory(FACTORY_DEFAULT, classLoader);
> > +            factory = newFactory(FACTORY_DEFAULT,
> > LogFactory.class.getClassLoader());
> >          }
> >
> >          if( props!=null ) {
> >

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [LOGGING] ClassLoader Problems

Posted by "Craig R. McClanahan" <cr...@apache.org>.

On Mon, 22 Apr 2002, Scott Sanders wrote:

> Date: Mon, 22 Apr 2002 09:48:54 -0700
> From: Scott Sanders <ss...@nextance.com>
> Reply-To: Jakarta Commons Developers List <co...@jakarta.apache.org>
> To: Jakarta Commons Developers List <co...@jakarta.apache.org>
> Subject: [LOGGING] ClassLoader Problems
>
> Hi all,
>
> I am working on vindico, a replacement for Alexandria over in the
> Alexandria CVS, and I am using commons-digester, which in turn uses
> commons-logging.
>
> When running Vindico as an Ant task,  it starts running and then says
> that it cannot find the LogFactoryImpl class.
>
> I have traced this down to the getClassLoader() function in LogFactory,
> which returns the Thread.getContextClassLoader() if there is one.
>
> The problem that I have is the LogFactoryImpl class is in the same class
> loader as LogFactory, so I am suggesting a change to LogFactory, such
> that the default class loader is used.  Patch is below.  Are there other
> problems that this does not address?
>

Haven't looked at the patch yet, but from your description it sounds like
this would break the use of commons-logging in a servlet container where
the commons-logging.jar file was in a shared directory and the desired
custom LogFactory implementation class was in the webapp.  Please make
sure that this use case works correctly.

> Any questions, comments, suggestions?
>
> If not, I will commit this.
>
> Scott
>

Craig


> Index: src/java/org/apache/commons/logging/LogFactory.java
> ===================================================================
> RCS file:
> /home/cvs/jakarta-commons/logging/src/java/org/apache/commons/logging/Lo
> gFactory.java,v
> retrieving revision 1.6
> diff -u -r1.6 LogFactory.java
> --- src/java/org/apache/commons/logging/LogFactory.java 15 Mar 2002
> 22:57:36 -0000      1.6
> +++ src/java/org/apache/commons/logging/LogFactory.java 22 Apr 2002
> 16:48:12 -0000
> @@ -347,7 +347,7 @@
>
>          // Fourth, try the fallback implementation class
>          if (factory == null) {
> -            factory = newFactory(FACTORY_DEFAULT, classLoader);
> +            factory = newFactory(FACTORY_DEFAULT,
> LogFactory.class.getClassLoader());
>          }
>
>          if( props!=null ) {
>
> --
> 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>