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 15866f9..006a4f9 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 @@ -24,4 +24,7 @@ public class CircuitBreakerConfiguration { @Builder.Default Function exceptionBuilder = reason -> Status.RESOURCE_EXHAUSTED.withDescription(reason).asRuntimeException(); + + // config key to access non default circuit breaker thresholds + String configKey; } 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 28cfd0c..c5c262f 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 @@ -18,7 +18,7 @@ public static Map getCircuitBreakerConfigs( public static List getDisabledKeys( Map configurationMap) { return configurationMap.entrySet().stream() - .filter(entry -> entry.getValue().isEnabled()) + .filter(entry -> !entry.getValue().isEnabled()) .map(Map.Entry::getKey) .collect(Collectors.toList()); } 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 fb47045..d9ff1f5 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 @@ -22,7 +22,8 @@ public static ResilienceCircuitBreakerInterceptor getResilienceCircuitBreakerInt resilienceCircuitBreakerConfigMap, ResilienceCircuitBreakerConfigConverter.getDisabledKeys( circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()), - circuitBreakerConfiguration.getDefaultThresholds().isEnabled()); + circuitBreakerConfiguration.getDefaultThresholds().isEnabled(), + circuitBreakerConfiguration.getConfigKey()); 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 a834130..df37dd2 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 @@ -8,6 +8,7 @@ import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.TimeUnit; import lombok.extern.slf4j.Slf4j; @@ -21,6 +22,7 @@ class ResilienceCircuitBreakerProvider { private final Map circuitBreakerConfigMap; private final List disabledKeys; private final boolean defaultEnabled; + private final String configKey; // LoadingCache to manage CircuitBreaker instances with automatic loading and eviction private final LoadingCache> circuitBreakerCache = @@ -33,17 +35,23 @@ public ResilienceCircuitBreakerProvider( CircuitBreakerRegistry circuitBreakerRegistry, Map circuitBreakerConfigMap, List disabledKeys, - boolean defaultEnabled) { + boolean defaultEnabled, + String configKey) { this.circuitBreakerRegistry = circuitBreakerRegistry; this.circuitBreakerConfigMap = circuitBreakerConfigMap; this.disabledKeys = disabledKeys; this.defaultEnabled = defaultEnabled; + this.configKey = configKey; } public Optional getCircuitBreaker(String circuitBreakerKey) { - if (disabledKeys.contains(circuitBreakerKey)) { + if (disabledKeys.contains(circuitBreakerKey) + || (Objects.nonNull(configKey) + && !circuitBreakerConfigMap.containsKey(circuitBreakerKey) + && disabledKeys.contains(configKey))) { return Optional.empty(); } + return circuitBreakerCache.getUnchecked(circuitBreakerKey); } @@ -75,6 +83,7 @@ private static void attachListeners(CircuitBreaker circuitBreaker) { private Optional buildNewCircuitBreaker(String circuitBreakerKey) { return Optional.ofNullable(circuitBreakerConfigMap.get(circuitBreakerKey)) + .or(() -> Optional.ofNullable(configKey).map(circuitBreakerConfigMap::get)) .map(config -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey, config)) .or( () -> diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParserTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParserTest.java new file mode 100644 index 0000000..23986ed --- /dev/null +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParserTest.java @@ -0,0 +1,80 @@ +package org.hypertrace.circuitbreaker.grpcutils; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.typesafe.config.Config; +import com.typesafe.config.ConfigFactory; +import java.time.Duration; +import java.util.Map; +import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerThresholds.SlidingWindowType; +import org.junit.jupiter.api.Test; + +class CircuitBreakerConfigParserTest { + + @Test + void testParseConfig() { + String configStr = + "enabled = true\n" + + "defaultThresholds {\n" + + " failureRateThreshold = 50.0\n" + + " slowCallRateThreshold = 30.0\n" + + " slowCallDurationThreshold = 1s\n" + + " slidingWindowSize = 100\n" + + " slidingWindowType = COUNT_BASED\n" + + "}\n" + + "service1 {\n" + + " failureRateThreshold = 60.0\n" + + " slowCallRateThreshold = 40.0\n" + + " slowCallDurationThreshold = 2s\n" + + " slidingWindowSize = 200\n" + + " slidingWindowType = TIME_BASED\n" + + "}"; + + Config config = ConfigFactory.parseString(configStr); + CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder builder = + CircuitBreakerConfigParser.parseConfig(config); + CircuitBreakerConfiguration configuration = builder.build(); + + // Test enabled flag + assertTrue(configuration.isEnabled()); + + // Test default thresholds + CircuitBreakerThresholds defaultThresholds = configuration.getDefaultThresholds(); + assertNotNull(defaultThresholds); + assertEquals(50.0f, defaultThresholds.getFailureRateThreshold()); + assertEquals(30.0f, defaultThresholds.getSlowCallRateThreshold()); + assertEquals(Duration.ofSeconds(1), defaultThresholds.getSlowCallDurationThreshold()); + assertEquals(100, defaultThresholds.getSlidingWindowSize()); + assertEquals(SlidingWindowType.COUNT_BASED, defaultThresholds.getSlidingWindowType()); + + // Test service specific thresholds + Map thresholdsMap = + configuration.getCircuitBreakerThresholdsMap(); + assertNotNull(thresholdsMap); + assertTrue(thresholdsMap.containsKey("service1")); + + CircuitBreakerThresholds service1Thresholds = thresholdsMap.get("service1"); + assertEquals(60.0f, service1Thresholds.getFailureRateThreshold()); + assertEquals(40.0f, service1Thresholds.getSlowCallRateThreshold()); + assertEquals(Duration.ofSeconds(2), service1Thresholds.getSlowCallDurationThreshold()); + assertEquals(200, service1Thresholds.getSlidingWindowSize()); + assertEquals(SlidingWindowType.TIME_BASED, service1Thresholds.getSlidingWindowType()); + } + + @Test + void testParseConfigWithMinimalConfig() { + String configStr = "{}"; + Config config = ConfigFactory.parseString(configStr); + CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder builder = + CircuitBreakerConfigParser.parseConfig(config); + CircuitBreakerConfiguration configuration = builder.build(); + + // Test that defaults are used when no config is provided + assertFalse(configuration.isEnabled()); + assertNotNull(configuration.getDefaultThresholds()); + assertTrue(configuration.getCircuitBreakerThresholdsMap().isEmpty()); + } +} diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverterTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverterTest.java index b49f3b3..46f27d1 100644 --- a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverterTest.java +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerConfigConverterTest.java @@ -2,10 +2,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; import java.time.Duration; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.hypertrace.circuitbreaker.grpcutils.CircuitBreakerThresholds; import org.junit.jupiter.api.Assertions; @@ -51,4 +53,35 @@ void shouldThrowExceptionWhenConfigurationIsNull() { NullPointerException.class, () -> ResilienceCircuitBreakerConfigConverter.convertConfig(null)); } + + @Test + void shouldGetDisabledKeys() { + // Create a mix of enabled and disabled configurations + CircuitBreakerThresholds enabledThresholds = + CircuitBreakerThresholds.builder().enabled(true).failureRateThreshold(50.0f).build(); + + CircuitBreakerThresholds disabledThresholds1 = + CircuitBreakerThresholds.builder().enabled(false).failureRateThreshold(50.0f).build(); + + CircuitBreakerThresholds disabledThresholds2 = + CircuitBreakerThresholds.builder().enabled(false).failureRateThreshold(60.0f).build(); + + Map configMap = new HashMap<>(); + configMap.put("enabledService", enabledThresholds); + configMap.put("disabledService1", disabledThresholds1); + configMap.put("disabledService2", disabledThresholds2); + + List disabledKeys = ResilienceCircuitBreakerConfigConverter.getDisabledKeys(configMap); + + assertEquals(2, disabledKeys.size()); + assertTrue(disabledKeys.contains("disabledService1")); + assertTrue(disabledKeys.contains("disabledService2")); + } + + @Test + void shouldGetEmptyDisabledKeysForEmptyConfig() { + Map configMap = new HashMap<>(); + List disabledKeys = ResilienceCircuitBreakerConfigConverter.getDisabledKeys(configMap); + assertTrue(disabledKeys.isEmpty()); + } } diff --git a/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProviderTest.java b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProviderTest.java new file mode 100644 index 0000000..db195c3 --- /dev/null +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProviderTest.java @@ -0,0 +1,184 @@ +package org.hypertrace.circuitbreaker.grpcutils.resilience; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import io.github.resilience4j.circuitbreaker.CircuitBreaker; +import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; +import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class ResilienceCircuitBreakerProviderTest { + + @Mock private CircuitBreakerRegistry circuitBreakerRegistry; + @Mock private CircuitBreaker mockCircuitBreaker; + @Mock private CircuitBreaker.EventPublisher eventPublisher; + + private ResilienceCircuitBreakerProvider provider; + private Map configMap; + private List disabledKeys; + private static final String CONFIG_KEY = "defaultConfig"; + private static final String ENABLED_SERVICE = "enabledService"; + private static final String DISABLED_SERVICE = "disabledService"; + private static final String NON_CONFIGURED_SERVICE = "nonConfiguredService"; + private CircuitBreakerConfig defaultConfig; + private CircuitBreakerConfig serviceConfig; + + @BeforeEach + void setup() { + defaultConfig = CircuitBreakerConfig.custom().build(); + serviceConfig = CircuitBreakerConfig.custom().build(); + + configMap = new HashMap<>(); + configMap.put(ENABLED_SERVICE, serviceConfig); + configMap.put(CONFIG_KEY, defaultConfig); + + disabledKeys = Arrays.asList(DISABLED_SERVICE); + + // Use lenient() for all mocks since they might not be used in every test + lenient().when(mockCircuitBreaker.getEventPublisher()).thenReturn(eventPublisher); + lenient().when(eventPublisher.onStateTransition(any())).thenReturn(eventPublisher); + lenient().when(eventPublisher.onCallNotPermitted(any())).thenReturn(eventPublisher); + lenient().doNothing().when(eventPublisher).onEvent(any()); + + lenient() + .when(circuitBreakerRegistry.circuitBreaker(eq(ENABLED_SERVICE), eq(serviceConfig))) + .thenReturn(mockCircuitBreaker); + lenient() + .when(circuitBreakerRegistry.circuitBreaker(eq(NON_CONFIGURED_SERVICE), eq(defaultConfig))) + .thenReturn(mockCircuitBreaker); + + provider = + new ResilienceCircuitBreakerProvider( + circuitBreakerRegistry, configMap, disabledKeys, true, CONFIG_KEY); + } + + @Test + void shouldReturnCircuitBreakerForEnabledKey() { + Optional result = provider.getCircuitBreaker(ENABLED_SERVICE); + assertTrue(result.isPresent()); + assertEquals(mockCircuitBreaker, result.get()); + + // Verify cache behavior by getting the same key again + Optional secondResult = provider.getCircuitBreaker(ENABLED_SERVICE); + assertTrue(secondResult.isPresent()); + assertEquals(mockCircuitBreaker, secondResult.get()); + + // Verify circuit breaker was created only once + verify(circuitBreakerRegistry, times(1)).circuitBreaker(eq(ENABLED_SERVICE), eq(serviceConfig)); + } + + @Test + void shouldReturnEmptyForDisabledKey() { + Optional result = provider.getCircuitBreaker(DISABLED_SERVICE); + assertFalse(result.isPresent()); + } + + @Test + void shouldReturnEmptyForNonConfiguredServiceWhenConfigKeyDisabled() { + // Create provider with disabled config key + ResilienceCircuitBreakerProvider providerWithDisabledConfig = + new ResilienceCircuitBreakerProvider( + circuitBreakerRegistry, + configMap, + Arrays.asList(DISABLED_SERVICE, CONFIG_KEY), // CONFIG_KEY is disabled + true, + CONFIG_KEY); + + Optional result = + providerWithDisabledConfig.getCircuitBreaker(NON_CONFIGURED_SERVICE); + assertFalse(result.isPresent()); + } + + @Test + void shouldReturnCircuitBreakerForNonConfiguredServiceWhenConfigKeyEnabled() { + Optional result = provider.getCircuitBreaker(NON_CONFIGURED_SERVICE); + assertTrue(result.isPresent()); + assertEquals(mockCircuitBreaker, result.get()); + + // Verify it used the default config + verify(circuitBreakerRegistry, times(1)) + .circuitBreaker(eq(NON_CONFIGURED_SERVICE), eq(defaultConfig)); + } + + @Test + void shouldCacheCircuitBreakerInstances() { + // Get the same circuit breaker multiple times + provider.getCircuitBreaker(ENABLED_SERVICE); + provider.getCircuitBreaker(ENABLED_SERVICE); + provider.getCircuitBreaker(ENABLED_SERVICE); + + // Verify circuit breaker was created only once despite multiple calls + verify(circuitBreakerRegistry, times(1)).circuitBreaker(eq(ENABLED_SERVICE), eq(serviceConfig)); + } + + @Test + void shouldHandleAllBuildNewCircuitBreakerScenarios() { + // Case 1: Service with specific config against circuit breaker key + Optional serviceSpecificResult = provider.getCircuitBreaker(ENABLED_SERVICE); + assertTrue(serviceSpecificResult.isPresent()); + assertEquals(mockCircuitBreaker, serviceSpecificResult.get()); + verify(circuitBreakerRegistry).circuitBreaker(ENABLED_SERVICE, serviceConfig); + + // Case 2: Service without specific config against circuit breaker key but with config present + // against config key + String fallbackService = "fallbackService"; + lenient() + .when(circuitBreakerRegistry.circuitBreaker(eq(fallbackService), eq(defaultConfig))) + .thenReturn(mockCircuitBreaker); + Optional fallbackResult = provider.getCircuitBreaker(fallbackService); + assertTrue(fallbackResult.isPresent()); + assertEquals(mockCircuitBreaker, fallbackResult.get()); + verify(circuitBreakerRegistry).circuitBreaker(fallbackService, defaultConfig); + + // Case 3: Service with no config present against circuit breaker key and no config key, but + // defaultEnabled=true + String noConfigService = "noConfigService"; + ResilienceCircuitBreakerProvider defaultEnabledProvider = + new ResilienceCircuitBreakerProvider( + circuitBreakerRegistry, + configMap, + disabledKeys, + true, // defaultEnabled = true + null); // no config key + + lenient() + .when(circuitBreakerRegistry.circuitBreaker(eq(noConfigService))) + .thenReturn(mockCircuitBreaker); + Optional noConfigResult = + defaultEnabledProvider.getCircuitBreaker(noConfigService); + assertTrue(noConfigResult.isPresent()); + assertEquals(mockCircuitBreaker, noConfigResult.get()); + verify(circuitBreakerRegistry).circuitBreaker(noConfigService); + + // Case 4: Service with no config present against circuit breaker key and no config key, and + // defaultEnabled=false + ResilienceCircuitBreakerProvider disabledDefaultProvider = + new ResilienceCircuitBreakerProvider( + circuitBreakerRegistry, + configMap, + disabledKeys, + false, // defaultEnabled = false + null); // no config key + + Optional disabledDefaultResult = + disabledDefaultProvider.getCircuitBreaker(noConfigService); + assertFalse(disabledDefaultResult.isPresent()); + } +}