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

Make hystrix metrics abstract #843

Merged

Conversation

mattrjacobs
Copy link
Contributor

This addresses the concern of #333. This PR is intended to be a PoC to demonstrate that there can be varying implementations of metric collection (command/thread pool/collapser). The version which summarizes via HystrixRollingNumber/HystrixRollingPercentile is one such concrete implementation.

This PR is not ready for merging - I still would like to:

  • Clean up the Javadoc
  • Add Wiki documentation around the new plugin
  • Add no-op metrics strategies to evaluate perf impact
  • Make sure I didn't impact any public methods
  • Convince myself that new abstract methods are the proper set for concrete impls

If anyone has feedback along those lines, it would be appreciated.

@cloudbees-pull-request-builder

NetflixOSS » Hystrix » Hystrix-pull-requests #146 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

NetflixOSS » Hystrix » Hystrix-pull-requests #148 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

NetflixOSS » Hystrix » Hystrix-pull-requests #149 SUCCESS
This pull request looks good

@mattrjacobs
Copy link
Contributor Author

I've just branched off 1.4.x, and this is the first commit past that point into master. Once I am comfortable this works as intended, I will modify the Wiki

mattrjacobs added a commit that referenced this pull request Aug 7, 2015
@mattrjacobs mattrjacobs merged commit 7c3fc13 into Netflix:master Aug 7, 2015
@mattrjacobs mattrjacobs deleted the make-hystrix-command-metrics-abstract branch August 7, 2015 23:21
mattrjacobs added a commit that referenced this pull request Oct 14, 2015
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.

2 participants