On 10/23/07, Trustin Lee <[EMAIL PROTECTED]> wrote: > > On 10/23/07, Maarten Bosteels <[EMAIL PROTECTED]> wrote: > > Hi, > > > > Some comments: > > > > 1) I understand your worries about performance impact but I think we > should > > to a little benchmark before jumping to conclusions. Maybe you already > did > > ? > > Good idea. I didn't do any benchmark yet. But my primary concern is > not performance now. It's impedance mismatch. I feel like we are > working around the problem. We can go as we discussed so far, but > it's also a lot of fun to explore another possibility. Please don't > get me wrong. :) > > > 2) I was planning to create an Enum of all the keys that > MdcInjectionFilter > > supports (remoteAddress, remotePort,...) > > and add a constructor that takes an EnumSet, that way users can minimize > the > > size of the map. > > That would be really nice.
Done, please review. regards Maarten > 3) Currently, when using the MdcInjectionFilter, all logging events > > generated by code down the call-stack (or is it up ?) will also have the > MDC > > set, even when that code is completely unwaware of MINA. That's a big > > advantage IMO. > > And AFAICS this can only be done using a ThreadLocal ? > > Very true. Backward compatibility! Please consider my previous post > as a fundamental question about the current logging frameworks. > > > I think the solution we discussed previously, is reasonable: > > > > 1) IoSessionLogger constructor takes an org.slf4j.Logger and an > IoSession > > 2) use a session attribute " IoSessionLogger.usePrefix" that determines > if a > > prefix should be prepended > > 3) IoSessionLogger forwards all messages to the org.slf4j.Logger and > > optionally prepends a prefix > > > > * people using the MdcInjectionFilter will see the remoteAddress for ALL > log > > messages > > * people not using MdcInjectionFilter will only see the remoteAddress > for > > log messages generated via IoSessionLogger > > Yep. Let me go ahead. > > > On the other hand, when performance impact of MdcInjectionFilter is > really > > small, and when MDC support is added for JUL > > I think we could just drop IoSessionLogger, as you wrote on the slf4j > > mailing list. > > I can't agree more. Unfortunately Ceki is going for vacation, so we > might need to wait for a while. Anyways, we can keep working on it. > :) > > Cheers, > Trustin > -- > what we call human nature is actually human habit > -- > http://gleamynode.net/ > -- > PGP Key ID: 0x0255ECA6 >
