Hi,
so I did the switch on my local checkout and after
a little fiddling I'm the happy owner of a GeoServer
whose logging levels can be modified on the fly,
and correctly too. Moreover, the code needed to
configure logging has been drastically simplified (it's
maybe 1/10th of what it used to be), since all I need
to do now is to have log4j reload it configuration
(instead of trying to force every java logger to
change its logging level and whatnot).
To make this work smoothly I had to do a couple
of modifications on the log4j log level mapping, turning
it into:
/**
* Returns the Log4J level for the given Java level.
*/
private static org.apache.log4j.Level toLog4JLevel(final Level level) {
final int n = level.intValue();
switch (n / 100) {
default: {
// MAX_VALUE is a special value for Level.OFF. Otherwise and
// if positive, log to fatal since we are greater than SEVERE.
switch (n) {
default: if (n >= 0) return org.apache.log4j.Level.FATAL; // fallthrough otherwise.
case Integer.MAX_VALUE: return org.apache.log4j.Level.OFF;
case Integer.MIN_VALUE: return org.apache.log4j.Level.ALL;
}
}
case 10: return org.apache.log4j.Level.ERROR; // SEVERE
case 9: return org.apache.log4j.Level.WARN; // WARNING
case 8: // INFO
case 7: return org.apache.log4j.Level.INFO; // CONFIG
case 6: // (not allocated)
case 5: return org.apache.log4j.Level.DEBUG; // FINE
case 4: return org.apache.log4j.Level.TRACE; // FINER
case 3: // FINEST
case 2: // (not allocated)
case 1: // (not allocated)
case 0: return org.apache.log4j.Level.ALL; // ALL
}
}
The changes are:
* moved TRACE to finer level. This is because many geotools modules
don't ever go lower than FINER and they log a huge number of
messages at the FINER level (renderer and shapefile renderer do
so, making the FINER level usage almost impractical unless a well
hidden rendering issue is to be spotted)
* used ALL for the lastest level, not OFF. OFF is meant to be
the highest level, the one at no-one should be logging anything.
* the opposite method, toJavaLevel, has been changed accordingly.
What do you think, can I go forward and commit? What about the commons
logger, I think I should make the same change there?
Cheers
Andrea
Andrea Aime a écrit :
* moved TRACE to finer level. This is because many geotools modules
don't ever go lower than FINER and they log a huge number of
messages at the FINER level (renderer and shapefile renderer do
so, making the FINER level usage almost impractical unless a well
hidden rendering issue is to be spotted)
No problem, but I believe that we should also kick down rendering logs from
FINER to FINEST.
* used ALL for the lastest level, not OFF. OFF is meant to be
the highest level, the one at no-one should be logging anything.
Right.
What do you think, can I go forward and commit? What about the commons
logger, I think I should make the same change there?
Sure (and commons-logger should also be updated).
Thanks for the feedback,
Martin
Martin Desruisseaux ha scritto:
What do you think, can I go forward and commit? What about the commons
logger, I think I should make the same change there?
Sure (and commons-logger should also be updated).
Updates landed both on gt2 trunk and 2.4.x
Cheers
Andrea
Andrea Aime a écrit :
Updates landed both on gt2 trunk and 2.4.x
Thanks. I just did some experiment with that. Since we removed LoggingFramework,
can I add "throws ClassNotFoundException" declaration in LoggerFactory
constructor and 'getInstance()' method? Since it is a checked exception, it will
require updates in GeoServer.
Rational: first, the "catch" blocks in GeoTools.init(...) code are ineffective.
The throwable that we may get is NoClassDefFoundError (not to be confused with
ClassNotFoundException), which is an Error, not an Exception, and consequently
is not handled in the "catch" blocks provided in GeoTools.init(...).
Second, we are at risk to get NoClassDefFoundError right at GeoTools class
loading even if GeoTools.init(...) is never invoked. It is JVM-implementation
dependent (depends how deep the ClassLoader checks GeoTools class dependencies).
Fortunatly Java 6 behave well, but we have no garantee that all JVM behave as well.
Actually Java 6 behave so well in deferring dependencies checks that
'setLoggerFactory(Log4JLogger.getInstance())' always succeed, and we get
NoClassDefFoundError only the first time we try to log a message (I just tried
that).
The LoggingFramework stuff was there exactly for handling those issues through
reflections. Since we removed it, I would like to:
- Adds some code in LoggerFactory for checking that the logging framework
is available in the classpath (i.e. reinsert some of LoggingFramework
code into LoggerFactory).
- Throws ClassNotFound checked exception if the above test fails.
Note that it does not completly eliminate the risk of NoClassDefFoundError (only
LoggingFramework completly eliminated it), but it should greatly reduce it.
Martin
Martin Desruisseaux ha scritto:
Andrea Aime a écrit :
Updates landed both on gt2 trunk and 2.4.x
Thanks. I just did some experiment with that. Since we removed LoggingFramework,
can I add "throws ClassNotFoundException" declaration in LoggerFactory
constructor and 'getInstance()' method? Since it is a checked exception, it will
require updates in GeoServer.
Rational: first, the "catch" blocks in GeoTools.init(...) code are ineffective.
The throwable that we may get is NoClassDefFoundError (not to be confused with
ClassNotFoundException), which is an Error, not an Exception, and consequently
is not handled in the "catch" blocks provided in GeoTools.init(...).
Second, we are at risk to get NoClassDefFoundError right at GeoTools class
loading even if GeoTools.init(...) is never invoked. It is JVM-implementation
dependent (depends how deep the ClassLoader checks GeoTools class dependencies).
Fortunatly Java 6 behave well, but we have no garantee that all JVM behave as well.
Actually Java 6 behave so well in deferring dependencies checks that
'setLoggerFactory(Log4JLogger.getInstance())' always succeed, and we get
NoClassDefFoundError only the first time we try to log a message (I just tried
that).
The LoggingFramework stuff was there exactly for handling those issues through
reflections. Since we removed it, I would like to:
- Adds some code in LoggerFactory for checking that the logging framework
is available in the classpath (i.e. reinsert some of LoggingFramework
code into LoggerFactory).
- Throws ClassNotFound checked exception if the above test fails.
Note that it does not completly eliminate the risk of NoClassDefFoundError (only
LoggingFramework completly eliminated it), but it should greatly reduce it.
Sure, sounds fine to me
Cheers
Andrea