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

Extended metrics #125

Closed
robholland opened this issue Jul 11, 2016 · 6 comments
Closed

Extended metrics #125

robholland opened this issue Jul 11, 2016 · 6 comments
Milestone

Comments

@robholland
Copy link

We're interested in measuring the impact of switching a system over to fabio versus a haproxy setup, it would be very useful to have a Counter() per HTTP status code so we can check for behaviour versus our current setup, ensuring the 200/202/404 rates are equivalent.

Also it would be useful to have a Counter() for route-not-found responses (proxy.noroutestatus) as they won't be associated with a route target.

@magiconair
Copy link
Contributor

Sounds good. I'll see what I can do.

magiconair added a commit that referenced this issue Jul 16, 2016
* Add 'noroute' counter for requests to non-existing routes
* Add 'http.status.xxx' timer for responses with a specific status code
* Add 'ws.conn' counter for counting number of open websockets
@magiconair
Copy link
Contributor

@robholland The patch I've uploaded should do the trick.

    * Add 'noroute' counter for requests to non-existing routes
    * Add 'http.status.xxx' timer for responses with a specific status code
    * Add 'ws.conn' counter for counting number of open websockets

I'm a bit torn by keeping track of all status codes since this code is in the hot path. The generic implementation has to create the metric name and lookup the timer for every request which means 1-2 allocs and accessing a (global?) lock.

A possible optimization would be to limit the number of status codes fabio keeps track of and make that configurable. Then I can pre-alloc the timers and don't have to look them up.

If it is easy for you to run a load test to see whether that makes a difference then that would help a lot. I'll see what I can do on our end. For now, I've opted for the more useful implementation but I'll merge this after I've seen some numbers.

@magiconair magiconair added this to the 1.2.1 milestone Jul 16, 2016
@robholland
Copy link
Author

Frank, we're going to have to halt our fabio experiments as our priorities have changed. If you're keen on getting this merged and need some testing then I'll try and make some time for it in a few weeks.

Thanks for your efforts getting us this far, hopefully we'll be back :)

@magiconair
Copy link
Contributor

@robholland No problem. I think this one has enough value. I'll run this in our environment first to see how it behaves. Let me know if/when I can help again.

magiconair added a commit that referenced this issue Aug 29, 2016
* Add 'noroute' counter for requests to non-existing routes
* Add 'http.status.xxx' timer for responses with a specific status code
* Add 'ws.conn' counter for counting number of open websockets
@magiconair
Copy link
Contributor

Rebased this on top of #147

magiconair added a commit that referenced this issue Aug 29, 2016
@magiconair magiconair removed this from the 1.2.1 milestone Aug 29, 2016
magiconair added a commit that referenced this issue Aug 29, 2016
@magiconair
Copy link
Contributor

Merged to master.

@magiconair magiconair added this to the 1.3 milestone Sep 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants