[logback-dev] Layout setter/getter methods in AppenderBase

[Maarten Bosteels]

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

On Tue, Feb 10, 2009 at 1:01 AM, Joern Huxhorn <jhuxhorn at googlemail.com>wrote:

>
> On 09.02.2009, at 23:15, Ceki Gulcu wrote:
>
>
>> Hello Joern,
>>
>> At an earlier time, certain classes expected the Appender interface to
>> have a setter/getter for the layout property. AbstractLayoutAction is one
>> such class. However, it is no longer used. (I just removed it.)
>>
>>
> Oh, I see, "for historic reasons" :)
>
>  Many logback appenders do not require a layout and can have their layout
>> property set to null. Having a layout property in Appender and AppenderBase
>> is just noise for such appenders but otherwise not harmful. They have a
>> property, i.e. layout, which they don't use...
>>
>
> Well, yes, you are right. It's not downright harmful... but it is somewhat
> confusing for users (not developers) of the appenders and makes them wonder
> what the layout is about. People actually asked me about that...
>
>
>> If the Appender interface did not contain the layout property, we would
>> probably  have two distinct appender base implementations, AppenderBase and
>> AppenderBaseWithLayout (possibly with an additional interface such as
>> AppenderWithLayout). Come to think of it, we would also need to handle the
>> UnsyncronizedAppenderBase branch of the class hierarchy.  Adding
>> UnsyncronizedAppenderBaseWithLayout would be too unwieldy. :-)
>>
>>
> I don't think that this would be necessary because it would be enough if
> the mentioned classes would simply implement Appender (without the layout
> methods) and appenders requiring the layout would additionally implement the
> extended interface, in just the way that is necessary for their desired
> behavior (e.g. even synchronized/locked if required, which isn't the case
> right now).
>
> This is more or less an aesthetic reasoning and I wouldn't consider it very
> important.
>
> Concerning your original question: the new implementation is definitely
> better than before!
>
>  For developers coming from log4j, AppenderBase having the same
>> implementation for setLayout and getLayout as log4j's AppenderSkeleton class
>> would help them migrate to logback with a little bit more ease, at least
>> with less surprise.
>>
>> I wonder what I was thinking when I implemented layout setter and getters
>> in LayoutBase as nop.
>>
>
> I know this feeling all too well...
> But this is actually a good thing because it's a sign that we keep
> developing our skills ;)
>
> Joern.
>
>
>
>>
>>
>> Joern Huxhorn wrote:
>>
>>> Hi Ceki,
>>> the only thing that I don't understand is why Appender requires a layout
>>> at all.
>>> It would by cleaner if there was a sub-interface, e.g.
>>> LayoutAwareAooender (just a spontaneous suggestion), that extended Appender
>>> and would add said methods.
>>> Some appenders, like SocketAppender or some fictitious appender that
>>> would simply serialize the events to a file (I'm planning to implement such
>>> an appender, btw), just don't need a layout at all.
>>> AppenderBase would then just implement the basic Appender interface,
>>> leaving the layout implementation to appenders that would really require it.
>>> This topic is covered by both http://jira.qos.ch/browse/LBCORE-1 and
>>> http://jira.qos.ch/browse/LBCORE-56
>>> Regards,
>>> Joern.
>>>
>>
>>
>>
>> --
>> Ceki Gülcü
>> Logback: The reliable, generic, fast and flexible logging framework for
>> Java.
>> http://logback.qos.ch
>> _______________________________________________
>> logback-dev mailing list
>> logback-dev at qos.ch
>> http://qos.ch/mailman/listinfo/logback-dev
>>
>
> _______________________________________________
> logback-dev mailing list
> logback-dev at qos.ch
> http://qos.ch/mailman/listinfo/logback-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://qos.ch/pipermail/logback-dev/attachments/20090213/1adc310a/attachment.htm>