Daniel Kulp wrote:
[...] The good thing is that it's very easy to get checkstyle to error out if Logger.getLogger(..) is ever called. It's quite enforceable.

True.

* I
couldn't find a solution that handles j.u.l specific Levels like
Level.CONFIG which do not exist in log4j. Same with user defined
levels.

Well.   That would be a problem if we wrote our own logging API as well.

No, because Level would not appear in the Logger interface as a class. The API would only contain the methods, like info(), warning(), etc.

If we wrote our own logging API, I would want at LEAST as many levels as j.u.l. Making a "lowest common denominator" type API would be a big mistake.


Agreed. For example there should be two levels above warning, ERROR and FATAL.

[...] Out of curiosity: can I ask why any of our code would EVER call "setLevel(...)"?

It shouldn't, for the reasons you mention below.

Shouldn't configuring the log levels and such be part of the logging configuration by the user? I don't think we should pretend to assume what the user wants and override it by calling setLevel(...).

Right, but the user can for example call setLevel() on the log4j logger backend *or* the j.u.l logger frontend while our software is running. Then the adapter needs to map that back and forth, and I think it's hard to implement that in a way that never produces surprising behaviour.

    * The implementation of addHandler() is unclear, probably that
      operation should not be supported. Delegating to log4j means that
      j.u.l Handlers will be ignored. This violates the API contract of
      j.u.l.Logger - logging messages are no longer forwarded to
      registered Handlers instead they are forwarded to log4j.

As above, I'm not sure why any of our code would ever call get/setHandler(..). That seems to again be a configuration thing. In anycase, I THINK we can prevent that with a checkstyle rule as well. (This is a little harder as it's not a static method.)


I'm on the checkstyle dev team, and I implemented large portions of checkstyle. I'm pretty sure that such a rule is impossible to implement, basically because checkstyle cannot determine the type of an expression. Hence, it can't tell whether foo.setHandler() is legal or not, because checkstyle doesn't have type info about foo, it might be something other than a Logger.

But like with setLevel(), it's the user who calls setHandler(), not yoko. From the user's perspective, when yoko documents that it logs using j.u.l why not set a ConsoleHandler and just use log4j for the trickier appenders? Lots of "usage pattern X won't work" in our docs...

[...] The only good news is that I
managed to get some log messages out of log4j.

That's perfect.   That's the news I wanted to hear.

The only good news about Notepad as a Java IDE is that it allows you to type code. :-)

To me, the code of the j.u.l.Logger subclass seems like a fragile hack.
Yes, I could probably bridge to log4j and get some yoko logging output,
but the code will continue to work only if the yoko dev team is
extremely disciplined and it is very clear which parts of j.u.l must
not be used.

Or enforce it with checkstyle

I don't think this will work because of the limitations that checkstyle has.

or accept that if you use some of the "advanced" things, they might not work the same (or at all) with a different backend.

My guess is that 80+% of all developers use log4j, not j.u.l.

I don't think we can tell all those people "Look, your logs contain only nonsense. You just need to accept that".

[...]
I still think that using our own interface would be the best solution.
No external dependencies and maximum flexibility. I could live with
slf4j, but it seems that j.u.l is not a viable option for those of us
who need to integrate with log4j, even when using a LoggerFactory as
suggested above.

Huh? You just stated that the only thing that doesn't seem to work is the Handlers. So how is it it not a viable option?

Because it's a fragile solution that requires a lot of constant attention from yoko developers. Usage restrictions are not enforced by the API. The prototype code I wrote just sucks, it doesn't feel right.

[...]
-1 to defining our own API that is a "lowest common denominator" type API. I STRONGLY don't believe in "lcd" approaches. It's always the worst of everything.

