From bc60d8e6f42b8e3a77d838cbabcaceeb5c8dd080 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Tue, 11 Mar 2025 17:36:27 +0530 Subject: [PATCH 01/15] Add grpc circuit breaker utility using interceptors --- grpc-circuitbreaker-utils/build.gradle.kts | 39 ++++++ .../CircuitBreakerConfigProvider.java | 112 +++++++++++++++ .../CircuitBreakerEventListener.java | 38 +++++ .../grpcutils/CircuitBreakerInterceptor.java | 130 ++++++++++++++++++ .../CircuitBreakerMetricsNotifier.java | 33 +++++ .../grpcutils/CircuitBreakerModule.java | 67 +++++++++ settings.gradle.kts | 1 + 7 files changed, 420 insertions(+) create mode 100644 grpc-circuitbreaker-utils/build.gradle.kts create mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProvider.java create mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerEventListener.java create mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerInterceptor.java create mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerMetricsNotifier.java create mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerModule.java diff --git a/grpc-circuitbreaker-utils/build.gradle.kts b/grpc-circuitbreaker-utils/build.gradle.kts new file mode 100644 index 0000000..5364c0b --- /dev/null +++ b/grpc-circuitbreaker-utils/build.gradle.kts @@ -0,0 +1,39 @@ +plugins { + `java-library` + jacoco + id("org.hypertrace.publish-plugin") + id("org.hypertrace.jacoco-report-plugin") +} + +dependencies { + + api(platform("io.grpc:grpc-bom:1.68.3")) + api("io.grpc:grpc-context") + api("io.grpc:grpc-api") + api("io.grpc:grpc-inprocess") + api(platform("io.netty:netty-bom:4.1.118.Final")) + constraints { + api("com.google.protobuf:protobuf-java:3.25.5") { + because("https://nvd.nist.gov/vuln/detail/CVE-2024-7254") + } + } + + implementation(project(":grpc-context-utils")) + implementation("org.slf4j:slf4j-api:1.7.36") + implementation("io.grpc:grpc-core") + implementation("io.github.resilience4j:resilience4j-circuitbreaker:1.7.1") + implementation("com.typesafe:config:1.4.2") + implementation("com.google.inject:guice:7.0.0") + implementation("org.hypertrace.core.serviceframework:platform-metrics:0.1.87") + + annotationProcessor("org.projectlombok:lombok:1.18.24") + compileOnly("org.projectlombok:lombok:1.18.24") + + testImplementation("org.junit.jupiter:junit-jupiter:5.8.2") + testImplementation("org.mockito:mockito-core:5.8.0") + testRuntimeOnly("io.grpc:grpc-netty") +} + +tasks.test { + useJUnitPlatform() +} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProvider.java new file mode 100644 index 0000000..da597fa --- /dev/null +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProvider.java @@ -0,0 +1,112 @@ +package org.hypertrace.circuitbreaker.grpcutils; + +import com.typesafe.config.Config; +import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class CircuitBreakerConfigProvider { + + public static final String CIRCUIT_BREAKER_CONFIG = "circuit.breaker.config"; + public static final String DEFAULT_CONFIG_KEY = "default"; + + // Whether to enable circuit breaker or not. + private static final String ENABLED = "enabled"; + + // Percentage of failures to trigger OPEN state + private static final String FAILURE_RATE_THRESHOLD = "failureRateThreshold"; + // Percentage of slow calls to trigger OPEN state + private static final String SLOW_CALL_RATE_THRESHOLD = "slowCallRateThreshold"; + // Define what a "slow" call is + private static final String SLOW_CALL_DURATION_THRESHOLD = "slowCallDurationThreshold"; + // Number of calls to consider in the sliding window + private static final String SLIDING_WINDOW_SIZE = "slidingWindowSize"; + // Time before retrying after OPEN state + private static final String WAIT_DURATION_IN_OPEN_STATE = "waitDurationInOpenState"; + // Minimum calls before evaluating failure rate + private static final String MINIMUM_NUMBER_OF_CALLS = "minimumNumberOfCalls"; + // Calls allowed in HALF_OPEN state before deciding to + // CLOSE or OPEN again + private static final String PERMITTED_NUMBER_OF_CALLS_IN_HALF_OPEN_STATE = + "permittedNumberOfCallsInHalfOpenState"; + private static final String SLIDING_WINDOW_TYPE = "slidingWindowType"; + + // Cache for storing CircuitBreakerConfig instances + private static final ConcurrentHashMap configCache = + new ConcurrentHashMap<>(); + + // Global flag for circuit breaker enablement + private boolean circuitBreakerEnabled = false; + + public CircuitBreakerConfigProvider(Config config) { + initialize(config); + } + + public CircuitBreakerConfigProvider() {} + + /** Initializes and caches all CircuitBreaker configurations. */ + public void initialize(Config config) { + if (!config.hasPath(CIRCUIT_BREAKER_CONFIG)) { + log.warn("No circuit breaker configurations found in the config file."); + return; + } + + Config circuitBreakerConfig = config.getConfig(CIRCUIT_BREAKER_CONFIG); + + // Read global enabled flag (default to false if not provided) + circuitBreakerEnabled = + circuitBreakerConfig.hasPath(ENABLED) && circuitBreakerConfig.getBoolean(ENABLED); + + // Load all circuit breaker configurations and cache them + Map allConfigs = + circuitBreakerConfig.root().keySet().stream() + .filter(key -> !key.equals(ENABLED)) // Ignore the global enabled flag + .collect( + Collectors.toMap( + key -> key, // Circuit breaker key + key -> createCircuitBreakerConfig(circuitBreakerConfig.getConfig(key)))); + + // Store in cache + configCache.putAll(allConfigs); + + log.info( + "Loaded {} circuit breaker configurations, Global Enabled: {}. Configs: {}", + allConfigs.size(), + circuitBreakerEnabled, + allConfigs); + } + + /** + * Retrieves the CircuitBreakerConfig for a specific key. Falls back to default if key-specific + * config is not found. + */ + public CircuitBreakerConfig getConfig(String circuitBreakerKey) { + return configCache.getOrDefault(circuitBreakerKey, configCache.get(DEFAULT_CONFIG_KEY)); + } + + /** Checks if Circuit Breaker is globally enabled. */ + public boolean isCircuitBreakerEnabled() { + return circuitBreakerEnabled; + } + + private CircuitBreakerConfig createCircuitBreakerConfig(Config config) { + return CircuitBreakerConfig.custom() + .failureRateThreshold((float) config.getDouble(FAILURE_RATE_THRESHOLD)) + .slowCallRateThreshold((float) config.getDouble(SLOW_CALL_RATE_THRESHOLD)) + .slowCallDurationThreshold(config.getDuration(SLOW_CALL_DURATION_THRESHOLD)) + .slidingWindowType(getSlidingWindowType(config.getString(SLIDING_WINDOW_TYPE))) + .slidingWindowSize(config.getInt(SLIDING_WINDOW_SIZE)) + .waitDurationInOpenState(config.getDuration(WAIT_DURATION_IN_OPEN_STATE)) + .permittedNumberOfCallsInHalfOpenState( + config.getInt(PERMITTED_NUMBER_OF_CALLS_IN_HALF_OPEN_STATE)) + .minimumNumberOfCalls(config.getInt(MINIMUM_NUMBER_OF_CALLS)) + .build(); + } + + private CircuitBreakerConfig.SlidingWindowType getSlidingWindowType(String slidingWindowType) { + return CircuitBreakerConfig.SlidingWindowType.valueOf(slidingWindowType); + } +} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerEventListener.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerEventListener.java new file mode 100644 index 0000000..47815fa --- /dev/null +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerEventListener.java @@ -0,0 +1,38 @@ +package org.hypertrace.circuitbreaker.grpcutils; + +import io.github.resilience4j.circuitbreaker.CircuitBreaker; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class CircuitBreakerEventListener { + private static final Set attachedCircuitBreakers = ConcurrentHashMap.newKeySet(); + + public static synchronized void attachListeners(CircuitBreaker circuitBreaker) { + if (!attachedCircuitBreakers.add( + circuitBreaker.getName())) { // Ensures only one listener is attached + return; + } + circuitBreaker + .getEventPublisher() + .onStateTransition( + event -> + log.info( + "State transition: {} for circuit breaker {} ", + event.getStateTransition(), + event.getCircuitBreakerName())) + .onCallNotPermitted( + event -> + log.debug( + "Call not permitted: Circuit is OPEN for circuit breaker {} ", + event.getCircuitBreakerName())) + .onEvent( + event -> { + log.debug( + "Circuit breaker event type {} for circuit breaker name {} ", + event.getEventType(), + event.getCircuitBreakerName()); + }); + } +} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerInterceptor.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerInterceptor.java new file mode 100644 index 0000000..b15ea78 --- /dev/null +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerInterceptor.java @@ -0,0 +1,130 @@ +package org.hypertrace.circuitbreaker.grpcutils; + +import io.github.resilience4j.circuitbreaker.CircuitBreaker; +import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import io.grpc.CallOptions; +import io.grpc.Channel; +import io.grpc.ClientCall; +import io.grpc.ClientInterceptor; +import io.grpc.ForwardingClientCall; +import io.grpc.ForwardingClientCallListener; +import io.grpc.Metadata; +import io.grpc.MethodDescriptor; +import io.grpc.Status; +import java.util.concurrent.TimeUnit; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class CircuitBreakerInterceptor implements ClientInterceptor { + + public static final CallOptions.Key CIRCUIT_BREAKER_KEY = + CallOptions.Key.createWithDefault("circuitBreakerKey", "default"); + private final CircuitBreakerRegistry circuitBreakerRegistry; + private final CircuitBreakerConfigProvider circuitBreakerConfigProvider; + private final CircuitBreakerMetricsNotifier circuitBreakerMetricsNotifier; + + public CircuitBreakerInterceptor( + CircuitBreakerRegistry circuitBreakerRegistry, + CircuitBreakerConfigProvider circuitBreakerConfigProvider, + CircuitBreakerMetricsNotifier circuitBreakerMetricsNotifier) { + this.circuitBreakerRegistry = circuitBreakerRegistry; + this.circuitBreakerConfigProvider = circuitBreakerConfigProvider; + this.circuitBreakerMetricsNotifier = circuitBreakerMetricsNotifier; + } + + // Intercepts the call and applies circuit breaker logic + @Override + public ClientCall interceptCall( + MethodDescriptor method, CallOptions callOptions, Channel next) { + if (!circuitBreakerConfigProvider.isCircuitBreakerEnabled()) { + return next.newCall(method, callOptions); + } + + // Get circuit breaker key from CallOptions + String circuitBreakerKey = callOptions.getOption(CIRCUIT_BREAKER_KEY); + CircuitBreaker circuitBreaker = getCircuitBreaker(circuitBreakerKey); + return new ForwardingClientCall.SimpleForwardingClientCall<>( + next.newCall(method, callOptions)) { + @Override + public void start(Listener responseListener, Metadata headers) { + long startTime = System.nanoTime(); + + // Wrap response listener to track failures + Listener wrappedListener = + new ForwardingClientCallListener.SimpleForwardingClientCallListener<>( + responseListener) { + @Override + public void onClose(Status status, Metadata trailers) { + long duration = System.nanoTime() - startTime; + if (status.isOk()) { + circuitBreaker.onSuccess(duration, TimeUnit.NANOSECONDS); + } else { + log.debug( + "Circuit Breaker '{}' detected failure. Status: {}, Description: {}", + circuitBreaker.getName(), + status.getCode(), + status.getDescription()); + circuitBreaker.onError( + duration, TimeUnit.NANOSECONDS, status.asRuntimeException()); + } + super.onClose(status, trailers); + } + }; + + super.start(wrappedListener, headers); + } + + @Override + public void sendMessage(ReqT message) { + if (!circuitBreaker.tryAcquirePermission()) { + handleCircuitBreakerRejection(circuitBreakerKey, circuitBreaker); + String rejectionReason = + circuitBreaker.getState() == CircuitBreaker.State.HALF_OPEN + ? "Circuit Breaker is HALF-OPEN and rejecting excess requests" + : "Circuit Breaker is OPEN and blocking requests"; + throw Status.UNAVAILABLE.withDescription(rejectionReason).asRuntimeException(); + } + super.sendMessage(message); + } + }; + } + + private void handleCircuitBreakerRejection( + String circuitBreakerKey, CircuitBreaker circuitBreaker) { + String tenantId = getTenantId(circuitBreakerKey); + if (circuitBreaker.getState() == CircuitBreaker.State.HALF_OPEN) { + circuitBreakerMetricsNotifier.incrementCount(tenantId, "circuitbreaker.halfopen.rejected"); + log.debug( + "Circuit Breaker '{}' is HALF-OPEN and rejecting excess requests for tenant '{}'.", + circuitBreakerKey, + tenantId); + } else if (circuitBreaker.getState() == CircuitBreaker.State.OPEN) { + circuitBreakerMetricsNotifier.incrementCount(tenantId, "circuitbreaker.open.blocked"); + log.debug( + "Circuit Breaker '{}' is OPEN. Blocking request for tenant '{}'.", + circuitBreakerKey, + tenantId); + } else { + log.debug( // Added unexpected state handling for safety + "Unexpected Circuit Breaker state '{}' for '{}'. Blocking request.", + circuitBreaker.getState(), + circuitBreakerKey); + } + } + + private static String getTenantId(String circuitBreakerKey) { + if (!circuitBreakerKey.contains(".")) { + return "Unknown"; + } + return circuitBreakerKey.split("\\.", 2)[0]; // Ensures only the first split + } + + /** Retrieve the Circuit Breaker based on the key. */ + private CircuitBreaker getCircuitBreaker(String circuitBreakerKey) { + CircuitBreaker circuitBreaker = + circuitBreakerRegistry.circuitBreaker( + circuitBreakerKey, circuitBreakerConfigProvider.getConfig(circuitBreakerKey)); + CircuitBreakerEventListener.attachListeners(circuitBreaker); + return circuitBreaker; + } +} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerMetricsNotifier.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerMetricsNotifier.java new file mode 100644 index 0000000..b7926d2 --- /dev/null +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerMetricsNotifier.java @@ -0,0 +1,33 @@ +package org.hypertrace.circuitbreaker.grpcutils; + +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.Meter; +import io.micrometer.core.instrument.Tags; +import io.micrometer.core.instrument.noop.NoopCounter; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import org.hypertrace.core.serviceframework.metrics.PlatformMetricsRegistry; + +public class CircuitBreakerMetricsNotifier { + private static final ConcurrentHashMap counterMap = new ConcurrentHashMap<>(); + public static final String UNKNOWN_TENANT = "unknown"; + + public void incrementCount(String tenantId, String counterName) { + getCounter(tenantId, counterName).increment(); + } + + public Counter getCounter(String tenantId, String counterName) { + if (tenantId == null || tenantId.equals(UNKNOWN_TENANT)) { + return getNoopCounter(); + } + return counterMap.computeIfAbsent( + tenantId + counterName, + (unused) -> + PlatformMetricsRegistry.registerCounter(counterName, Map.of("tenantId", tenantId))); + } + + private NoopCounter getNoopCounter() { + Meter.Id dummyId = new Meter.Id("noopCounter", Tags.empty(), null, null, Meter.Type.COUNTER); + return new NoopCounter(dummyId); + } +} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerModule.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerModule.java new file mode 100644 index 0000000..075e96c --- /dev/null +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerModule.java @@ -0,0 +1,67 @@ +package org.hypertrace.circuitbreaker.grpcutils; + +import com.google.inject.AbstractModule; +import com.google.inject.Provides; +import com.google.inject.Singleton; +import com.typesafe.config.Config; +import io.github.resilience4j.circuitbreaker.CircuitBreaker; +import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class CircuitBreakerModule extends AbstractModule { + + private final Config config; + + public CircuitBreakerModule(Config config) { + this.config = config; + } + + @Provides + @Singleton + public CircuitBreakerMetricsNotifier providesCircuitBreakerMetricsNotifier() { + return new CircuitBreakerMetricsNotifier(); + } + + @Provides + @Singleton + public CircuitBreakerConfigProvider providesCircuitBreakerConfigProvider() { + return new CircuitBreakerConfigProvider(config); + } + + @Provides + @Singleton + public CircuitBreakerRegistry providesCircuitBreakerRegistry( + CircuitBreakerConfigProvider circuitBreakerConfigProvider) { + if (!circuitBreakerConfigProvider.isCircuitBreakerEnabled()) { + return CircuitBreakerRegistry.ofDefaults(); + } + CircuitBreakerRegistry circuitBreakerRegistry = + CircuitBreakerRegistry.of( + circuitBreakerConfigProvider.getConfig( + CircuitBreakerConfigProvider.DEFAULT_CONFIG_KEY)); + circuitBreakerRegistry + .getEventPublisher() + .onEntryAdded( + entryAddedEvent -> { + CircuitBreaker addedCircuitBreaker = entryAddedEvent.getAddedEntry(); + log.info("CircuitBreaker {} added", addedCircuitBreaker.getName()); + }) + .onEntryRemoved( + entryRemovedEvent -> { + CircuitBreaker removedCircuitBreaker = entryRemovedEvent.getRemovedEntry(); + log.info("CircuitBreaker {} removed", removedCircuitBreaker.getName()); + }); + return circuitBreakerRegistry; + } + + @Provides + @Singleton + public CircuitBreakerInterceptor providesCircuitBreakerInterceptor( + CircuitBreakerRegistry circuitBreakerRegistry, + CircuitBreakerConfigProvider circuitBreakerConfigProvider, + CircuitBreakerMetricsNotifier circuitBreakerMetricsNotifier) { + return new CircuitBreakerInterceptor( + circuitBreakerRegistry, circuitBreakerConfigProvider, circuitBreakerMetricsNotifier); + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index c13b603..94ab61c 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -18,3 +18,4 @@ include(":grpc-server-rx-utils") include(":grpc-context-utils") include(":grpc-server-utils") include(":grpc-validation-utils") +include(":grpc-circuitbreaker-utils") From faa0e02ddc754d418e363923a232d779c6f19d29 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Thu, 13 Mar 2025 14:54:13 +0530 Subject: [PATCH 02/15] Refactored classes and added test cases --- grpc-circuitbreaker-utils/build.gradle.kts | 12 -- .../CircuitBreakerConfigProvider.java | 60 +++----- .../CircuitBreakerConfiguration.java | 33 +++++ .../CircuitBreakerEventListener.java | 38 ----- .../grpcutils/CircuitBreakerInterceptor.java | 119 +-------------- .../CircuitBreakerMetricsNotifier.java | 33 ----- .../grpcutils/CircuitBreakerModule.java | 67 --------- .../ResilienceCircuitBreakerConfigParser.java | 41 ++++++ .../ResilienceCircuitBreakerInterceptor.java | 137 ++++++++++++++++++ .../ResilienceCircuitBreakerProvider.java | 56 +++++++ ...ilienceCircuitBreakerRegistryProvider.java | 39 +++++ .../CircuitBreakerConfigProviderTest.java | 56 +++++++ ...ilienceCircuitBreakerConfigParserTest.java | 64 ++++++++ ...silienceCircuitBreakerInterceptorTest.java | 110 ++++++++++++++ 14 files changed, 559 insertions(+), 306 deletions(-) create mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java delete mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerEventListener.java delete mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerMetricsNotifier.java delete mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerModule.java create mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParser.java create mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java create mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java create mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java create mode 100644 grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProviderTest.java create mode 100644 grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParserTest.java create mode 100644 grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java diff --git a/grpc-circuitbreaker-utils/build.gradle.kts b/grpc-circuitbreaker-utils/build.gradle.kts index 5364c0b..ca10605 100644 --- a/grpc-circuitbreaker-utils/build.gradle.kts +++ b/grpc-circuitbreaker-utils/build.gradle.kts @@ -8,30 +8,18 @@ plugins { dependencies { api(platform("io.grpc:grpc-bom:1.68.3")) - api("io.grpc:grpc-context") api("io.grpc:grpc-api") - api("io.grpc:grpc-inprocess") - api(platform("io.netty:netty-bom:4.1.118.Final")) - constraints { - api("com.google.protobuf:protobuf-java:3.25.5") { - because("https://nvd.nist.gov/vuln/detail/CVE-2024-7254") - } - } - implementation(project(":grpc-context-utils")) implementation("org.slf4j:slf4j-api:1.7.36") - implementation("io.grpc:grpc-core") implementation("io.github.resilience4j:resilience4j-circuitbreaker:1.7.1") implementation("com.typesafe:config:1.4.2") implementation("com.google.inject:guice:7.0.0") - implementation("org.hypertrace.core.serviceframework:platform-metrics:0.1.87") annotationProcessor("org.projectlombok:lombok:1.18.24") compileOnly("org.projectlombok:lombok:1.18.24") testImplementation("org.junit.jupiter:junit-jupiter:5.8.2") testImplementation("org.mockito:mockito-core:5.8.0") - testRuntimeOnly("io.grpc:grpc-netty") } tasks.test { diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProvider.java index da597fa..0d95734 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProvider.java @@ -1,16 +1,13 @@ package org.hypertrace.circuitbreaker.grpcutils; import com.typesafe.config.Config; -import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; @Slf4j public class CircuitBreakerConfigProvider { - public static final String CIRCUIT_BREAKER_CONFIG = "circuit.breaker.config"; public static final String DEFAULT_CONFIG_KEY = "default"; // Whether to enable circuit breaker or not. @@ -34,66 +31,42 @@ public class CircuitBreakerConfigProvider { "permittedNumberOfCallsInHalfOpenState"; private static final String SLIDING_WINDOW_TYPE = "slidingWindowType"; - // Cache for storing CircuitBreakerConfig instances - private static final ConcurrentHashMap configCache = - new ConcurrentHashMap<>(); - // Global flag for circuit breaker enablement private boolean circuitBreakerEnabled = false; + private Map circuitBreakerConfigurationMap; public CircuitBreakerConfigProvider(Config config) { - initialize(config); + this.initialize(config); } - public CircuitBreakerConfigProvider() {} - - /** Initializes and caches all CircuitBreaker configurations. */ - public void initialize(Config config) { - if (!config.hasPath(CIRCUIT_BREAKER_CONFIG)) { - log.warn("No circuit breaker configurations found in the config file."); - return; - } - - Config circuitBreakerConfig = config.getConfig(CIRCUIT_BREAKER_CONFIG); + /** Checks if Circuit Breaker is globally enabled. */ + public boolean isCircuitBreakerEnabled() { + return circuitBreakerEnabled; + } - // Read global enabled flag (default to false if not provided) + private void initialize(Config circuitBreakerConfig) { circuitBreakerEnabled = circuitBreakerConfig.hasPath(ENABLED) && circuitBreakerConfig.getBoolean(ENABLED); - - // Load all circuit breaker configurations and cache them - Map allConfigs = + this.circuitBreakerConfigurationMap = circuitBreakerConfig.root().keySet().stream() .filter(key -> !key.equals(ENABLED)) // Ignore the global enabled flag .collect( Collectors.toMap( key -> key, // Circuit breaker key key -> createCircuitBreakerConfig(circuitBreakerConfig.getConfig(key)))); - - // Store in cache - configCache.putAll(allConfigs); - log.info( "Loaded {} circuit breaker configurations, Global Enabled: {}. Configs: {}", - allConfigs.size(), + circuitBreakerConfigurationMap.size(), circuitBreakerEnabled, - allConfigs); - } - - /** - * Retrieves the CircuitBreakerConfig for a specific key. Falls back to default if key-specific - * config is not found. - */ - public CircuitBreakerConfig getConfig(String circuitBreakerKey) { - return configCache.getOrDefault(circuitBreakerKey, configCache.get(DEFAULT_CONFIG_KEY)); + circuitBreakerConfigurationMap); } - /** Checks if Circuit Breaker is globally enabled. */ - public boolean isCircuitBreakerEnabled() { - return circuitBreakerEnabled; + public Map getConfigMap() { + return circuitBreakerConfigurationMap; } - private CircuitBreakerConfig createCircuitBreakerConfig(Config config) { - return CircuitBreakerConfig.custom() + private CircuitBreakerConfiguration createCircuitBreakerConfig(Config config) { + return CircuitBreakerConfiguration.builder() .failureRateThreshold((float) config.getDouble(FAILURE_RATE_THRESHOLD)) .slowCallRateThreshold((float) config.getDouble(SLOW_CALL_RATE_THRESHOLD)) .slowCallDurationThreshold(config.getDuration(SLOW_CALL_DURATION_THRESHOLD)) @@ -106,7 +79,8 @@ private CircuitBreakerConfig createCircuitBreakerConfig(Config config) { .build(); } - private CircuitBreakerConfig.SlidingWindowType getSlidingWindowType(String slidingWindowType) { - return CircuitBreakerConfig.SlidingWindowType.valueOf(slidingWindowType); + private CircuitBreakerConfiguration.SlidingWindowType getSlidingWindowType( + String slidingWindowType) { + return CircuitBreakerConfiguration.SlidingWindowType.valueOf(slidingWindowType); } } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java new file mode 100644 index 0000000..355a24d --- /dev/null +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java @@ -0,0 +1,33 @@ +package org.hypertrace.circuitbreaker.grpcutils; + +import java.time.Duration; +import lombok.Builder; +import lombok.Setter; +import lombok.Value; + +@Value +@Builder +@Setter +public class CircuitBreakerConfiguration { + // Percentage of failures to trigger OPEN state + float failureRateThreshold; + // Percentage of slow calls to trigger OPEN state + float slowCallRateThreshold; + // Define what a "slow" call is + Duration slowCallDurationThreshold; + // Number of calls to consider in the sliding window + SlidingWindowType slidingWindowType; + int slidingWindowSize; + // Time before retrying after OPEN state + Duration waitDurationInOpenState; + // Minimum calls before evaluating failure rate + int minimumNumberOfCalls; + // Calls allowed in HALF_OPEN state before deciding to + // CLOSE or OPEN again + int permittedNumberOfCallsInHalfOpenState; + + public enum SlidingWindowType { + COUNT_BASED, + TIME_BASED + } +} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerEventListener.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerEventListener.java deleted file mode 100644 index 47815fa..0000000 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerEventListener.java +++ /dev/null @@ -1,38 +0,0 @@ -package org.hypertrace.circuitbreaker.grpcutils; - -import io.github.resilience4j.circuitbreaker.CircuitBreaker; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import lombok.extern.slf4j.Slf4j; - -@Slf4j -public class CircuitBreakerEventListener { - private static final Set attachedCircuitBreakers = ConcurrentHashMap.newKeySet(); - - public static synchronized void attachListeners(CircuitBreaker circuitBreaker) { - if (!attachedCircuitBreakers.add( - circuitBreaker.getName())) { // Ensures only one listener is attached - return; - } - circuitBreaker - .getEventPublisher() - .onStateTransition( - event -> - log.info( - "State transition: {} for circuit breaker {} ", - event.getStateTransition(), - event.getCircuitBreakerName())) - .onCallNotPermitted( - event -> - log.debug( - "Call not permitted: Circuit is OPEN for circuit breaker {} ", - event.getCircuitBreakerName())) - .onEvent( - event -> { - log.debug( - "Circuit breaker event type {} for circuit breaker name {} ", - event.getEventType(), - event.getCircuitBreakerName()); - }); - } -} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerInterceptor.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerInterceptor.java index b15ea78..b94cce8 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerInterceptor.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerInterceptor.java @@ -1,130 +1,23 @@ package org.hypertrace.circuitbreaker.grpcutils; -import io.github.resilience4j.circuitbreaker.CircuitBreaker; -import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; -import io.grpc.ForwardingClientCall; -import io.grpc.ForwardingClientCallListener; -import io.grpc.Metadata; import io.grpc.MethodDescriptor; -import io.grpc.Status; -import java.util.concurrent.TimeUnit; -import lombok.extern.slf4j.Slf4j; -@Slf4j -public class CircuitBreakerInterceptor implements ClientInterceptor { - - public static final CallOptions.Key CIRCUIT_BREAKER_KEY = - CallOptions.Key.createWithDefault("circuitBreakerKey", "default"); - private final CircuitBreakerRegistry circuitBreakerRegistry; - private final CircuitBreakerConfigProvider circuitBreakerConfigProvider; - private final CircuitBreakerMetricsNotifier circuitBreakerMetricsNotifier; - - public CircuitBreakerInterceptor( - CircuitBreakerRegistry circuitBreakerRegistry, - CircuitBreakerConfigProvider circuitBreakerConfigProvider, - CircuitBreakerMetricsNotifier circuitBreakerMetricsNotifier) { - this.circuitBreakerRegistry = circuitBreakerRegistry; - this.circuitBreakerConfigProvider = circuitBreakerConfigProvider; - this.circuitBreakerMetricsNotifier = circuitBreakerMetricsNotifier; - } - - // Intercepts the call and applies circuit breaker logic +public abstract class CircuitBreakerInterceptor implements ClientInterceptor { @Override public ClientCall interceptCall( MethodDescriptor method, CallOptions callOptions, Channel next) { - if (!circuitBreakerConfigProvider.isCircuitBreakerEnabled()) { + if (!isCircuitBreakerEnabled()) { return next.newCall(method, callOptions); } - - // Get circuit breaker key from CallOptions - String circuitBreakerKey = callOptions.getOption(CIRCUIT_BREAKER_KEY); - CircuitBreaker circuitBreaker = getCircuitBreaker(circuitBreakerKey); - return new ForwardingClientCall.SimpleForwardingClientCall<>( - next.newCall(method, callOptions)) { - @Override - public void start(Listener responseListener, Metadata headers) { - long startTime = System.nanoTime(); - - // Wrap response listener to track failures - Listener wrappedListener = - new ForwardingClientCallListener.SimpleForwardingClientCallListener<>( - responseListener) { - @Override - public void onClose(Status status, Metadata trailers) { - long duration = System.nanoTime() - startTime; - if (status.isOk()) { - circuitBreaker.onSuccess(duration, TimeUnit.NANOSECONDS); - } else { - log.debug( - "Circuit Breaker '{}' detected failure. Status: {}, Description: {}", - circuitBreaker.getName(), - status.getCode(), - status.getDescription()); - circuitBreaker.onError( - duration, TimeUnit.NANOSECONDS, status.asRuntimeException()); - } - super.onClose(status, trailers); - } - }; - - super.start(wrappedListener, headers); - } - - @Override - public void sendMessage(ReqT message) { - if (!circuitBreaker.tryAcquirePermission()) { - handleCircuitBreakerRejection(circuitBreakerKey, circuitBreaker); - String rejectionReason = - circuitBreaker.getState() == CircuitBreaker.State.HALF_OPEN - ? "Circuit Breaker is HALF-OPEN and rejecting excess requests" - : "Circuit Breaker is OPEN and blocking requests"; - throw Status.UNAVAILABLE.withDescription(rejectionReason).asRuntimeException(); - } - super.sendMessage(message); - } - }; - } - - private void handleCircuitBreakerRejection( - String circuitBreakerKey, CircuitBreaker circuitBreaker) { - String tenantId = getTenantId(circuitBreakerKey); - if (circuitBreaker.getState() == CircuitBreaker.State.HALF_OPEN) { - circuitBreakerMetricsNotifier.incrementCount(tenantId, "circuitbreaker.halfopen.rejected"); - log.debug( - "Circuit Breaker '{}' is HALF-OPEN and rejecting excess requests for tenant '{}'.", - circuitBreakerKey, - tenantId); - } else if (circuitBreaker.getState() == CircuitBreaker.State.OPEN) { - circuitBreakerMetricsNotifier.incrementCount(tenantId, "circuitbreaker.open.blocked"); - log.debug( - "Circuit Breaker '{}' is OPEN. Blocking request for tenant '{}'.", - circuitBreakerKey, - tenantId); - } else { - log.debug( // Added unexpected state handling for safety - "Unexpected Circuit Breaker state '{}' for '{}'. Blocking request.", - circuitBreaker.getState(), - circuitBreakerKey); - } + return createInterceptedCall(method, callOptions, next); } - private static String getTenantId(String circuitBreakerKey) { - if (!circuitBreakerKey.contains(".")) { - return "Unknown"; - } - return circuitBreakerKey.split("\\.", 2)[0]; // Ensures only the first split - } + protected abstract boolean isCircuitBreakerEnabled(); - /** Retrieve the Circuit Breaker based on the key. */ - private CircuitBreaker getCircuitBreaker(String circuitBreakerKey) { - CircuitBreaker circuitBreaker = - circuitBreakerRegistry.circuitBreaker( - circuitBreakerKey, circuitBreakerConfigProvider.getConfig(circuitBreakerKey)); - CircuitBreakerEventListener.attachListeners(circuitBreaker); - return circuitBreaker; - } + protected abstract ClientCall createInterceptedCall( + MethodDescriptor method, CallOptions callOptions, Channel next); } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerMetricsNotifier.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerMetricsNotifier.java deleted file mode 100644 index b7926d2..0000000 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerMetricsNotifier.java +++ /dev/null @@ -1,33 +0,0 @@ -package org.hypertrace.circuitbreaker.grpcutils; - -import io.micrometer.core.instrument.Counter; -import io.micrometer.core.instrument.Meter; -import io.micrometer.core.instrument.Tags; -import io.micrometer.core.instrument.noop.NoopCounter; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import org.hypertrace.core.serviceframework.metrics.PlatformMetricsRegistry; - -public class CircuitBreakerMetricsNotifier { - private static final ConcurrentHashMap counterMap = new ConcurrentHashMap<>(); - public static final String UNKNOWN_TENANT = "unknown"; - - public void incrementCount(String tenantId, String counterName) { - getCounter(tenantId, counterName).increment(); - } - - public Counter getCounter(String tenantId, String counterName) { - if (tenantId == null || tenantId.equals(UNKNOWN_TENANT)) { - return getNoopCounter(); - } - return counterMap.computeIfAbsent( - tenantId + counterName, - (unused) -> - PlatformMetricsRegistry.registerCounter(counterName, Map.of("tenantId", tenantId))); - } - - private NoopCounter getNoopCounter() { - Meter.Id dummyId = new Meter.Id("noopCounter", Tags.empty(), null, null, Meter.Type.COUNTER); - return new NoopCounter(dummyId); - } -} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerModule.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerModule.java deleted file mode 100644 index 075e96c..0000000 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerModule.java +++ /dev/null @@ -1,67 +0,0 @@ -package org.hypertrace.circuitbreaker.grpcutils; - -import com.google.inject.AbstractModule; -import com.google.inject.Provides; -import com.google.inject.Singleton; -import com.typesafe.config.Config; -import io.github.resilience4j.circuitbreaker.CircuitBreaker; -import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; -import lombok.extern.slf4j.Slf4j; - -@Slf4j -public class CircuitBreakerModule extends AbstractModule { - - private final Config config; - - public CircuitBreakerModule(Config config) { - this.config = config; - } - - @Provides - @Singleton - public CircuitBreakerMetricsNotifier providesCircuitBreakerMetricsNotifier() { - return new CircuitBreakerMetricsNotifier(); - } - - @Provides - @Singleton - public CircuitBreakerConfigProvider providesCircuitBreakerConfigProvider() { - return new CircuitBreakerConfigProvider(config); - } - - @Provides - @Singleton - public CircuitBreakerRegistry providesCircuitBreakerRegistry( - CircuitBreakerConfigProvider circuitBreakerConfigProvider) { - if (!circuitBreakerConfigProvider.isCircuitBreakerEnabled()) { - return CircuitBreakerRegistry.ofDefaults(); - } - CircuitBreakerRegistry circuitBreakerRegistry = - CircuitBreakerRegistry.of( - circuitBreakerConfigProvider.getConfig( - CircuitBreakerConfigProvider.DEFAULT_CONFIG_KEY)); - circuitBreakerRegistry - .getEventPublisher() - .onEntryAdded( - entryAddedEvent -> { - CircuitBreaker addedCircuitBreaker = entryAddedEvent.getAddedEntry(); - log.info("CircuitBreaker {} added", addedCircuitBreaker.getName()); - }) - .onEntryRemoved( - entryRemovedEvent -> { - CircuitBreaker removedCircuitBreaker = entryRemovedEvent.getRemovedEntry(); - log.info("CircuitBreaker {} removed", removedCircuitBreaker.getName()); - }); - return circuitBreakerRegistry; - } - - @Provides - @Singleton - public CircuitBreakerInterceptor providesCircuitBreakerInterceptor( - CircuitBreakerRegistry circuitBreakerRegistry, - CircuitBreakerConfigProvider circuitBreakerConfigProvider, - CircuitBreakerMetricsNotifier circuitBreakerMetricsNotifier) { - return new CircuitBreakerInterceptor( - circuitBreakerRegistry, circuitBreakerConfigProvider, circuitBreakerMetricsNotifier); - } -} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParser.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParser.java new file mode 100644 index 0000000..ca1e1ff --- /dev/null +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParser.java @@ -0,0 +1,41 @@ +package org.hypertrace.circuitbreaker.grpcutils.resilience; + +import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; +import java.util.Map; +import java.util.stream.Collectors; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration; + +/** Utility class to parse CircuitBreakerConfiguration to Resilience4j CircuitBreakerConfig */ +public class ResilienceCircuitBreakerConfigParser { + + public static Map getCircuitBreakerConfigs( + Map configurationMap) { + return configurationMap.entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, entry -> getConfig(entry.getValue()))); + } + + static CircuitBreakerConfig getConfig(CircuitBreakerConfiguration configuration) { + return CircuitBreakerConfig.custom() + .failureRateThreshold(configuration.getFailureRateThreshold()) + .slowCallRateThreshold(configuration.getSlowCallRateThreshold()) + .slowCallDurationThreshold(configuration.getSlowCallDurationThreshold()) + .slidingWindowType(getSlidingWindowType(configuration)) + .slidingWindowSize(configuration.getSlidingWindowSize()) + .waitDurationInOpenState(configuration.getWaitDurationInOpenState()) + .permittedNumberOfCallsInHalfOpenState( + configuration.getPermittedNumberOfCallsInHalfOpenState()) + .minimumNumberOfCalls(configuration.getMinimumNumberOfCalls()) + .build(); + } + + private static CircuitBreakerConfig.SlidingWindowType getSlidingWindowType( + CircuitBreakerConfiguration configuration) { + switch (configuration.getSlidingWindowType()) { + case COUNT_BASED: + return CircuitBreakerConfig.SlidingWindowType.COUNT_BASED; + case TIME_BASED: + default: + return CircuitBreakerConfig.SlidingWindowType.TIME_BASED; + } + } +} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java new file mode 100644 index 0000000..525b140 --- /dev/null +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java @@ -0,0 +1,137 @@ +package org.hypertrace.circuitbreaker.grpcutils.resilience; + +import com.google.common.annotations.VisibleForTesting; +import com.google.inject.Singleton; +import com.typesafe.config.Config; +import io.github.resilience4j.circuitbreaker.CircuitBreaker; +import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; +import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import io.grpc.CallOptions; +import io.grpc.Channel; +import io.grpc.ClientCall; +import io.grpc.ForwardingClientCall; +import io.grpc.ForwardingClientCallListener; +import io.grpc.Metadata; +import io.grpc.MethodDescriptor; +import io.grpc.Status; +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import lombok.extern.slf4j.Slf4j; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigProvider; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerInterceptor; + +@Slf4j +@Singleton +public class ResilienceCircuitBreakerInterceptor extends CircuitBreakerInterceptor { + + public static final CallOptions.Key CIRCUIT_BREAKER_KEY = + CallOptions.Key.createWithDefault("circuitBreakerKey", "default"); + private final CircuitBreakerRegistry resilicenceCircuitBreakerRegistry; + private final CircuitBreakerConfigProvider circuitBreakerConfigProvider; + private final Map resilienceCircuitBreakerConfig; + private final ResilienceCircuitBreakerProvider resilienceCircuitBreakerProvider; + private final Clock clock; + + public ResilienceCircuitBreakerInterceptor(Config config, Clock clock) { + this.circuitBreakerConfigProvider = new CircuitBreakerConfigProvider(config); + this.resilienceCircuitBreakerConfig = + ResilienceCircuitBreakerConfigParser.getCircuitBreakerConfigs( + circuitBreakerConfigProvider.getConfigMap()); + this.resilicenceCircuitBreakerRegistry = + new ResilienceCircuitBreakerRegistryProvider(resilienceCircuitBreakerConfig) + .getCircuitBreakerRegistry(); + this.resilienceCircuitBreakerProvider = + new ResilienceCircuitBreakerProvider( + resilicenceCircuitBreakerRegistry, resilienceCircuitBreakerConfig); + this.clock = clock; + } + + @VisibleForTesting + public ResilienceCircuitBreakerInterceptor( + Config config, + Clock clock, + CircuitBreakerRegistry resilicenceCircuitBreakerRegistry, + ResilienceCircuitBreakerProvider resilienceCircuitBreakerProvider) { + this.circuitBreakerConfigProvider = new CircuitBreakerConfigProvider(config); + this.resilienceCircuitBreakerConfig = + ResilienceCircuitBreakerConfigParser.getCircuitBreakerConfigs( + circuitBreakerConfigProvider.getConfigMap()); + this.resilicenceCircuitBreakerRegistry = resilicenceCircuitBreakerRegistry; + this.resilienceCircuitBreakerProvider = resilienceCircuitBreakerProvider; + this.clock = clock; + } + + @Override + protected boolean isCircuitBreakerEnabled() { + return circuitBreakerConfigProvider.isCircuitBreakerEnabled(); + } + + @Override + protected ClientCall createInterceptedCall( + MethodDescriptor method, CallOptions callOptions, Channel next) { + // Get circuit breaker key from CallOptions + String circuitBreakerKey = callOptions.getOption(CIRCUIT_BREAKER_KEY); + CircuitBreaker circuitBreaker = + resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey); + return new ForwardingClientCall.SimpleForwardingClientCall<>( + next.newCall(method, callOptions)) { + @Override + public void start(Listener responseListener, Metadata headers) { + Instant startTime = clock.instant(); + // Wrap response listener to track failures + Listener wrappedListener = + wrapListenerWithCircuitBreaker(responseListener, startTime); + super.start(wrappedListener, headers); + } + + @Override + public void sendMessage(ReqT message) { + if (!circuitBreaker.tryAcquirePermission()) { + logCircuitBreakerRejection(circuitBreakerKey, circuitBreaker); + String rejectionReason = + circuitBreaker.getState() == CircuitBreaker.State.HALF_OPEN + ? "Circuit Breaker is HALF-OPEN and rejecting excess requests" + : "Circuit Breaker is OPEN and blocking requests"; + throw Status.RESOURCE_EXHAUSTED.withDescription(rejectionReason).asRuntimeException(); + } + super.sendMessage(message); + } + + private ForwardingClientCallListener.SimpleForwardingClientCallListener + wrapListenerWithCircuitBreaker(Listener responseListener, Instant startTime) { + return new ForwardingClientCallListener.SimpleForwardingClientCallListener<>( + responseListener) { + @Override + public void onClose(Status status, Metadata trailers) { + long duration = Duration.between(startTime, clock.instant()).toNanos(); + if (status.isOk()) { + circuitBreaker.onSuccess(duration, TimeUnit.NANOSECONDS); + } else { + log.debug( + "Circuit Breaker '{}' detected failure. Status: {}, Description: {}", + circuitBreaker.getName(), + status.getCode(), + status.getDescription()); + circuitBreaker.onError(duration, TimeUnit.NANOSECONDS, status.asRuntimeException()); + } + super.onClose(status, trailers); + } + }; + } + }; + } + + private void logCircuitBreakerRejection(String circuitBreakerKey, CircuitBreaker circuitBreaker) { + Map stateMessages = + Map.of( + CircuitBreaker.State.HALF_OPEN, "is HALF-OPEN and rejecting excess requests.", + CircuitBreaker.State.OPEN, "is OPEN and blocking requests"); + log.debug( + "Circuit Breaker '{}' {}", + circuitBreakerKey, + stateMessages.getOrDefault(circuitBreaker.getState(), "is in an unexpected state")); + } +} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java new file mode 100644 index 0000000..6192d74 --- /dev/null +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java @@ -0,0 +1,56 @@ +package org.hypertrace.circuitbreaker.grpcutils.resilience; + +import io.github.resilience4j.circuitbreaker.CircuitBreaker; +import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; +import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import lombok.extern.slf4j.Slf4j; + +/** Utility class to provide Resilience4j CircuitBreaker */ +@Slf4j +public class ResilienceCircuitBreakerProvider { + + private final CircuitBreakerRegistry circuitBreakerRegistry; + private final Map circuitBreakerConfigMap; + private static final Set attachedCircuitBreakers = ConcurrentHashMap.newKeySet(); + + public ResilienceCircuitBreakerProvider( + CircuitBreakerRegistry circuitBreakerRegistry, + Map circuitBreakerConfigMap) { + this.circuitBreakerRegistry = circuitBreakerRegistry; + this.circuitBreakerConfigMap = circuitBreakerConfigMap; + } + + public CircuitBreaker getCircuitBreaker(String circuitBreakerKey) { + CircuitBreaker circuitBreaker = + circuitBreakerRegistry.circuitBreaker( + circuitBreakerKey, + circuitBreakerConfigMap.getOrDefault( + circuitBreakerKey, circuitBreakerConfigMap.get("default"))); + + if (attachedCircuitBreakers.add(circuitBreakerKey)) { + circuitBreaker + .getEventPublisher() + .onStateTransition( + event -> + log.info( + "State transition: {} for circuit breaker {}", + event.getStateTransition(), + event.getCircuitBreakerName())) + .onCallNotPermitted( + event -> + log.debug( + "Call not permitted: Circuit is OPEN for circuit breaker {}", + event.getCircuitBreakerName())) + .onEvent( + event -> + log.debug( + "Circuit breaker event type {} for circuit breaker name {}", + event.getEventType(), + event.getCircuitBreakerName())); + } + return circuitBreaker; + } +} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java new file mode 100644 index 0000000..85f07c9 --- /dev/null +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java @@ -0,0 +1,39 @@ +package org.hypertrace.circuitbreaker.grpcutils.resilience; + +import io.github.resilience4j.circuitbreaker.CircuitBreaker; +import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; +import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import java.util.Map; +import lombok.extern.slf4j.Slf4j; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigProvider; + +/** Utility class to provide Resilience4j CircuitBreakerRegistry */ +@Slf4j +public class ResilienceCircuitBreakerRegistryProvider { + private final Map circuitBreakerConfigMap; + + public ResilienceCircuitBreakerRegistryProvider( + Map circuitBreakerConfigMap) { + this.circuitBreakerConfigMap = circuitBreakerConfigMap; + } + + public CircuitBreakerRegistry getCircuitBreakerRegistry() { + CircuitBreakerRegistry circuitBreakerRegistry = + CircuitBreakerRegistry.of( + this.circuitBreakerConfigMap.get(CircuitBreakerConfigProvider.DEFAULT_CONFIG_KEY)); + + circuitBreakerRegistry + .getEventPublisher() + .onEntryAdded( + entryAddedEvent -> { + CircuitBreaker addedCircuitBreaker = entryAddedEvent.getAddedEntry(); + log.info("CircuitBreaker {} added", addedCircuitBreaker.getName()); + }) + .onEntryRemoved( + entryRemovedEvent -> { + CircuitBreaker removedCircuitBreaker = entryRemovedEvent.getRemovedEntry(); + log.info("CircuitBreaker {} removed", removedCircuitBreaker.getName()); + }); + return circuitBreakerRegistry; + } +} diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProviderTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProviderTest.java new file mode 100644 index 0000000..4cf405f --- /dev/null +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProviderTest.java @@ -0,0 +1,56 @@ +package org.hypertrace.circuitbreaker.grpcutils; + +import static org.junit.jupiter.api.Assertions.*; + +import com.typesafe.config.Config; +import com.typesafe.config.ConfigFactory; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class CircuitBreakerConfigProviderTest { + + private CircuitBreakerConfigProvider configProvider; + + @BeforeEach + public void setUp() { + Config config = + ConfigFactory.parseString( + "enabled=true\n" + + "default {\n" + + " failureRateThreshold=50.0\n" + + " slowCallRateThreshold=100.0\n" + + " slowCallDurationThreshold=5s\n" + + " slidingWindowSize=10\n" + + " waitDurationInOpenState=1m\n" + + " minimumNumberOfCalls=5\n" + + " permittedNumberOfCallsInHalfOpenState=3\n" + + " slidingWindowType=COUNT_BASED\n" + + "}"); + configProvider = new CircuitBreakerConfigProvider(config); + } + + @Test + public void testIsCircuitBreakerEnabled() { + assertTrue(configProvider.isCircuitBreakerEnabled()); + } + + @Test + public void testGetConfigMap() { + Map configMap = configProvider.getConfigMap(); + assertEquals(1, configMap.size()); + assertTrue(configMap.containsKey("default")); + + CircuitBreakerConfiguration defaultConfig = configMap.get("default"); + assertEquals(50.0f, defaultConfig.getFailureRateThreshold()); + assertEquals(100.0f, defaultConfig.getSlowCallRateThreshold()); + assertEquals(java.time.Duration.ofSeconds(5), defaultConfig.getSlowCallDurationThreshold()); + assertEquals(10, defaultConfig.getSlidingWindowSize()); + assertEquals(java.time.Duration.ofMinutes(1), defaultConfig.getWaitDurationInOpenState()); + assertEquals(5, defaultConfig.getMinimumNumberOfCalls()); + assertEquals(3, defaultConfig.getPermittedNumberOfCallsInHalfOpenState()); + assertEquals( + CircuitBreakerConfiguration.SlidingWindowType.COUNT_BASED, + defaultConfig.getSlidingWindowType()); + } +} diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParserTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParserTest.java new file mode 100644 index 0000000..b899025 --- /dev/null +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParserTest.java @@ -0,0 +1,64 @@ +package org.hypertrace.circuitbreaker.grpcutils.resilience; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; +import java.time.Duration; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration.SlidingWindowType; +import org.junit.jupiter.api.Test; + +public class ResilienceCircuitBreakerConfigParserTest { + + @Test + void testGetConfigWithCountBasedSlidingWindow() { + CircuitBreakerConfiguration configuration = + CircuitBreakerConfiguration.builder() + .failureRateThreshold(50.0f) + .slowCallRateThreshold(30.0f) + .slowCallDurationThreshold(Duration.ofSeconds(2)) + .slidingWindowType(SlidingWindowType.COUNT_BASED) + .slidingWindowSize(100) + .waitDurationInOpenState(Duration.ofSeconds(30)) + .permittedNumberOfCallsInHalfOpenState(10) + .minimumNumberOfCalls(20) + .build(); + + CircuitBreakerConfig config = ResilienceCircuitBreakerConfigParser.getConfig(configuration); + + assertEquals(50.0f, config.getFailureRateThreshold()); + assertEquals(30.0f, config.getSlowCallRateThreshold()); + assertEquals(Duration.ofSeconds(2), config.getSlowCallDurationThreshold()); + assertEquals(CircuitBreakerConfig.SlidingWindowType.COUNT_BASED, config.getSlidingWindowType()); + assertEquals(100, config.getSlidingWindowSize()); + assertEquals(Duration.ofSeconds(30), config.getWaitDurationInOpenState()); + assertEquals(10, config.getPermittedNumberOfCallsInHalfOpenState()); + assertEquals(20, config.getMinimumNumberOfCalls()); + } + + @Test + void testGetConfigWithTimeBasedSlidingWindow() { + CircuitBreakerConfiguration configuration = + CircuitBreakerConfiguration.builder() + .failureRateThreshold(70.0f) + .slowCallRateThreshold(40.0f) + .slowCallDurationThreshold(Duration.ofSeconds(5)) + .slidingWindowType(SlidingWindowType.TIME_BASED) + .slidingWindowSize(60) + .waitDurationInOpenState(Duration.ofMinutes(1)) + .permittedNumberOfCallsInHalfOpenState(5) + .minimumNumberOfCalls(15) + .build(); + + CircuitBreakerConfig config = ResilienceCircuitBreakerConfigParser.getConfig(configuration); + + assertEquals(70.0f, config.getFailureRateThreshold()); + assertEquals(40.0f, config.getSlowCallRateThreshold()); + assertEquals(Duration.ofSeconds(5), config.getSlowCallDurationThreshold()); + assertEquals(CircuitBreakerConfig.SlidingWindowType.TIME_BASED, config.getSlidingWindowType()); + assertEquals(60, config.getSlidingWindowSize()); + assertEquals(Duration.ofMinutes(1), config.getWaitDurationInOpenState()); + assertEquals(5, config.getPermittedNumberOfCallsInHalfOpenState()); + assertEquals(15, config.getMinimumNumberOfCalls()); + } +} diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java new file mode 100644 index 0000000..c7c4f04 --- /dev/null +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java @@ -0,0 +1,110 @@ +package org.hypertrace.circuitbreaker.grpcutils.resilience; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.typesafe.config.Config; +import com.typesafe.config.ConfigFactory; +import io.github.resilience4j.circuitbreaker.CircuitBreaker; +import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import io.grpc.CallOptions; +import io.grpc.Channel; +import io.grpc.ClientCall; +import io.grpc.ForwardingClientCall; +import io.grpc.Metadata; +import io.grpc.MethodDescriptor; +import io.grpc.StatusRuntimeException; +import java.time.Clock; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +class ResilienceCircuitBreakerInterceptorTest { + + private final Config config = + ConfigFactory.parseString( + "enabled=true\n" + + "default {\n" + + " failureRateThreshold=50.0\n" + + " slowCallRateThreshold=100.0\n" + + " slowCallDurationThreshold=5s\n" + + " slidingWindowSize=10\n" + + " waitDurationInOpenState=1m\n" + + " minimumNumberOfCalls=5\n" + + " permittedNumberOfCallsInHalfOpenState=3\n" + + " slidingWindowType=COUNT_BASED\n" + + "}"); + private final Clock clock = Clock.systemUTC(); + private final CircuitBreakerRegistry mockRegistry = Mockito.mock(CircuitBreakerRegistry.class); + private final CircuitBreaker mockCircuitBreaker = Mockito.mock(CircuitBreaker.class); + private final Channel mockChannel = Mockito.mock(Channel.class); + private final ClientCall.Listener mockListener = mock(ClientCall.Listener.class); + private final ResilienceCircuitBreakerProvider mockCircuitBreakerProvider = + Mockito.mock(ResilienceCircuitBreakerProvider.class); + + @Test + void testCircuitBreakerEnabled_InterceptsCall() { + MethodDescriptor methodDescriptor = mock(MethodDescriptor.class); + CallOptions callOptions = + CallOptions.DEFAULT.withOption( + ResilienceCircuitBreakerInterceptor.CIRCUIT_BREAKER_KEY, "test-key"); + when(mockCircuitBreakerProvider.getCircuitBreaker("test-key")).thenReturn(mockCircuitBreaker); + ResilienceCircuitBreakerInterceptor interceptor = + new ResilienceCircuitBreakerInterceptor( + config, clock, mockRegistry, mockCircuitBreakerProvider); + + ClientCall interceptedCall = + spy(interceptor.interceptCall(methodDescriptor, callOptions, mockChannel)); + doNothing().when(interceptedCall).start(any(), any()); + assertNotNull(interceptedCall); + assertDoesNotThrow(() -> interceptedCall.start(mockListener, new Metadata())); + verify(interceptedCall).start(eq(mockListener), any(Metadata.class)); + } + + @Test + void testCircuitBreakerRejectsRequest() { + MethodDescriptor methodDescriptor = mock(MethodDescriptor.class); + CallOptions callOptions = + CallOptions.DEFAULT.withOption( + ResilienceCircuitBreakerInterceptor.CIRCUIT_BREAKER_KEY, "test-key"); + when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(false); + when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.OPEN); + when(mockCircuitBreakerProvider.getCircuitBreaker("test-key")).thenReturn(mockCircuitBreaker); + ResilienceCircuitBreakerInterceptor interceptor = + new ResilienceCircuitBreakerInterceptor( + config, clock, mockRegistry, mockCircuitBreakerProvider); + + ClientCall interceptedCall = + interceptor.interceptCall(methodDescriptor, callOptions, mockChannel); + assertThrows(StatusRuntimeException.class, () -> interceptedCall.sendMessage(new Object())); + } + + @Test + void testCircuitBreakerSuccess() { + MethodDescriptor methodDescriptor = mock(MethodDescriptor.class); + CallOptions callOptions = + CallOptions.DEFAULT.withOption( + ResilienceCircuitBreakerInterceptor.CIRCUIT_BREAKER_KEY, "test-key"); + when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); + when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.CLOSED); + when(mockCircuitBreakerProvider.getCircuitBreaker("test-key")).thenReturn(mockCircuitBreaker); + ResilienceCircuitBreakerInterceptor interceptor = + spy( + new ResilienceCircuitBreakerInterceptor( + config, clock, mockRegistry, mockCircuitBreakerProvider)); + ClientCall interceptedCall = + spy(interceptor.createInterceptedCall(methodDescriptor, callOptions, mockChannel)); + Mockito.doNothing().when((ForwardingClientCall) interceptedCall).sendMessage(Mockito.any()); + interceptedCall.sendMessage(new Object()); + // Assert + verify(interceptedCall, times(1)).sendMessage(any()); + } +} From 6a058f7899c3e52832bb6f1bc96d1c03fe30c1c5 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Sat, 15 Mar 2025 17:27:10 +0530 Subject: [PATCH 03/15] Changed design of CircuitBreakerInterceptor --- grpc-circuitbreaker-utils/build.gradle.kts | 1 + ...r.java => CircuitBreakerConfigParser.java} | 52 +++------- .../CircuitBreakerConfiguration.java | 33 ++----- .../grpcutils/CircuitBreakerThresholds.java | 31 ++++++ .../ResilienceCircuitBreakerConfigParser.java | 15 +-- .../ResilienceCircuitBreakerInterceptor.java | 60 +++++++----- .../ResilienceCircuitBreakerProvider.java | 62 ++++++------ ...ilienceCircuitBreakerRegistryProvider.java | 7 +- .../CircuitBreakerConfigProviderTest.java | 56 ----------- ...ilienceCircuitBreakerConfigParserTest.java | 71 +++++++------- ...silienceCircuitBreakerInterceptorTest.java | 98 +++++++++++++++---- 11 files changed, 251 insertions(+), 235 deletions(-) rename grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/{CircuitBreakerConfigProvider.java => CircuitBreakerConfigParser.java} (56%) create mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerThresholds.java delete mode 100644 grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProviderTest.java diff --git a/grpc-circuitbreaker-utils/build.gradle.kts b/grpc-circuitbreaker-utils/build.gradle.kts index ca10605..989024e 100644 --- a/grpc-circuitbreaker-utils/build.gradle.kts +++ b/grpc-circuitbreaker-utils/build.gradle.kts @@ -9,6 +9,7 @@ dependencies { api(platform("io.grpc:grpc-bom:1.68.3")) api("io.grpc:grpc-api") + api(project(":grpc-context-utils")) implementation("org.slf4j:slf4j-api:1.7.36") implementation("io.github.resilience4j:resilience4j-circuitbreaker:1.7.1") diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java similarity index 56% rename from grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProvider.java rename to grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java index 0d95734..09062fe 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java @@ -6,13 +6,10 @@ import lombok.extern.slf4j.Slf4j; @Slf4j -public class CircuitBreakerConfigProvider { +public class CircuitBreakerConfigParser { public static final String DEFAULT_CONFIG_KEY = "default"; - // Whether to enable circuit breaker or not. - private static final String ENABLED = "enabled"; - // Percentage of failures to trigger OPEN state private static final String FAILURE_RATE_THRESHOLD = "failureRateThreshold"; // Percentage of slow calls to trigger OPEN state @@ -31,42 +28,23 @@ public class CircuitBreakerConfigProvider { "permittedNumberOfCallsInHalfOpenState"; private static final String SLIDING_WINDOW_TYPE = "slidingWindowType"; - // Global flag for circuit breaker enablement - private boolean circuitBreakerEnabled = false; - private Map circuitBreakerConfigurationMap; - - public CircuitBreakerConfigProvider(Config config) { - this.initialize(config); - } - - /** Checks if Circuit Breaker is globally enabled. */ - public boolean isCircuitBreakerEnabled() { - return circuitBreakerEnabled; - } - - private void initialize(Config circuitBreakerConfig) { - circuitBreakerEnabled = - circuitBreakerConfig.hasPath(ENABLED) && circuitBreakerConfig.getBoolean(ENABLED); - this.circuitBreakerConfigurationMap = - circuitBreakerConfig.root().keySet().stream() - .filter(key -> !key.equals(ENABLED)) // Ignore the global enabled flag + public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder parseConfig( + Config config) { + CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder builder = + CircuitBreakerConfiguration.builder(); + Map circuitBreakerThresholdsMap = + config.root().keySet().stream() .collect( Collectors.toMap( key -> key, // Circuit breaker key - key -> createCircuitBreakerConfig(circuitBreakerConfig.getConfig(key)))); - log.info( - "Loaded {} circuit breaker configurations, Global Enabled: {}. Configs: {}", - circuitBreakerConfigurationMap.size(), - circuitBreakerEnabled, - circuitBreakerConfigurationMap); - } - - public Map getConfigMap() { - return circuitBreakerConfigurationMap; + key -> buildCircuitBreakerThresholds(config.getConfig(key)))); + builder.circuitBreakerThresholdsMap(circuitBreakerThresholdsMap); + log.info("Loaded circuit breaker configs: {}", circuitBreakerThresholdsMap); + return builder; } - private CircuitBreakerConfiguration createCircuitBreakerConfig(Config config) { - return CircuitBreakerConfiguration.builder() + private static CircuitBreakerThresholds buildCircuitBreakerThresholds(Config config) { + return CircuitBreakerThresholds.builder() .failureRateThreshold((float) config.getDouble(FAILURE_RATE_THRESHOLD)) .slowCallRateThreshold((float) config.getDouble(SLOW_CALL_RATE_THRESHOLD)) .slowCallDurationThreshold(config.getDuration(SLOW_CALL_DURATION_THRESHOLD)) @@ -79,8 +57,8 @@ private CircuitBreakerConfiguration createCircuitBreakerConfig(Config config) { .build(); } - private CircuitBreakerConfiguration.SlidingWindowType getSlidingWindowType( + private static CircuitBreakerThresholds.SlidingWindowType getSlidingWindowType( String slidingWindowType) { - return CircuitBreakerConfiguration.SlidingWindowType.valueOf(slidingWindowType); + return CircuitBreakerThresholds.SlidingWindowType.valueOf(slidingWindowType); } } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java index 355a24d..6aabbef 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java @@ -1,33 +1,16 @@ package org.hypertrace.circuitbreaker.grpcutils; -import java.time.Duration; +import java.util.Map; +import java.util.function.BiFunction; import lombok.Builder; -import lombok.Setter; import lombok.Value; +import org.hypertrace.core.grpcutils.context.RequestContext; @Value @Builder -@Setter -public class CircuitBreakerConfiguration { - // Percentage of failures to trigger OPEN state - float failureRateThreshold; - // Percentage of slow calls to trigger OPEN state - float slowCallRateThreshold; - // Define what a "slow" call is - Duration slowCallDurationThreshold; - // Number of calls to consider in the sliding window - SlidingWindowType slidingWindowType; - int slidingWindowSize; - // Time before retrying after OPEN state - Duration waitDurationInOpenState; - // Minimum calls before evaluating failure rate - int minimumNumberOfCalls; - // Calls allowed in HALF_OPEN state before deciding to - // CLOSE or OPEN again - int permittedNumberOfCallsInHalfOpenState; - - public enum SlidingWindowType { - COUNT_BASED, - TIME_BASED - } +public class CircuitBreakerConfiguration { + Class requestClass; + BiFunction keyFunction; + boolean enabled; + Map circuitBreakerThresholdsMap; } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerThresholds.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerThresholds.java new file mode 100644 index 0000000..74b918a --- /dev/null +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerThresholds.java @@ -0,0 +1,31 @@ +package org.hypertrace.circuitbreaker.grpcutils; + +import java.time.Duration; +import lombok.Builder; +import lombok.Value; + +@Value +@Builder +public class CircuitBreakerThresholds { + // Percentage of failures to trigger OPEN state + float failureRateThreshold; + // Percentage of slow calls to trigger OPEN state + float slowCallRateThreshold; + // Define what a "slow" call is + Duration slowCallDurationThreshold; + // Number of calls to consider in the sliding window + SlidingWindowType slidingWindowType; + int slidingWindowSize; + // Time before retrying after OPEN state + Duration waitDurationInOpenState; + // Minimum calls before evaluating failure rate + int minimumNumberOfCalls; + // Calls allowed in HALF_OPEN state before deciding to + // CLOSE or OPEN again + int permittedNumberOfCallsInHalfOpenState; + + public enum SlidingWindowType { + COUNT_BASED, + TIME_BASED + } +} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParser.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParser.java index ca1e1ff..bfa3e7e 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParser.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParser.java @@ -3,23 +3,23 @@ import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; import java.util.Map; import java.util.stream.Collectors; -import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerThresholds; /** Utility class to parse CircuitBreakerConfiguration to Resilience4j CircuitBreakerConfig */ public class ResilienceCircuitBreakerConfigParser { public static Map getCircuitBreakerConfigs( - Map configurationMap) { + Map configurationMap) { return configurationMap.entrySet().stream() .collect(Collectors.toMap(Map.Entry::getKey, entry -> getConfig(entry.getValue()))); } - static CircuitBreakerConfig getConfig(CircuitBreakerConfiguration configuration) { + static CircuitBreakerConfig getConfig(CircuitBreakerThresholds configuration) { return CircuitBreakerConfig.custom() .failureRateThreshold(configuration.getFailureRateThreshold()) .slowCallRateThreshold(configuration.getSlowCallRateThreshold()) .slowCallDurationThreshold(configuration.getSlowCallDurationThreshold()) - .slidingWindowType(getSlidingWindowType(configuration)) + .slidingWindowType(getSlidingWindowType(configuration.getSlidingWindowType())) .slidingWindowSize(configuration.getSlidingWindowSize()) .waitDurationInOpenState(configuration.getWaitDurationInOpenState()) .permittedNumberOfCallsInHalfOpenState( @@ -29,8 +29,11 @@ static CircuitBreakerConfig getConfig(CircuitBreakerConfiguration configuration) } private static CircuitBreakerConfig.SlidingWindowType getSlidingWindowType( - CircuitBreakerConfiguration configuration) { - switch (configuration.getSlidingWindowType()) { + CircuitBreakerThresholds.SlidingWindowType slidingWindowType) { + if (slidingWindowType == null) { + return CircuitBreakerConfig.SlidingWindowType.TIME_BASED; + } + switch (slidingWindowType) { case COUNT_BASED: return CircuitBreakerConfig.SlidingWindowType.COUNT_BASED; case TIME_BASED: diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java index 525b140..42b3f6d 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java @@ -1,8 +1,6 @@ package org.hypertrace.circuitbreaker.grpcutils.resilience; import com.google.common.annotations.VisibleForTesting; -import com.google.inject.Singleton; -import com.typesafe.config.Config; import io.github.resilience4j.circuitbreaker.CircuitBreaker; import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; @@ -20,45 +18,44 @@ import java.util.Map; import java.util.concurrent.TimeUnit; import lombok.extern.slf4j.Slf4j; -import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigProvider; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration; import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerInterceptor; +import org.hypertrace.core.grpcutils.context.RequestContext; @Slf4j -@Singleton public class ResilienceCircuitBreakerInterceptor extends CircuitBreakerInterceptor { - public static final CallOptions.Key CIRCUIT_BREAKER_KEY = - CallOptions.Key.createWithDefault("circuitBreakerKey", "default"); private final CircuitBreakerRegistry resilicenceCircuitBreakerRegistry; - private final CircuitBreakerConfigProvider circuitBreakerConfigProvider; - private final Map resilienceCircuitBreakerConfig; + private final Map resilienceCircuitBreakerConfigMap; private final ResilienceCircuitBreakerProvider resilienceCircuitBreakerProvider; + private final CircuitBreakerConfiguration circuitBreakerConfiguration; private final Clock clock; - public ResilienceCircuitBreakerInterceptor(Config config, Clock clock) { - this.circuitBreakerConfigProvider = new CircuitBreakerConfigProvider(config); - this.resilienceCircuitBreakerConfig = + public ResilienceCircuitBreakerInterceptor( + CircuitBreakerConfiguration circuitBreakerConfiguration, Clock clock) { + this.circuitBreakerConfiguration = circuitBreakerConfiguration; + this.clock = clock; + this.resilienceCircuitBreakerConfigMap = ResilienceCircuitBreakerConfigParser.getCircuitBreakerConfigs( - circuitBreakerConfigProvider.getConfigMap()); + circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()); this.resilicenceCircuitBreakerRegistry = - new ResilienceCircuitBreakerRegistryProvider(resilienceCircuitBreakerConfig) + new ResilienceCircuitBreakerRegistryProvider(resilienceCircuitBreakerConfigMap) .getCircuitBreakerRegistry(); this.resilienceCircuitBreakerProvider = new ResilienceCircuitBreakerProvider( - resilicenceCircuitBreakerRegistry, resilienceCircuitBreakerConfig); - this.clock = clock; + resilicenceCircuitBreakerRegistry, resilienceCircuitBreakerConfigMap); } @VisibleForTesting public ResilienceCircuitBreakerInterceptor( - Config config, Clock clock, CircuitBreakerRegistry resilicenceCircuitBreakerRegistry, - ResilienceCircuitBreakerProvider resilienceCircuitBreakerProvider) { - this.circuitBreakerConfigProvider = new CircuitBreakerConfigProvider(config); - this.resilienceCircuitBreakerConfig = + ResilienceCircuitBreakerProvider resilienceCircuitBreakerProvider, + CircuitBreakerConfiguration circuitBreakerConfiguration) { + this.circuitBreakerConfiguration = circuitBreakerConfiguration; + this.resilienceCircuitBreakerConfigMap = ResilienceCircuitBreakerConfigParser.getCircuitBreakerConfigs( - circuitBreakerConfigProvider.getConfigMap()); + circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()); this.resilicenceCircuitBreakerRegistry = resilicenceCircuitBreakerRegistry; this.resilienceCircuitBreakerProvider = resilienceCircuitBreakerProvider; this.clock = clock; @@ -66,18 +63,17 @@ public ResilienceCircuitBreakerInterceptor( @Override protected boolean isCircuitBreakerEnabled() { - return circuitBreakerConfigProvider.isCircuitBreakerEnabled(); + return circuitBreakerConfiguration.isEnabled(); } @Override protected ClientCall createInterceptedCall( MethodDescriptor method, CallOptions callOptions, Channel next) { - // Get circuit breaker key from CallOptions - String circuitBreakerKey = callOptions.getOption(CIRCUIT_BREAKER_KEY); - CircuitBreaker circuitBreaker = - resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey); return new ForwardingClientCall.SimpleForwardingClientCall<>( next.newCall(method, callOptions)) { + CircuitBreaker circuitBreaker; + String circuitBreakerKey; + @Override public void start(Listener responseListener, Metadata headers) { Instant startTime = clock.instant(); @@ -87,8 +83,22 @@ public void start(Listener responseListener, Metadata headers) { super.start(wrappedListener, headers); } + @SuppressWarnings("unchecked") @Override public void sendMessage(ReqT message) { + CircuitBreakerConfiguration config = + (CircuitBreakerConfiguration) circuitBreakerConfiguration; + if (config.getRequestClass() == null + || (!message.getClass().equals(config.getRequestClass()))) { + log.warn("Invalid config for message type: {}", message.getClass()); + super.sendMessage(message); + } + if (config.getKeyFunction() != null) { + circuitBreakerKey = config.getKeyFunction().apply(RequestContext.CURRENT.get(), message); + } else { + circuitBreakerKey = "default"; + } + circuitBreaker = resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey); if (!circuitBreaker.tryAcquirePermission()) { logCircuitBreakerRejection(circuitBreakerKey, circuitBreaker); String rejectionReason = diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java index 6192d74..4f83478 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java @@ -3,18 +3,19 @@ import io.github.resilience4j.circuitbreaker.CircuitBreaker; import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import jakarta.inject.Singleton; import java.util.Map; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import lombok.extern.slf4j.Slf4j; /** Utility class to provide Resilience4j CircuitBreaker */ @Slf4j +@Singleton public class ResilienceCircuitBreakerProvider { private final CircuitBreakerRegistry circuitBreakerRegistry; private final Map circuitBreakerConfigMap; - private static final Set attachedCircuitBreakers = ConcurrentHashMap.newKeySet(); + private final Map circuitBreakerCache = new ConcurrentHashMap<>(); public ResilienceCircuitBreakerProvider( CircuitBreakerRegistry circuitBreakerRegistry, @@ -24,33 +25,34 @@ public ResilienceCircuitBreakerProvider( } public CircuitBreaker getCircuitBreaker(String circuitBreakerKey) { - CircuitBreaker circuitBreaker = - circuitBreakerRegistry.circuitBreaker( - circuitBreakerKey, - circuitBreakerConfigMap.getOrDefault( - circuitBreakerKey, circuitBreakerConfigMap.get("default"))); - - if (attachedCircuitBreakers.add(circuitBreakerKey)) { - circuitBreaker - .getEventPublisher() - .onStateTransition( - event -> - log.info( - "State transition: {} for circuit breaker {}", - event.getStateTransition(), - event.getCircuitBreakerName())) - .onCallNotPermitted( - event -> - log.debug( - "Call not permitted: Circuit is OPEN for circuit breaker {}", - event.getCircuitBreakerName())) - .onEvent( - event -> - log.debug( - "Circuit breaker event type {} for circuit breaker name {}", - event.getEventType(), - event.getCircuitBreakerName())); - } - return circuitBreaker; + return circuitBreakerCache.computeIfAbsent( + circuitBreakerKey, + key -> { + CircuitBreaker circuitBreaker = + circuitBreakerRegistry.circuitBreaker( + circuitBreakerKey, + circuitBreakerConfigMap.getOrDefault( + circuitBreakerKey, circuitBreakerConfigMap.get("default"))); + circuitBreaker + .getEventPublisher() + .onStateTransition( + event -> + log.info( + "State transition: {} for circuit breaker {}", + event.getStateTransition(), + event.getCircuitBreakerName())) + .onCallNotPermitted( + event -> + log.debug( + "Call not permitted: Circuit is OPEN for circuit breaker {}", + event.getCircuitBreakerName())) + .onEvent( + event -> + log.debug( + "Circuit breaker event type {} for circuit breaker name {}", + event.getEventType(), + event.getCircuitBreakerName())); + return circuitBreaker; + }); } } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java index 85f07c9..92243e0 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java @@ -3,12 +3,14 @@ import io.github.resilience4j.circuitbreaker.CircuitBreaker; import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import jakarta.inject.Singleton; import java.util.Map; import lombok.extern.slf4j.Slf4j; -import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigProvider; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigParser; /** Utility class to provide Resilience4j CircuitBreakerRegistry */ @Slf4j +@Singleton public class ResilienceCircuitBreakerRegistryProvider { private final Map circuitBreakerConfigMap; @@ -20,8 +22,7 @@ public ResilienceCircuitBreakerRegistryProvider( public CircuitBreakerRegistry getCircuitBreakerRegistry() { CircuitBreakerRegistry circuitBreakerRegistry = CircuitBreakerRegistry.of( - this.circuitBreakerConfigMap.get(CircuitBreakerConfigProvider.DEFAULT_CONFIG_KEY)); - + this.circuitBreakerConfigMap.get(CircuitBreakerConfigParser.DEFAULT_CONFIG_KEY)); circuitBreakerRegistry .getEventPublisher() .onEntryAdded( diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProviderTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProviderTest.java deleted file mode 100644 index 4cf405f..0000000 --- a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigProviderTest.java +++ /dev/null @@ -1,56 +0,0 @@ -package org.hypertrace.circuitbreaker.grpcutils; - -import static org.junit.jupiter.api.Assertions.*; - -import com.typesafe.config.Config; -import com.typesafe.config.ConfigFactory; -import java.util.Map; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -public class CircuitBreakerConfigProviderTest { - - private CircuitBreakerConfigProvider configProvider; - - @BeforeEach - public void setUp() { - Config config = - ConfigFactory.parseString( - "enabled=true\n" - + "default {\n" - + " failureRateThreshold=50.0\n" - + " slowCallRateThreshold=100.0\n" - + " slowCallDurationThreshold=5s\n" - + " slidingWindowSize=10\n" - + " waitDurationInOpenState=1m\n" - + " minimumNumberOfCalls=5\n" - + " permittedNumberOfCallsInHalfOpenState=3\n" - + " slidingWindowType=COUNT_BASED\n" - + "}"); - configProvider = new CircuitBreakerConfigProvider(config); - } - - @Test - public void testIsCircuitBreakerEnabled() { - assertTrue(configProvider.isCircuitBreakerEnabled()); - } - - @Test - public void testGetConfigMap() { - Map configMap = configProvider.getConfigMap(); - assertEquals(1, configMap.size()); - assertTrue(configMap.containsKey("default")); - - CircuitBreakerConfiguration defaultConfig = configMap.get("default"); - assertEquals(50.0f, defaultConfig.getFailureRateThreshold()); - assertEquals(100.0f, defaultConfig.getSlowCallRateThreshold()); - assertEquals(java.time.Duration.ofSeconds(5), defaultConfig.getSlowCallDurationThreshold()); - assertEquals(10, defaultConfig.getSlidingWindowSize()); - assertEquals(java.time.Duration.ofMinutes(1), defaultConfig.getWaitDurationInOpenState()); - assertEquals(5, defaultConfig.getMinimumNumberOfCalls()); - assertEquals(3, defaultConfig.getPermittedNumberOfCallsInHalfOpenState()); - assertEquals( - CircuitBreakerConfiguration.SlidingWindowType.COUNT_BASED, - defaultConfig.getSlidingWindowType()); - } -} diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParserTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParserTest.java index b899025..a9c5286 100644 --- a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParserTest.java +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParserTest.java @@ -1,64 +1,69 @@ package org.hypertrace.circuitbreaker.grpcutils.resilience; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; import java.time.Duration; -import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration; -import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration.SlidingWindowType; +import java.util.HashMap; +import java.util.Map; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerThresholds; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; public class ResilienceCircuitBreakerConfigParserTest { @Test - void testGetConfigWithCountBasedSlidingWindow() { - CircuitBreakerConfiguration configuration = - CircuitBreakerConfiguration.builder() + void shouldParseValidConfiguration() { + CircuitBreakerThresholds thresholds = + CircuitBreakerThresholds.builder() .failureRateThreshold(50.0f) .slowCallRateThreshold(30.0f) .slowCallDurationThreshold(Duration.ofSeconds(2)) - .slidingWindowType(SlidingWindowType.COUNT_BASED) + .slidingWindowType(CircuitBreakerThresholds.SlidingWindowType.TIME_BASED) .slidingWindowSize(100) - .waitDurationInOpenState(Duration.ofSeconds(30)) - .permittedNumberOfCallsInHalfOpenState(10) + .waitDurationInOpenState(Duration.ofSeconds(60)) + .permittedNumberOfCallsInHalfOpenState(5) .minimumNumberOfCalls(20) .build(); - CircuitBreakerConfig config = ResilienceCircuitBreakerConfigParser.getConfig(configuration); + Map configMap = new HashMap<>(); + configMap.put("testService", thresholds); + + Map result = + ResilienceCircuitBreakerConfigParser.getCircuitBreakerConfigs(configMap); + + Assertions.assertTrue(result.containsKey("testService")); + CircuitBreakerConfig config = result.get("testService"); assertEquals(50.0f, config.getFailureRateThreshold()); assertEquals(30.0f, config.getSlowCallRateThreshold()); assertEquals(Duration.ofSeconds(2), config.getSlowCallDurationThreshold()); - assertEquals(CircuitBreakerConfig.SlidingWindowType.COUNT_BASED, config.getSlidingWindowType()); + assertEquals(CircuitBreakerConfig.SlidingWindowType.TIME_BASED, config.getSlidingWindowType()); assertEquals(100, config.getSlidingWindowSize()); - assertEquals(Duration.ofSeconds(30), config.getWaitDurationInOpenState()); - assertEquals(10, config.getPermittedNumberOfCallsInHalfOpenState()); + assertEquals(5, config.getPermittedNumberOfCallsInHalfOpenState()); assertEquals(20, config.getMinimumNumberOfCalls()); } @Test - void testGetConfigWithTimeBasedSlidingWindow() { - CircuitBreakerConfiguration configuration = - CircuitBreakerConfiguration.builder() - .failureRateThreshold(70.0f) - .slowCallRateThreshold(40.0f) - .slowCallDurationThreshold(Duration.ofSeconds(5)) - .slidingWindowType(SlidingWindowType.TIME_BASED) - .slidingWindowSize(60) - .waitDurationInOpenState(Duration.ofMinutes(1)) + void shouldUseDefaultSlidingWindowTypeForInvalidType() { + CircuitBreakerThresholds thresholds = + CircuitBreakerThresholds.builder() + .failureRateThreshold(50.0f) + .slowCallRateThreshold(30.0f) + .slowCallDurationThreshold(Duration.ofSeconds(2)) + .slidingWindowSize(100) + .waitDurationInOpenState(Duration.ofSeconds(60)) .permittedNumberOfCallsInHalfOpenState(5) - .minimumNumberOfCalls(15) - .build(); - - CircuitBreakerConfig config = ResilienceCircuitBreakerConfigParser.getConfig(configuration); - - assertEquals(70.0f, config.getFailureRateThreshold()); - assertEquals(40.0f, config.getSlowCallRateThreshold()); - assertEquals(Duration.ofSeconds(5), config.getSlowCallDurationThreshold()); + .minimumNumberOfCalls(20) + .build(); // Invalid type scenario + CircuitBreakerConfig config = ResilienceCircuitBreakerConfigParser.getConfig(thresholds); assertEquals(CircuitBreakerConfig.SlidingWindowType.TIME_BASED, config.getSlidingWindowType()); - assertEquals(60, config.getSlidingWindowSize()); - assertEquals(Duration.ofMinutes(1), config.getWaitDurationInOpenState()); - assertEquals(5, config.getPermittedNumberOfCallsInHalfOpenState()); - assertEquals(15, config.getMinimumNumberOfCalls()); + } + + @Test + void shouldThrowExceptionWhenConfigurationIsNull() { + assertThrows( + NullPointerException.class, () -> ResilienceCircuitBreakerConfigParser.getConfig(null)); } } diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java index c7c4f04..3bad85d 100644 --- a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java @@ -8,7 +8,6 @@ import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -24,6 +23,10 @@ import io.grpc.MethodDescriptor; import io.grpc.StatusRuntimeException; import java.time.Clock; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigParser; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration; +import org.hypertrace.core.grpcutils.context.RequestContext; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -31,8 +34,7 @@ class ResilienceCircuitBreakerInterceptorTest { private final Config config = ConfigFactory.parseString( - "enabled=true\n" - + "default {\n" + "default {\n" + " failureRateThreshold=50.0\n" + " slowCallRateThreshold=100.0\n" + " slowCallDurationThreshold=5s\n" @@ -53,14 +55,22 @@ class ResilienceCircuitBreakerInterceptorTest { @Test void testCircuitBreakerEnabled_InterceptsCall() { MethodDescriptor methodDescriptor = mock(MethodDescriptor.class); - CallOptions callOptions = - CallOptions.DEFAULT.withOption( - ResilienceCircuitBreakerInterceptor.CIRCUIT_BREAKER_KEY, "test-key"); when(mockCircuitBreakerProvider.getCircuitBreaker("test-key")).thenReturn(mockCircuitBreaker); + CircuitBreakerConfiguration circuitBreakerConfiguration = + CircuitBreakerConfigParser.parseConfig(config) + .enabled(true) + .keyFunction( + (requestContext, request) -> { + GetGithubIntegrationsRequest getGithubIntegrationsRequest = + (GetGithubIntegrationsRequest) request; + return requestContext.getTenantId() + "-" + getGithubIntegrationsRequest.getUrl(); + }) + .build(); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( - config, clock, mockRegistry, mockCircuitBreakerProvider); + clock, mockRegistry, mockCircuitBreakerProvider, circuitBreakerConfiguration); + CallOptions callOptions = Mockito.mock(CallOptions.class); ClientCall interceptedCall = spy(interceptor.interceptCall(methodDescriptor, callOptions, mockChannel)); doNothing().when(interceptedCall).start(any(), any()); @@ -72,39 +82,87 @@ void testCircuitBreakerEnabled_InterceptsCall() { @Test void testCircuitBreakerRejectsRequest() { MethodDescriptor methodDescriptor = mock(MethodDescriptor.class); - CallOptions callOptions = - CallOptions.DEFAULT.withOption( - ResilienceCircuitBreakerInterceptor.CIRCUIT_BREAKER_KEY, "test-key"); + CallOptions callOptions = Mockito.mock(CallOptions.class); when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(false); when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.OPEN); - when(mockCircuitBreakerProvider.getCircuitBreaker("test-key")).thenReturn(mockCircuitBreaker); + when(mockCircuitBreakerProvider.getCircuitBreaker("tenant1-http://localhost:9000")) + .thenReturn(mockCircuitBreaker); + CircuitBreakerConfiguration circuitBreakerConfiguration = + CircuitBreakerConfigParser.parseConfig(config) + .enabled(true) + .requestClass(GetGithubIntegrationsRequest.class) + .keyFunction( + (requestContext, request) -> { + return requestContext.getTenantId().get() + "-" + request.getUrl(); + }) + .build(); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( - config, clock, mockRegistry, mockCircuitBreakerProvider); + clock, mockRegistry, mockCircuitBreakerProvider, circuitBreakerConfiguration); ClientCall interceptedCall = interceptor.interceptCall(methodDescriptor, callOptions, mockChannel); - assertThrows(StatusRuntimeException.class, () -> interceptedCall.sendMessage(new Object())); + assertThrows( + StatusRuntimeException.class, + () -> { + RequestContext.forTenantId("tenant1") + .call( + () -> { + interceptedCall.sendMessage( + new GetGithubIntegrationsRequest("http://localhost:9000")); + return null; + }); + }); } @Test + @Disabled void testCircuitBreakerSuccess() { MethodDescriptor methodDescriptor = mock(MethodDescriptor.class); - CallOptions callOptions = - CallOptions.DEFAULT.withOption( - ResilienceCircuitBreakerInterceptor.CIRCUIT_BREAKER_KEY, "test-key"); + CallOptions callOptions = Mockito.mock(CallOptions.class); when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.CLOSED); when(mockCircuitBreakerProvider.getCircuitBreaker("test-key")).thenReturn(mockCircuitBreaker); + CircuitBreakerConfiguration circuitBreakerConfiguration = + CircuitBreakerConfigParser.parseConfig(config) + .enabled(true) + .requestClass(GetGithubIntegrationsRequest.class) + .keyFunction( + (requestContext, request) -> { + return requestContext.getTenantId().get() + "-" + request.getUrl(); + }) + .build(); ResilienceCircuitBreakerInterceptor interceptor = spy( new ResilienceCircuitBreakerInterceptor( - config, clock, mockRegistry, mockCircuitBreakerProvider)); + clock, mockRegistry, mockCircuitBreakerProvider, circuitBreakerConfiguration)); + ClientCall interceptedCall = - spy(interceptor.createInterceptedCall(methodDescriptor, callOptions, mockChannel)); + interceptor.createInterceptedCall(methodDescriptor, callOptions, mockChannel); + ClientCall spyCall = spy(interceptedCall); Mockito.doNothing().when((ForwardingClientCall) interceptedCall).sendMessage(Mockito.any()); - interceptedCall.sendMessage(new Object()); + // Act + RequestContext.forTenantId("tenant1") + .call( + () -> { + spyCall.sendMessage(new Object()); + return null; + }); + // Assert - verify(interceptedCall, times(1)).sendMessage(any()); + verify(spyCall).sendMessage(any()); + verify(mockCircuitBreaker).tryAcquirePermission(); + } + + private static class GetGithubIntegrationsRequest { + private final String url; + + public GetGithubIntegrationsRequest(String url) { + this.url = url; + } + + public String getUrl() { + return url; + } } } From 878a8cfa1069d6a3e5e1c00a74a3c80009bd16c4 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Sun, 16 Mar 2025 17:39:13 +0530 Subject: [PATCH 04/15] Fixed tests for CircuitBreakerInterceptor --- grpc-circuitbreaker-utils/build.gradle.kts | 1 + .../ResilienceCircuitBreakerInterceptor.java | 16 +- ...silienceCircuitBreakerInterceptorTest.java | 254 +++++++++--------- 3 files changed, 137 insertions(+), 134 deletions(-) diff --git a/grpc-circuitbreaker-utils/build.gradle.kts b/grpc-circuitbreaker-utils/build.gradle.kts index 989024e..e838cbc 100644 --- a/grpc-circuitbreaker-utils/build.gradle.kts +++ b/grpc-circuitbreaker-utils/build.gradle.kts @@ -21,6 +21,7 @@ dependencies { testImplementation("org.junit.jupiter:junit-jupiter:5.8.2") testImplementation("org.mockito:mockito-core:5.8.0") + testImplementation("org.mockito:mockito-junit-jupiter:5.8.0") } tasks.test { diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java index 42b3f6d..0930e9b 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java @@ -88,15 +88,15 @@ public void start(Listener responseListener, Metadata headers) { public void sendMessage(ReqT message) { CircuitBreakerConfiguration config = (CircuitBreakerConfiguration) circuitBreakerConfiguration; - if (config.getRequestClass() == null - || (!message.getClass().equals(config.getRequestClass()))) { - log.warn("Invalid config for message type: {}", message.getClass()); - super.sendMessage(message); - } - if (config.getKeyFunction() != null) { - circuitBreakerKey = config.getKeyFunction().apply(RequestContext.CURRENT.get(), message); - } else { + if (config.getRequestClass() == null || config.getKeyFunction() == null) { + log.debug("Circuit breaker will apply to all requests as config is not set"); circuitBreakerKey = "default"; + } else { + if (!message.getClass().equals(config.getRequestClass())) { + log.warn("Invalid config for message type: {}", message.getClass()); + super.sendMessage(message); + } + circuitBreakerKey = config.getKeyFunction().apply(RequestContext.CURRENT.get(), message); } circuitBreaker = resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey); if (!circuitBreaker.tryAcquirePermission()) { diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java index 3bad85d..7af3240 100644 --- a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java @@ -1,168 +1,170 @@ package org.hypertrace.circuitbreaker.grpcutils.resilience; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import com.typesafe.config.Config; -import com.typesafe.config.ConfigFactory; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.*; + import io.github.resilience4j.circuitbreaker.CircuitBreaker; import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; -import io.grpc.CallOptions; -import io.grpc.Channel; -import io.grpc.ClientCall; -import io.grpc.ForwardingClientCall; -import io.grpc.Metadata; -import io.grpc.MethodDescriptor; -import io.grpc.StatusRuntimeException; +import io.grpc.*; import java.time.Clock; -import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigParser; +import java.time.Instant; +import java.time.ZoneOffset; +import java.util.concurrent.TimeUnit; import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration; -import org.hypertrace.core.grpcutils.context.RequestContext; -import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.*; +import org.mockito.junit.jupiter.MockitoExtension; +@ExtendWith(MockitoExtension.class) class ResilienceCircuitBreakerInterceptorTest { - private final Config config = - ConfigFactory.parseString( - "default {\n" - + " failureRateThreshold=50.0\n" - + " slowCallRateThreshold=100.0\n" - + " slowCallDurationThreshold=5s\n" - + " slidingWindowSize=10\n" - + " waitDurationInOpenState=1m\n" - + " minimumNumberOfCalls=5\n" - + " permittedNumberOfCallsInHalfOpenState=3\n" - + " slidingWindowType=COUNT_BASED\n" - + "}"); - private final Clock clock = Clock.systemUTC(); - private final CircuitBreakerRegistry mockRegistry = Mockito.mock(CircuitBreakerRegistry.class); - private final CircuitBreaker mockCircuitBreaker = Mockito.mock(CircuitBreaker.class); - private final Channel mockChannel = Mockito.mock(Channel.class); - private final ClientCall.Listener mockListener = mock(ClientCall.Listener.class); - private final ResilienceCircuitBreakerProvider mockCircuitBreakerProvider = - Mockito.mock(ResilienceCircuitBreakerProvider.class); + @Mock private Channel mockChannel; + @Mock private ClientCall mockClientCall; + @Mock private CircuitBreaker mockCircuitBreaker; + @Mock private Metadata mockMetadata; + @Mock private ClientCall.Listener mockListener; + @Mock private ResilienceCircuitBreakerProvider mockCircuitBreakerProvider; + @Mock private CircuitBreakerConfiguration mockCircuitBreakerConfig; + @Mock private CircuitBreakerRegistry mockCircuitBreakerRegistry; + + @Mock private Clock fixedClock; + + @BeforeEach + void setUp() { + MockitoAnnotations.openMocks(this); + + fixedClock = Clock.fixed(Instant.now(), ZoneOffset.UTC); + when(mockChannel.newCall(any(), any())).thenReturn(mockClientCall); + when(mockCircuitBreakerProvider.getCircuitBreaker(anyString())).thenReturn(mockCircuitBreaker); + } @Test - void testCircuitBreakerEnabled_InterceptsCall() { - MethodDescriptor methodDescriptor = mock(MethodDescriptor.class); - when(mockCircuitBreakerProvider.getCircuitBreaker("test-key")).thenReturn(mockCircuitBreaker); - CircuitBreakerConfiguration circuitBreakerConfiguration = - CircuitBreakerConfigParser.parseConfig(config) - .enabled(true) - .keyFunction( - (requestContext, request) -> { - GetGithubIntegrationsRequest getGithubIntegrationsRequest = - (GetGithubIntegrationsRequest) request; - return requestContext.getTenantId() + "-" + getGithubIntegrationsRequest.getUrl(); - }) - .build(); + void testSendMessage_CallsSuperSendMessage_Success() { + doNothing().when(mockClientCall).sendMessage(any()); + when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); + ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( - clock, mockRegistry, mockCircuitBreakerProvider, circuitBreakerConfiguration); + fixedClock, + mockCircuitBreakerRegistry, + mockCircuitBreakerProvider, + mockCircuitBreakerConfig); - CallOptions callOptions = Mockito.mock(CallOptions.class); ClientCall interceptedCall = - spy(interceptor.interceptCall(methodDescriptor, callOptions, mockChannel)); - doNothing().when(interceptedCall).start(any(), any()); - assertNotNull(interceptedCall); - assertDoesNotThrow(() -> interceptedCall.start(mockListener, new Metadata())); - verify(interceptedCall).start(eq(mockListener), any(Metadata.class)); + interceptor.createInterceptedCall( + mock(MethodDescriptor.class), CallOptions.DEFAULT, mockChannel); + + interceptedCall.start(mockListener, mockMetadata); + interceptedCall.sendMessage(new Object()); + + verify(mockClientCall).sendMessage(any()); } @Test - void testCircuitBreakerRejectsRequest() { - MethodDescriptor methodDescriptor = mock(MethodDescriptor.class); - CallOptions callOptions = Mockito.mock(CallOptions.class); + void testSendMessage_CircuitBreakerRejectsRequest() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(false); when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.OPEN); - when(mockCircuitBreakerProvider.getCircuitBreaker("tenant1-http://localhost:9000")) - .thenReturn(mockCircuitBreaker); - CircuitBreakerConfiguration circuitBreakerConfiguration = - CircuitBreakerConfigParser.parseConfig(config) - .enabled(true) - .requestClass(GetGithubIntegrationsRequest.class) - .keyFunction( - (requestContext, request) -> { - return requestContext.getTenantId().get() + "-" + request.getUrl(); - }) - .build(); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( - clock, mockRegistry, mockCircuitBreakerProvider, circuitBreakerConfiguration); + fixedClock, + mockCircuitBreakerRegistry, + mockCircuitBreakerProvider, + mockCircuitBreakerConfig); + + ClientCall interceptedCall = + interceptor.createInterceptedCall( + mock(MethodDescriptor.class), CallOptions.DEFAULT, mockChannel); + + interceptedCall.start(mockListener, mockMetadata); + + assertThrows( + StatusRuntimeException.class, + () -> interceptedCall.sendMessage(new Object()), + "Circuit Breaker should reject request"); + + verify(mockClientCall, never()).sendMessage(any()); + } + + @Test + void testSendMessage_CircuitBreakerInHalfOpenState() { + when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(false); + when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.HALF_OPEN); + ResilienceCircuitBreakerInterceptor interceptor = + new ResilienceCircuitBreakerInterceptor( + fixedClock, + mockCircuitBreakerRegistry, + mockCircuitBreakerProvider, + mockCircuitBreakerConfig); ClientCall interceptedCall = - interceptor.interceptCall(methodDescriptor, callOptions, mockChannel); + interceptor.createInterceptedCall( + mock(MethodDescriptor.class), CallOptions.DEFAULT, mockChannel); + + interceptedCall.start(mockListener, mockMetadata); + assertThrows( StatusRuntimeException.class, - () -> { - RequestContext.forTenantId("tenant1") - .call( - () -> { - interceptedCall.sendMessage( - new GetGithubIntegrationsRequest("http://localhost:9000")); - return null; - }); - }); + () -> interceptedCall.sendMessage(new Object()), + "Circuit Breaker should reject requests when in HALF-OPEN state"); + + verify(mockClientCall, never()).sendMessage(any()); } @Test - @Disabled - void testCircuitBreakerSuccess() { - MethodDescriptor methodDescriptor = mock(MethodDescriptor.class); - CallOptions callOptions = Mockito.mock(CallOptions.class); + void testWrapListenerWithCircuitBreaker_Success() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); - when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.CLOSED); - when(mockCircuitBreakerProvider.getCircuitBreaker("test-key")).thenReturn(mockCircuitBreaker); - CircuitBreakerConfiguration circuitBreakerConfiguration = - CircuitBreakerConfigParser.parseConfig(config) - .enabled(true) - .requestClass(GetGithubIntegrationsRequest.class) - .keyFunction( - (requestContext, request) -> { - return requestContext.getTenantId().get() + "-" + request.getUrl(); - }) - .build(); ResilienceCircuitBreakerInterceptor interceptor = - spy( - new ResilienceCircuitBreakerInterceptor( - clock, mockRegistry, mockCircuitBreakerProvider, circuitBreakerConfiguration)); + new ResilienceCircuitBreakerInterceptor( + fixedClock, + mockCircuitBreakerRegistry, + mockCircuitBreakerProvider, + mockCircuitBreakerConfig); ClientCall interceptedCall = - interceptor.createInterceptedCall(methodDescriptor, callOptions, mockChannel); - ClientCall spyCall = spy(interceptedCall); - Mockito.doNothing().when((ForwardingClientCall) interceptedCall).sendMessage(Mockito.any()); - // Act - RequestContext.forTenantId("tenant1") - .call( - () -> { - spyCall.sendMessage(new Object()); - return null; - }); - - // Assert - verify(spyCall).sendMessage(any()); - verify(mockCircuitBreaker).tryAcquirePermission(); + interceptor.createInterceptedCall( + mock(MethodDescriptor.class), CallOptions.DEFAULT, mockChannel); + + interceptedCall.start(mockListener, mockMetadata); + interceptedCall.sendMessage(new Object()); + + // Trigger `onClose` directly to mimic gRPC's flow + ArgumentCaptor> listenerCaptor = + ArgumentCaptor.forClass(ForwardingClientCallListener.class); + verify(mockClientCall).start(listenerCaptor.capture(), any()); + listenerCaptor.getValue().onClose(Status.OK, mockMetadata); + + verify(mockClientCall).sendMessage(any()); + verify(mockCircuitBreaker).onSuccess(anyLong(), eq(TimeUnit.NANOSECONDS)); } - private static class GetGithubIntegrationsRequest { - private final String url; + @Test + void testWrapListenerWithCircuitBreaker_Failure() { + when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); + ResilienceCircuitBreakerInterceptor interceptor = + new ResilienceCircuitBreakerInterceptor( + fixedClock, + mockCircuitBreakerRegistry, + mockCircuitBreakerProvider, + mockCircuitBreakerConfig); + + ClientCall interceptedCall = + interceptor.createInterceptedCall( + mock(MethodDescriptor.class), CallOptions.DEFAULT, mockChannel); + + interceptedCall.start(mockListener, mockMetadata); + interceptedCall.sendMessage(new Object()); - public GetGithubIntegrationsRequest(String url) { - this.url = url; - } + // Trigger `onClose` directly to mimic gRPC's flow + ArgumentCaptor> listenerCaptor = + ArgumentCaptor.forClass(ForwardingClientCallListener.class); + verify(mockClientCall).start(listenerCaptor.capture(), any()); + listenerCaptor.getValue().onClose(Status.UNKNOWN, mockMetadata); - public String getUrl() { - return url; - } + verify(mockClientCall).sendMessage(any()); + verify(mockCircuitBreaker).onError(anyLong(), eq(TimeUnit.NANOSECONDS), any()); } } From 275c8544ecfbc22fe5af8548d038160841fdee7a Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Mon, 17 Mar 2025 13:52:47 +0530 Subject: [PATCH 05/15] Addressed comments --- grpc-circuitbreaker-utils/build.gradle.kts | 1 - .../grpcutils/CircuitBreakerConfigParser.java | 38 ++++++++++++++++++- .../CircuitBreakerConfiguration.java | 9 ++++- ...ilienceCircuitBreakerConfigConverter.java} | 9 ++--- .../ResilienceCircuitBreakerInterceptor.java | 23 +++++------ .../ResilienceCircuitBreakerProvider.java | 11 +++--- ...ilienceCircuitBreakerRegistryProvider.java | 14 +++---- ...nceCircuitBreakerConfigConverterTest.java} | 23 ++--------- ...silienceCircuitBreakerInterceptorTest.java | 29 +++++++++----- 9 files changed, 89 insertions(+), 68 deletions(-) rename grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/{ResilienceCircuitBreakerConfigParser.java => ResilienceCircuitBreakerConfigConverter.java} (83%) rename grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/{ResilienceCircuitBreakerConfigParserTest.java => ResilienceCircuitBreakerConfigConverterTest.java} (66%) diff --git a/grpc-circuitbreaker-utils/build.gradle.kts b/grpc-circuitbreaker-utils/build.gradle.kts index e838cbc..c892067 100644 --- a/grpc-circuitbreaker-utils/build.gradle.kts +++ b/grpc-circuitbreaker-utils/build.gradle.kts @@ -14,7 +14,6 @@ dependencies { implementation("org.slf4j:slf4j-api:1.7.36") implementation("io.github.resilience4j:resilience4j-circuitbreaker:1.7.1") implementation("com.typesafe:config:1.4.2") - implementation("com.google.inject:guice:7.0.0") annotationProcessor("org.projectlombok:lombok:1.18.24") compileOnly("org.projectlombok:lombok:1.18.24") diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java index 09062fe..99efbd9 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java @@ -1,7 +1,9 @@ package org.hypertrace.circuitbreaker.grpcutils; import com.typesafe.config.Config; +import java.time.Duration; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; @@ -9,6 +11,8 @@ public class CircuitBreakerConfigParser { public static final String DEFAULT_CONFIG_KEY = "default"; + private static final Set nonThresholdKeys = + Set.of("enabled", "defaultCircuitBreakerKey", "defaultThresholds"); // Percentage of failures to trigger OPEN state private static final String FAILURE_RATE_THRESHOLD = "failureRateThreshold"; @@ -27,19 +31,38 @@ public class CircuitBreakerConfigParser { private static final String PERMITTED_NUMBER_OF_CALLS_IN_HALF_OPEN_STATE = "permittedNumberOfCallsInHalfOpenState"; private static final String SLIDING_WINDOW_TYPE = "slidingWindowType"; + public static final String ENABLED = "enabled"; + public static final String DEFAULT_CIRCUIT_BREAKER_KEY = "defaultCircuitBreakerKey"; + public static final String DEFAULT_THRESHOLDS = "defaultThresholds"; public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder parseConfig( Config config) { CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder builder = CircuitBreakerConfiguration.builder(); + if (config.hasPath(ENABLED)) { + builder.enabled(config.getBoolean(ENABLED)); + } + + if (config.hasPath(DEFAULT_CIRCUIT_BREAKER_KEY)) { + builder.defaultCircuitBreakerKey(config.getString(DEFAULT_CIRCUIT_BREAKER_KEY)); + } + + if (config.hasPath(DEFAULT_THRESHOLDS)) { + builder.defaultThresholds( + buildCircuitBreakerThresholds(config.getConfig(DEFAULT_THRESHOLDS))); + } else { + builder.defaultThresholds(buildCircuitBreakerDefaultThresholds()); + } + Map circuitBreakerThresholdsMap = config.root().keySet().stream() + .filter(key -> !nonThresholdKeys.contains(key)) // Filter out non-threshold keys .collect( Collectors.toMap( key -> key, // Circuit breaker key key -> buildCircuitBreakerThresholds(config.getConfig(key)))); builder.circuitBreakerThresholdsMap(circuitBreakerThresholdsMap); - log.info("Loaded circuit breaker configs: {}", circuitBreakerThresholdsMap); + log.info("Loaded circuit breaker configs: {}", builder); return builder; } @@ -57,6 +80,19 @@ private static CircuitBreakerThresholds buildCircuitBreakerThresholds(Config con .build(); } + public static CircuitBreakerThresholds buildCircuitBreakerDefaultThresholds() { + return CircuitBreakerThresholds.builder() + .failureRateThreshold(50f) + .slowCallRateThreshold(50f) + .slowCallDurationThreshold(Duration.ofSeconds(2)) + .slidingWindowType(CircuitBreakerThresholds.SlidingWindowType.TIME_BASED) + .slidingWindowSize(60) + .waitDurationInOpenState(Duration.ofSeconds(60)) + .permittedNumberOfCallsInHalfOpenState(5) + .minimumNumberOfCalls(10) + .build(); + } + private static CircuitBreakerThresholds.SlidingWindowType getSlidingWindowType( String slidingWindowType) { return CircuitBreakerThresholds.SlidingWindowType.valueOf(slidingWindowType); diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java index 6aabbef..55f7be1 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java @@ -11,6 +11,11 @@ public class CircuitBreakerConfiguration { Class requestClass; BiFunction keyFunction; - boolean enabled; - Map circuitBreakerThresholdsMap; + @Builder.Default boolean enabled = false; + // Default value be "global" if not override. + @Builder.Default String defaultCircuitBreakerKey = "global"; + // Standard/default thresholds + CircuitBreakerThresholds defaultThresholds; + // Custom overrides for specific cases (less common) + @Builder.Default Map circuitBreakerThresholdsMap = Map.of(); } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParser.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverter.java similarity index 83% rename from grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParser.java rename to grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverter.java index bfa3e7e..29e1fb3 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParser.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverter.java @@ -6,15 +6,15 @@ import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerThresholds; /** Utility class to parse CircuitBreakerConfiguration to Resilience4j CircuitBreakerConfig */ -public class ResilienceCircuitBreakerConfigParser { +public class ResilienceCircuitBreakerConfigConverter { public static Map getCircuitBreakerConfigs( Map configurationMap) { return configurationMap.entrySet().stream() - .collect(Collectors.toMap(Map.Entry::getKey, entry -> getConfig(entry.getValue()))); + .collect(Collectors.toMap(Map.Entry::getKey, entry -> convertConfig(entry.getValue()))); } - static CircuitBreakerConfig getConfig(CircuitBreakerThresholds configuration) { + static CircuitBreakerConfig convertConfig(CircuitBreakerThresholds configuration) { return CircuitBreakerConfig.custom() .failureRateThreshold(configuration.getFailureRateThreshold()) .slowCallRateThreshold(configuration.getSlowCallRateThreshold()) @@ -30,9 +30,6 @@ static CircuitBreakerConfig getConfig(CircuitBreakerThresholds configuration) { private static CircuitBreakerConfig.SlidingWindowType getSlidingWindowType( CircuitBreakerThresholds.SlidingWindowType slidingWindowType) { - if (slidingWindowType == null) { - return CircuitBreakerConfig.SlidingWindowType.TIME_BASED; - } switch (slidingWindowType) { case COUNT_BASED: return CircuitBreakerConfig.SlidingWindowType.COUNT_BASED; diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java index 0930e9b..e25149f 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java @@ -1,6 +1,5 @@ package org.hypertrace.circuitbreaker.grpcutils.resilience; -import com.google.common.annotations.VisibleForTesting; import io.github.resilience4j.circuitbreaker.CircuitBreaker; import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; @@ -36,25 +35,25 @@ public ResilienceCircuitBreakerInterceptor( this.circuitBreakerConfiguration = circuitBreakerConfiguration; this.clock = clock; this.resilienceCircuitBreakerConfigMap = - ResilienceCircuitBreakerConfigParser.getCircuitBreakerConfigs( + ResilienceCircuitBreakerConfigConverter.getCircuitBreakerConfigs( circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()); this.resilicenceCircuitBreakerRegistry = - new ResilienceCircuitBreakerRegistryProvider(resilienceCircuitBreakerConfigMap) + new ResilienceCircuitBreakerRegistryProvider( + circuitBreakerConfiguration.getDefaultThresholds()) .getCircuitBreakerRegistry(); this.resilienceCircuitBreakerProvider = new ResilienceCircuitBreakerProvider( resilicenceCircuitBreakerRegistry, resilienceCircuitBreakerConfigMap); } - @VisibleForTesting public ResilienceCircuitBreakerInterceptor( + CircuitBreakerConfiguration circuitBreakerConfiguration, Clock clock, CircuitBreakerRegistry resilicenceCircuitBreakerRegistry, - ResilienceCircuitBreakerProvider resilienceCircuitBreakerProvider, - CircuitBreakerConfiguration circuitBreakerConfiguration) { + ResilienceCircuitBreakerProvider resilienceCircuitBreakerProvider) { this.circuitBreakerConfiguration = circuitBreakerConfiguration; this.resilienceCircuitBreakerConfigMap = - ResilienceCircuitBreakerConfigParser.getCircuitBreakerConfigs( + ResilienceCircuitBreakerConfigConverter.getCircuitBreakerConfigs( circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()); this.resilicenceCircuitBreakerRegistry = resilicenceCircuitBreakerRegistry; this.resilienceCircuitBreakerProvider = resilienceCircuitBreakerProvider; @@ -88,14 +87,10 @@ public void start(Listener responseListener, Metadata headers) { public void sendMessage(ReqT message) { CircuitBreakerConfiguration config = (CircuitBreakerConfiguration) circuitBreakerConfiguration; - if (config.getRequestClass() == null || config.getKeyFunction() == null) { - log.debug("Circuit breaker will apply to all requests as config is not set"); - circuitBreakerKey = "default"; + if (config.getKeyFunction() == null) { + log.debug("Circuit breaker will apply to all requests as keyFunction config is not set"); + circuitBreakerKey = config.getDefaultCircuitBreakerKey(); } else { - if (!message.getClass().equals(config.getRequestClass())) { - log.warn("Invalid config for message type: {}", message.getClass()); - super.sendMessage(message); - } circuitBreakerKey = config.getKeyFunction().apply(RequestContext.CURRENT.get(), message); } circuitBreaker = resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey); diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java index 4f83478..e2bbdba 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java @@ -3,14 +3,13 @@ import io.github.resilience4j.circuitbreaker.CircuitBreaker; import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; -import jakarta.inject.Singleton; import java.util.Map; +import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import lombok.extern.slf4j.Slf4j; /** Utility class to provide Resilience4j CircuitBreaker */ @Slf4j -@Singleton public class ResilienceCircuitBreakerProvider { private final CircuitBreakerRegistry circuitBreakerRegistry; @@ -29,10 +28,10 @@ public CircuitBreaker getCircuitBreaker(String circuitBreakerKey) { circuitBreakerKey, key -> { CircuitBreaker circuitBreaker = - circuitBreakerRegistry.circuitBreaker( - circuitBreakerKey, - circuitBreakerConfigMap.getOrDefault( - circuitBreakerKey, circuitBreakerConfigMap.get("default"))); + Optional.ofNullable(circuitBreakerConfigMap.get(circuitBreakerKey)) + .map(config -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey, config)) + .orElseGet(() -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey)); + circuitBreaker .getEventPublisher() .onStateTransition( diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java index 92243e0..7a0b454 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java @@ -1,28 +1,24 @@ package org.hypertrace.circuitbreaker.grpcutils.resilience; import io.github.resilience4j.circuitbreaker.CircuitBreaker; -import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; -import jakarta.inject.Singleton; -import java.util.Map; import lombok.extern.slf4j.Slf4j; -import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigParser; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerThresholds; /** Utility class to provide Resilience4j CircuitBreakerRegistry */ @Slf4j -@Singleton public class ResilienceCircuitBreakerRegistryProvider { - private final Map circuitBreakerConfigMap; + private final CircuitBreakerThresholds circuitBreakerThresholds; public ResilienceCircuitBreakerRegistryProvider( - Map circuitBreakerConfigMap) { - this.circuitBreakerConfigMap = circuitBreakerConfigMap; + CircuitBreakerThresholds circuitBreakerThresholds) { + this.circuitBreakerThresholds = circuitBreakerThresholds; } public CircuitBreakerRegistry getCircuitBreakerRegistry() { CircuitBreakerRegistry circuitBreakerRegistry = CircuitBreakerRegistry.of( - this.circuitBreakerConfigMap.get(CircuitBreakerConfigParser.DEFAULT_CONFIG_KEY)); + ResilienceCircuitBreakerConfigConverter.convertConfig(circuitBreakerThresholds)); circuitBreakerRegistry .getEventPublisher() .onEntryAdded( diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParserTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverterTest.java similarity index 66% rename from grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParserTest.java rename to grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverterTest.java index a9c5286..b49f3b3 100644 --- a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigParserTest.java +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverterTest.java @@ -11,7 +11,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -public class ResilienceCircuitBreakerConfigParserTest { +public class ResilienceCircuitBreakerConfigConverterTest { @Test void shouldParseValidConfiguration() { @@ -31,7 +31,7 @@ void shouldParseValidConfiguration() { configMap.put("testService", thresholds); Map result = - ResilienceCircuitBreakerConfigParser.getCircuitBreakerConfigs(configMap); + ResilienceCircuitBreakerConfigConverter.getCircuitBreakerConfigs(configMap); Assertions.assertTrue(result.containsKey("testService")); @@ -45,25 +45,10 @@ void shouldParseValidConfiguration() { assertEquals(20, config.getMinimumNumberOfCalls()); } - @Test - void shouldUseDefaultSlidingWindowTypeForInvalidType() { - CircuitBreakerThresholds thresholds = - CircuitBreakerThresholds.builder() - .failureRateThreshold(50.0f) - .slowCallRateThreshold(30.0f) - .slowCallDurationThreshold(Duration.ofSeconds(2)) - .slidingWindowSize(100) - .waitDurationInOpenState(Duration.ofSeconds(60)) - .permittedNumberOfCallsInHalfOpenState(5) - .minimumNumberOfCalls(20) - .build(); // Invalid type scenario - CircuitBreakerConfig config = ResilienceCircuitBreakerConfigParser.getConfig(thresholds); - assertEquals(CircuitBreakerConfig.SlidingWindowType.TIME_BASED, config.getSlidingWindowType()); - } - @Test void shouldThrowExceptionWhenConfigurationIsNull() { assertThrows( - NullPointerException.class, () -> ResilienceCircuitBreakerConfigParser.getConfig(null)); + NullPointerException.class, + () -> ResilienceCircuitBreakerConfigConverter.convertConfig(null)); } } diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java index 7af3240..49f5df0 100644 --- a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java @@ -11,7 +11,9 @@ import java.time.Clock; import java.time.Instant; import java.time.ZoneOffset; +import java.util.Map; import java.util.concurrent.TimeUnit; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigParser; import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -27,6 +29,7 @@ class ResilienceCircuitBreakerInterceptorTest { @Mock private CircuitBreaker mockCircuitBreaker; @Mock private Metadata mockMetadata; @Mock private ClientCall.Listener mockListener; + @Mock private ResilienceCircuitBreakerRegistryProvider mockCircuitBreakerRegistryProvider; @Mock private ResilienceCircuitBreakerProvider mockCircuitBreakerProvider; @Mock private CircuitBreakerConfiguration mockCircuitBreakerConfig; @Mock private CircuitBreakerRegistry mockCircuitBreakerRegistry; @@ -39,7 +42,13 @@ void setUp() { fixedClock = Clock.fixed(Instant.now(), ZoneOffset.UTC); when(mockChannel.newCall(any(), any())).thenReturn(mockClientCall); + // when(mockCircuitBreakerRegistryProvider.getCircuitBreakerRegistry()) + // .thenReturn(mockCircuitBreakerRegistry); when(mockCircuitBreakerProvider.getCircuitBreaker(anyString())).thenReturn(mockCircuitBreaker); + when(mockCircuitBreakerConfig.getDefaultCircuitBreakerKey()).thenReturn("global"); + when(mockCircuitBreakerConfig.getCircuitBreakerThresholdsMap()) + .thenReturn( + Map.of("global", CircuitBreakerConfigParser.buildCircuitBreakerDefaultThresholds())); } @Test @@ -49,10 +58,10 @@ void testSendMessage_CallsSuperSendMessage_Success() { ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( + mockCircuitBreakerConfig, fixedClock, mockCircuitBreakerRegistry, - mockCircuitBreakerProvider, - mockCircuitBreakerConfig); + mockCircuitBreakerProvider); ClientCall interceptedCall = interceptor.createInterceptedCall( @@ -70,10 +79,10 @@ void testSendMessage_CircuitBreakerRejectsRequest() { when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.OPEN); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( + mockCircuitBreakerConfig, fixedClock, mockCircuitBreakerRegistry, - mockCircuitBreakerProvider, - mockCircuitBreakerConfig); + mockCircuitBreakerProvider); ClientCall interceptedCall = interceptor.createInterceptedCall( @@ -95,10 +104,10 @@ void testSendMessage_CircuitBreakerInHalfOpenState() { when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.HALF_OPEN); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( + mockCircuitBreakerConfig, fixedClock, mockCircuitBreakerRegistry, - mockCircuitBreakerProvider, - mockCircuitBreakerConfig); + mockCircuitBreakerProvider); ClientCall interceptedCall = interceptor.createInterceptedCall( @@ -119,10 +128,10 @@ void testWrapListenerWithCircuitBreaker_Success() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( + mockCircuitBreakerConfig, fixedClock, mockCircuitBreakerRegistry, - mockCircuitBreakerProvider, - mockCircuitBreakerConfig); + mockCircuitBreakerProvider); ClientCall interceptedCall = interceptor.createInterceptedCall( @@ -146,10 +155,10 @@ void testWrapListenerWithCircuitBreaker_Failure() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( + mockCircuitBreakerConfig, fixedClock, mockCircuitBreakerRegistry, - mockCircuitBreakerProvider, - mockCircuitBreakerConfig); + mockCircuitBreakerProvider); ClientCall interceptedCall = interceptor.createInterceptedCall( From 04ed4872e68959256862c5a055b6754bb0186c7f Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Tue, 18 Mar 2025 10:51:40 +0530 Subject: [PATCH 06/15] Add ResilienceCircuitBreakerFactory class --- .../grpcutils/CircuitBreakerConfigParser.java | 8 +--- .../CircuitBreakerConfiguration.java | 9 +++- ...silienceCircuitBreakerConfigConverter.java | 2 +- .../ResilienceCircuitBreakerFactory.java | 25 +++++++++++ .../ResilienceCircuitBreakerInterceptor.java | 31 ++------------ .../ResilienceCircuitBreakerProvider.java | 14 ++++--- ...ilienceCircuitBreakerRegistryProvider.java | 6 +-- ...silienceCircuitBreakerInterceptorTest.java | 42 +++++++------------ 8 files changed, 64 insertions(+), 73 deletions(-) create mode 100644 grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java index 99efbd9..e372231 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java @@ -10,7 +10,6 @@ @Slf4j public class CircuitBreakerConfigParser { - public static final String DEFAULT_CONFIG_KEY = "default"; private static final Set nonThresholdKeys = Set.of("enabled", "defaultCircuitBreakerKey", "defaultThresholds"); @@ -32,7 +31,6 @@ public class CircuitBreakerConfigParser { "permittedNumberOfCallsInHalfOpenState"; private static final String SLIDING_WINDOW_TYPE = "slidingWindowType"; public static final String ENABLED = "enabled"; - public static final String DEFAULT_CIRCUIT_BREAKER_KEY = "defaultCircuitBreakerKey"; public static final String DEFAULT_THRESHOLDS = "defaultThresholds"; public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder parseConfig( @@ -43,10 +41,6 @@ public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder builder.enabled(config.getBoolean(ENABLED)); } - if (config.hasPath(DEFAULT_CIRCUIT_BREAKER_KEY)) { - builder.defaultCircuitBreakerKey(config.getString(DEFAULT_CIRCUIT_BREAKER_KEY)); - } - if (config.hasPath(DEFAULT_THRESHOLDS)) { builder.defaultThresholds( buildCircuitBreakerThresholds(config.getConfig(DEFAULT_THRESHOLDS))); @@ -62,7 +56,7 @@ public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder key -> key, // Circuit breaker key key -> buildCircuitBreakerThresholds(config.getConfig(key)))); builder.circuitBreakerThresholdsMap(circuitBreakerThresholdsMap); - log.info("Loaded circuit breaker configs: {}", builder); + log.debug("Loaded circuit breaker configs: {}", builder); return builder; } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java index 55f7be1..62264c0 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java @@ -1,7 +1,9 @@ package org.hypertrace.circuitbreaker.grpcutils; +import io.grpc.Status; import java.util.Map; import java.util.function.BiFunction; +import java.util.function.Function; import lombok.Builder; import lombok.Value; import org.hypertrace.core.grpcutils.context.RequestContext; @@ -13,9 +15,14 @@ public class CircuitBreakerConfiguration { BiFunction keyFunction; @Builder.Default boolean enabled = false; // Default value be "global" if not override. - @Builder.Default String defaultCircuitBreakerKey = "global"; + String defaultCircuitBreakerKey = "global"; // Standard/default thresholds CircuitBreakerThresholds defaultThresholds; // Custom overrides for specific cases (less common) @Builder.Default Map circuitBreakerThresholdsMap = Map.of(); + + // New exception builder logic + @Builder.Default + Function exceptionBuilder = + reason -> Status.RESOURCE_EXHAUSTED.withDescription(reason).asRuntimeException(); } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverter.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverter.java index 29e1fb3..0fa128e 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverter.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverter.java @@ -6,7 +6,7 @@ import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerThresholds; /** Utility class to parse CircuitBreakerConfiguration to Resilience4j CircuitBreakerConfig */ -public class ResilienceCircuitBreakerConfigConverter { +class ResilienceCircuitBreakerConfigConverter { public static Map getCircuitBreakerConfigs( Map configurationMap) { diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java new file mode 100644 index 0000000..22788c9 --- /dev/null +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java @@ -0,0 +1,25 @@ +package org.hypertrace.circuitbreaker.grpcutils.resilience; + +import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; +import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import java.time.Clock; +import java.util.Map; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration; + +public class ResilienceCircuitBreakerFactory { + public static ResilienceCircuitBreakerInterceptor getResilienceCircuitBreakerInterceptor( + CircuitBreakerConfiguration circuitBreakerConfiguration, Clock clock) { + Map resilienceCircuitBreakerConfigMap = + ResilienceCircuitBreakerConfigConverter.getCircuitBreakerConfigs( + circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()); + CircuitBreakerRegistry resilicenceCircuitBreakerRegistry = + new ResilienceCircuitBreakerRegistryProvider( + circuitBreakerConfiguration.getDefaultThresholds()) + .getCircuitBreakerRegistry(); + ResilienceCircuitBreakerProvider resilienceCircuitBreakerProvider = + new ResilienceCircuitBreakerProvider( + resilicenceCircuitBreakerRegistry, resilienceCircuitBreakerConfigMap); + return new ResilienceCircuitBreakerInterceptor( + circuitBreakerConfiguration, clock, resilienceCircuitBreakerProvider); + } +} diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java index e25149f..55df16b 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java @@ -1,8 +1,6 @@ package org.hypertrace.circuitbreaker.grpcutils.resilience; import io.github.resilience4j.circuitbreaker.CircuitBreaker; -import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; -import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ClientCall; @@ -24,40 +22,17 @@ @Slf4j public class ResilienceCircuitBreakerInterceptor extends CircuitBreakerInterceptor { - private final CircuitBreakerRegistry resilicenceCircuitBreakerRegistry; - private final Map resilienceCircuitBreakerConfigMap; private final ResilienceCircuitBreakerProvider resilienceCircuitBreakerProvider; private final CircuitBreakerConfiguration circuitBreakerConfiguration; private final Clock clock; - public ResilienceCircuitBreakerInterceptor( - CircuitBreakerConfiguration circuitBreakerConfiguration, Clock clock) { - this.circuitBreakerConfiguration = circuitBreakerConfiguration; - this.clock = clock; - this.resilienceCircuitBreakerConfigMap = - ResilienceCircuitBreakerConfigConverter.getCircuitBreakerConfigs( - circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()); - this.resilicenceCircuitBreakerRegistry = - new ResilienceCircuitBreakerRegistryProvider( - circuitBreakerConfiguration.getDefaultThresholds()) - .getCircuitBreakerRegistry(); - this.resilienceCircuitBreakerProvider = - new ResilienceCircuitBreakerProvider( - resilicenceCircuitBreakerRegistry, resilienceCircuitBreakerConfigMap); - } - - public ResilienceCircuitBreakerInterceptor( + ResilienceCircuitBreakerInterceptor( CircuitBreakerConfiguration circuitBreakerConfiguration, Clock clock, - CircuitBreakerRegistry resilicenceCircuitBreakerRegistry, ResilienceCircuitBreakerProvider resilienceCircuitBreakerProvider) { this.circuitBreakerConfiguration = circuitBreakerConfiguration; - this.resilienceCircuitBreakerConfigMap = - ResilienceCircuitBreakerConfigConverter.getCircuitBreakerConfigs( - circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()); - this.resilicenceCircuitBreakerRegistry = resilicenceCircuitBreakerRegistry; - this.resilienceCircuitBreakerProvider = resilienceCircuitBreakerProvider; this.clock = clock; + this.resilienceCircuitBreakerProvider = resilienceCircuitBreakerProvider; } @Override @@ -100,7 +75,7 @@ public void sendMessage(ReqT message) { circuitBreaker.getState() == CircuitBreaker.State.HALF_OPEN ? "Circuit Breaker is HALF-OPEN and rejecting excess requests" : "Circuit Breaker is OPEN and blocking requests"; - throw Status.RESOURCE_EXHAUSTED.withDescription(rejectionReason).asRuntimeException(); + throw config.getExceptionBuilder().apply(rejectionReason); } super.sendMessage(message); } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java index e2bbdba..f54e034 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java @@ -10,7 +10,7 @@ /** Utility class to provide Resilience4j CircuitBreaker */ @Slf4j -public class ResilienceCircuitBreakerProvider { +class ResilienceCircuitBreakerProvider { private final CircuitBreakerRegistry circuitBreakerRegistry; private final Map circuitBreakerConfigMap; @@ -27,11 +27,7 @@ public CircuitBreaker getCircuitBreaker(String circuitBreakerKey) { return circuitBreakerCache.computeIfAbsent( circuitBreakerKey, key -> { - CircuitBreaker circuitBreaker = - Optional.ofNullable(circuitBreakerConfigMap.get(circuitBreakerKey)) - .map(config -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey, config)) - .orElseGet(() -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey)); - + CircuitBreaker circuitBreaker = getCircuitBreakerFromConfigMap(circuitBreakerKey); circuitBreaker .getEventPublisher() .onStateTransition( @@ -54,4 +50,10 @@ public CircuitBreaker getCircuitBreaker(String circuitBreakerKey) { return circuitBreaker; }); } + + private CircuitBreaker getCircuitBreakerFromConfigMap(String circuitBreakerKey) { + return Optional.ofNullable(circuitBreakerConfigMap.get(circuitBreakerKey)) + .map(config -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey, config)) + .orElseGet(() -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey)); + } } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java index 7a0b454..329c909 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerRegistryProvider.java @@ -7,7 +7,7 @@ /** Utility class to provide Resilience4j CircuitBreakerRegistry */ @Slf4j -public class ResilienceCircuitBreakerRegistryProvider { +class ResilienceCircuitBreakerRegistryProvider { private final CircuitBreakerThresholds circuitBreakerThresholds; public ResilienceCircuitBreakerRegistryProvider( @@ -24,12 +24,12 @@ public CircuitBreakerRegistry getCircuitBreakerRegistry() { .onEntryAdded( entryAddedEvent -> { CircuitBreaker addedCircuitBreaker = entryAddedEvent.getAddedEntry(); - log.info("CircuitBreaker {} added", addedCircuitBreaker.getName()); + log.debug("CircuitBreaker {} added", addedCircuitBreaker.getName()); }) .onEntryRemoved( entryRemovedEvent -> { CircuitBreaker removedCircuitBreaker = entryRemovedEvent.getRemovedEntry(); - log.info("CircuitBreaker {} removed", removedCircuitBreaker.getName()); + log.debug("CircuitBreaker {} removed", removedCircuitBreaker.getName()); }); return circuitBreakerRegistry; } diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java index 49f5df0..79840b9 100644 --- a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java @@ -11,9 +11,7 @@ import java.time.Clock; import java.time.Instant; import java.time.ZoneOffset; -import java.util.Map; import java.util.concurrent.TimeUnit; -import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigParser; import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -42,13 +40,8 @@ void setUp() { fixedClock = Clock.fixed(Instant.now(), ZoneOffset.UTC); when(mockChannel.newCall(any(), any())).thenReturn(mockClientCall); - // when(mockCircuitBreakerRegistryProvider.getCircuitBreakerRegistry()) - // .thenReturn(mockCircuitBreakerRegistry); when(mockCircuitBreakerProvider.getCircuitBreaker(anyString())).thenReturn(mockCircuitBreaker); when(mockCircuitBreakerConfig.getDefaultCircuitBreakerKey()).thenReturn("global"); - when(mockCircuitBreakerConfig.getCircuitBreakerThresholdsMap()) - .thenReturn( - Map.of("global", CircuitBreakerConfigParser.buildCircuitBreakerDefaultThresholds())); } @Test @@ -58,10 +51,7 @@ void testSendMessage_CallsSuperSendMessage_Success() { ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( - mockCircuitBreakerConfig, - fixedClock, - mockCircuitBreakerRegistry, - mockCircuitBreakerProvider); + mockCircuitBreakerConfig, fixedClock, mockCircuitBreakerProvider); ClientCall interceptedCall = interceptor.createInterceptedCall( @@ -77,12 +67,14 @@ void testSendMessage_CallsSuperSendMessage_Success() { void testSendMessage_CircuitBreakerRejectsRequest() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(false); when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.OPEN); + when(mockCircuitBreakerConfig.getExceptionBuilder()) + .thenReturn( + reason -> + new StatusRuntimeException( + Status.RESOURCE_EXHAUSTED.withDescription(reason), mock(Metadata.class))); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( - mockCircuitBreakerConfig, - fixedClock, - mockCircuitBreakerRegistry, - mockCircuitBreakerProvider); + mockCircuitBreakerConfig, fixedClock, mockCircuitBreakerProvider); ClientCall interceptedCall = interceptor.createInterceptedCall( @@ -102,12 +94,14 @@ void testSendMessage_CircuitBreakerRejectsRequest() { void testSendMessage_CircuitBreakerInHalfOpenState() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(false); when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.HALF_OPEN); + when(mockCircuitBreakerConfig.getExceptionBuilder()) + .thenReturn( + reason -> + new StatusRuntimeException( + Status.RESOURCE_EXHAUSTED.withDescription(reason), mock(Metadata.class))); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( - mockCircuitBreakerConfig, - fixedClock, - mockCircuitBreakerRegistry, - mockCircuitBreakerProvider); + mockCircuitBreakerConfig, fixedClock, mockCircuitBreakerProvider); ClientCall interceptedCall = interceptor.createInterceptedCall( @@ -128,10 +122,7 @@ void testWrapListenerWithCircuitBreaker_Success() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( - mockCircuitBreakerConfig, - fixedClock, - mockCircuitBreakerRegistry, - mockCircuitBreakerProvider); + mockCircuitBreakerConfig, fixedClock, mockCircuitBreakerProvider); ClientCall interceptedCall = interceptor.createInterceptedCall( @@ -155,10 +146,7 @@ void testWrapListenerWithCircuitBreaker_Failure() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( - mockCircuitBreakerConfig, - fixedClock, - mockCircuitBreakerRegistry, - mockCircuitBreakerProvider); + mockCircuitBreakerConfig, fixedClock, mockCircuitBreakerProvider); ClientCall interceptedCall = interceptor.createInterceptedCall( From fa3a3c3ee162220d3a33857204fb4923a5e24d3a Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Tue, 18 Mar 2025 11:09:25 +0530 Subject: [PATCH 07/15] Addressed comments --- .../resilience/ResilienceCircuitBreakerInterceptor.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java index 55df16b..204a64e 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java @@ -62,6 +62,13 @@ public void start(Listener responseListener, Metadata headers) { public void sendMessage(ReqT message) { CircuitBreakerConfiguration config = (CircuitBreakerConfiguration) circuitBreakerConfiguration; + // Type check for message class compatibility + if (config.getRequestClass() != null && !config.getRequestClass().isInstance(message)) { + throw new IllegalArgumentException( + String.format( + "Message type mismatch: Expected %s but received %s", + config.getRequestClass().getName(), message.getClass().getName())); + } if (config.getKeyFunction() == null) { log.debug("Circuit breaker will apply to all requests as keyFunction config is not set"); circuitBreakerKey = config.getDefaultCircuitBreakerKey(); From 90ca22bf6283f0d55eb6f391a95bc74a3eb0d006 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Tue, 18 Mar 2025 22:57:46 +0530 Subject: [PATCH 08/15] Addressed comments --- .../grpcutils/CircuitBreakerConfigParser.java | 66 +++++++++++-------- .../grpcutils/CircuitBreakerThresholds.java | 16 ++--- 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java index e372231..52a572e 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java @@ -1,7 +1,6 @@ package org.hypertrace.circuitbreaker.grpcutils; import com.typesafe.config.Config; -import java.time.Duration; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -10,9 +9,6 @@ @Slf4j public class CircuitBreakerConfigParser { - private static final Set nonThresholdKeys = - Set.of("enabled", "defaultCircuitBreakerKey", "defaultThresholds"); - // Percentage of failures to trigger OPEN state private static final String FAILURE_RATE_THRESHOLD = "failureRateThreshold"; // Percentage of slow calls to trigger OPEN state @@ -32,6 +28,7 @@ public class CircuitBreakerConfigParser { private static final String SLIDING_WINDOW_TYPE = "slidingWindowType"; public static final String ENABLED = "enabled"; public static final String DEFAULT_THRESHOLDS = "defaultThresholds"; + private static final Set NON_THRESHOLD_KEYS = Set.of(ENABLED, DEFAULT_THRESHOLDS); public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder parseConfig( Config config) { @@ -50,7 +47,7 @@ public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder Map circuitBreakerThresholdsMap = config.root().keySet().stream() - .filter(key -> !nonThresholdKeys.contains(key)) // Filter out non-threshold keys + .filter(key -> !NON_THRESHOLD_KEYS.contains(key)) // Filter out non-threshold keys .collect( Collectors.toMap( key -> key, // Circuit breaker key @@ -61,30 +58,47 @@ public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder } private static CircuitBreakerThresholds buildCircuitBreakerThresholds(Config config) { - return CircuitBreakerThresholds.builder() - .failureRateThreshold((float) config.getDouble(FAILURE_RATE_THRESHOLD)) - .slowCallRateThreshold((float) config.getDouble(SLOW_CALL_RATE_THRESHOLD)) - .slowCallDurationThreshold(config.getDuration(SLOW_CALL_DURATION_THRESHOLD)) - .slidingWindowType(getSlidingWindowType(config.getString(SLIDING_WINDOW_TYPE))) - .slidingWindowSize(config.getInt(SLIDING_WINDOW_SIZE)) - .waitDurationInOpenState(config.getDuration(WAIT_DURATION_IN_OPEN_STATE)) - .permittedNumberOfCallsInHalfOpenState( - config.getInt(PERMITTED_NUMBER_OF_CALLS_IN_HALF_OPEN_STATE)) - .minimumNumberOfCalls(config.getInt(MINIMUM_NUMBER_OF_CALLS)) - .build(); + CircuitBreakerThresholds.CircuitBreakerThresholdsBuilder builder = + CircuitBreakerThresholds.builder(); + + if (config.hasPath(FAILURE_RATE_THRESHOLD)) { + builder.failureRateThreshold((float) config.getDouble(FAILURE_RATE_THRESHOLD)); + } + + if (config.hasPath(SLOW_CALL_RATE_THRESHOLD)) { + builder.slowCallRateThreshold((float) config.getDouble(SLOW_CALL_RATE_THRESHOLD)); + } + + if (config.hasPath(SLOW_CALL_DURATION_THRESHOLD)) { + builder.slowCallDurationThreshold(config.getDuration(SLOW_CALL_DURATION_THRESHOLD)); + } + + if (config.hasPath(SLIDING_WINDOW_TYPE)) { + builder.slidingWindowType(getSlidingWindowType(config.getString(SLIDING_WINDOW_TYPE))); + } + + if (config.hasPath(SLIDING_WINDOW_SIZE)) { + builder.slidingWindowSize(config.getInt(SLIDING_WINDOW_SIZE)); + } + + if (config.hasPath(WAIT_DURATION_IN_OPEN_STATE)) { + builder.waitDurationInOpenState(config.getDuration(WAIT_DURATION_IN_OPEN_STATE)); + } + + if (config.hasPath(PERMITTED_NUMBER_OF_CALLS_IN_HALF_OPEN_STATE)) { + builder.permittedNumberOfCallsInHalfOpenState( + config.getInt(PERMITTED_NUMBER_OF_CALLS_IN_HALF_OPEN_STATE)); + } + + if (config.hasPath(MINIMUM_NUMBER_OF_CALLS)) { + builder.minimumNumberOfCalls(config.getInt(MINIMUM_NUMBER_OF_CALLS)); + } + + return builder.build(); } public static CircuitBreakerThresholds buildCircuitBreakerDefaultThresholds() { - return CircuitBreakerThresholds.builder() - .failureRateThreshold(50f) - .slowCallRateThreshold(50f) - .slowCallDurationThreshold(Duration.ofSeconds(2)) - .slidingWindowType(CircuitBreakerThresholds.SlidingWindowType.TIME_BASED) - .slidingWindowSize(60) - .waitDurationInOpenState(Duration.ofSeconds(60)) - .permittedNumberOfCallsInHalfOpenState(5) - .minimumNumberOfCalls(10) - .build(); + return CircuitBreakerThresholds.builder().build(); } private static CircuitBreakerThresholds.SlidingWindowType getSlidingWindowType( diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerThresholds.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerThresholds.java index 74b918a..eba1a52 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerThresholds.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerThresholds.java @@ -8,21 +8,21 @@ @Builder public class CircuitBreakerThresholds { // Percentage of failures to trigger OPEN state - float failureRateThreshold; + @Builder.Default float failureRateThreshold = 50f; // Percentage of slow calls to trigger OPEN state - float slowCallRateThreshold; + @Builder.Default float slowCallRateThreshold = 50f; // Define what a "slow" call is - Duration slowCallDurationThreshold; + @Builder.Default Duration slowCallDurationThreshold = Duration.ofSeconds(2); // Number of calls to consider in the sliding window - SlidingWindowType slidingWindowType; - int slidingWindowSize; + @Builder.Default SlidingWindowType slidingWindowType = SlidingWindowType.TIME_BASED; + @Builder.Default int slidingWindowSize = 60; // Time before retrying after OPEN state - Duration waitDurationInOpenState; + @Builder.Default Duration waitDurationInOpenState = Duration.ofSeconds(60); // Minimum calls before evaluating failure rate - int minimumNumberOfCalls; + @Builder.Default int minimumNumberOfCalls = 10; // Calls allowed in HALF_OPEN state before deciding to // CLOSE or OPEN again - int permittedNumberOfCallsInHalfOpenState; + @Builder.Default int permittedNumberOfCallsInHalfOpenState = 5; public enum SlidingWindowType { COUNT_BASED, From 5f0ecd0dd627062551005464ae3e0f40f83f7a82 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Wed, 19 Mar 2025 10:24:40 +0530 Subject: [PATCH 09/15] Addressed comments --- .../resilience/ResilienceCircuitBreakerInterceptor.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java index 204a64e..a3ad283 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java @@ -64,10 +64,8 @@ public void sendMessage(ReqT message) { (CircuitBreakerConfiguration) circuitBreakerConfiguration; // Type check for message class compatibility if (config.getRequestClass() != null && !config.getRequestClass().isInstance(message)) { - throw new IllegalArgumentException( - String.format( - "Message type mismatch: Expected %s but received %s", - config.getRequestClass().getName(), message.getClass().getName())); + super.sendMessage(message); + return; } if (config.getKeyFunction() == null) { log.debug("Circuit breaker will apply to all requests as keyFunction config is not set"); From 8ca26b7084e3b4bf8a65403b8b3b629d32f06f3f Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Wed, 19 Mar 2025 21:33:51 +0530 Subject: [PATCH 10/15] Addressed comments --- .../grpcutils/CircuitBreakerConfigParser.java | 19 +++--- .../CircuitBreakerConfiguration.java | 5 +- .../grpcutils/CircuitBreakerThresholds.java | 1 + .../ResilienceCircuitBreakerFactory.java | 4 +- .../ResilienceCircuitBreakerInterceptor.java | 32 +++++---- .../ResilienceCircuitBreakerProvider.java | 68 +++++++++++-------- ...silienceCircuitBreakerInterceptorTest.java | 13 ++-- 7 files changed, 87 insertions(+), 55 deletions(-) diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java index 52a572e..7e3b006 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java @@ -28,7 +28,7 @@ public class CircuitBreakerConfigParser { private static final String SLIDING_WINDOW_TYPE = "slidingWindowType"; public static final String ENABLED = "enabled"; public static final String DEFAULT_THRESHOLDS = "defaultThresholds"; - private static final Set NON_THRESHOLD_KEYS = Set.of(ENABLED, DEFAULT_THRESHOLDS); + private static final Set NON_THRESHOLD_KEYS = Set.of(ENABLED); public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder parseConfig( Config config) { @@ -38,13 +38,6 @@ public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder builder.enabled(config.getBoolean(ENABLED)); } - if (config.hasPath(DEFAULT_THRESHOLDS)) { - builder.defaultThresholds( - buildCircuitBreakerThresholds(config.getConfig(DEFAULT_THRESHOLDS))); - } else { - builder.defaultThresholds(buildCircuitBreakerDefaultThresholds()); - } - Map circuitBreakerThresholdsMap = config.root().keySet().stream() .filter(key -> !NON_THRESHOLD_KEYS.contains(key)) // Filter out non-threshold keys @@ -52,6 +45,12 @@ public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder Collectors.toMap( key -> key, // Circuit breaker key key -> buildCircuitBreakerThresholds(config.getConfig(key)))); + + if (!config.hasPath(DEFAULT_THRESHOLDS)) { + builder.defaultThresholds(buildCircuitBreakerDefaultThresholds()); + circuitBreakerThresholdsMap.put(DEFAULT_THRESHOLDS, buildCircuitBreakerDefaultThresholds()); + } + builder.circuitBreakerThresholdsMap(circuitBreakerThresholdsMap); log.debug("Loaded circuit breaker configs: {}", builder); return builder; @@ -94,6 +93,10 @@ private static CircuitBreakerThresholds buildCircuitBreakerThresholds(Config con builder.minimumNumberOfCalls(config.getInt(MINIMUM_NUMBER_OF_CALLS)); } + if (config.hasPath(ENABLED)) { + builder.enabled(config.getBoolean(ENABLED)); + } + return builder.build(); } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java index 62264c0..15866f9 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java @@ -1,6 +1,7 @@ package org.hypertrace.circuitbreaker.grpcutils; import io.grpc.Status; +import io.grpc.StatusRuntimeException; import java.util.Map; import java.util.function.BiFunction; import java.util.function.Function; @@ -14,8 +15,6 @@ public class CircuitBreakerConfiguration { Class requestClass; BiFunction keyFunction; @Builder.Default boolean enabled = false; - // Default value be "global" if not override. - String defaultCircuitBreakerKey = "global"; // Standard/default thresholds CircuitBreakerThresholds defaultThresholds; // Custom overrides for specific cases (less common) @@ -23,6 +22,6 @@ public class CircuitBreakerConfiguration { // New exception builder logic @Builder.Default - Function exceptionBuilder = + Function exceptionBuilder = reason -> Status.RESOURCE_EXHAUSTED.withDescription(reason).asRuntimeException(); } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerThresholds.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerThresholds.java index eba1a52..3b7f89e 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerThresholds.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerThresholds.java @@ -23,6 +23,7 @@ public class CircuitBreakerThresholds { // Calls allowed in HALF_OPEN state before deciding to // CLOSE or OPEN again @Builder.Default int permittedNumberOfCallsInHalfOpenState = 5; + @Builder.Default boolean enabled = true; public enum SlidingWindowType { COUNT_BASED, diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java index 22788c9..645894b 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java @@ -18,7 +18,9 @@ public static ResilienceCircuitBreakerInterceptor getResilienceCircuitBreakerInt .getCircuitBreakerRegistry(); ResilienceCircuitBreakerProvider resilienceCircuitBreakerProvider = new ResilienceCircuitBreakerProvider( - resilicenceCircuitBreakerRegistry, resilienceCircuitBreakerConfigMap); + resilicenceCircuitBreakerRegistry, + resilienceCircuitBreakerConfigMap, + circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()); return new ResilienceCircuitBreakerInterceptor( circuitBreakerConfiguration, clock, resilienceCircuitBreakerProvider); } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java index a3ad283..7f2cacf 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java @@ -13,6 +13,7 @@ import java.time.Duration; import java.time.Instant; import java.util.Map; +import java.util.Optional; import java.util.concurrent.TimeUnit; import lombok.extern.slf4j.Slf4j; import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration; @@ -45,7 +46,7 @@ protected ClientCall createInterceptedCall( MethodDescriptor method, CallOptions callOptions, Channel next) { return new ForwardingClientCall.SimpleForwardingClientCall<>( next.newCall(method, callOptions)) { - CircuitBreaker circuitBreaker; + Optional circuitBreaker; String circuitBreakerKey; @Override @@ -67,17 +68,21 @@ public void sendMessage(ReqT message) { super.sendMessage(message); return; } - if (config.getKeyFunction() == null) { - log.debug("Circuit breaker will apply to all requests as keyFunction config is not set"); - circuitBreakerKey = config.getDefaultCircuitBreakerKey(); - } else { + if (config.getKeyFunction() != null) { circuitBreakerKey = config.getKeyFunction().apply(RequestContext.CURRENT.get(), message); + circuitBreaker = resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey); + } else { + log.debug("Circuit breaker will apply to all requests as keyFunction config is not set"); + circuitBreaker = resilienceCircuitBreakerProvider.getDefaultCircuitBreaker(); + } + if (circuitBreaker.isEmpty()) { + super.sendMessage(message); + return; } - circuitBreaker = resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey); - if (!circuitBreaker.tryAcquirePermission()) { - logCircuitBreakerRejection(circuitBreakerKey, circuitBreaker); + if (!circuitBreaker.get().tryAcquirePermission()) { + logCircuitBreakerRejection(circuitBreakerKey, circuitBreaker.get()); String rejectionReason = - circuitBreaker.getState() == CircuitBreaker.State.HALF_OPEN + circuitBreaker.get().getState() == CircuitBreaker.State.HALF_OPEN ? "Circuit Breaker is HALF-OPEN and rejecting excess requests" : "Circuit Breaker is OPEN and blocking requests"; throw config.getExceptionBuilder().apply(rejectionReason); @@ -89,18 +94,21 @@ public void sendMessage(ReqT message) { wrapListenerWithCircuitBreaker(Listener responseListener, Instant startTime) { return new ForwardingClientCallListener.SimpleForwardingClientCallListener<>( responseListener) { + @SuppressWarnings("OptionalGetWithoutIsPresent") @Override public void onClose(Status status, Metadata trailers) { long duration = Duration.between(startTime, clock.instant()).toNanos(); if (status.isOk()) { - circuitBreaker.onSuccess(duration, TimeUnit.NANOSECONDS); + circuitBreaker.get().onSuccess(duration, TimeUnit.NANOSECONDS); } else { log.debug( "Circuit Breaker '{}' detected failure. Status: {}, Description: {}", - circuitBreaker.getName(), + circuitBreaker.get().getName(), status.getCode(), status.getDescription()); - circuitBreaker.onError(duration, TimeUnit.NANOSECONDS, status.asRuntimeException()); + circuitBreaker + .get() + .onError(duration, TimeUnit.NANOSECONDS, status.asRuntimeException()); } super.onClose(status, trailers); } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java index f54e034..8bcee98 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java @@ -7,6 +7,8 @@ import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import lombok.extern.slf4j.Slf4j; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigParser; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerThresholds; /** Utility class to provide Resilience4j CircuitBreaker */ @Slf4j @@ -14,41 +16,53 @@ class ResilienceCircuitBreakerProvider { private final CircuitBreakerRegistry circuitBreakerRegistry; private final Map circuitBreakerConfigMap; + private final Map circuitBreakerThresholdsMap; private final Map circuitBreakerCache = new ConcurrentHashMap<>(); public ResilienceCircuitBreakerProvider( CircuitBreakerRegistry circuitBreakerRegistry, - Map circuitBreakerConfigMap) { + Map circuitBreakerConfigMap, + Map circuitBreakerThresholdsMap) { this.circuitBreakerRegistry = circuitBreakerRegistry; this.circuitBreakerConfigMap = circuitBreakerConfigMap; + this.circuitBreakerThresholdsMap = circuitBreakerThresholdsMap; } - public CircuitBreaker getCircuitBreaker(String circuitBreakerKey) { - return circuitBreakerCache.computeIfAbsent( - circuitBreakerKey, - key -> { - CircuitBreaker circuitBreaker = getCircuitBreakerFromConfigMap(circuitBreakerKey); - circuitBreaker - .getEventPublisher() - .onStateTransition( - event -> - log.info( - "State transition: {} for circuit breaker {}", - event.getStateTransition(), - event.getCircuitBreakerName())) - .onCallNotPermitted( - event -> - log.debug( - "Call not permitted: Circuit is OPEN for circuit breaker {}", - event.getCircuitBreakerName())) - .onEvent( - event -> - log.debug( - "Circuit breaker event type {} for circuit breaker name {}", - event.getEventType(), - event.getCircuitBreakerName())); - return circuitBreaker; - }); + public Optional getCircuitBreaker(String circuitBreakerKey) { + if (circuitBreakerThresholdsMap.containsKey(circuitBreakerKey) + && !circuitBreakerThresholdsMap.get(circuitBreakerKey).isEnabled()) { + return Optional.empty(); + } + return Optional.of( + circuitBreakerCache.computeIfAbsent( + circuitBreakerKey, + key -> { + CircuitBreaker circuitBreaker = getCircuitBreakerFromConfigMap(circuitBreakerKey); + circuitBreaker + .getEventPublisher() + .onStateTransition( + event -> + log.info( + "State transition: {} for circuit breaker {}", + event.getStateTransition(), + event.getCircuitBreakerName())) + .onCallNotPermitted( + event -> + log.debug( + "Call not permitted: Circuit is OPEN for circuit breaker {}", + event.getCircuitBreakerName())) + .onEvent( + event -> + log.debug( + "Circuit breaker event type {} for circuit breaker name {}", + event.getEventType(), + event.getCircuitBreakerName())); + return circuitBreaker; + })); + } + + public Optional getDefaultCircuitBreaker() { + return getCircuitBreaker(CircuitBreakerConfigParser.DEFAULT_THRESHOLDS); } private CircuitBreaker getCircuitBreakerFromConfigMap(String circuitBreakerKey) { diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java index 79840b9..f750913 100644 --- a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java @@ -2,7 +2,6 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.*; import io.github.resilience4j.circuitbreaker.CircuitBreaker; @@ -11,6 +10,7 @@ import java.time.Clock; import java.time.Instant; import java.time.ZoneOffset; +import java.util.Optional; import java.util.concurrent.TimeUnit; import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfiguration; import org.junit.jupiter.api.BeforeEach; @@ -40,14 +40,11 @@ void setUp() { fixedClock = Clock.fixed(Instant.now(), ZoneOffset.UTC); when(mockChannel.newCall(any(), any())).thenReturn(mockClientCall); - when(mockCircuitBreakerProvider.getCircuitBreaker(anyString())).thenReturn(mockCircuitBreaker); - when(mockCircuitBreakerConfig.getDefaultCircuitBreakerKey()).thenReturn("global"); } @Test void testSendMessage_CallsSuperSendMessage_Success() { doNothing().when(mockClientCall).sendMessage(any()); - when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( @@ -67,6 +64,8 @@ void testSendMessage_CallsSuperSendMessage_Success() { void testSendMessage_CircuitBreakerRejectsRequest() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(false); when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.OPEN); + when(mockCircuitBreakerProvider.getDefaultCircuitBreaker()) + .thenReturn(Optional.of(mockCircuitBreaker)); when(mockCircuitBreakerConfig.getExceptionBuilder()) .thenReturn( reason -> @@ -94,6 +93,8 @@ void testSendMessage_CircuitBreakerRejectsRequest() { void testSendMessage_CircuitBreakerInHalfOpenState() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(false); when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.HALF_OPEN); + when(mockCircuitBreakerProvider.getDefaultCircuitBreaker()) + .thenReturn(Optional.of(mockCircuitBreaker)); when(mockCircuitBreakerConfig.getExceptionBuilder()) .thenReturn( reason -> @@ -120,6 +121,8 @@ void testSendMessage_CircuitBreakerInHalfOpenState() { @Test void testWrapListenerWithCircuitBreaker_Success() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); + when(mockCircuitBreakerProvider.getDefaultCircuitBreaker()) + .thenReturn(Optional.of(mockCircuitBreaker)); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( mockCircuitBreakerConfig, fixedClock, mockCircuitBreakerProvider); @@ -144,6 +147,8 @@ void testWrapListenerWithCircuitBreaker_Success() { @Test void testWrapListenerWithCircuitBreaker_Failure() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); + when(mockCircuitBreakerProvider.getDefaultCircuitBreaker()) + .thenReturn(Optional.of(mockCircuitBreaker)); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( mockCircuitBreakerConfig, fixedClock, mockCircuitBreakerProvider); From 75fb8b19571f115cdd428eb1e905b24cee711511 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Thu, 20 Mar 2025 12:57:13 +0530 Subject: [PATCH 11/15] Addressed comments --- .../grpcutils/CircuitBreakerConfigParser.java | 10 +++---- ...silienceCircuitBreakerConfigConverter.java | 9 ++++++ .../ResilienceCircuitBreakerFactory.java | 7 +++-- .../ResilienceCircuitBreakerInterceptor.java | 30 +++++++++++-------- .../ResilienceCircuitBreakerProvider.java | 11 ++++--- 5 files changed, 40 insertions(+), 27 deletions(-) diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java index 7e3b006..dc4867f 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java @@ -28,7 +28,7 @@ public class CircuitBreakerConfigParser { private static final String SLIDING_WINDOW_TYPE = "slidingWindowType"; public static final String ENABLED = "enabled"; public static final String DEFAULT_THRESHOLDS = "defaultThresholds"; - private static final Set NON_THRESHOLD_KEYS = Set.of(ENABLED); + private static final Set NON_THRESHOLD_KEYS = Set.of(ENABLED, DEFAULT_THRESHOLDS); public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder parseConfig( Config config) { @@ -46,10 +46,10 @@ public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder key -> key, // Circuit breaker key key -> buildCircuitBreakerThresholds(config.getConfig(key)))); - if (!config.hasPath(DEFAULT_THRESHOLDS)) { - builder.defaultThresholds(buildCircuitBreakerDefaultThresholds()); - circuitBreakerThresholdsMap.put(DEFAULT_THRESHOLDS, buildCircuitBreakerDefaultThresholds()); - } + builder.defaultThresholds( + config.hasPath(DEFAULT_THRESHOLDS) + ? buildCircuitBreakerThresholds(config.getConfig(DEFAULT_THRESHOLDS)) + : buildCircuitBreakerDefaultThresholds()); builder.circuitBreakerThresholdsMap(circuitBreakerThresholdsMap); log.debug("Loaded circuit breaker configs: {}", builder); diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverter.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverter.java index 0fa128e..28cfd0c 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverter.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverter.java @@ -1,6 +1,7 @@ package org.hypertrace.circuitbreaker.grpcutils.resilience; import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; +import java.util.List; import java.util.Map; import java.util.stream.Collectors; import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerThresholds; @@ -14,6 +15,14 @@ public static Map getCircuitBreakerConfigs( .collect(Collectors.toMap(Map.Entry::getKey, entry -> convertConfig(entry.getValue()))); } + public static List getDisabledKeys( + Map configurationMap) { + return configurationMap.entrySet().stream() + .filter(entry -> entry.getValue().isEnabled()) + .map(Map.Entry::getKey) + .collect(Collectors.toList()); + } + static CircuitBreakerConfig convertConfig(CircuitBreakerThresholds configuration) { return CircuitBreakerConfig.custom() .failureRateThreshold(configuration.getFailureRateThreshold()) diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java index 645894b..d4add9e 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java @@ -12,15 +12,16 @@ public static ResilienceCircuitBreakerInterceptor getResilienceCircuitBreakerInt Map resilienceCircuitBreakerConfigMap = ResilienceCircuitBreakerConfigConverter.getCircuitBreakerConfigs( circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()); - CircuitBreakerRegistry resilicenceCircuitBreakerRegistry = + CircuitBreakerRegistry resilienceCircuitBreakerRegistry = new ResilienceCircuitBreakerRegistryProvider( circuitBreakerConfiguration.getDefaultThresholds()) .getCircuitBreakerRegistry(); ResilienceCircuitBreakerProvider resilienceCircuitBreakerProvider = new ResilienceCircuitBreakerProvider( - resilicenceCircuitBreakerRegistry, + resilienceCircuitBreakerRegistry, resilienceCircuitBreakerConfigMap, - circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()); + ResilienceCircuitBreakerConfigConverter.getDisabledKeys( + circuitBreakerConfiguration.getCircuitBreakerThresholdsMap())); return new ResilienceCircuitBreakerInterceptor( circuitBreakerConfiguration, clock, resilienceCircuitBreakerProvider); } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java index 7f2cacf..f5c1280 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java @@ -46,7 +46,7 @@ protected ClientCall createInterceptedCall( MethodDescriptor method, CallOptions callOptions, Channel next) { return new ForwardingClientCall.SimpleForwardingClientCall<>( next.newCall(method, callOptions)) { - Optional circuitBreaker; + Optional optionalCircuitBreaker; String circuitBreakerKey; @Override @@ -70,19 +70,21 @@ public void sendMessage(ReqT message) { } if (config.getKeyFunction() != null) { circuitBreakerKey = config.getKeyFunction().apply(RequestContext.CURRENT.get(), message); - circuitBreaker = resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey); + optionalCircuitBreaker = + resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey); } else { log.debug("Circuit breaker will apply to all requests as keyFunction config is not set"); - circuitBreaker = resilienceCircuitBreakerProvider.getDefaultCircuitBreaker(); + optionalCircuitBreaker = resilienceCircuitBreakerProvider.getDefaultCircuitBreaker(); } - if (circuitBreaker.isEmpty()) { + CircuitBreaker circuitBreaker = optionalCircuitBreaker.orElse(null); + if (circuitBreaker == null) { super.sendMessage(message); return; } - if (!circuitBreaker.get().tryAcquirePermission()) { - logCircuitBreakerRejection(circuitBreakerKey, circuitBreaker.get()); + if (!circuitBreaker.tryAcquirePermission()) { + logCircuitBreakerRejection(circuitBreakerKey, circuitBreaker); String rejectionReason = - circuitBreaker.get().getState() == CircuitBreaker.State.HALF_OPEN + circuitBreaker.getState() == CircuitBreaker.State.HALF_OPEN ? "Circuit Breaker is HALF-OPEN and rejecting excess requests" : "Circuit Breaker is OPEN and blocking requests"; throw config.getExceptionBuilder().apply(rejectionReason); @@ -94,21 +96,23 @@ public void sendMessage(ReqT message) { wrapListenerWithCircuitBreaker(Listener responseListener, Instant startTime) { return new ForwardingClientCallListener.SimpleForwardingClientCallListener<>( responseListener) { - @SuppressWarnings("OptionalGetWithoutIsPresent") @Override public void onClose(Status status, Metadata trailers) { long duration = Duration.between(startTime, clock.instant()).toNanos(); + CircuitBreaker circuitBreaker = optionalCircuitBreaker.orElse(null); + if (circuitBreaker == null) { + super.onClose(status, trailers); + return; + } if (status.isOk()) { - circuitBreaker.get().onSuccess(duration, TimeUnit.NANOSECONDS); + circuitBreaker.onSuccess(duration, TimeUnit.NANOSECONDS); } else { log.debug( "Circuit Breaker '{}' detected failure. Status: {}, Description: {}", - circuitBreaker.get().getName(), + circuitBreaker.getName(), status.getCode(), status.getDescription()); - circuitBreaker - .get() - .onError(duration, TimeUnit.NANOSECONDS, status.asRuntimeException()); + circuitBreaker.onError(duration, TimeUnit.NANOSECONDS, status.asRuntimeException()); } super.onClose(status, trailers); } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java index 8bcee98..437e4dc 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java @@ -3,12 +3,12 @@ import io.github.resilience4j.circuitbreaker.CircuitBreaker; import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import lombok.extern.slf4j.Slf4j; import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigParser; -import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerThresholds; /** Utility class to provide Resilience4j CircuitBreaker */ @Slf4j @@ -16,21 +16,20 @@ class ResilienceCircuitBreakerProvider { private final CircuitBreakerRegistry circuitBreakerRegistry; private final Map circuitBreakerConfigMap; - private final Map circuitBreakerThresholdsMap; private final Map circuitBreakerCache = new ConcurrentHashMap<>(); + private final List disabledKeys; public ResilienceCircuitBreakerProvider( CircuitBreakerRegistry circuitBreakerRegistry, Map circuitBreakerConfigMap, - Map circuitBreakerThresholdsMap) { + List disabledKeys) { this.circuitBreakerRegistry = circuitBreakerRegistry; this.circuitBreakerConfigMap = circuitBreakerConfigMap; - this.circuitBreakerThresholdsMap = circuitBreakerThresholdsMap; + this.disabledKeys = disabledKeys; } public Optional getCircuitBreaker(String circuitBreakerKey) { - if (circuitBreakerThresholdsMap.containsKey(circuitBreakerKey) - && !circuitBreakerThresholdsMap.get(circuitBreakerKey).isEnabled()) { + if (disabledKeys.contains(circuitBreakerKey)) { return Optional.empty(); } return Optional.of( From 77b0aac3f3fe63ced9d41c131832f30f70316e70 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Thu, 20 Mar 2025 13:24:57 +0530 Subject: [PATCH 12/15] Addressed comments --- .../circuitbreaker/grpcutils/CircuitBreakerConfigParser.java | 2 +- .../grpcutils/resilience/ResilienceCircuitBreakerProvider.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java index dc4867f..deee9bf 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java @@ -28,7 +28,7 @@ public class CircuitBreakerConfigParser { private static final String SLIDING_WINDOW_TYPE = "slidingWindowType"; public static final String ENABLED = "enabled"; public static final String DEFAULT_THRESHOLDS = "defaultThresholds"; - private static final Set NON_THRESHOLD_KEYS = Set.of(ENABLED, DEFAULT_THRESHOLDS); + private static final Set NON_THRESHOLD_KEYS = Set.of(ENABLED); public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder parseConfig( Config config) { diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java index 437e4dc..3610b96 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java @@ -29,7 +29,8 @@ public ResilienceCircuitBreakerProvider( } public Optional getCircuitBreaker(String circuitBreakerKey) { - if (disabledKeys.contains(circuitBreakerKey)) { + if (disabledKeys.contains(circuitBreakerKey) + || !circuitBreakerConfigMap.containsKey(circuitBreakerKey)) { return Optional.empty(); } return Optional.of( From ea47f32227b13a90dd06fa825735ecf0c95da6cf Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Thu, 20 Mar 2025 19:43:14 +0530 Subject: [PATCH 13/15] Addressed comments --- build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index ffc733c..10adac6 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -7,7 +7,7 @@ plugins { id("org.hypertrace.publish-plugin") version "1.0.5" apply false id("org.hypertrace.jacoco-report-plugin") version "0.2.1" apply false id("org.hypertrace.code-style-plugin") version "2.0.0" apply false - id("org.owasp.dependencycheck") version "10.0.3" + id("org.owasp.dependencycheck") version "12.1.0" } subprojects { From f163a9d3b52e226dc222f4f383078330da9e3a31 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Thu, 20 Mar 2025 22:11:39 +0530 Subject: [PATCH 14/15] Fixed corner cases --- .../ResilienceCircuitBreakerInterceptor.java | 12 +-- .../ResilienceCircuitBreakerProvider.java | 80 ++++++++++++------- ...silienceCircuitBreakerInterceptorTest.java | 8 +- 3 files changed, 63 insertions(+), 37 deletions(-) diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java index f5c1280..64f4082 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java @@ -47,7 +47,6 @@ protected ClientCall createInterceptedCall( return new ForwardingClientCall.SimpleForwardingClientCall<>( next.newCall(method, callOptions)) { Optional optionalCircuitBreaker; - String circuitBreakerKey; @Override public void start(Listener responseListener, Metadata headers) { @@ -68,14 +67,15 @@ public void sendMessage(ReqT message) { super.sendMessage(message); return; } + String circuitBreakerKey = null; if (config.getKeyFunction() != null) { circuitBreakerKey = config.getKeyFunction().apply(RequestContext.CURRENT.get(), message); - optionalCircuitBreaker = - resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey); - } else { - log.debug("Circuit breaker will apply to all requests as keyFunction config is not set"); - optionalCircuitBreaker = resilienceCircuitBreakerProvider.getDefaultCircuitBreaker(); } + optionalCircuitBreaker = + circuitBreakerKey != null + ? resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey) + : resilienceCircuitBreakerProvider.getSharedCircuitBreaker(); + CircuitBreaker circuitBreaker = optionalCircuitBreaker.orElse(null); if (circuitBreaker == null) { super.sendMessage(message); diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java index 3610b96..a4806e0 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java @@ -14,10 +14,12 @@ @Slf4j class ResilienceCircuitBreakerProvider { + private static final String SHARED_KEY = "SHARED_KEY"; private final CircuitBreakerRegistry circuitBreakerRegistry; private final Map circuitBreakerConfigMap; private final Map circuitBreakerCache = new ConcurrentHashMap<>(); private final List disabledKeys; + private final boolean defaultEnabled; public ResilienceCircuitBreakerProvider( CircuitBreakerRegistry circuitBreakerRegistry, @@ -26,48 +28,72 @@ public ResilienceCircuitBreakerProvider( this.circuitBreakerRegistry = circuitBreakerRegistry; this.circuitBreakerConfigMap = circuitBreakerConfigMap; this.disabledKeys = disabledKeys; + this.defaultEnabled = !disabledKeys.contains(CircuitBreakerConfigParser.DEFAULT_THRESHOLDS); } public Optional getCircuitBreaker(String circuitBreakerKey) { - if (disabledKeys.contains(circuitBreakerKey) - || !circuitBreakerConfigMap.containsKey(circuitBreakerKey)) { + if (disabledKeys.contains(circuitBreakerKey)) { return Optional.empty(); } - return Optional.of( + return Optional.ofNullable( circuitBreakerCache.computeIfAbsent( circuitBreakerKey, key -> { - CircuitBreaker circuitBreaker = getCircuitBreakerFromConfigMap(circuitBreakerKey); - circuitBreaker - .getEventPublisher() - .onStateTransition( - event -> - log.info( - "State transition: {} for circuit breaker {}", - event.getStateTransition(), - event.getCircuitBreakerName())) - .onCallNotPermitted( - event -> - log.debug( - "Call not permitted: Circuit is OPEN for circuit breaker {}", - event.getCircuitBreakerName())) - .onEvent( - event -> - log.debug( - "Circuit breaker event type {} for circuit breaker name {}", - event.getEventType(), - event.getCircuitBreakerName())); + CircuitBreaker circuitBreaker = + getCircuitBreakerFromConfigMap(circuitBreakerKey, defaultEnabled); + // If no circuit breaker is created return empty + if (circuitBreaker == null) { + return null; // Ensures cache does not store null entries + } + attachListeners(circuitBreaker); + return circuitBreaker; + })); + } + + public Optional getSharedCircuitBreaker() { + if (!defaultEnabled) { + return Optional.empty(); + } + return Optional.of( + circuitBreakerCache.computeIfAbsent( + SHARED_KEY, + key -> { + CircuitBreaker circuitBreaker = circuitBreakerRegistry.circuitBreaker(SHARED_KEY); + attachListeners(circuitBreaker); return circuitBreaker; })); } - public Optional getDefaultCircuitBreaker() { - return getCircuitBreaker(CircuitBreakerConfigParser.DEFAULT_THRESHOLDS); + private static void attachListeners(CircuitBreaker circuitBreaker) { + circuitBreaker + .getEventPublisher() + .onStateTransition( + event -> + log.info( + "State transition: {} for circuit breaker {}", + event.getStateTransition(), + event.getCircuitBreakerName())) + .onCallNotPermitted( + event -> + log.debug( + "Call not permitted: Circuit is OPEN for circuit breaker {}", + event.getCircuitBreakerName())) + .onEvent( + event -> + log.debug( + "Circuit breaker event type {} for circuit breaker name {}", + event.getEventType(), + event.getCircuitBreakerName())); } - private CircuitBreaker getCircuitBreakerFromConfigMap(String circuitBreakerKey) { + private CircuitBreaker getCircuitBreakerFromConfigMap( + String circuitBreakerKey, boolean defaultEnabled) { return Optional.ofNullable(circuitBreakerConfigMap.get(circuitBreakerKey)) .map(config -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey, config)) - .orElseGet(() -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey)); + .orElseGet( + () -> + defaultEnabled + ? circuitBreakerRegistry.circuitBreaker(circuitBreakerKey) + : null); // Return null if default is disabled } } diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java index f750913..c136b95 100644 --- a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java @@ -64,7 +64,7 @@ void testSendMessage_CallsSuperSendMessage_Success() { void testSendMessage_CircuitBreakerRejectsRequest() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(false); when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.OPEN); - when(mockCircuitBreakerProvider.getDefaultCircuitBreaker()) + when(mockCircuitBreakerProvider.getSharedCircuitBreaker()) .thenReturn(Optional.of(mockCircuitBreaker)); when(mockCircuitBreakerConfig.getExceptionBuilder()) .thenReturn( @@ -93,7 +93,7 @@ void testSendMessage_CircuitBreakerRejectsRequest() { void testSendMessage_CircuitBreakerInHalfOpenState() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(false); when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.HALF_OPEN); - when(mockCircuitBreakerProvider.getDefaultCircuitBreaker()) + when(mockCircuitBreakerProvider.getSharedCircuitBreaker()) .thenReturn(Optional.of(mockCircuitBreaker)); when(mockCircuitBreakerConfig.getExceptionBuilder()) .thenReturn( @@ -121,7 +121,7 @@ void testSendMessage_CircuitBreakerInHalfOpenState() { @Test void testWrapListenerWithCircuitBreaker_Success() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); - when(mockCircuitBreakerProvider.getDefaultCircuitBreaker()) + when(mockCircuitBreakerProvider.getSharedCircuitBreaker()) .thenReturn(Optional.of(mockCircuitBreaker)); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( @@ -147,7 +147,7 @@ void testWrapListenerWithCircuitBreaker_Success() { @Test void testWrapListenerWithCircuitBreaker_Failure() { when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true); - when(mockCircuitBreakerProvider.getDefaultCircuitBreaker()) + when(mockCircuitBreakerProvider.getSharedCircuitBreaker()) .thenReturn(Optional.of(mockCircuitBreaker)); ResilienceCircuitBreakerInterceptor interceptor = new ResilienceCircuitBreakerInterceptor( From 5f3b287f30f28560448c2c1126ad522b8fc74fb1 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Kolamuri Date: Fri, 21 Mar 2025 21:54:57 +0530 Subject: [PATCH 15/15] Addressed comments --- grpc-circuitbreaker-utils/build.gradle.kts | 1 + .../grpcutils/CircuitBreakerConfigParser.java | 2 +- .../ResilienceCircuitBreakerFactory.java | 3 +- .../ResilienceCircuitBreakerProvider.java | 79 ++++++++++--------- 4 files changed, 46 insertions(+), 39 deletions(-) diff --git a/grpc-circuitbreaker-utils/build.gradle.kts b/grpc-circuitbreaker-utils/build.gradle.kts index c892067..fadc1cf 100644 --- a/grpc-circuitbreaker-utils/build.gradle.kts +++ b/grpc-circuitbreaker-utils/build.gradle.kts @@ -14,6 +14,7 @@ dependencies { implementation("org.slf4j:slf4j-api:1.7.36") implementation("io.github.resilience4j:resilience4j-circuitbreaker:1.7.1") implementation("com.typesafe:config:1.4.2") + implementation("com.google.guava:guava:32.0.1-jre") annotationProcessor("org.projectlombok:lombok:1.18.24") compileOnly("org.projectlombok:lombok:1.18.24") diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java index deee9bf..dc4867f 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParser.java @@ -28,7 +28,7 @@ public class CircuitBreakerConfigParser { private static final String SLIDING_WINDOW_TYPE = "slidingWindowType"; public static final String ENABLED = "enabled"; public static final String DEFAULT_THRESHOLDS = "defaultThresholds"; - private static final Set NON_THRESHOLD_KEYS = Set.of(ENABLED); + private static final Set NON_THRESHOLD_KEYS = Set.of(ENABLED, DEFAULT_THRESHOLDS); public static CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder parseConfig( Config config) { diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java index d4add9e..fb47045 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java @@ -21,7 +21,8 @@ public static ResilienceCircuitBreakerInterceptor getResilienceCircuitBreakerInt resilienceCircuitBreakerRegistry, resilienceCircuitBreakerConfigMap, ResilienceCircuitBreakerConfigConverter.getDisabledKeys( - circuitBreakerConfiguration.getCircuitBreakerThresholdsMap())); + circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()), + circuitBreakerConfiguration.getDefaultThresholds().isEnabled()); return new ResilienceCircuitBreakerInterceptor( circuitBreakerConfiguration, clock, resilienceCircuitBreakerProvider); } diff --git a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java index a4806e0..897a3de 100644 --- a/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java +++ b/grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java @@ -1,14 +1,16 @@ package org.hypertrace.circuitbreaker.grpcutils.resilience; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; import io.github.resilience4j.circuitbreaker.CircuitBreaker; import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import lombok.extern.slf4j.Slf4j; -import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerConfigParser; /** Utility class to provide Resilience4j CircuitBreaker */ @Slf4j @@ -17,51 +19,42 @@ class ResilienceCircuitBreakerProvider { private static final String SHARED_KEY = "SHARED_KEY"; private final CircuitBreakerRegistry circuitBreakerRegistry; private final Map circuitBreakerConfigMap; - private final Map circuitBreakerCache = new ConcurrentHashMap<>(); private final List disabledKeys; private final boolean defaultEnabled; + // LoadingCache to manage CircuitBreaker instances with automatic loading and eviction + private final LoadingCache> circuitBreakerCache = + CacheBuilder.newBuilder() + .expireAfterAccess(60, TimeUnit.MINUTES) // Auto-evict after 60 minutes + .maximumSize(10000) // Limit max cache size + .build( + new CacheLoader<>() { + @Override + public Optional load(String key) { + return buildNewCircuitBreaker(key); + } + }); + public ResilienceCircuitBreakerProvider( CircuitBreakerRegistry circuitBreakerRegistry, Map circuitBreakerConfigMap, - List disabledKeys) { + List disabledKeys, + boolean defaultEnabled) { this.circuitBreakerRegistry = circuitBreakerRegistry; this.circuitBreakerConfigMap = circuitBreakerConfigMap; this.disabledKeys = disabledKeys; - this.defaultEnabled = !disabledKeys.contains(CircuitBreakerConfigParser.DEFAULT_THRESHOLDS); + this.defaultEnabled = defaultEnabled; } public Optional getCircuitBreaker(String circuitBreakerKey) { if (disabledKeys.contains(circuitBreakerKey)) { return Optional.empty(); } - return Optional.ofNullable( - circuitBreakerCache.computeIfAbsent( - circuitBreakerKey, - key -> { - CircuitBreaker circuitBreaker = - getCircuitBreakerFromConfigMap(circuitBreakerKey, defaultEnabled); - // If no circuit breaker is created return empty - if (circuitBreaker == null) { - return null; // Ensures cache does not store null entries - } - attachListeners(circuitBreaker); - return circuitBreaker; - })); + return circuitBreakerCache.getUnchecked(circuitBreakerKey); } public Optional getSharedCircuitBreaker() { - if (!defaultEnabled) { - return Optional.empty(); - } - return Optional.of( - circuitBreakerCache.computeIfAbsent( - SHARED_KEY, - key -> { - CircuitBreaker circuitBreaker = circuitBreakerRegistry.circuitBreaker(SHARED_KEY); - attachListeners(circuitBreaker); - return circuitBreaker; - })); + return defaultEnabled ? getCircuitBreaker(SHARED_KEY) : Optional.empty(); } private static void attachListeners(CircuitBreaker circuitBreaker) { @@ -86,14 +79,26 @@ private static void attachListeners(CircuitBreaker circuitBreaker) { event.getCircuitBreakerName())); } - private CircuitBreaker getCircuitBreakerFromConfigMap( - String circuitBreakerKey, boolean defaultEnabled) { + private Optional buildNewCircuitBreaker(String circuitBreakerKey) { return Optional.ofNullable(circuitBreakerConfigMap.get(circuitBreakerKey)) - .map(config -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey, config)) - .orElseGet( - () -> - defaultEnabled - ? circuitBreakerRegistry.circuitBreaker(circuitBreakerKey) - : null); // Return null if default is disabled + .map( + config -> { + CircuitBreaker circuitBreaker = + circuitBreakerRegistry.circuitBreaker(circuitBreakerKey, config); + attachListeners(circuitBreaker); // Attach listeners here + return circuitBreaker; + }) + .or( + () -> { + if (defaultEnabled) { + CircuitBreaker circuitBreaker = + circuitBreakerRegistry.circuitBreaker(circuitBreakerKey); + attachListeners( + circuitBreaker); // Attach listeners here for default circuit breaker + return Optional.of(circuitBreaker); + } else { + return Optional.empty(); + } + }); } }