Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [smila-dev] code conventions : logging

thanks for the feedback!

 

regarding:

> top 2:  0

> don’t think that we really need a strict rule here how to format the logging texts....

 

that was meant to be a recommendation, not  strict rule. but I admit, I should have written it softer then.

 

 

Thomas Menzel @ brox IT-Solutions GmbH

 

From: smila-dev-bounces@xxxxxxxxxxx [mailto:smila-dev-bounces@xxxxxxxxxxx] On Behalf Of Andreas.Weber@xxxxxxxxxxxxx
Sent: Dienstag, 8. März 2011 17:42
To: smila-dev@xxxxxxxxxxx
Subject: Re: [smila-dev] code conventions : logging

 

Hi,

 

top 1:  +1

BTW, one could also ask why to check for “if (_log.isErrorEnabled())“  in case of errors.

Is there really a szenario where I don’t want to log errors (or even warnings)?

For trace/debug/info I would still leave a general recommendation for “if (log.is...Enabled)” in the guide.

 

top 2:  0

don’t think that we really need a strict rule here how to format the logging texts....

 

top 3:  +1

one should always log the whole exception (stacktrace) in case of error

 

Cheers,

Andreas

 

Von: smila-dev-bounces@xxxxxxxxxxx [mailto:smila-dev-bounces@xxxxxxxxxxx] Im Auftrag von Thomas Menzel
Gesendet: Dienstag, 8. März 2011 12:24
An: Smila project developer mailing list
Betreff: [smila-dev] code conventions : logging

 

hi,

 

since we discussed some checkstyle issues Im propsing the following for logging so u have smth. to vote on and leave comments:

 

 

top 1 : check log level

 

the guide states:

  • always check log level before logging, e.g.

if (_log.isErrorEnabled()) {

    _log.error("Your error message", e);

}

 

the problems I have with this is, that it hinders readability/bloats the code and changing the log level means having to change it 2x.

 

I would want to relax this to:

"check log level before logging when computing the log message is potentially costly and may happen a lot in a short period of time under normal circumstances."

 

in the exact case given by the guide, there is no extra computational cost (apart from executing the call to the log method and putting the args on the stack) so I wouldn’t do it here at all. even for calls like this:

 

_log.info("do smth. for with id: " + id );

 

I wouldn’t check it here either as the computational cost is low.

 

all in all it’s a judgment call of the dev. for the exact situation. when in doubt however, check it.

 

 

top 2 format of log statements

 

when logging, build the log message such that dynamic values are append at the end of a prefixed string and don’t put quotes, braces or such around it.

 

rationale:

- a constant log message start makes it much easier to find it in the log and also in bug reposts etc.

- there is hardly ever a reason to quote a value when the preceding log message is well formulated and introduces the value with a colon. the braces often just make it hard to read and copy/paste from the log

 

recommended:               _log.info("do smth. for with id: " + id );

discouraged:                    _log.info("do smth. for with id [" + id + " ]" );

 

 

log messages containing a lot of dynamic parameters are better readable in code when String.format is used but it is relatively slow compared to just + concatenation . (see http://jevopisdeveloperblog.blogspot.com/2009/09/stringformat-vs-messageformatformat-vs.html).

 

this however is usually only needed in the context of logging errors (see below) and hence, then it is OK to take the time to produce a helpful log message, assuming that the exception occurs seldom.

 

 

top 3 logging errors

 

if u don’t want to prefix the error in the log with an own message use:  _log.error("", e);

log.error(e) will just write the e.getMessage() which is usually not enough to help u along finding the problem and it can be easily overlooked the log.

 

if u really want to log just the message, then comment this unusual case in the code or write _log.error(e.getMessage() ); to set it apart. in most of these cases however, these cases don’t indicate an error to be logged but a warning or info, or even smth. less.

 

it is also good to put in as much info as possible in the log message of an error if it is not contained in the error message itself, but avoid duplication.

for instance: logging a FileNotFoundException is nice put the vanilla message and stacktrace doesn’t tell u which file was a miss, so that should best go into the message of the exception itself or at least the log msg of the error along with all other relevant key values, that u can get hold of that helps u qualify the source and cause of an error just from the log without having to debug the code.

sometimes this even means to put in extra  code into the catch clause to get that info. if so. do it.

 

 

Thomas Menzel @ brox IT-Solutions GmbH

 

Taglocity Tags: smila, devenv


Back to the top