feign
feign copied to clipboard
Use SLF4J for library logging.
Logging in Feign today is managed entirely behind our Logger
abstraction. This wrapper is a simplistic form of logging facade similar to Commons Logging and SLF4J.
We should consider adopting a logging facade and replace the Logger
with a more distinct LogConfiguration
or other such abstraction. This change will allow users to control with logging subsystem they want to use without the need to configure Feign separately.
This will allow us to use the logger facade instances throughout the library, increasing our ability to log additional internal details while removing our current requirement to implement logger implementation specific Logger
instances.
Here is an example of what the resulting changes to Feign.Builder
could look like:
Feign.builder()
.log(LogConfiguration.Builder()
.withRequest()
.withResponse()
.withHeaders()
.build())
.build();
In addition, configuring what information is logged during Feign operations will become more explicit, leading to increased understanding and ease of use.
The potential draw backs here are that whatever facade we choose will require users to include that dependency in their project and we will need to include documentation on how to manage conflicts with Commons Logging for users whose projects intersect/conflict. This information is readily available however and shouldn't be considered a major blocker to this approach.
The alternative is to adopt something similar to what Spring has done, which is to maintain the Logger
abstraction and place ourselves in the middle, co-opting the detection of the logging framework and adapt accordingly. More information can be found here: https://github.com/spring-projects/spring-framework/issues/19081
While this does provide the most flexibility, it does mean that we will be explicitly choosing Commons Logging over SLF4J as SLF4J is more opinionated and expresses that users will need to configure SLF4J and the appropriate bridges. Also, it does require that we, the Feign maintainers, also maintain our own mini-facade. For this reason alone, I feel that the above suggestion of using SLF4J and adding it as a dependency, it worthwhile.
Just for note: some modules depend on JCL
- (Apache) httpclient
- hystrix
- googlehttpclient
- ribbon
- spring4
What if log()
will get varargs like:
log(HEADERS, REQUEST, RESPONSE)
In my opinion that it allows to configure the logging but the builder is more hard-coded way.
As far as I understand
-
withRequest()
equalswithHeaders()
(only request headers) plus a request body -
withResponse()
equalswithHeaders()
(only response headers) plus a response body -
withHeaders()
for all headers, response and request
withRequest()
, withResponse()
and withHeaders()
we could use simultaniuosly and in any order.
We also need something like withBasic()
.
But withBasic()
should re-write configuration or vice versa should be ignored if any other with...()
was used earlier.
How about logging as another capability?
Now we use capability for metrics and for Hystrix.
@radio-rogal sound very interesting.... hystrix is using capabilities, man I'm lossing track of things =)
Do you wanna play around with logs + capabilities and make a proposal? May or may not be approved, but, could be nice
I have started with Slf4jCapability + builder like @kdavisk6 Kevin wrote. It implements client and retryer wrappers so I add withRetryer
too.
I am going to use the DEBUG level as it old implementation does. From logging prospective it should look the same.
sounds awesome