Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions grpc-circuitbreaker-utils/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> NON_THRESHOLD_KEYS = Set.of(ENABLED);
private static final Set<String> NON_THRESHOLD_KEYS = Set.of(ENABLED, DEFAULT_THRESHOLDS);

public static <T> CircuitBreakerConfiguration.CircuitBreakerConfigurationBuilder<T> parseConfig(
Config config) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -17,51 +19,42 @@ class ResilienceCircuitBreakerProvider {
private static final String SHARED_KEY = "SHARED_KEY";
private final CircuitBreakerRegistry circuitBreakerRegistry;
private final Map<String, CircuitBreakerConfig> circuitBreakerConfigMap;
private final Map<String, CircuitBreaker> circuitBreakerCache = new ConcurrentHashMap<>();
private final List<String> disabledKeys;
private final boolean defaultEnabled;

// LoadingCache to manage CircuitBreaker instances with automatic loading and eviction
private final LoadingCache<String, Optional<CircuitBreaker>> circuitBreakerCache =
CacheBuilder.newBuilder()
.expireAfterAccess(60, TimeUnit.MINUTES) // Auto-evict after 60 minutes
.maximumSize(10000) // Limit max cache size
.build(
new CacheLoader<>() {
@Override
public Optional<CircuitBreaker> load(String key) {
return buildNewCircuitBreaker(key);
}
});
Comment on lines +30 to +36
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
.build(
new CacheLoader<>() {
@Override
public Optional<CircuitBreaker> load(String key) {
return buildNewCircuitBreaker(key);
}
});
.build(CacheLoader.from(this::buildNewCircuitBreaker));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix in immediate PR, merging this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


public ResilienceCircuitBreakerProvider(
CircuitBreakerRegistry circuitBreakerRegistry,
Map<String, CircuitBreakerConfig> circuitBreakerConfigMap,
List<String> disabledKeys) {
List<String> disabledKeys,
boolean defaultEnabled) {
this.circuitBreakerRegistry = circuitBreakerRegistry;
this.circuitBreakerConfigMap = circuitBreakerConfigMap;
this.disabledKeys = disabledKeys;
this.defaultEnabled = !disabledKeys.contains(CircuitBreakerConfigParser.DEFAULT_THRESHOLDS);
this.defaultEnabled = defaultEnabled;
}

public Optional<CircuitBreaker> getCircuitBreaker(String circuitBreakerKey) {
Comment thread
aaron-steinfeld marked this conversation as resolved.
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<CircuitBreaker> 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) {
Expand All @@ -86,14 +79,26 @@ private static void attachListeners(CircuitBreaker circuitBreaker) {
event.getCircuitBreakerName()));
}

private CircuitBreaker getCircuitBreakerFromConfigMap(
String circuitBreakerKey, boolean defaultEnabled) {
private Optional<CircuitBreaker> 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();
}
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - can do shared things at the end once you've built the circuit breaker

     return Optional.ofNullable(circuitBreakerConfigMap.get(circuitBreakerKey))
        .map(config -> circuitBreakerRegistry.circuitBreaker(circuitBreakerKey, config))
        .or(() -> {
              if (defaultEnabled) {
                return Optional.of(circuitBreakerRegistry.circuitBreaker(circuitBreakerKey));
              } else {
                return Optional.empty();
              }
            })
        .map(this::attachListeners); // assuming attach listeners can be made fluent and return the circuitbreaker

}
}