mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Trustin Lee" <trus...@gmail.com>
Subject Re: result poll logging frameworks
Date Mon, 15 Oct 2007 10:42:40 GMT
On 10/15/07, Maarten Bosteels <mbosteels.dns@gmail.com> wrote:
> 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

Uh, i didn't know that.  Thanks for the education! :D

> > 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 ?

Yeah exactly.

> > * 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 :-)

Yeah he liked the idea but he didn't look like he want to implement it
by himself. :)

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

Mime
View raw message