spring-cloud / spring-cloud-circuitbreaker

Spring Cloud Circuit Breaker API and Implementations
Apache License 2.0
332 stars 111 forks source link

Resilience4J Configurations from application.yml aren't Supported #61

Closed timothymathison closed 3 years ago

timothymathison commented 4 years ago

Description:

Application: spring-cloud-gateway:Hoxton.SR3 Circuit-Breaker version: 1.0.2.RELEASE

As far as I can tell, yaml configurations are ignored. The provided ReactiveResilience4JCircuitBreakerFactory bean uses the default CircuitBreakerRegistry which only uses the hard-coded defaults. I tried creating my own ReactiveResilience4JCircuitBreakerFactory bean which uses the CircuitBreakerRegistry from here. However, spring-cloud-circuit-breaker seems to ignore resiliency4j configurations defined for the registry. I think that is because of this logic which pulls configurations from here; but it seems like configure is never called, so that configurations map is always empty. Therefore, it always falls back to the defaults. As a result, every CircuitBreaker is created with those defaults. Does anyone know why the circuitBreaker(id, configs) method is being used instead of one of the other methods like circuitBreaker(id) which uses the configurations set for the CircuitBreakerRegistry?

Is there some existing logic I'm missing? If not would it be possible to change the implementation to support providing resilience4J configurations from application.yml?

ryanjbaxter commented 4 years ago

You can provide default configuration for all circuit breakers as outlined here in the documentation https://cloud.spring.io/spring-cloud-static/spring-cloud-circuitbreaker/1.0.0.RELEASE/reference/html/#default-configuration

You can also provide configuration for a specific circuit breaker

https://cloud.spring.io/spring-cloud-static/spring-cloud-circuitbreaker/1.0.0.RELEASE/reference/html/#specific-circuit-breaker-configuration

We do not currently use properties from application properties.

timothymathison commented 4 years ago

Unfortunately, there isn't a straight forward way to configure different environments separately using the code approach outlined in the documentation (without implementing our own Configuration class/logic).

Is there any future plan to implement support for application properties?

timothymathison commented 4 years ago

Closed by accident.

ryanjbaxter commented 4 years ago

I am unsure what you mean by "environments"

timothymathison commented 4 years ago

I am unsure what you mean by "environments"

Sorry for the confusion. I mean't "different profiles".

spencergibb commented 4 years ago

Have you tried @Profile?

timothymathison commented 4 years ago

@spencergibb, No I haven't; I forgot that @Profile is an option in this case. I think in-code configurations should be sufficient for our current uses.

However, wouldn't support for external/remote configurations still be a worthwhile feature to have, for the same reasons that external configurations are supported by other Spring packages?

Romeh commented 4 years ago

@timothymathison here you can find an example how to mix resilience4j spring boot starter and spring cloud circuit breaker starter to enable external application yaml configuration of your circuit breaker and time limiter as well : https://github.com/Romeh/spring-cloud-gateway-resilience4j

rajashashankmuppirala commented 4 years ago

@timothymathison , I had a similar requirement and I was able to configure it as below.

        @Bean
    public ReactiveResilience4JCircuitBreakerFactory reactiveResilience4JCircuitBreakerFactory(final CircuitBreakerRegistry circuitBreakerRegistry, final TimeLimiterRegistry timeLimiterRegistry) {
        ReactiveResilience4JCircuitBreakerFactory reactiveResilience4JCircuitBreakerFactory = new ReactiveResilience4JCircuitBreakerFactory();
reactiveResilience4JCircuitBreakerFactory.configureCircuitBreakerRegistry(circuitBreakerRegistry);
        reactiveResilience4JCircuitBreakerFactory.configureDefault(id -> {
            CircuitBreakerConfig circuitBreakerConfig = circuitBreakerRegistry.find(id).isPresent() ? circuitBreakerRegistry.find(id).get().getCircuitBreakerConfig()
                    : circuitBreakerRegistry.getDefaultConfig();
            TimeLimiterConfig timeLimiterConfig = timeLimiterRegistry.find(id).isPresent() ? timeLimiterRegistry.find(id).get().getTimeLimiterConfig()
                    : timeLimiterRegistry.getDefaultConfig();

            return new Resilience4JConfigBuilder(id)
                    .circuitBreakerConfig(circuitBreakerConfig)
                    .timeLimiterConfig(timeLimiterConfig)
                    .build();
        });
        return reactiveResilience4JCircuitBreakerFactory;
    }
ryanjbaxter commented 3 years ago

@Romeh thanks for the example!

Basically all you need is the r4j spring boot starter on the classpath and then you can configure the CB and time limiter via configuration properties. The starter creates the beans and then those beans are used by your custom config.

Is that the basic idea?

spring-cloud-issues commented 3 years ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

spring-cloud-issues commented 3 years ago

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

dalegaspi commented 3 years ago

@Romeh actually that doesn't really work 100%. for https://github.com/Romeh/spring-cloud-gateway-resilience4j/blob/f74cdcdc226fb782e2adc8af9dd84f0f32ab482a/src/main/java/io/github/romeh/services/gateway/GatewayApplication.java#L28 the timeLimiterRegistry.getConfiguration("backendB") will actually return Optional.Empty even if you have backendB defined in your application.yml (i know because i tried) so it will always fall back to the .orElse(...) part.

dalegaspi commented 3 years ago

@rajashashankmuppirala's solution is (IMO) better and it actually works (i just tried it). it would at least defer fully on application.yml for configuration (and overrides) for the timeout.

Ferioney commented 3 years ago

@ryanjbaxter We have a lot of configuration properties for Hystrix (like Hystrix command properties). And it would be nice to have a possibility to migrate Hystrix properties to the Resilience4J properties. Now I see just one way to configure CircuitBreaker/TimeTimit by using Customizer<Resilience4JCircuitBreakerFactory>. It very difficult to migrate Hystrix's properties to java Customizer.

