slf4j: add slf4j integration module (#94) #96
Conversation
|
feign-pull-requests #129 SUCCESS |
|
I considered adding support for the SLF4J MDC. My conclusion was that the right way to integrate that would be at a different level than the Logger implementation, and thus should be treated as a separate feature. Ideally, the MDC entries would be set once at the beginning of handling a request/response/retry loop, and cleared at the end (possibly with some additional keys per-response/retry). Otherwise, there's the possibility that some logging entries would have the MDC properly set, and others wouldn't. Good examples of things that might be "missed" are retry messages and IO exceptions. Currently, the only place to integrate at this level would be within SynchronousMethodHandler. To do this properly, I think we'd either need to adopt SLF4J as our only logging library (and likely eliminate our current Logger extension point) or introduce a new extension point that provides the desired hooks. |
|
Good work, and thanks for explaining the rationale on MDC. |
|
Can you rebase this so that I can click merge? :) |
Adds a new "slf4j" module. A few methods in Logger are now protected rather than package protected to allow access by Logger subclasses that aren't inner classes of Logger.
|
@adriancole rebased |
|
feign-pull-requests #131 SUCCESS |
slf4j: add slf4j integration module (#94)
slf4j: add slf4j integration module (#94)
3 participants
Adds a new "slf4j" module. A few methods in Logger are now protected rather
than package protected to allow access by Logger subclasses that aren't
inner classes of Logger.