-
Notifications
You must be signed in to change notification settings - Fork 902
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
circuit breaker metrics tracking #139
Conversation
5ea93ac
to
0273698
Compare
ec3aadc
to
fbbc9a1
Compare
@@ -223,7 +240,7 @@ public CircuitBreaker build() { | |||
new CircuitBreakerConfig(name, failureRateThreshold, minimumRequestThreshold, | |||
circuitOpenWindow, trialRequestInterval, | |||
counterSlidingWindow, counterUpdateInterval, | |||
exceptionFilter)); | |||
exceptionFilter, listeners)); |
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.
Wrap with Collections.unmodifiableList()
?
8dcab5b
to
f1fdb7e
Compare
/** | ||
* Invoked when the circuit state is changed. | ||
*/ | ||
void onStateChanged(CircuitBreaker circuitBreaker, CircuitState state); |
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.
How about adding throws Exception
to all listener methods so that a user doesn't need to catch boring checked exceptions?
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.
Because CircuitBreakerListener
will be called from the core of the circuit breaker directly, That should be carefully implemented. IMO, It would be better that listeners handle the exceptions by them self.
But It is reasonable to add the throws
to be consistent with armeria's policy!
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.
There's always a chance of throwing a RuntimeException
regardless we added throws Exception
or not. :-)
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.
I see
f1fdb7e
to
9dd89e6
Compare
Changes
CircuitBreakerListener
interface to track circuit breaker stats.CircuitState
andEventCount
public to use fromCircuitBreakerListener
definition.