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