spring-cloud / spring-cloud-openfeign

Support for using OpenFeign in Spring Cloud apps
Apache License 2.0
1.18k stars 758 forks source link

Support circuit breaker configuration per Feign client #487

Open fdw opened 3 years ago

fdw commented 3 years ago

Is your feature request related to a problem? Please describe. Currently, there seems to be no way to configure different circuit breakers for different Feign clients without listing all their methods. I can either customize the default configuration, but then this configuration will be applied to all Feign clients. On the other hand, if I want to customize a specific configuration, I must do so for all its methods, as the circuit breaker name is <feignClientName>-<methodName>.

Describe the solution you'd like A simple solution would be to also check for a circuit breaker called feignClientName if no more specific one is found.

An ideal solution would be similar to the other Feign configuration, i.e. I can specify a Customizer Bean in the Feign configuration, like ErrorDecoder or Retryer. This customizer would then be applied to just this Feign client.

Describe alternatives you've considered As I said, the current alternatives would be to list all methods in a client, which is quite error-prone.

fdw commented 3 years ago

If you think this is a good idea, I might try to implement it. I just need some pointers how I would go about implementing that.

tjuchniewicz commented 2 years ago

I agree with @fdw. Configuration is difficult and error-prone. In Hystrix era we used SetterFactory to customize command names. We could simply use Feign class name only or class and method. Currently we have to use: "resilience4j.timelimiter.instances.\"[ExampleServiceClient#readData(Long, String,Integer)]\".timeoutDuration=1s",

It would be great if we could easily get the same with Spring Cloud Circuit Breaker/Resilience4j. Maybe something similar to feign.circuitbreaker.group.enabled=true that groups all methods into one configuration for bulkhead configuration? Another solution would be to allow to customize circuitName in FeignCircuitBreakerInvocationHandler:

class FeignCircuitBreakerInvocationHandler implements InvocationHandler {
...
   @Override
   public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable {
      ...
      String circuitName = Feign.configKey(target.type(), method); // allow to customize name
      ...
   }
}

My current small improvement for this issue is to display all possible circuit breaker names during app startup:

@Bean
Capability customCapability() {
      return new ConfigKeysPrintingCapability();
}

@Slf4j
public class ConfigKeysPrintingCapability implements Capability {

    @Override
    public InvocationHandlerFactory enrich(InvocationHandlerFactory invocationHandlerFactory) {
        return (target, dispatch) -> {
            Set<Method> keySet = dispatch.keySet();
            for (Method method : keySet) {
                log.info("Method key: {}", Feign.configKey(target.type(), method));
            }
            return invocationHandlerFactory.create(target, dispatch);
        };
    }
}
tjuchniewicz commented 2 years ago

Fixed by https://github.com/spring-cloud/spring-cloud-openfeign/pull/575. Now we can use CircuitBreakerNameResolver to customize circuit name.