mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maarten Bosteels" <mbosteels....@gmail.com>
Subject Re: result poll logging frameworks
Date Mon, 15 Oct 2007 10:38:57 GMT
Hi Trustin,

On 10/15/07, Trustin Lee <trustin@gmail.com> wrote:
> Hi Maarten,
>
> On 10/10/07, Maarten Bosteels <mbosteels.dns@gmail.com> wrote:
> > > > Trustin, maybe we should reopen the discussion about removing IoSessionLogger
> > > > in favour of MdcInjectionFilter :-)
> > >
> > > Well, there's at least one person who's using java.util.logging in the
> > > mailing list, and there should be more people who's not in here.  If
> > > we are going to drop good support for JUL, what is the use of SLF4J?
> >
> > Well, SLF4J gives people choice and it still would, people could still choose
> > any of the supported logging systems, BUT when the logging framework
> > of their choice doesn't support MDC, they wouldn't see the
> > remoteAddress in their logs.
> > But ok, I can understand that you find that unacceptable.
> >
> > On the other hand, I would prefer if logging events generated by
> > ProtocolCodecFilter
> > would have org.apache.mina.filter.codec.ProtocolCodecFilter as their logger.
> > I know this is not required but it is a very widely used convention.
>
> Very true.  We need to provide an easy easy way to customize the behavior.
>
> > I thought of another solution:
> >
> > * the MdcInjectionFilter stores the remoteAddress in a ThreadLocal
> > * we create a decorator for org.slf4j.Logger that retrieves the
> > remoteAddress from the ThreadLocal and prepends it to the log-message
> > (optionally of course, only needed when the logging framework doesn't
> > support MDC)
> > * all mina-classes use this decorator instead of IoSessionLogger
> >
> > I haven't tried this idea, but I think it would work ok.
> > Normally, I am wary of using ThreadLocal's but for logging I consider
> > it acceptable.
>
> Wouldn't ThreadLocal cause some kind of bottleneck in a high performance server?

I don't think so, but a benchmark with/without mdcInjectionFilter
could be interesting of course.

Since jdk 1.3 there is no synchronization needed for ThreadLocal,
see this article by Brian Goetz:

http://www.ibm.com/developerworks/java/library/j-threads3.html

see also:
http://www.ibm.com/developerworks/forums/dw_thread.jsp?message=13929885&cat=10&thread=155193&treeDisplayType=threadmode1&forum=181#13929885


> I'm in favor of doing the following:
>
> * Make IoSessionLoggger implement SLF4J Logger
> * Add IoSessionLogger.enableDefaultPrefix (true by default) static property
> * A user will create a logger by calling 'new
> IoSessionLogger(org.slf4j.Logger)'.
> * IoSessionLogger forwards all calls to the underlying SLF4J logger if
> enableDefaultPrefix is false.  Otherwise, IoSessionLogger instance
> prefixes the default prefix for all messages.
>
> WDYT?  By doing this, we can incorporate all the suggested changes we discussed:

Looks good !
I assume you mean that IoSessionLogger would *always* delegate to the
underlying SLF4J logger, but only prepend the prefix when
enableDefaultPrefix is true ?


>
> * Users can display meaningful information without using MdcInjectionFilter.
> * Advanced users can disable the default prefix very easily and use
> MdcInjectionFilter.
> * IoSessionLogger will implement SLF4J Logger, so it's much more flexible.
>
> > In fact, I think this functionality could be added to SLF4J to make
> > all SLF4J implementations MDC-capable. I'll ask Ceki what he thinks
> > about it.
>
> Exactly.  I saw your message in the SLF4J mailing list, though I felt
> like Ceki is not that interested in your suggestion.   :(

Well, that wasn't my impression. I was happy that he responded so quickly and
was willing to add MDC support to SLF4J.
I had the impression he liked the idea. We'll see when I provide the patches :-)

Maarten

>
> Cheers,
> Trustin
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
> --
> PGP Key ID: 0x0255ECA6
>

Mime
View raw message