-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Adding support for Default Haproxy Stats Page #1034
Conversation
@pasancario you appear to have broke a unit test with this change and changed the default behavior. Can you explain why this was necessary? |
@pasancario I think I understand the problem now. But how about we just change this to have users specify the full path? guessing and parsing URLs is a recipe for failure. So rather than parsing, just have the servers list look like this: servers = [
"http://myhaproxy.com:1936/;csv",
"http://anotherhaproxy.com:1936/haproxy?stats;csv"
] |
HI @sparrc , I just tried with this configuration: I was only trying to give support to default haproxy configuration URI. We can change then URI passed by configuration file, and let haproxy plugin use it as a final request, without modifying or appending anything. |
@pasancario I can merge this if you make the changes to have users specify the full URL |
I changed it made user specify full URL, and also change default behaviour (with ;csv too) |
you need to run |
So what does this change for users exactly? Does the SampleConfig need to change? Also please update the changelog |
Added full URI to parse haproxy stats
@pasancario Thanks for the readme, but I meant the SampleConfig() function, this needs to change to return the correct URLs. You'll also need to rebase because there was a separate PR for haproxy unix socket support. |
closing for lack of updates, @pasancario please feel free to open a new PR with your changes rebased off of master. |
Related to #1019