Skip to content
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

WIP: Metrics collector for Circuit Breaker #137

Closed
wants to merge 2 commits into from

Conversation

slayful
Copy link

@slayful slayful commented Aug 1, 2018

No description provided.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.4%) to 85.972% when pulling 3b4a0aa on slayful:metrics-collector into e3c37eb on jhalterman:master.

@whiskeysierra
Copy link

I wouldn't design/implement this with just metrics in mind. Why not add more extensive listener/callback support to the circuit breaker (similar to retries) and people can use that for metrics or anything else then.

@vgoldin
Copy link

vgoldin commented Aug 22, 2018

Whenever as a generic mechanism, or specific to circuit breakers, metrics would be highly valuable. In addition, creating hystrix compatible stream would allow plugging it into hystrix dashboard

@sunng87
Copy link
Contributor

sunng87 commented Aug 24, 2018

I wish we can make this generic and keep failsafe self-contained. Give listener to user allows integration with any metrics provider.

@jhalterman
Copy link
Member

@sunng87 @whiskeysierra Any particular listeners you had in mind for this?

import net.jodah.failsafe.internal.ClosedState;
import net.jodah.failsafe.internal.HalfOpenState;
import net.jodah.failsafe.internal.OpenState;
import net.jodah.failsafe.internal.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably this should be made explicit?

Copy link
Member

@jhalterman jhalterman Dec 28, 2018

Choose a reason for hiding this comment

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

You mean the wildcard import? My IDE does this too if there are several imports from the same package. The runtime cost should be nil.

@MALPI
Copy link
Contributor

MALPI commented Dec 28, 2018

@jhalterman I would make it more generic so that we can measure as well how long an attempt took.

@jhalterman
Copy link
Member

@MALPI #104 currently looks to add elapsed attempt time to ExecutionContext, so we'd just need an onComplete listener for CircuitBreaker in order to access that.

This gets back to #154 where it might be nice to have separate event listeners for each Policy (such as CircuitBreaker) :)

@jhalterman jhalterman force-pushed the master branch 28 times, most recently from 9259312 to 431e4f9 Compare August 14, 2019 03:25
@jhalterman
Copy link
Member

Closing as obsolete for now.

@jhalterman jhalterman closed this Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants