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

Implement a badRequests counter #32

Closed
wants to merge 1 commit into from

Conversation

hugowetterberg
Copy link

I just tried to use the Hystrix Dashboard and it broke because of a missing rollingCountBadRequests property.

This adds that counter, and the required stuff around it.

What I'm unsure about here is how a bad request should be tracked, it's not very well documented in Hystrix itself (...or, I just couldn't find the docs). I found this issue: Netflix/Hystrix#186 that discussed bad requests in relationship to the dash.

BREAKING CHANGE: This is a breaking change for anyone implementing custom collectors as the collector interface has a new function IncrementBadRequests().

I just tried to use the Hystrix Dashboard and it broke because of a missing `rollingCountBadRequests` property.

This adds that counter, and the required stuff around it.

What I'm unsure about here is how a bad request should be tracked, it's not very well documented in Hystrix itself (...or, I just couldn't find the docs). I found this issue: Netflix/Hystrix#186 that discussed bad requests in relationship to the dash.

BREAKING CHANGE: This is a breaking change for anyone implementing custom collectors as the collector interface has a new function `IncrementBadRequests()`.
@afex
Copy link
Owner

afex commented Jun 26, 2015

could you share some more details about how the hystrix dashboard "broke"? what version of the dashboard? turbine?

i currently run the dashboard with no issues

@hugowetterberg
Copy link
Author

It was in the templating layer and stopped all graphs except one or two, from rendering. I can check the dash version when I get to my computer. Regardless it's a hystrix metric that should be collected, I might be running of the dash master though.

@hugowetterberg
Copy link
Author

Ah, sorry for not providing all info. I was running the dash directly against the application. Running it trough turbine would likely fill in the zero value and make it safe for dash consumption. Will get back to you about the exact dash version.

@afex
Copy link
Owner

afex commented Jun 28, 2015

thanks, i'll wait to hear about your turbine investigation

@hugowetterberg
Copy link
Author

Ok, I had "trying out turbine" on my summer schedule anyhow, but this is something that should be addressed regardless of whether running the stream through turbine "fixes" it or not.

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