On Fri, Feb 13, 2009 at 3:25 PM, Ceki Gulcu <ceki@qos.ch> wrote:
Hi Maarteen,
I really like the idea of pluggable encoders.
Great.
Putting UnsyncronizedAppenderBase aside for a second, I could imagine the following class hierarchy:
interface LayoutAware extends Appender; interface EncoderAware extends Appender;
I don't understand. In my opinion LayoutAware and EncoderAware should be standalone interface with just a getter and a setter, and not extend Appender. All appenders that need a layout should implement LayoutAware. They can implement the interface themselves or - when its practical - inherit from LayoutAwareAppenderBase Why would you tie LayoutAware to the Appender interface. Imagine this Encoder implementation: it implements layoutAware but it's not an Appender public class LayoutBasedEncoder implements Encoder, LayoutAware { private Layout layout; public Layout getLayout() {...} public void setLayout(...) {...} public ByteBuffer encode(LoggingEvent event) { String str = layout.format(event); byte[] bytes = getUtf8Bytes(str); ByteBuffer buffer = ByteBuffer.allocate(bytes.length + 4); buffer.putInt(bytes.length); buffer.put(bytes); return buffer; } } PS: Not yet sure whether the encode method should return a byte[] or a java.nio.ByteBuffer, I think the latter is easier to use. regards, Maarten
abstract class AppenderBase implements Appender; abstract class LayoutAwareAppenderBase ext. AppenderBase implements LayoutAware; abstract class EncodrAwareAppenderBase ext. AppenderBase implements LayoutAware;
class WriterAppender extends LayoutAwareAppenderBase; class FileAppender extends WriterAppender; ... class SocketAppender extends EncoderAwareAppenderBase;
Appenders such as DBAppender and SMTPAppender, where LoggingEvent to byte[] encoding nor layouts make sense, could extend AppenderBase directly.
We should pursue this....
Maarten Bosteels wrote:
Hello,
I agree with Joern, it would be cleaner to have a LayoutAware interface, and only appenders that use a Layout should implement it. The way it is now, people can set a layout on the SocketAppender, they don't get an exception, but the layout would never be used.
I can understand the "historical" reasons, but IMO things like this can be changed as long as logback doesn't reach 1.0Maarten
Related idea/proposal: an Encoder interface similar to Layout but returning a byte array instead of a String:
public interface Encoder { byte[] encode(LoggingEvent event) }
I recently worked on an AsyncSocketAppender (extending UnsyncronizedAppenderBase) and with this interface the wire-format would be pluggable.
Some wire-formats I am thinking about: Apache Thrift, Google protobuf and of course Java serialization.
I still have to implement these encoders and compare their perfomance. I will let you know when I get there.
It would be really cool if we could define a wire-format based on Protobuf and/or Thrift that could also be used for encoding log4j events. But I guess it would be better to do this in a separate project ...
regards, Maarten
-- Ceki Gülcü Logback: The reliable, generic, fast and flexible logging framework for Java. http://logback.qos.ch _______________________________________________ logback-dev mailing list logback-dev@qos.ch http://qos.ch/mailman/listinfo/logback-dev