You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Per Steffensen <st...@designware.dk> on 2013/10/15 09:46:25 UTC
SolrException.log not used all over?
Hi
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?
Regards, Per Steffensen
Re: SolrException.log not used all over?
Posted by Per Steffensen <st...@designware.dk>.
On 10/15/13 7:33 PM, Shawn Heisey wrote:
> On 10/15/2013 8:11 AM, Yonik Seeley wrote:
>> On Tue, Oct 15, 2013 at 3:46 AM, Per Steffensen <st...@designware.dk>
>> 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).
>> Perhaps we would be better off tackling customizations at the logger
>> level?
>
> I saw calls to SolrException#log when I was finding things for
> SOLR-5342. One of the arguments is a logger object. Doesn't that mean
> that the log message will actually be output with the class of that
> logger?
No
>
> From what I can tell, the only thing you get by using this logging
> method is doIgnore. Looking at all the pieces, I don't see what
> doIgnore is good for or what exactly it is accomplishing. Can someone
> with a better understanding tell me what I'm missing?
I do not see much usage of doIgnore either, but in general I see a
benefit in sending all (exception-)logging through a central/common
place in the code, where you can implement general rules for logging,
that it not easy to do in log-configuration. I gave an example earlier -
that, of course, is only to be used in my local version of Solr (at
least until Apache accept my solution to
optimistic-locking/version-control, but that is probably not going to
happen), but the idea about central/common logging-code in useful in
general.
>
> There seems to be a lot of code duplication between the different log
> methods in SolrException, that should be addressed. I could probably
> do that.
That would be nice!
>
> Thanks,
> Shawn
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: SolrException.log not used all over?
Posted by Shawn Heisey <so...@elyograg.org>.
On 10/15/2013 8:11 AM, Yonik Seeley wrote:
> On Tue, Oct 15, 2013 at 3:46 AM, Per Steffensen <st...@designware.dk> 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).
> Perhaps we would be better off tackling customizations at the logger level?
I saw calls to SolrException#log when I was finding things for
SOLR-5342. One of the arguments is a logger object. Doesn't that mean
that the log message will actually be output with the class of that logger?
From what I can tell, the only thing you get by using this logging
method is doIgnore. Looking at all the pieces, I don't see what
doIgnore is good for or what exactly it is accomplishing. Can someone
with a better understanding tell me what I'm missing?
There seems to be a lot of code duplication between the different log
methods in SolrException, that should be addressed. I could probably do
that.
Thanks,
Shawn
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: SolrException.log not used all over?
Posted by Per Steffensen <st...@designware.dk>.
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?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: SolrException.log not used all over?
Posted by Per Steffensen <st...@designware.dk>.
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 <st...@designware.dk> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: SolrException.log not used all over?
Posted by Yonik Seeley <ys...@gmail.com>.
On Tue, Oct 15, 2013 at 3:46 AM, Per Steffensen <st...@designware.dk> 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).
Perhaps we would be better off tackling customizations at the logger level?
-Yonik
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org