tags:

views:

187

answers:

8

I came across one very good library for parsing CUE files. But when I started to read its source code, I realized that it is almost unreadable:

public void setParent(final CueSheet parent) {
    FileData.logger.entering(FileData.class.getCanonicalName(), "setParent(CueSheet)", parent);
    this.parent = parent;
    FileData.logger.exiting(FileData.class.getCanonicalName(), "setParent(CueSheet)");
}

every method has logger.entering() and logger.exiting() messages. Isn't that too much?

There's another java library for parsing audio tags. It also had like 15 log messages for each file it read. It was annoying so I commented out every call to logger. And the library became twice as fast, because they used a lot of string concatenation for log messages.

So the question is: should I really log everything, even if it is not large enterprise application? Because these libraries obviously don't need any logging, except for error messages. And my experience shows that loggers are terrible tool for debugging. Why should I use it?

+1  A: 

Ideally, you use a logger that allows logging levels; log4j has fatal/error/warn/debug/info, for example. That way, if you set the level to "only show errors", you don't lose speed to the software building log messages you didn't need.

That said, it's only too much logging until you wind up needing something that would have been logged. It sounds like most of the logging that's slowing you down should be "trace" level, though; it's showing you what a profiler would have.

Dean J
Ok, for example you have something like this: logger.info("Checking from start:" + newAudioHeader). And you have thousand calls to this line. Even if you disable info messages, the String will be created anyways.
tulskiy
It may be better to manually incorporate logging levels in that case.if (LOG_LEVEL == DEBUG) { FileData.logger.exiting(FileData.class.getCanonicalName(), "setParent(CueSheet)");}
Dean J
Yes, that's what the second library is using sometimes and it sounds reasonable. But in case of entering and exiting messages it would create even more unreadable code :)
tulskiy
A: 

It depends, it this code for an application, or a library? For an application, logger are useful once the code is in production. It should not be used to debug, but to help you replicate a bug. When a user tells you that your application crashed, you always want the maximum logging information.

I agree that it makes the code less readable. It even make the application slower!

It's a total different game for a library. You should have consistent logging with adequate level. The library SHOULD inform the development team when an error occurs.

Thierry-Dimitri Roy
+2  A: 

How to know when is too much logging? When you know that the logged information isn't important in the long term, being for debug actions or bug correction, or the application doesn't deal with too much important information.

Sometimes you need to log almost everything. Is performance or full possibility of analysis the most important part of an application? It really depends.

I've worked in the past with some integration with a lot of different webservices, like 10 in a same app. We logged all xml requests and responses. Is this an overhead? In the long term, I don't think so because we worked with a lot of credit card operations and should have every process made with the server logged. How to know what happened when there was a bug?

You wouldn't believe what I've saw in some of the xml responses. I've even received a xml without closing tags, from a BIG airplane company. Were the "excessive logs" a bad practice? Say that to your clients when you have to prove that the error came from the other vendor.

GmonC
A: 

Logging should provide you with information that a stack trace can't in order to track down a problem. This usually means that the info is some kind of historical trace of what the program did, as opposed to what state it's in at the time of failure.

Too much historical data will be ignored. If you can safely deduce that a method was called without having to actually log its entry and exit, then it's safe to remove those logging entries.

Another bad sign is if your logging files start to use up a huge amounts of disk space. You're not only sacrificing space, you're probably slowing down the app too much.

Loadmaster
A: 

To answer the question, why should I use loggers?

Have you ever encountered a piece of software where the only error indicated presented to the end user is Fatal error occured. Would it not be nice to find out what have caused it?

Logging is a tool that can really help you narrow these kind of problems in the field. Remember, end-user systems don't have nice IDE's to debug and the end-users usually are not knowledgeable enough to run these tools. However end-users, in most cases, are capable of copying log configuration files ( written by us, clever programmers ) into predefined location and fetch log files and email them back to us ( poor soles for having to parse megabytes of log output ) when they encounter problems.

Having said this, logging should be highly configurable and under normal conditions produce minimal output. Also, guards should protect finer level logging from consuming too many resources.

I think in the example that you have provided all logging should have been done on a TRACE level. Also, because nothing bad can really happen between function entry point and exit, it probably make sense to have only one log statement there.

Alexander Pogrebnyak
+1  A: 

Most logging libraries incorporate a means to confirm that logging is enabled before processing an instruction:

For example:

public void foo(ComplicatedObject bar) {
    Logger.getInstance(Foo.class).trace("Entering foo(" + bar + ")");
}

Could be quite costly depending on the efficiency of the bar.toString() method. However, if you instead wrap that in a check for the logging level before doing the string concatenation:

static {
    Logger log = Logger.getInstance(Foo.class);

public void foo(ComplicatedObject bar) {
    if (log.isTraceEnabled()) {
        log.trace("Entering foo(" + bar + ")");
    }
}

Then the string concatenation only occurs if at least one appender for the class is set to Trace. Any complicated log message should do this to avoid unnecessary String creation.

Brian M. Carr
Check the {}-construct in slf4j. It allows you to skip the guarding if-statement
Thorbjørn Ravn Andersen
A: 

Over the years I've swayed backwards and forwards between promoting logging everything at the appropriate levels (trace, info, etc...) and thinking that any is a complete waste of time. In reality it depends on what is going to be useful to track down or required (logging can be a cheap way of maintaining an audit trail).

Personally, I tend to log entry/exit at a component or service level and then log significant points in the processing such as a business logic decision or a call on another service/component. Of course errors are always logged, but once only and at the place they were handled (the stack trace and exception message should have sufficient info to diagnose the problem) and any service/component interface should always handle an errors (even if it is just converting it into another more appropriate to the caller).

The problem with logging stuff on the off chance something goes wrong is that you end up with too much information that it is impossible to identify the issue, especially if it is running under a server as you end up with loads of intertwined log entries. Obviously you can get around that by incorporating a request id in the entry and using some software to filter on that. Of course you also have the case where your application is distributed and/or cluster and you have multiple logs.

Nowadays I would never actually write trace entering/exiting entries code, the code just gets in the way and it is so much easier to use something like aspectj if it is really needed. Using aspectj also would guarantee to be consistent (you can change the log format in one place rather than having to change every operation) and accurate (in case some refactoring adds a new paramater and teh developer forgets to add it to the logging).

One thing I have thought about doing or looking to see if someone already has is a logger that will hold the entries in memory, then if an error is encountered they are written, if the operation succeeds the entries are just discarded. If anyone knows of one (ideally for log4j) please let me know, alternatively I have a few ideas on how to implement this if anyone is interested in doing one.

vickirk
A: 

This level of logging is canonically bad - in fact, I saw code exactly like this in the Daily WTF a few days ago.

But logging is in general a Very Good Thing.

CPerkins