You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sk...@apache.org on 2005/05/22 12:43:54 UTC
svn commit: r171301 -
/jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
Author: skitching
Date: Sun May 22 03:43:52 2005
New Revision: 171301
URL: http://svn.apache.org/viewcvs?rev=171301&view=rev
Log:
Added internal diagnostics
Modified:
jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
Modified: jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java?rev=171301&r1=171300&r2=171301&view=diff
==============================================================================
--- jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java (original)
+++ jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java Sun May 22 03:43:52 2005
@@ -76,6 +76,8 @@
*/
public LogFactoryImpl() {
super();
+ initDiagnostics(); // method on this object
+ logDiagnostic("Instance created.");
}
@@ -109,6 +111,11 @@
/**
+ * The string prefixed to every message output by the logDiagnostic method.
+ */
+ private String diagnosticPrefix;
+
+ /**
* Configuration attributes.
*/
protected Hashtable attributes = new Hashtable();
@@ -160,7 +167,6 @@
// --------------------------------------------------------- Public Methods
-
/**
* Return the configuration attribute with the specified name (if any),
* or <code>null</code> if there is no such attribute.
@@ -250,6 +256,7 @@
*/
public void release() {
+ logDiagnostic("Releasing all known loggers");
instances.clear();
}
@@ -287,13 +294,90 @@
}
+ // ------------------------------------------------------
+ // Static Methods
+ //
+ // These methods only defined as workarounds for a java 1.2 bug;
+ // theoretically none of these are needed.
+ // ------------------------------------------------------
+
+ /**
+ * Gets the context classloader.
+ * This method is a workaround for a java 1.2 compiler bug.
+ */
+ protected static ClassLoader getContextClassLoader() throws LogConfigurationException {
+ return LogFactory.getContextClassLoader();
+ }
+
+ /**
+ * Workaround for bug in Java1.2; in theory this method is not needed.
+ * See LogFactory.isInternalLoggingEnabled.
+ */
+ protected static boolean isDiagnosticsEnabled() {
+ return LogFactory.isDiagnosticsEnabled();
+ }
+
+ /**
+ * Workaround for bug in Java1.2; in theory this method is not needed.
+ * See LogFactory.getClassLoader.
+ */
+ protected static ClassLoader getClassLoader(Class clazz) {
+ return LogFactory.getClassLoader(clazz);
+ }
+
// ------------------------------------------------------ Protected Methods
+ /**
+ * Calculate and cache a string that uniquely identifies this instance,
+ * including which classloader the object was loaded from.
+ * <p>
+ * This string will later be prefixed to each "internal logging" message
+ * emitted, so that users can clearly see any unexpected behaviour.
+ * <p>
+ * Note that this method does not detect whether internal logging is
+ * enabled or not, nor where to output stuff if it is; that is all
+ * handled by the parent LogFactory class. This method just computes
+ * its own unique prefix for log messages.
+ */
+ private void initDiagnostics() {
+ // It would be nice to include an identifier of the context classloader
+ // that this LogFactoryImpl object is responsible for. However that
+ // isn't possible as that information isn't available. It is possible
+ // to figure this out by looking at the logging from LogFactory to
+ // see the context & impl ids from when this object was instantiated,
+ // in order to link the impl id output as this object's prefix back to
+ // the context it is intended to manage.
+ Class clazz = this.getClass();
+ ClassLoader classLoader = getClassLoader(clazz);
+ diagnosticPrefix = clazz.getName() + "@" + classLoader.toString() + ":";
+ }
+ /**
+ * Output a diagnostic message to a user-specified destination (if the
+ * user has enabled diagnostic logging).
+ *
+ * @param msg
+ */
+ protected void logDiagnostic(String msg) {
+ if (isDiagnosticsEnabled()) {
+ logRawDiagnostic(diagnosticPrefix + msg);
+ }
+ }
/**
* Return the fully qualified Java classname of the {@link Log}
* implementation we will be using.
+ * <p>
+ * This method looks in the following places:
+ * <ul>
+ * <li>Looks for an attribute LOG_PROPERTY or LOG_PROPERTY_OLD in the
+ * "attributes" associated with this class, as set earlier by method
+ * setAttribute.
+ * <li>Looks for a property LOG_PROPERTY or LOG_PROPERTY_OLD in the
+ * system properties.
+ * <li>Looks for log4j, jdk logging and jdk13lumberjack classes in
+ * the classpath.
+ * </ul>
*/
protected String getLogClassName() {
@@ -302,14 +386,19 @@
return logClassName;
}
+ logDiagnostic("Determining the name for the Log implementation.");
+
+ logDiagnostic("Trying to get log class from attribute " + LOG_PROPERTY);
logClassName = (String) getAttribute(LOG_PROPERTY);
if (logClassName == null) { // @deprecated
+ logDiagnostic("Trying to get log class from attribute " + LOG_PROPERTY_OLD);
logClassName = (String) getAttribute(LOG_PROPERTY_OLD);
}
if (logClassName == null) {
try {
+ logDiagnostic("Trying to get log class from system property " + LOG_PROPERTY);
logClassName = System.getProperty(LOG_PROPERTY);
} catch (SecurityException e) {
;
@@ -318,12 +407,15 @@
if (logClassName == null) { // @deprecated
try {
+ logDiagnostic("Trying to get log class from system property " + LOG_PROPERTY_OLD);
logClassName = System.getProperty(LOG_PROPERTY_OLD);
} catch (SecurityException e) {
;
}
}
+ // no need for internalLog calls below; they are done inside the
+ // various isXXXAvailable methods.
if ((logClassName == null) && isLog4JAvailable()) {
logClassName = "org.apache.commons.logging.impl.Log4JLogger";
}
@@ -340,6 +432,7 @@
logClassName = "org.apache.commons.logging.impl.SimpleLog";
}
+ logDiagnostic("Using log class " + logClassName);
return (logClassName);
}
@@ -368,24 +461,48 @@
String logClassName = getLogClassName();
// Attempt to load the Log implementation class
+ //
+ // Question: why is the loginterface being loaded dynamically?
+ // Isn't the code below exactly the same as this?
+ // Class logInterface = Log.class;
+
Class logClass = null;
Class logInterface = null;
try {
- ClassLoader cl = this.getClass().getClassLoader();
- // handle the case if getClassLoader() returns null
- // It may mean this class was loaded from the bootstrap classloader
- logInterface = (cl == null) ? loadClass(LOG_INTERFACE) :
- cl.loadClass(LOG_INTERFACE);
+ ClassLoader cl = getClassLoader(this.getClass());
+ if (cl == null) {
+ // we are probably in Java 1.1, but may also be running in
+ // some sort of embedded system..
+ logInterface = loadClass(LOG_INTERFACE);
+ } else {
+ // normal situation
+ logInterface = cl.loadClass(LOG_INTERFACE);
+ }
+
logClass = loadClass(logClassName);
if (logClass == null) {
+ logDiagnostic(
+ "Unable to find any class named [" + logClassName + "]"
+ + " in either the context classloader"
+ + " or the classloader that loaded this class.");
+
throw new LogConfigurationException
("No suitable Log implementation for " + logClassName);
}
+
if (!logInterface.isAssignableFrom(logClass)) {
- LogConfigurationException ex = reportInvalidLogAdapter(logInterface, logClass);
+ // oops, we need to cast this logClass we have loaded into
+ // a Log object in order to return it. But we won't be
+ // able to. See method reportInvalidLogAdapter for more
+ // information.
+ LogConfigurationException ex =
+ reportInvalidLogAdapter(logInterface, logClass);
throw ex;
}
} catch (Throwable t) {
+ logDiagnostic(
+ "An unexpected problem occurred while loading the"
+ + " log adapter class: " + t.getMessage());
throw new LogConfigurationException(t);
}
@@ -411,9 +528,23 @@
/**
* Report a problem loading the log adapter, then <i>always</i> throw
* a LogConfigurationException.
- *
- * @param logInterface
- * @param logClass
+ * <p>
+ * There are two possible reasons why we successfully loaded the
+ * specified log adapter class then failed to cast it to a Log object:
+ * <ol>
+ * <li>the specific class just doesn't implement the Log interface
+ * (user screwed up), or
+ * <li> the specified class has bound to a Log class loaded by some other
+ * classloader; Log@classloaderX cannot be cast to Log@classloaderY.
+ * </ol>
+ * <p>
+ * Here we try to figure out which case has occurred so we can give the
+ * user some reasonable feedback.
+ *
+ * @param logInterface is the class that this LogFactoryImpl class needs
+ * to return the adapter as.
+ * @param logClass is the adapter class we successfully loaded (but which
+ * could not be cast to type logInterface).
*/
private LogConfigurationException reportInvalidLogAdapter(
Class logInterface, Class logClass) {
@@ -421,6 +552,21 @@
Class interfaces[] = logClass.getInterfaces();
for (int i = 0; i < interfaces.length; i++) {
if (LOG_INTERFACE.equals(interfaces[i].getName())) {
+
+ if (isDiagnosticsEnabled()) {
+ ClassLoader logInterfaceClassLoader = getClassLoader(logInterface);
+ ClassLoader logAdapterClassLoader = getClassLoader(logClass);
+ Class logAdapterInterface = interfaces[i];
+ ClassLoader logAdapterInterfaceClassLoader = getClassLoader(logAdapterInterface);
+ logDiagnostic(
+ "Class " + logClassName + " was found in classloader "
+ + objectId(logAdapterClassLoader)
+ + " but it implements the Log interface as loaded"
+ + " from classloader " + objectId(logAdapterInterfaceClassLoader)
+ + " not the one loaded by this class's classloader "
+ + objectId(logInterfaceClassLoader));
+ }
+
throw new LogConfigurationException
("Invalid class loader hierarchy. " +
"You have more than one version of '" +
@@ -433,16 +579,7 @@
("Class " + logClassName + " does not implement '" +
LOG_INTERFACE + "'.");
}
-
- /**
- * Gets the context classloader.
- * This method is a workaround for a java 1.2 compiler bug.
- */
- protected static ClassLoader getContextClassLoader() throws LogConfigurationException {
- return LogFactory.getContextClassLoader();
- }
-
-
+
/**
* MUST KEEP THIS METHOD PRIVATE.
*
@@ -453,7 +590,10 @@
* </p>
*
* Load a class, try first the thread class loader, and
- * if it fails use the loader that loaded this class.
+ * if it fails use the loader that loaded this class. Actually, as
+ * the thread (context) classloader should always be the same as or a
+ * child of the classloader that loaded this class, the fallback should
+ * never be used.
*/
private static Class loadClass( final String name )
throws ClassNotFoundException
@@ -489,12 +629,17 @@
*/
protected boolean isJdk13LumberjackAvailable() {
+ // note: the algorithm here is different from isLog4JAvailable.
+ // I think isLog4JAvailable is correct....see bugzilla#31597
+ logDiagnostic("Checking for Jdk13Lumberjack.");
try {
loadClass("java.util.logging.Logger");
loadClass("org.apache.commons.logging.impl.Jdk13LumberjackLogger");
- return (true);
+ logDiagnostic("Found Jdk13Lumberjack.");
+ return true;
} catch (Throwable t) {
- return (false);
+ logDiagnostic("Did not find Jdk13Lumberjack.");
+ return false;
}
}
@@ -508,6 +653,9 @@
*/
protected boolean isJdk14Available() {
+ // note: the algorithm here is different from isLog4JAvailable.
+ // I think isLog4JAvailable is correct....
+ logDiagnostic("Checking for Jdk14.");
try {
loadClass("java.util.logging.Logger");
loadClass("org.apache.commons.logging.impl.Jdk14Logger");
@@ -515,9 +663,11 @@
if (throwable.getDeclaredMethod("getStackTrace", null) == null) {
return (false);
}
- return (true);
+ logDiagnostic("Found Jdk14.");
+ return true;
} catch (Throwable t) {
- return (false);
+ logDiagnostic("Did not find Jdk14.");
+ return false;
}
}
@@ -527,12 +677,16 @@
*/
protected boolean isLog4JAvailable() {
+ logDiagnostic("Checking for Log4J");
try {
- loadClass("org.apache.commons.logging.impl.Log4JLogger").getClassLoader()
- .loadClass("org.apache.log4j.Logger" );
- return (true);
+ Class adapterClass = loadClass("org.apache.commons.logging.impl.Log4JLogger");
+ ClassLoader cl = getClassLoader(adapterClass);
+ Class loggerClass = cl.loadClass("org.apache.log4j.Logger" );
+ logDiagnostic("Found Log4J");
+ return true;
} catch (Throwable t) {
- return (false);
+ logDiagnostic("Did not find Log4J");
+ return false;
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: svn commit: r171301 -
/jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Sun, 2005-05-22 at 10:43 +0000, skitching@apache.org wrote:
> Author: skitching
<snip>
> @@ -368,24 +461,48 @@
> String logClassName = getLogClassName();
>
> // Attempt to load the Log implementation class
> + //
> + // Question: why is the loginterface being loaded dynamically?
> + // Isn't the code below exactly the same as this?
> + // Class logInterface = Log.class;
> +
i suspect that the reason it was coded this way is there are some
security gymnastics performed and it's the LogFactory (rather than
LogFactoryImpl) class classloader that is used to load the class.
i'm not convinced that this actually serves any useful purpose: in the
end the instance created will have to be cast to Log as referenced by
LogFactoryImpl.
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [logging] detecting logging libs
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Mon, 2005-05-23 at 11:24 +1200, Simon Kitching wrote:
> On Sun, 2005-05-22 at 21:01 +0100, robert burrell donkin wrote:
> > On Sun, 2005-05-22 at 10:43 +0000, skitching@apache.org wrote:
> >
> > <snip>
> >
> > > protected boolean isJdk13LumberjackAvailable() {
> > >
> > > + // note: the algorithm here is different from isLog4JAvailable.
> > > + // I think isLog4JAvailable is correct....see bugzilla#31597
> >
> > +1
> >
> > could do with going through all the isAvailable's and checking whether
> > their algorithms are correct. however, i suspect that brian's approach
> > will be needed to deal correctly with some circumstances. if no one
> > feels like volunteering should probably record this in bugzilla so we
> > don't lose track...
>
> If by "brian's approach" you mean creating an instance of the logger
> class in order to test whether that logging lib is *really* available, I
> agree.
yes
(hopefully brian will jump in here and correct any misunderstandings)
> Not only is it more reliable, but it's a cleaner solution; currently the
> LogFactoryImpl class is making *assumptions* about what classes the
> various logging adapters depend on. That information should be only in
> the logging adapter class.
+1
in addition, the specification allows variation as to the timing of
error reporting. i believe that creating an instances would give more
consistency across JVMs.
> The only problem with creating an instance of the logger is that we
> would have to pass a category string to the logger constructor, and
> therefore must build an assumption into LogFactoryImpl about what
> category names are valid for the underlying logger. Can we assume that
> an empty string is a valid category for all logger libraries? Can we
> assume that "apache" or "org.apache.commons.logging" are valid category
> strings? Perhaps some loggers only accept valid URLs as
> categories...yes, I'm playing devil's advocate a bit here. I guess we
> could always say that the writer of the logging adapter is required to
> return a valid logger instance for category "", even if that is not
> normally a category that is valid to the underlying library.
IIRC brian's patch refactored the code so that the test also constructed
the correct logger instance (or something like that). if you don't beat
me to it, i'll commit the patch onto one of the branches so that
everyone can easily take a look at the approach.
(again hopefully brian will jump in here if i've made any mistakes)
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
[logging] detecting logging libs
Posted by Simon Kitching <sk...@apache.org>.
On Sun, 2005-05-22 at 21:01 +0100, robert burrell donkin wrote:
> On Sun, 2005-05-22 at 10:43 +0000, skitching@apache.org wrote:
>
> <snip>
>
> > protected boolean isJdk13LumberjackAvailable() {
> >
> > + // note: the algorithm here is different from isLog4JAvailable.
> > + // I think isLog4JAvailable is correct....see bugzilla#31597
>
> +1
>
> could do with going through all the isAvailable's and checking whether
> their algorithms are correct. however, i suspect that brian's approach
> will be needed to deal correctly with some circumstances. if no one
> feels like volunteering should probably record this in bugzilla so we
> don't lose track...
If by "brian's approach" you mean creating an instance of the logger
class in order to test whether that logging lib is *really* available, I
agree.
Not only is it more reliable, but it's a cleaner solution; currently the
LogFactoryImpl class is making *assumptions* about what classes the
various logging adapters depend on. That information should be only in
the logging adapter class.
The only problem with creating an instance of the logger is that we
would have to pass a category string to the logger constructor, and
therefore must build an assumption into LogFactoryImpl about what
category names are valid for the underlying logger. Can we assume that
an empty string is a valid category for all logger libraries? Can we
assume that "apache" or "org.apache.commons.logging" are valid category
strings? Perhaps some loggers only accept valid URLs as
categories...yes, I'm playing devil's advocate a bit here. I guess we
could always say that the writer of the logging adapter is required to
return a valid logger instance for category "", even if that is not
normally a category that is valid to the underlying library.
Regards,
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: svn commit: r171301 -
/jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Sun, 2005-05-22 at 10:43 +0000, skitching@apache.org wrote:
<snip>
> protected boolean isJdk13LumberjackAvailable() {
>
> + // note: the algorithm here is different from isLog4JAvailable.
> + // I think isLog4JAvailable is correct....see bugzilla#31597
+1
could do with going through all the isAvailable's and checking whether
their algorithms are correct. however, i suspect that brian's approach
will be needed to deal correctly with some circumstances. if no one
feels like volunteering should probably record this in bugzilla so we
don't lose track...
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org