It would be nice to add this possibility to spring-cloud-starter-circuitbreaker-resilience4j module. I think it pretty simple (I can help with contributing). All that we need is add a dependency to the resilience4j-spring-boot2 module and add CircuitBreakerRegistry (TimeLimitRegistry and BulkheadRegistry in the future) bean to the Resilience4JCircuitBreakerFactory. To my mind it will be very helpful for all people who want to migrate from Hystrix to Resilience4j

ryanjbaxter commented 3 years ago

@Ferioney always happy to have contributions!

timothymathison commented 3 years ago

It looks like #88 didn't include changes for ReactiveResilience4JCircuitBreakerFactory which was the context for my original request when I opened this issue. So I don't think this issue should have been closed. However, I see that #108 has been opened specifically for the reactive version.

atulkumar-mittal commented 2 years ago

Hi,

I have defined below config in my project:

org.springframework.cloud spring-cloud-starter-circuitbreaker-reactor-resilience4j 2.0.2 In application.yml I am using the config from https://github.com/resilience4j/resilience4j-spring-cloud2-demo project yml file.

If I just use @Autowired ReactiveCircuitBreakerFactory in my class, I get the default config not the one defined inside application.yml file.

If I create a bean of Customizer defaultCustomizer() and customize the config programatically then I just see the changed properties on top of default config.

But the application.yml is never working. And same is with Metrics also. As per https://docs.spring.io/spring-cloud-circuitbreaker/docs/current/reference/html/ documentation, the metrics requires actuator and resilience4j-metrics dependencies but when I define resilience4j-metrics along with above spring-cloud dependency, I get error in initializing the MicrometerReactiveResilience4JCustomizerConfiguration defined inside ReactiveResilience4JAutoConfiguration class.

The documentation on https://resilience4j.readme.io/docs/getting-started-3#section-metrics-endpoint page expects only micrometer and actuator dependencies in classpath and saying that it will automatically register circuitbreaker with micrometer. While the documentation on https://docs.spring.io/spring-cloud-circuitbreaker/docs/current/reference/html/ expects other dependency which when defined gives error in starting the application context.

Kindly help in getting these two things resolved. @ryanjbaxter

ryanjbaxter commented 2 years ago

Please open a separate issue with a sample that reproduces the issue(s)

atulkumar-mittal commented 2 years ago

Hi @ryanjbaxter ,

I found the issue. This was because of version mismatch between spring-boot, spring-cloud-dependencies and resilience-4j. Once I synced them, it worked.

Similar to https://github.com/resilience4j/resilience4j/issues/1211

Thanks anyways

ryanjbaxter commented 2 years ago

@atulkumar-mittal can you elaborate on what versions you were using that were incompatible?

atulkumar-mittal commented 2 years ago

Hey @ryanjbaxter ,

There are still some issues.

  1. The initial config which was not in sync and could not even read the configuration from application.yml spring.boot.version-2.3.4.RELEASE spring.cloud.version-Hoxton.SR7 spring.cloud.sleuth.core-2.2.5 -- this is also having spring-cloud-circuitbreaker-1.0.4 spring-cloud-starter-circuitbreaker-reactor-resilience4j-2.0.2 but it did not have spring-boot2 dependencies in classpath. Due to which the configuration could not be read

  2. In the same config excluded spring-cloud-circuitbreaker from spring.cloud.sleuth and added io.githun.resilience4j.spring-boot2.1.7.0 but it gave exception while initialising MicrometerRegistry inside ReactiveResilience4jAutoConfiguration

  3. Changed io.github.resilience4j.spring-boot2 to 1.3.1 Now it is reading the configuration when @CircuitBreakerRegistry annotation or constructor dependency is defined. The ReactiveCircuitBreakerFactory autowiring or constructor dependency was fetching the default config even though configuration was read from application.yml After defining @CircuitBreakerRegistry dependency and creating a @Bean of ReactiveCircuitBreakerFactory is having configuration as defined in application.yml but when I fetch an object using reactiveCircuitBreakerFactory.create('service').run(), it still provides the default config. Even though the circuitBreakerRegistry is having the configuration from application.yml

Regards, Atul

ryanjbaxter commented 2 years ago

Can you try with Spring Boot 2.5.5 and Spring Cloud 2020.0.4? There should be no need to mess with version numbers to make things work.

atulkumar-mittal commented 2 years ago

In a standalone project it works with spring-boot version higher than or equal to 2.4.0 with spring-cloud-reactive-resilience4j.2.0.2. But we have a constraint of using the spring-boot version due to some other dependencies. I tried the solution by @rajashashankmuppirala and it works well.

The prometheus metrics are however not getting populated correctly. I have defined two configs in application.yml (serv1, serv2). I created instance of serv1 like reactiveCircuitBreakerFactory.create("serv1") which I suppose is returning the same instance for each call as defined in application.yml.

The state of circuit-breaker is correctly populated resilience4j_circuitbreaker_state{name="serv1",state="open",} 1.0 but some metrics are not like resilience4j_circuitbreaker_failure_rate{name="serv1",} -1.0 and resilience4j_circuitbreaker_slow_calls{kind="failed",name="serv1",} 0.0 resilience4j_circuitbreaker_slow_calls{kind="successful",name="serv1",} 0.0 while resilience4j_circuitbreaker_calls_seconds_count{kind="failed",name="serv1",} 52.0 resilience4j_circuitbreaker_calls_seconds_sum{kind="failed",name="serv1",} 910.19755384 is populated with some data

ryanjbaxter commented 2 years ago

Please open a separate issue