Results 1 to 3 of 3

Thread: zimbra logging is a mess

  1. #1
    jelmd is offline Junior Member
    Join Date
    Mar 2009
    Posts
    8
    Rep Power
    6

    Default zimbra logging is a mess

    Currently evaluating zimbra and also had a look at the source code. Wrt. other more or less big projects it seems pretty modern (beside the refs to old oswego concurrent utils), but logging is a real mess.

    Did somebody already made a concept?

    The main concern is: Neither an end user nor an admin needs stack traces. What the need is a clear indication of the problem and they need to find it fast!
    The only one, which is able to make use of an stack trace is usually a zimbra developer, only.

    Actually I know a lot of admins, which actually hate java because of its stacktraces - one needs to look through tons of stacktraces 'til one perhaps finds the line, which really indicates the problem.

    So IMHO stacktraces should be logged only on debug/trace level - never never on info/warn/error. To make it more transparent what I mean:
    public class Account extends MailTarget {
    private static final Logger log = LoggerFactory.getLogger(Account.class);
    ...
    } catch (ServiceException e) {
    ZimbraLog.account.warn("unable to get domain for account "
    + getName() + ": " + e.getLocalizedMessage());
    log.debug("getAccountStatus", e);
    return accountStatus;
    }

    As you can see, I would also prefer to be able to set debug level wrt. class namespace instead of the artificial "zimbra.*" because I'm usually not interested in all the annoying aka the problem cluttering stuff in other classes, which also log to e.g. zimbra.account.

    Further more I think, a lot of messages are more or less debug messages, but logged at info level, which result in huge log files - without any valueable information -- admins really hate that, too.
    If they want more information, there should be the right knobs, to turn it on (e.g. with logback + JMX/jconsole no problem at all).

    Another anti-pattern I've seen wrt. logging is the catch/log/rethrow one.
    This should be used very very rarely. Rule of thumb: If you catch it and can't recover, i.e. need to re-throw it, don't log it (and especially not with stacktraces). Otherwise the user/admin is overwelmed by the output and has a big problem to find out, what the real problem is / whether he has several different problems (remember, the admin/enduser is usually not the application developer and is much hapier, when it does not need to study the inner guts of the app).

    The last anti-pattern observed (for now ;-)) is the tunneling of log messages through an artificial Logger. This defeats one of the most valueable debugging info features one usually gets: the linenumber and class, where the problem really occured (because the "native" Logger determines the stacktrace wrong - i.e. reports always the method/class/linenumber of the tuneling class). There is no need to do that. If one is lazy to add the same code snippet to the log statement again and again, one should use a "formatter" which returns the message to log instead of routing everything to the method itself. E.g.:

    Instead of:
    xfailed("bla", "fahsel", src, dst, msg, e)
    ...
    private static void xfailed(String op, String why, String destAddr, String rcptAddr, Message msg, Exception e) {
    StringBuffer sb = new StringBuffer(128);
    sb.append(op).append(" not sent (");
    sb.append(why).append(")");
    sb.append(" mid=").append(msg.getId());
    sb.append(" rcpt='").append(rcptAddr).append("'");
    if (destAddr != null) {
    sb.append(" dest='").append(destAddr).append("'");
    }
    ZimbraLog.mailbox.info(sb.toString(), e);
    }

    one should use something like that:
    ZimbraLog.mailbox.warn(failed("bla", "fahsel", src, dst, msg, e));
    log.debug("method_name", e);

    private static String failed(String op, String why, String destAddr,
    String rcptAddr, Message msg, Exception e) {
    if (!ZimbraLog.mailbox.isInfoEnabled()) {
    return "";
    }
    StringBuilder sb = new StringBuilder(128);
    sb.append(op).append(" not sent (");
    sb.append(why).append(")");
    sb.append(" mid=").append(msg.getId());
    sb.append(" rcpt='").append(rcptAddr).append("'");
    if (destAddr != null) {
    sb.append(" dest='").append(destAddr).append("'");
    }
    if (e != null) {
    sb.append(": ").append(e.getLocalizedMessage());
    }
    return sb.toString();
    }

    This much more beneficial for admins as well as developers and makes code even more readable ...

    What do you think?

  2. #2
    uxbod's Avatar
    uxbod is offline Moderator
    Join Date
    Nov 2006
    Location
    UK
    Posts
    8,017
    Rep Power
    24

    Default

    Please feel free to file a RFE as developers will more likely get to see your suggestion

  3. #3
    jelmd is offline Junior Member
    Join Date
    Mar 2009
    Posts
    8
    Rep Power
    6

    Default

    Okidoki - done ;-)

Thread Information

Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Similar Threads

  1. Replies: 8
    Last Post: 01-12-2012, 02:20 AM
  2. Replies: 12
    Last Post: 02-25-2008, 07:28 PM
  3. Replies: 31
    Last Post: 12-15-2007, 09:05 PM
  4. Zimbra shutdowns every n hours.
    By Andrewb in forum Administrators
    Replies: 13
    Last Post: 08-14-2007, 08:55 AM
  5. Fedora Core 3, Clean Install - Not working!
    By pcjackson in forum Installation
    Replies: 17
    Last Post: 03-05-2006, 07:38 PM

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •