Repository navigation
Add registry decorators for deadlines and service-keyed circuit breakers - #94
suresh-prakash wants to merge 9 commits into
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…gistry. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
gitignore "docs/superpowers".
|
|
||
| private CircuitBreakerChannelRegistry() {} | ||
|
|
||
| public static GrpcChannelRegistry wrap(GrpcChannelRegistry registry, Config config) { |
There was a problem hiding this comment.
Prefer to use final wherever feasible (to avoid errors like accidentally assigning a local variable instead of the field). Let's use that throughout this PR.
|
|
||
| static final String CIRCUIT_BREAKER_CONFIG = "circuit.breaker.config"; | ||
|
|
||
| private CircuitBreakerChannelRegistry() {} |
There was a problem hiding this comment.
Nit: Can also use lombok?
| for (String serviceName : config.root().keySet()) { | ||
| if (CircuitBreakerConfigParser.ENABLED.equals(serviceName) | ||
| || CircuitBreakerConfigParser.DEFAULT_THRESHOLDS.equals(serviceName)) { | ||
| continue; |
There was a problem hiding this comment.
Would be helpful to have "debug" logs at these early exist points so it can come handy for debugging.
| .map(Status.Code::name) | ||
| .collect(Collectors.joining(", ")); | ||
| return ConfigFactory.parseString( | ||
| "failureRateThreshold = " |
There was a problem hiding this comment.
Would be better if these configs were built more first-class instead of string concatenation. It's okay if that's not possible.
There was a problem hiding this comment.
Maybe, once we have the first-class model, the defaults can be specified directly in the model.
| Objects.requireNonNull(registry), Objects.requireNonNull(interceptor)); | ||
| } | ||
|
|
||
| @Delegate(excludes = ForAddress.class) |
There was a problem hiding this comment.
Do we need the excludes specifically? Or, can lombok automatically skip methods that are explicitly overridden?
| } | ||
|
|
||
| private static final class InterceptedManagedChannel extends ManagedChannel { | ||
| @Delegate(excludes = InterceptedCall.class) |
|
|
||
| Config timeoutConfig = config.getConfig(TIMEOUT_PATH); | ||
| if (!timeoutConfig.hasPath(ENABLED_KEY) || !timeoutConfig.getBoolean(ENABLED_KEY)) { | ||
| return registry; |
There was a problem hiding this comment.
Add a debug log at all early exist points, especially since the would be invoked just once per restart.
| return InterceptingChannelRegistry.wrap(registry, new DefaultTimeoutClientInterceptor(timeout)); | ||
| } | ||
|
|
||
| private TimeoutChannelRegistry() {} |
There was a problem hiding this comment.
Nit: Can be replaced with Lombok?
|
|
||
| .idea | ||
|
|
||
| .superpowers/ |
There was a problem hiding this comment.
docs/superpowers can also be ignored.
Parse service breakers into CircuitBreakerThresholds instead of rebuilding defaults as HOCON, and log when a decorator is skipped or a call is rejected. Lombok still excludes forAddress because that method is inherited, not overridden. Co-authored-by: Cursor <cursoragent@cursor.com>
|
|
||
| static final String CIRCUIT_BREAKER_CONFIG = CircuitBreakerConfigParser.CIRCUIT_BREAKER_CONFIG; | ||
|
|
||
| public static GrpcChannelRegistry wrap(final GrpcChannelRegistry registry, final Config config) { |
There was a problem hiding this comment.
Why not take in the first-class object here instead of typesafe?
| return new ServiceCircuitBreakerInterceptor(Map.of(), Map.of()); | ||
| } | ||
|
|
||
| // Resilience4j records onError as a success when recordException returns false. |
There was a problem hiding this comment.
This comment seems off place still.
Callers can install the breaker from CircuitBreakerThresholds without passing Typesafe Config. Parameters in the new code are final, and a breaker logs when it changes state. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
InterceptingChannelRegistryso an already-builtGrpcChannelRegistrycan carry one client interceptor, andTimeoutChannelRegistrysogrpc.client.timeoutapplies a default deadline when a call has none.CircuitBreakerInterceptorFactory.forServiceandCircuitBreakerChannelRegistry.wrap. The breaker name is the protobuf service name.failureStatusCodesdefaults toDEADLINE_EXCEEDED,UNAVAILABLE, andRESOURCE_EXHAUSTED.Test plan
./gradlew -Pversion=0.13.23 :grpc-client-utils:test :grpc-circuitbreaker-utils:testRESOURCE_EXHAUSTEDbefore the downstream callMade with Cursor