From ce95df7d1455fbd4d06127f4a5d049f4af01335a Mon Sep 17 00:00:00 2001 From: Gaurav Gupta Date: Thu, 3 Apr 2025 12:57:36 +0530 Subject: [PATCH 1/4] Add config key in circuit breaker config to fetch config level thresholds --- .../grpcutils/CircuitBreakerConfiguration.java | 3 +++ .../ResilienceCircuitBreakerConfigConverter.java | 2 +- .../resilience/ResilienceCircuitBreakerFactory.java | 3 ++- .../ResilienceCircuitBreakerProvider.java | 13 +++++++++++-- 4 files changed, 17 insertions(+), 4 deletions(-) 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( () -> From ab5ca7080457d7f2ecf5a62f7a71ea6ca70220db Mon Sep 17 00:00:00 2001 From: Gaurav Gupta Date: Fri, 4 Apr 2025 11:10:03 +0530 Subject: [PATCH 2/4] Added test cases --- .../CircuitBreakerConfigParserTest.java | 79 +++++++++++ ...enceCircuitBreakerConfigConverterTest.java | 42 ++++++ .../ResilienceCircuitBreakerProviderTest.java | 131 ++++++++++++++++++ 3 files changed, 252 insertions(+) create mode 100644 grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParserTest.java create mode 100644 grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProviderTest.java 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..56114a1 --- /dev/null +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfigParserTest.java @@ -0,0 +1,79 @@ +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..fd31cf2 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,44 @@ 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..f905027 --- /dev/null +++ b/grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProviderTest.java @@ -0,0 +1,131 @@ +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)); + } +} From 17bc5a01186e0341bdd1ba3045933b7aab72d2f7 Mon Sep 17 00:00:00 2001 From: Gaurav Gupta Date: Sun, 6 Apr 2025 10:58:51 +0530 Subject: [PATCH 3/4] Added test case for new circuit breaker builder --- .../ResilienceCircuitBreakerProviderTest.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) 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 index f905027..e04fe9f 100644 --- 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 @@ -128,4 +128,49 @@ void shouldCacheCircuitBreakerInstances() { 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()); + } } From 21756b5baf824178fc0ebe7c895f67bae7c87a32 Mon Sep 17 00:00:00 2001 From: Gaurav Gupta Date: Tue, 8 Apr 2025 18:42:23 +0530 Subject: [PATCH 4/4] fix code format violations --- .../CircuitBreakerConfigParserTest.java | 33 +++---- ...enceCircuitBreakerConfigConverterTest.java | 15 +--- .../ResilienceCircuitBreakerProviderTest.java | 90 ++++++++++--------- 3 files changed, 69 insertions(+), 69 deletions(-) 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 index 56114a1..23986ed 100644 --- 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 @@ -17,21 +17,21 @@ 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" + - "}"; + "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 = @@ -51,7 +51,8 @@ void testParseConfig() { assertEquals(SlidingWindowType.COUNT_BASED, defaultThresholds.getSlidingWindowType()); // Test service specific thresholds - Map thresholdsMap = configuration.getCircuitBreakerThresholdsMap(); + Map thresholdsMap = + configuration.getCircuitBreakerThresholdsMap(); assertNotNull(thresholdsMap); assertTrue(thresholdsMap.containsKey("service1")); 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 fd31cf2..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 @@ -58,22 +58,13 @@ void shouldThrowExceptionWhenConfigurationIsNull() { void shouldGetDisabledKeys() { // Create a mix of enabled and disabled configurations CircuitBreakerThresholds enabledThresholds = - CircuitBreakerThresholds.builder() - .enabled(true) - .failureRateThreshold(50.0f) - .build(); + CircuitBreakerThresholds.builder().enabled(true).failureRateThreshold(50.0f).build(); CircuitBreakerThresholds disabledThresholds1 = - CircuitBreakerThresholds.builder() - .enabled(false) - .failureRateThreshold(50.0f) - .build(); + CircuitBreakerThresholds.builder().enabled(false).failureRateThreshold(50.0f).build(); CircuitBreakerThresholds disabledThresholds2 = - CircuitBreakerThresholds.builder() - .enabled(false) - .failureRateThreshold(60.0f) - .build(); + CircuitBreakerThresholds.builder().enabled(false).failureRateThreshold(60.0f).build(); Map configMap = new HashMap<>(); configMap.put("enabledService", enabledThresholds); 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 index e04fe9f..db195c3 100644 --- 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 @@ -57,17 +57,16 @@ void setup() { lenient().when(eventPublisher.onCallNotPermitted(any())).thenReturn(eventPublisher); lenient().doNothing().when(eventPublisher).onEvent(any()); - lenient().when(circuitBreakerRegistry.circuitBreaker(eq(ENABLED_SERVICE), eq(serviceConfig))) + lenient() + .when(circuitBreakerRegistry.circuitBreaker(eq(ENABLED_SERVICE), eq(serviceConfig))) .thenReturn(mockCircuitBreaker); - lenient().when(circuitBreakerRegistry.circuitBreaker(eq(NON_CONFIGURED_SERVICE), eq(defaultConfig))) + lenient() + .when(circuitBreakerRegistry.circuitBreaker(eq(NON_CONFIGURED_SERVICE), eq(defaultConfig))) .thenReturn(mockCircuitBreaker); - provider = new ResilienceCircuitBreakerProvider( - circuitBreakerRegistry, - configMap, - disabledKeys, - true, - CONFIG_KEY); + provider = + new ResilienceCircuitBreakerProvider( + circuitBreakerRegistry, configMap, disabledKeys, true, CONFIG_KEY); } @Test @@ -82,8 +81,7 @@ void shouldReturnCircuitBreakerForEnabledKey() { assertEquals(mockCircuitBreaker, secondResult.get()); // Verify circuit breaker was created only once - verify(circuitBreakerRegistry, times(1)) - .circuitBreaker(eq(ENABLED_SERVICE), eq(serviceConfig)); + verify(circuitBreakerRegistry, times(1)).circuitBreaker(eq(ENABLED_SERVICE), eq(serviceConfig)); } @Test @@ -95,14 +93,16 @@ void shouldReturnEmptyForDisabledKey() { @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); + 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()); } @@ -125,8 +125,7 @@ void shouldCacheCircuitBreakerInstances() { provider.getCircuitBreaker(ENABLED_SERVICE); // Verify circuit breaker was created only once despite multiple calls - verify(circuitBreakerRegistry, times(1)) - .circuitBreaker(eq(ENABLED_SERVICE), eq(serviceConfig)); + verify(circuitBreakerRegistry, times(1)).circuitBreaker(eq(ENABLED_SERVICE), eq(serviceConfig)); } @Test @@ -137,40 +136,49 @@ void shouldHandleAllBuildNewCircuitBreakerScenarios() { 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 + // 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))) + 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 + // 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))) + 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); + 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); + // 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()); } }