Thanks for getting back to me, Yonik
On 10/15/13 4:11 PM, Yonik Seeley wrote:
On Tue, Oct 15, 2013 at 3:46 AM, Per Steffensen <[email protected]> wrote:
Is it deliberate that SolrException.log is not used everywhere in the code
where we log exceptions (Throwables)? Why? Or is it just by accident?
One problem with SolrException.log is that the logger uses that as the
class and method name (assuming those are logged).
Well, that can be dealt with. Instead of
----- old code - start ------
public void log(Logger log) { log(log,this); }
public static void log(Logger log, Throwable e) {
String stackTrace = toStr(e);
String ignore = doIgnore(e, stackTrace);
if (ignore != null) {
log.info(ignore);
return;
}
log.error(stackTrace);
}
public static void log(Logger log, String msg, Throwable e) {
String stackTrace = msg + ':' + toStr(e);
String ignore = doIgnore(e, stackTrace);
if (ignore != null) {
log.info(ignore);
return;
}
log.error(stackTrace);
}
public static void log(Logger log, String msg) {
String stackTrace = msg;
String ignore = doIgnore(null, stackTrace);
if (ignore != null) {
log.info(ignore);
return;
}
log.error(stackTrace);
}
----- old code - end ------
just do something a-la
----- new code - start ------
public void log(Logger log) { log(log,this); }
public static void log(Logger log, Throwable e) {
String stackTrace = toStr(e);
String ignore = doIgnore(e, stackTrace);
if (ignore != null) {
log(log, LocationAwareLogger.INFO_INT, ignore);
return;
}
log(log, LocationAwareLogger.ERROR_INT, stackTrace);
}
public static void log(Logger log, String msg, Throwable e) {
String stackTrace = msg + ':' + toStr(e);
String ignore = doIgnore(e, stackTrace);
if (ignore != null) {
log(log, LocationAwareLogger.INFO_INT, ignore);
return;
}
log(log, LocationAwareLogger.ERROR_INT, stackTrace);
}
public static void log(Logger log, String msg) {
String stackTrace = msg;
String ignore = doIgnore(null, stackTrace);
if (ignore != null) {
log(log, LocationAwareLogger.INFO_INT, ignore);
return;
}
log(log, LocationAwareLogger.ERROR_INT, stackTrace);
}
private static void log(Logger log, int level, String msg) {
if (log instanceof LocationAwareLogger) {
((LocationAwareLogger)log).log(null,
SolrException.class.getCanonicalName(), level, msg, null, null);
} else {
switch (level) {
case LocationAwareLogger.DEBUG_INT:
log.debug(msg);
break;
case LocationAwareLogger.ERROR_INT:
log.error(msg);
break;
case LocationAwareLogger.INFO_INT:
log.info(msg);
break;
case LocationAwareLogger.TRACE_INT:
log.trace(msg);
break;
case LocationAwareLogger.WARN_INT:
log.warn(msg);
break;
}
}
}
----- new code - end ------
That will do the trick, as long as we are on log4j (or any other
logging-framework that supports FQCN - and you never want to chose one
that doesnt :-) )
Perhaps we would be better off tackling customizations at the logger level?
Well, I think that the thing about doing customizations in
SolrException.log instead of making our own "Logger" (and use that all
over) is a little strange. But that is where we are at right now
-Yonik
The reason I am interested in making sure all exception logging is going
through a central/common place in the code (that could be
SolrException.log) is that I want to be able to decide on log-level
based on the type of exception - and SolrException.log seems to be the
current place for that. As you might know, in our implementation of
optimistic-locking/version-control, we throw actual exceptions
VersionConflict, DocumentAlreadyExists and DocumentDoesNotExist
(sub-sub-classes of SolrException) - exceptions that through our
implementation of transporting exception to client-side allows us to
just do the following on client-side
try {
cloudSolrServer.add(...);
} catch (VersionConflict e) }
...deal with version-conflict...
} catch (DocumentAlreadyExists e) }
...deal with document-already-exists...
} catch (DocumentDoesNotExist e) }
...deal with document-does-not-exist...
}
Well, never mind, but the thing is that VersionConflict,
DocumentAlreadyExists and DocumentDoesNotExist exceptions flow around
the system, and they will get logged here and there. It fills up our
log-files in production, because they are logged on error-level
(mostly), and of course we have error-level enabled in production. We
want those kinds of exceptions logged at debug-level, because they do
actually not indicate that something is wrong in the system, and that
the admin has to do something - they are a natural part of healthy
system. I would categorize them something like ApplicationExceptions in
JEE. Actually we have a super-class ApplicationException (inheriting
from SolrException) that VersionConflict, DocumentAlreadyExists and
DocumentDoesNotExist inherits from.
One way of achieving our goal is to make sure that all exception logging
in the system goes through SolrException.log (or some other central
place) and in there just do
if (AppcationException.class.isAssignableFrom(e.getClass()) {
log(log, LocationAwareLogger.DEBUG_INT, stackTrace);
return;
}
in the top of SolrException.log-methods.
Alternatively we could do it just using filters at log-config level, but
filters are really not good at changing (deciding on) log-level - they
are best at just stopping or letting-through loggings.
Locally I am playing around with making sure all exceptions are logged
through SolrException.log. Thats easy, but requires changes a lot of
places in the code. I have also made changes to forbidden-apis in
solr-projects where I deny usage of any of the org.slf4j.Logger-methods
that take an exception. That will catch anyone breaking the rule about
always doing exception logging through SolrException.log in the future.
It all works, but I am not sure I want to do it this way, if the
community does not want to take this change into Apache code-base. I
will just get a lot more diff in my code vs Apache code (I already have
penty :-) ), and that is just really irritating whenever we want our
Solr version to be upgraded to be based on a new Apache Solr release
(basically we merge changes from Apache into our SVN, and the number of
differences we have compared to Apache code, increase the number of
conflicts etc. I have to solve, when we do this).
Do you think Apache community will be interested in a
always-log-exceptions-through-SolrException.log-solution? Including the
FQCN trick I showed you above?
Regards, Per Steffensen
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
