Skip to content

Commit f163a9d

Browse files
pavan-traceablepavan-traceable
committed
Fixed corner cases
1 parent ea47f32 commit f163a9d

File tree

3 files changed

+63
-37
lines changed

3 files changed

+63
-37
lines changed

‎grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptor.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ protected <ReqT, RespT> ClientCall<ReqT, RespT> createInterceptedCall(
4747
return new ForwardingClientCall.SimpleForwardingClientCall<>(
4848
next.newCall(method, callOptions)) {
4949
Optional<CircuitBreaker> optionalCircuitBreaker;
50-
String circuitBreakerKey;
5150

5251
@Override
5352
public void start(Listener<RespT> responseListener, Metadata headers) {
@@ -68,14 +67,15 @@ public void sendMessage(ReqT message) {
6867
super.sendMessage(message);
6968
return;
7069
}
70+
String circuitBreakerKey = null;
7171
if (config.getKeyFunction() != null) {
7272
circuitBreakerKey = config.getKeyFunction().apply(RequestContext.CURRENT.get(), message);
73-
optionalCircuitBreaker =
74-
resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey);
75-
} else {
76-
log.debug("Circuit breaker will apply to all requests as keyFunction config is not set");
77-
optionalCircuitBreaker = resilienceCircuitBreakerProvider.getDefaultCircuitBreaker();
7873
}
74+
optionalCircuitBreaker =
75+
circuitBreakerKey != null
76+
? resilienceCircuitBreakerProvider.getCircuitBreaker(circuitBreakerKey)
77+
: resilienceCircuitBreakerProvider.getSharedCircuitBreaker();
78+
7979
CircuitBreaker circuitBreaker = optionalCircuitBreaker.orElse(null);
8080
if (circuitBreaker == null) {
8181
super.sendMessage(message);

‎grpc-circuitbreaker-utils/src/main/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerProvider.java‎

Lines changed: 53 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@
1414
@Slf4j
1515
class ResilienceCircuitBreakerProvider {
1616

17+
private static final String SHARED_KEY = "SHARED_KEY";
1718
private final CircuitBreakerRegistry circuitBreakerRegistry;
1819
private final Map<String, CircuitBreakerConfig> circuitBreakerConfigMap;
1920
private final Map<String, CircuitBreaker> circuitBreakerCache = new ConcurrentHashMap<>();
2021
private final List<String> disabledKeys;
22+
private final boolean defaultEnabled;
2123

2224
public ResilienceCircuitBreakerProvider(
2325
CircuitBreakerRegistry circuitBreakerRegistry,
@@ -26,48 +28,72 @@ public ResilienceCircuitBreakerProvider(
2628
this.circuitBreakerRegistry = circuitBreakerRegistry;
2729
this.circuitBreakerConfigMap = circuitBreakerConfigMap;
2830
this.disabledKeys = disabledKeys;
31+
this.defaultEnabled = !disabledKeys.contains(CircuitBreakerConfigParser.DEFAULT_THRESHOLDS);
2932
}
3033

3134
public Optional<CircuitBreaker> getCircuitBreaker(String circuitBreakerKey) {
32-
if (disabledKeys.contains(circuitBreakerKey)
33-
|| !circuitBreakerConfigMap.containsKey(circuitBreakerKey)) {
35+
if (disabledKeys.contains(circuitBreakerKey)) {
3436
return Optional.empty();
3537
}
36-
return Optional.of(
38+
return Optional.ofNullable(
3739
circuitBreakerCache.computeIfAbsent(
3840
circuitBreakerKey,
3941
key -> {
40-
CircuitBreaker circuitBreaker = getCircuitBreakerFromConfigMap(circuitBreakerKey);
41-
circuitBreaker
42-
.getEventPublisher()
43-
.onStateTransition(
44-
event ->
45-
log.info(
46-
"State transition: {} for circuit breaker {}",
47-
event.getStateTransition(),
48-
event.getCircuitBreakerName()))
49-
.onCallNotPermitted(
50-
event ->
51-
log.debug(
52-
"Call not permitted: Circuit is OPEN for circuit breaker {}",
53-
event.getCircuitBreakerName()))
54-
.onEvent(
55-
event ->
56-
log.debug(
57-
"Circuit breaker event type {} for circuit breaker name {}",
58-
event.getEventType(),
59-
event.getCircuitBreakerName()));
42+
CircuitBreaker circuitBreaker =
43+
getCircuitBreakerFromConfigMap(circuitBreakerKey, defaultEnabled);
44+
// If no circuit breaker is created return empty
45+
if (circuitBreaker == null) {
46+
return null; // Ensures cache does not store null entries
47+
}
48+
attachListeners(circuitBreaker);
49+
return circuitBreaker;
50+
}));
51+
}
52+
53+
public Optional<CircuitBreaker> getSharedCircuitBreaker() {
54+
if (!defaultEnabled) {
55+
return Optional.empty();
56+
}
57+
return Optional.of(
58+
circuitBreakerCache.computeIfAbsent(
59+
SHARED_KEY,
60+
key -> {
61+
CircuitBreaker circuitBreaker = circuitBreakerRegistry.circuitBreaker(SHARED_KEY);
62+
attachListeners(circuitBreaker);
6063
return circuitBreaker;
6164
}));
6265
}
6366

64-
public Optional<CircuitBreaker> getDefaultCircuitBreaker() {
65-
return getCircuitBreaker(CircuitBreakerConfigParser.DEFAULT_THRESHOLDS);
67+
private static void attachListeners(CircuitBreaker circuitBreaker) {
68+
circuitBreaker
69+
.getEventPublisher()
70+
.onStateTransition(
71+
event ->
72+
log.info(
73+
"State transition: {} for circuit breaker {}",
74+
event.getStateTransition(),
75+
event.getCircuitBreakerName()))
76+
.onCallNotPermitted(
77+
event ->
78+
log.debug(
79+
"Call not permitted: Circuit is OPEN for circuit breaker {}",
80+
event.getCircuitBreakerName()))
81+
.onEvent(
82+
event ->
83+
log.debug(
84+
"Circuit breaker event type {} for circuit breaker name {}",
85+
event.getEventType(),
86+
event.getCircuitBreakerName()));
6687
}
6788

68-
private CircuitBreaker getCircuitBreakerFromConfigMap(String circuitBreakerKey) {
89+
private CircuitBreaker getCircuitBreakerFromConfigMap(
90+
String circuitBreakerKey, boolean defaultEnabled) {
6991
return Optional.ofNullable(circuitBreakerConfigMap.get(circuitBreakerKey))
7092
.map(config -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey, config))
71-
.orElseGet(() -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey));
93+
.orElseGet(
94+
() ->
95+
defaultEnabled
96+
? circuitBreakerRegistry.circuitBreaker(circuitBreakerKey)
97+
: null); // Return null if default is disabled
7298
}
7399
}

‎grpc-circuitbreaker-utils/src/test/java/org/hypertrace/circuitbreaker/grpcutils/resilience/ResilienceCircuitBreakerInterceptorTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ void testSendMessage_CallsSuperSendMessage_Success() {
6464
void testSendMessage_CircuitBreakerRejectsRequest() {
6565
when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(false);
6666
when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.OPEN);
67-
when(mockCircuitBreakerProvider.getDefaultCircuitBreaker())
67+
when(mockCircuitBreakerProvider.getSharedCircuitBreaker())
6868
.thenReturn(Optional.of(mockCircuitBreaker));
6969
when(mockCircuitBreakerConfig.getExceptionBuilder())
7070
.thenReturn(
@@ -93,7 +93,7 @@ void testSendMessage_CircuitBreakerRejectsRequest() {
9393
void testSendMessage_CircuitBreakerInHalfOpenState() {
9494
when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(false);
9595
when(mockCircuitBreaker.getState()).thenReturn(CircuitBreaker.State.HALF_OPEN);
96-
when(mockCircuitBreakerProvider.getDefaultCircuitBreaker())
96+
when(mockCircuitBreakerProvider.getSharedCircuitBreaker())
9797
.thenReturn(Optional.of(mockCircuitBreaker));
9898
when(mockCircuitBreakerConfig.getExceptionBuilder())
9999
.thenReturn(
@@ -121,7 +121,7 @@ void testSendMessage_CircuitBreakerInHalfOpenState() {
121121
@Test
122122
void testWrapListenerWithCircuitBreaker_Success() {
123123
when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true);
124-
when(mockCircuitBreakerProvider.getDefaultCircuitBreaker())
124+
when(mockCircuitBreakerProvider.getSharedCircuitBreaker())
125125
.thenReturn(Optional.of(mockCircuitBreaker));
126126
ResilienceCircuitBreakerInterceptor interceptor =
127127
new ResilienceCircuitBreakerInterceptor(
@@ -147,7 +147,7 @@ void testWrapListenerWithCircuitBreaker_Success() {
147147
@Test
148148
void testWrapListenerWithCircuitBreaker_Failure() {
149149
when(mockCircuitBreaker.tryAcquirePermission()).thenReturn(true);
150-
when(mockCircuitBreakerProvider.getDefaultCircuitBreaker())
150+
when(mockCircuitBreakerProvider.getSharedCircuitBreaker())
151151
.thenReturn(Optional.of(mockCircuitBreaker));
152152
ResilienceCircuitBreakerInterceptor interceptor =
153153
new ResilienceCircuitBreakerInterceptor(

0 commit comments

Comments
 (0)