feign-reactive
feign-reactive copied to clipboard
Allow to opt-out completely from DefaultReactiveLogger
In our production environment we have rather strict requirements as to what's logged on ERROR
-level. Because of this we need to implement a custom ReactiveLoggerListener
, which is fine, but we also need to disable the logger-level for DefaultReactiveLogger
since it will always be enabled when we build our client with something like:
WebReactiveFeign.<CustomApi>builder()
.httpClient(httpClient)
.addLoggerListener(new CustomReactiveLogger(clock))
The DefaultReactiveLogger is registered here:
https://github.com/PlaytikaOSS/feign-reactive/blob/baba570b5039c649d5487883f3db1e183026773a/feign-reactor-core/src/main/java/reactivefeign/ReactiveFeign.java#L170
and applies because the CoreWebBuilder
extends ReactiveFeign.Builder
which registers the logger in its constructor.
This is inconvenient to us specifically because we use ReactiveFeign in an internal library that helps with constructing the ReactiveFeign-client, the http-client and related configuration for simpler consumption in our services, but the current solution requires us to also configure specific log-suppression in the various services. It would be more convenient to allow for opting out from DefaultReactiveLogger entirely, either by builder-methods like useDefaultLoggerListener()/disableDefaultLoggerListener()
or clearLoggerListeners()
followed by the currently available addLoggerListener()
.
An alternate, perhaps clearer change would be to add a constructor argument for the desired ReactiveLoggerListener(s), requiring consumers to specify logger. It would break existing code, but in an easy-to-fix manner.
I would be willing to provide a pull-request, if this is at all interesting, but I would appreciate some guidance as to which approach to prefer.
Hi Can you add more details about this approach?
An alternate, perhaps clearer change would be to add a constructor argument for the desired ReactiveLoggerListener(s), requiring consumers to specify logger. It would break existing code, but in an easy-to-fix manner.
The same for me. DefaultReactiveLogger always logs ERROR even if a request is retried successfully. While each ERROR is an alert for devops team. Agree with suggestion that DefaultReactiveLogger should be disabled by default.