spring-cloud / spring-cloud-circuitbreaker

Spring Cloud Circuit Breaker API and Implementations
Apache License 2.0
336 stars 110 forks source link

Document How To Customize The ExecutorService #161

Closed v-v-y closed 1 year ago

v-v-y commented 1 year ago

Describe the bug As a developer, I'd like to see tracing context from MDC during executing requests. Now if I enable circuit-breaker context disappears from logs.

Sample Example project: https://github.com/v-v-y/demo

Java: 17 Spring boot: 2.7.6 spring-cloud-starter-circuitbreaker-resilience4j: 2.0.2 spring-cloud-starter-openfeign: 3.1.5

application.yaml

spring:
  application:
    name: demo-application
  profiles:
    active: dev
  cloud:
    open-feign:
      client:
        config:
          default:
            loggerLevel: BASIC
feign:
  circuit-breaker:
    enabled: true

resilience4j:
  scheduled:
    executor:
      corePoolSize: 10
      contextPropagators:
        - com.example.demo.MdcContextPropagator
  circuit-breaker:
    configs:
      default:
        slidingWindowType: TIME_BASED
        failureRateThreshold: 50
        minimumNumberOfCalls: 3
        slidingWindowSize: 60
        waitDurationInOpenState: 30s
        permittedNumberOfCallsInHalfOpenState: 3
        recordFailurePredicate: com.example.demo.ErrorPredicate
        registerHealthIndicator: true

The difference in logs during calling example controller GET /v1/discovery

if feign.circuit-breaker.enabled: true

[2022-12-18 14:25:36.569] [pool-2-thread-1] com.example.demo.GoogleClient             []  DEBUG: [GoogleClient#discovery] ---> GET https://docs.googleapis.com/$discovery/rest?version=v1 HTTP/1.1 
[2022-12-18 14:25:37.077] [pool-2-thread-1] com.example.demo.GoogleClient             []  DEBUG: [GoogleClient#discovery] <--- HTTP/1.1 200 OK (506ms) 

if feign.circuit-breaker.enabled: false

[2022-12-18 14:26:09.812] [http-nio-8080-exec-1] com.example.demo.GoogleClient             [1f143097-3f5e-467d-9038-aa6d511c1895]  DEBUG: [GoogleClient#discovery] ---> GET https://docs.googleapis.com/$discovery/rest?version=v1 HTTP/1.1 
[2022-12-18 14:26:10.333] [http-nio-8080-exec-1] com.example.demo.GoogleClient             [1f143097-3f5e-467d-9038-aa6d511c1895]  DEBUG: [GoogleClient#discovery] <--- HTTP/1.1 200 OK (518ms) 

Look like ContextAwareScheduledThreadPoolExecutor created, but never used. I've also tried to provide specific contextPropagator but it didn't help too

ryanjbaxter commented 1 year ago

I only see context propagators being used within bulkheads with a thread pool not circuit breakers. @RobWin is that true?

RobWin commented 1 year ago

Yes, correct. A CircuitBreaker is not spawning any threads, but a TimeLimiter schedules a thread when a timeout occurs.

But the thread name pool-2-thread-1 doesn't match the name of threads spawned by resilience4j ThreadPoolBulkhead or ContextAwareScheduledThreadPoolExecutor.

See: https://github.com/resilience4j/resilience4j/blob/master/resilience4j-core/src/main/java/io/github/resilience4j/core/ContextAwareScheduledThreadPoolExecutor.java#L35

or

https://github.com/resilience4j/resilience4j/blob/master/resilience4j-bulkhead/src/main/java/io/github/resilience4j/bulkhead/internal/BulkheadNamingThreadFactory.java#L12

v-v-y commented 1 year ago

Is it posible to propagate MDC context to resilience4j requests?

ryanjbaxter commented 1 year ago

@v-v-y can you try setting spring.cloud.circuitbreaker.resilience4j.disableThreadPool=false?

v-v-y commented 1 year ago

spring.cloud.circuitbreaker.resilience4j.disableThreadPool=false

checked, it doesn't help

ryanjbaxter commented 1 year ago

Could you please provide a Java project? I am having trouble getting the Kotlin project to run.

spring-cloud-issues commented 1 year 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.

v-v-y commented 1 year ago

Here is java project https://github.com/v-v-y/demo-java

ryanjbaxter commented 1 year ago

Please use the Spring Boot BOM as well as the Spring Cloud BOM you have mismatched versions because you are trying to manage the versions yourself. After using the latest Spring Boot 2.7.x release and Spring Cloud 2021.0.x release please try the property I suggest above. If that does not work please correct your sample with the updated versions.

spring-cloud-issues commented 1 year 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.

doxic012 commented 1 year ago

@v-v-y can you try setting spring.cloud.circuitbreaker.resilience4j.disableThreadPool=false?

This works for me in version 2.1.4, but now the whole timelimiter is disabled, since the calls are now only executed with the circuitBreaker (see Resilience4JCircuitBreaker), because there will be no executorService provided when creating an instance in the Factory in Resilience4JCircuitBreakerFactory.

Is there no reliable other way to e.g. inject an executorService with MDC awareness, etc. or disable the threadpool but keep the timelimiter intact?

ryanjbaxter commented 1 year ago

Can you try creating a Customizer bean for Resilience4JCircuitBreakerFactory which sets the Executor?

@Bean
public Customizer<Resilience4JCircuitBreakerFactory> slowCustomizer() {
    return factory -> {
        ContextAwareScheduledThreadPoolExecutor executor = ContextAwareScheduledThreadPoolExecutor.newScheduledThreadPool().corePoolSize(5).contextPropagators(new TestContextPropogator())
            .build();
        factory.configureExecutorService(executor);
    };
}
spring-cloud-issues commented 1 year 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.

v-v-y commented 1 year ago

@ryanjbaxter Thanks a lot, this approach works as expected.

Actually we do not need to specify TestContextPropogator because ContextAwareScheduledThreadPoolExecutor propagate MDC context at all For me solution

    @Bean
    fun customResilience4JCircuitBreakerFactory(): Customizer<Resilience4JCircuitBreakerFactory> {
        return Customizer { factory: Resilience4JCircuitBreakerFactory ->
            val executor = ContextAwareScheduledThreadPoolExecutor.newScheduledThreadPool()
                .corePoolSize(10)
                .build()
            factory.configureExecutorService(executor)
        }
    }

Circuit breaker and timelimiter works well with such approach with proper MDC context

Been24 commented 1 year ago

Can you try creating a Customizer bean for Resilience4JCircuitBreakerFactory which sets the Executor?

@Bean
public Customizer<Resilience4JCircuitBreakerFactory> slowCustomizer() {
  return factory -> {
      ContextAwareScheduledThreadPoolExecutor executor = ContextAwareScheduledThreadPoolExecutor.newScheduledThreadPool().corePoolSize(5).contextPropagators(new TestContextPropogator())
          .build();
      factory.configureExecutorService(executor);
  };
}

@ryanjbaxter can I configure with circuit-breaker-properties-configuration like this ? resilience4j.scheduled.executor.core-pool-size=10 not worked

ryanjbaxter commented 1 year ago

No we use a different executor service