Skip to content

Grpc cb utility update #70

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,7 @@
@Builder.Default
Function<String, StatusRuntimeException> exceptionBuilder =
reason -> Status.RESOURCE_EXHAUSTED.withDescription(reason).asRuntimeException();

// config key to access non default circuit breaker thresholds
String configKey;

Check warning on line 29 in grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java

View check run for this annotation

Codecov / codecov/patch

grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/CircuitBreakerConfiguration.java#L29

Added line #L29 was not covered by tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public static Map<String, CircuitBreakerConfig> getCircuitBreakerConfigs(
public static List<String> getDisabledKeys(
Map<String, CircuitBreakerThresholds> configurationMap) {
return configurationMap.entrySet().stream()
.filter(entry -> entry.getValue().isEnabled())
.filter(entry -> !entry.getValue().isEnabled())
.map(Map.Entry::getKey)
.collect(Collectors.toList());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
resilienceCircuitBreakerConfigMap,
ResilienceCircuitBreakerConfigConverter.getDisabledKeys(
circuitBreakerConfiguration.getCircuitBreakerThresholdsMap()),
circuitBreakerConfiguration.getDefaultThresholds().isEnabled());
circuitBreakerConfiguration.getDefaultThresholds().isEnabled(),
circuitBreakerConfiguration.getConfigKey());

Check warning on line 26 in grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java

View check run for this annotation

Codecov / codecov/patch

grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerFactory.java#L25-L26

Added lines #L25 - L26 were not covered by tests
return new ResilienceCircuitBreakerInterceptor(
circuitBreakerConfiguration, clock, resilienceCircuitBreakerProvider);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -21,6 +22,7 @@ class ResilienceCircuitBreakerProvider {
private final Map<String, CircuitBreakerConfig> circuitBreakerConfigMap;
private final List<String> disabledKeys;
private final boolean defaultEnabled;
private final String configKey;

// LoadingCache to manage CircuitBreaker instances with automatic loading and eviction
private final LoadingCache<String, Optional<CircuitBreaker>> circuitBreakerCache =
Expand All @@ -33,17 +35,23 @@ public ResilienceCircuitBreakerProvider(
CircuitBreakerRegistry circuitBreakerRegistry,
Map<String, CircuitBreakerConfig> circuitBreakerConfigMap,
List<String> disabledKeys,
boolean defaultEnabled) {
boolean defaultEnabled,
String configKey) {
this.circuitBreakerRegistry = circuitBreakerRegistry;
this.circuitBreakerConfigMap = circuitBreakerConfigMap;
this.disabledKeys = disabledKeys;
this.defaultEnabled = defaultEnabled;
this.configKey = configKey;
}

public Optional<CircuitBreaker> 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);
}

Expand Down Expand Up @@ -75,6 +83,7 @@ private static void attachListeners(CircuitBreaker circuitBreaker) {

private Optional<CircuitBreaker> buildNewCircuitBreaker(String circuitBreakerKey) {
return Optional.ofNullable(circuitBreakerConfigMap.get(circuitBreakerKey))
.or(() -> Optional.ofNullable(configKey).map(circuitBreakerConfigMap::get))
.map(config -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey, config))
.or(
() ->
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Object> builder =
CircuitBreakerConfigParser.parseConfig(config);
CircuitBreakerConfiguration<Object> 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<String, CircuitBreakerThresholds> 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<Object> builder =
CircuitBreakerConfigParser.parseConfig(config);
CircuitBreakerConfiguration<Object> configuration = builder.build();

// Test that defaults are used when no config is provided
assertFalse(configuration.isEnabled());
assertNotNull(configuration.getDefaultThresholds());
assertTrue(configuration.getCircuitBreakerThresholdsMap().isEmpty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, CircuitBreakerThresholds> configMap = new HashMap<>();
configMap.put("enabledService", enabledThresholds);
configMap.put("disabledService1", disabledThresholds1);
configMap.put("disabledService2", disabledThresholds2);

List<String> disabledKeys = ResilienceCircuitBreakerConfigConverter.getDisabledKeys(configMap);

assertEquals(2, disabledKeys.size());
assertTrue(disabledKeys.contains("disabledService1"));
assertTrue(disabledKeys.contains("disabledService2"));
}

@Test
void shouldGetEmptyDisabledKeysForEmptyConfig() {
Map<String, CircuitBreakerThresholds> configMap = new HashMap<>();
List<String> disabledKeys = ResilienceCircuitBreakerConfigConverter.getDisabledKeys(configMap);
assertTrue(disabledKeys.isEmpty());
}
}
Original file line number Diff line number Diff line change
@@ -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<String, CircuitBreakerConfig> configMap;
private List<String> 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<CircuitBreaker> result = provider.getCircuitBreaker(ENABLED_SERVICE);
assertTrue(result.isPresent());
assertEquals(mockCircuitBreaker, result.get());

// Verify cache behavior by getting the same key again
Optional<CircuitBreaker> 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<CircuitBreaker> 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<CircuitBreaker> result =
providerWithDisabledConfig.getCircuitBreaker(NON_CONFIGURED_SERVICE);
assertFalse(result.isPresent());
}

@Test
void shouldReturnCircuitBreakerForNonConfiguredServiceWhenConfigKeyEnabled() {
Optional<CircuitBreaker> 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<CircuitBreaker> 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<CircuitBreaker> 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<CircuitBreaker> 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<CircuitBreaker> disabledDefaultResult =
disabledDefaultProvider.getCircuitBreaker(noConfigService);
assertFalse(disabledDefaultResult.isPresent());
}
}
Loading