-0 to defining our own API that is NOT a total "lcd". Basically, take the j.u.l.Logger methods (ALL of them except the statics and the Handler/Filter stuff that our code shouldn't call anyway) and put them on an Interface (or abstract class). Maybe even add a couple. Keep using the j.u.l.Level object.

Why do you need Level? Aren't methods like info(), warning(), etc. sufficient?

Could you live with the Logger interface that is attached (including implementations for jul and log4j, plus demo usage code) or is that too lcd? I think it provides all levels for normal usage and supports i18n of logfiles. Currently uses Java5 varargs to keep the API concise, but we could easily change that to achieve 1.4 compatibility.

No need to define that. This is -0.1 just because I think it's a waste of time as just using the Logger API directly works as you proved. Then again, the time we've spent discussing this......

Yeah, this discussion takes way too long - if only you weren't as stubborn as I am ;-)

Cheers,
Lars

package org.apache.yoko.orb.logging;

import java.io.Serializable;
import java.util.Locale;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
 * A yoko Logger implementation that delegates to a <code>java.util.logging.Logger</code>.
 * <p>
 * Limitations:
 * <li>
 * <ul>jul does not support logging both message args and a throwable, you need to chose one.
 * This implementation opts for the throwable and ignores args,
 * except for the error() method which demos a workaround (we do l10n in our own code).</ul>
 * <ul>Standard Levels do not provide a distinction between errors
 * (something bad happend, but the system can continue) and fatal errors
 * (the system needs to be taken down). This implemetation maps both to the jul SEVERE level.</ul>
 * </li>
 * @author lk
 *
 */
public class JulLogger implements org.apache.yoko.orb.logging.Logger {
    
    private final Logger delegate;

    public JulLogger(Logger delegate) {
        this.delegate = delegate;
    }

    public boolean isTraceEnabled() {
        return delegate.isLoggable(Level.FINEST);
    }

    public void trace(String msgKey, Serializable... args) {
        delegate.log(Level.FINEST, msgKey, args);
    }

    public boolean isDebugEnabled() {
        return delegate.isLoggable(Level.FINE);
    }

    public void debug(String msgKey, Serializable... args) {
        delegate.log(Level.FINE, msgKey, args);
    }

    public boolean isInfoEnabled() {
        return delegate.isLoggable(Level.INFO);
    }

    public void info(String msgKey, Serializable... args) {
        delegate.log(Level.INFO, msgKey, args);
    }

    public void info(Throwable throwable, String msgKey, Serializable... args) {
        delegate.log(Level.INFO, msgKey, throwable);
    }

    public boolean isWarningEnabled() {
        return delegate.isLoggable(Level.WARNING);
    }

    public void warning(String msgKey, Serializable... args) {
        delegate.log(Level.INFO, msgKey, args);
    }

    public void warning(Throwable throwable, String msgKey, Serializable... args) {
        delegate.log(Level.WARNING, msgKey, throwable);
    }

    public boolean isErrorEnabled() {
        return delegate.isLoggable(Level.SEVERE);
    }

    public void error(String msgKey, Serializable... args) {
        delegate.log(Level.INFO, msgKey, args);
    }

    public void error(Throwable throwable, String msgKey, Serializable... args) {
        delegate.log(Level.SEVERE, LogMessage.format(Locale.getDefault(), msgKey, args), throwable);
    }

    public boolean isFatalErrorEnabled() {
        return isErrorEnabled();
    }

    public void fatalError(String msgKey, Serializable... args) {
        error(msgKey, args);
    }

    public void fatalError(Throwable throwable, String msgKey, Serializable... args) {
        error(throwable, msgKey, args);
    }

    public org.apache.yoko.orb.logging.Logger getChildLogger(String name) {
        String childLoggerName = delegate.getName() + ',' + name;
        Logger childLogger = Logger.getLogger(childLoggerName);
        return new JulLogger(childLogger);
    }

    

}
package org.apache.yoko.orb.logging;

import java.io.Serializable;
import java.util.Locale;

import org.apache.log4j.Level;
import org.apache.log4j.Logger;

/**
 * A yoko Logger that delegates to a log4 logger.
 * 
 * @author lk
 */
public class Log4jLogger implements org.apache.yoko.orb.logging.Logger {

    private final Logger log4jLogger;
    private final Locale locale;

    /**
     * Creates a new Log4jLogger.
     * 
     * @param log4jLogger the delegate log4 logger
     * @param locale the locale to use for logging
     * @see #format(String, Serializable[])
     */
    public Log4jLogger(Logger log4jLogger, Locale locale) {
        this.log4jLogger = log4jLogger;
        this.locale = locale;
    }
    
    public boolean isTraceEnabled() {
        return log4jLogger.isTraceEnabled();
    }

    public void trace(String msgKey, Serializable... args) {
        log4jLogger.trace(format(msgKey, args));
    }

    public boolean isDebugEnabled() {
        return log4jLogger.isDebugEnabled();
    }

    public void debug(String msgKey, Serializable... args) {
        log4jLogger.debug(format(msgKey, args));
    }

    public boolean isInfoEnabled() {
        return log4jLogger.isInfoEnabled();
    }

    public void info(String msgKey, Serializable... args) {
        log4jLogger.info(format(msgKey, args));
    }

    public void info(Throwable throwable, String msgKey, Serializable... args) {
        log4jLogger.trace(format(msgKey, args), throwable);
    }

    public boolean isWarningEnabled() {
        return log4jLogger.isEnabledFor(Level.WARN);
    }

    public void warning(String msgKey, Serializable... args) {
        log4jLogger.warn(format(msgKey, args));
    }

    public void warning(Throwable throwable, String msgKey, Serializable... args) {
        log4jLogger.warn(format(msgKey, args), throwable);
    }

    public boolean isErrorEnabled() {
        return log4jLogger.isEnabledFor(Level.ERROR);
    }

    public void error(String msgKey, Serializable... args) {
        log4jLogger.error(format(msgKey, args));
    }

    public void error(Throwable throwable, String msgKey, Serializable... args) {
        log4jLogger.error(format(msgKey, args), throwable);
    }

    public boolean isFatalErrorEnabled() {
        return log4jLogger.isEnabledFor(Level.FATAL);
    }

    public void fatalError(String msgKey, Serializable... args) {
        log4jLogger.fatal(format(msgKey, args));
    }

    public void fatalError(Throwable throwable, String msgKey, Serializable... args) {
        log4jLogger.fatal(format(msgKey, args), throwable);
    }

    public org.apache.yoko.orb.logging.Logger getChildLogger(String name) {
        return new Log4jLogger(Logger.getLogger(log4jLogger.getName() + "." + name), locale);
    }

    /**
     * Creates a message from a message key and args.
     * Note that log4j supports logging arbitrary objects, not just Strings.
     * 
     * This means for example that subclasses can defer l10n, create a transport Object for the arguments
     * and create a log4 Layout that knows about such objects and perform l10n there. The effect is
     * that you can generate multiple logfiles, each using it's won language (e.g. local for local operators,
     * english for remote support). 
     *        
     * @param msgKey
     * @param args
     * @return
     */
    protected Object format(String msgKey, Serializable... args) {
        return LogMessage.format(locale, msgKey, args);
    }
}
package org.apache.yoko.orb.logging;

import java.io.Serializable;

/**
 * Logger interface for Yoko.
 * 
 * Modeled after Avalon's logger interface, but with a trace level and some Java5 enhancements. 
 *
 * @author lkuehne
 */
public interface Logger {

    boolean isTraceEnabled();
    void trace(String msgKey, Serializable... args);
    
    boolean isDebugEnabled();
    void debug(String msgKey, Serializable... args);

    boolean isInfoEnabled();
    void info(String msgKey, Serializable... args);
    void info(Throwable throwable, String msgKey, Serializable... args);

    boolean isWarningEnabled();
    void warning(String msgKey, Serializable... args);
    void warning(Throwable throwable, String msgKey, Serializable... args);
    
    boolean isErrorEnabled();
    void error(String msgKey, Serializable... args);
    void error(Throwable throwable, String msgKey, Serializable... args);
    
    boolean isFatalErrorEnabled();
    void fatalError(String msgKey, Serializable... args);
    void fatalError(Throwable throwable, String msgKey, Serializable... args);
    
    Logger getChildLogger(String name);
}
package org.apache.yoko.orb.logging;

import java.io.IOException;
import java.io.Serializable;
import java.text.MessageFormat;
import java.util.HashSet;
import java.util.Locale;
import java.util.ResourceBundle;
import java.util.Set;
import java.util.logging.Logger;

import org.apache.log4j.ConsoleAppender;
import org.apache.log4j.PatternLayout;

/**
 * Provides constants for resource keys (and currently also a demo main method).
 * 
 * @author lkuehne
 *
 */
public final class LogMessage {
    
    /**
     * This is a utility class, no instances should be created.
     */
    private LogMessage() {
    }

    private static final Set<String> KEYS = new HashSet<String>(); 
    
    public static final String BUNDLE_NAME = "org.apache.yoko.orb.logging.messages";
    
    public static final String MSG_SAMPLE = key("sample"); // a simple message
    public static final String MSG_NO_REPLY = key("noreply"); // a complex message with i18n placeholders and typically with an associated throwable

    private static String key(String base) {
        final String key = "yoko." + base;
        KEYS.add(key);
        return key;
    }
    
    
    public static String format(Locale locale, String msgkey, Serializable... args) {
        
        final String pattern;
        if (KEYS.contains(msgkey)) {
            final ResourceBundle bundle = ResourceBundle.getBundle(BUNDLE_NAME, locale);
            pattern = bundle.getString(msgkey);
        }
        else {
            pattern = msgkey;
        }
        final MessageFormat msgformat = new MessageFormat(pattern, locale);
        return msgformat.format(args);
    }
    
    /**
     * Demo how to use the logger/message stuff.
     */
    public static void main(String[] args) {

        final IOException sampleException = new IOException("No route to host");

        // java.util.logging
        JulLogger logger = new JulLogger(Logger.getLogger("org.apache.yoko.orb", BUNDLE_NAME));
        logger.info(MSG_SAMPLE);
        logger.debug("no need to go through i18n every time"); // doesn' work because debug() doesnt have the same workaround as error()
        logger.error(sampleException, MSG_NO_REPLY, "serverhost.example.com", 500);
        
        // log4j example
        final ConsoleAppender consoleAppender = new ConsoleAppender(new PatternLayout(PatternLayout.TTCC_CONVERSION_PATTERN));
        consoleAppender.setName("console");
        org.apache.log4j.Logger.getRootLogger().addAppender(consoleAppender);
        // log4j typical usage (same as above, surprise, surprise)
        Log4jLogger log4jLogger = new Log4jLogger(org.apache.log4j.Logger.getLogger("org.apache.yoko.orb"), Locale.US);
        log4jLogger.info(MSG_SAMPLE);
        log4jLogger.debug("no need to go through i18n every time");
        log4jLogger.error(sampleException, MSG_NO_REPLY, "serverhost.example.com", 500);
    }
}
yoko.sample=Beispielmeldung
yoko.noreply=Keine Rückmeldung von {0} nach {1} Millisekunden
yoko.sample=Sample Message
yoko.noreply=No reply from {0} after {1} milliseconds

Reply via email to