-
Notifications
You must be signed in to change notification settings - Fork 4
Add grpc circuit breaker utility using interceptors #68
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
bc60d8e
Add grpc circuit breaker utility using interceptors
pavan-traceable faa0e02
Refactored classes and added test cases
pavan-traceable 6a058f7
Changed design of CircuitBreakerInterceptor
pavan-traceable 878a8cf
Fixed tests for CircuitBreakerInterceptor
pavan-traceable 275c854
Addressed comments
pavan-traceable 04ed487
Add ResilienceCircuitBreakerFactory class
pavan-traceable fa3a3c3
Addressed comments
pavan-traceable 90ca22b
Addressed comments
pavan-traceable 5f0ecd0
Addressed comments
pavan-traceable 8ca26b7
Addressed comments
pavan-traceable 75fb8b1
Addressed comments
pavan-traceable 77b0aac
Addressed comments
pavan-traceable ea47f32
Addressed comments
pavan-traceable f163a9d
Fixed corner cases
pavan-traceable 5f3b287
Addressed comments
pavan-traceable File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 | ||
|
|
@@ -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); | ||
| } | ||
| }); | ||
|
|
||
| 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) { | ||
|
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) { | ||
|
|
@@ -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(); | ||
| } | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/hypertrace/java-grpc-utils/pull/69/files Please review