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 the user able to specify full path for HAproxy stats #1499

Closed
wants to merge 1 commit into from
Closed

Make the user able to specify full path for HAproxy stats #1499

wants to merge 1 commit into from

Conversation

tgermain
Copy link

@tgermain tgermain commented Jul 15, 2016

Do no try to guess HAproxy stats url, just add ";csv" at the end of the
url if not present.

It inspired by #1034 (comment) and should fix #1019 .

Signed-off-by: tgermain timothee.germain@corp.ovh.com

@phemmer
Copy link
Contributor

phemmer commented Jul 16, 2016

The only effective change this makes is that it removes the trailing /.

[[inputs.haproxy]]
    servers = ["http://localhost:1234/foo/bar"]

master (300d9ad):

GET /foo/bar/;csv HTTP/1.1
Host: localhost:1234
User-Agent: Go-http-client/1.1
Accept-Encoding: gzip

this PR (ff82b1c)

GET /foo/bar;csv HTTP/1.1
Host: localhost:1234
User-Agent: Go-http-client/1.1
Accept-Encoding: gzip

@tgermain
Copy link
Author

Not only, it also take into account the query string part of the url which was dropped before.

Configuration :

[[inputs.haproxy]]
  servers = ["http://localhost:5656/haproxy?stats"]

Master branch:

127.0.0.1 - - [18/Jul/2016 07:52:10] "GET /haproxy/;csv HTTP/1.1" 404 -

This PR:

127.0.0.1 - - [18/Jul/2016 08:30:40] "GET /haproxy?stats;csv HTTP/1.1" 404 -

Since the default uri of HAproxy stats is /haproxy?stats I think that making the nominal case works is important.

For the trailing / I can add it, both /haproxy?stats;csv and /haproxy?stats/;csv are valid.

@sparrc
Copy link
Contributor

sparrc commented Jul 18, 2016

@tgermain the latest change appears to have created a test failure, and you'll also need to rebase your change before I can merge.

Do no try to guess HAproxy stats url, just add ";csv" at the end of the
url if not present.

Signed-off-by: tgermain <timothee.germain@corp.ovh.com>
@tgermain
Copy link
Author

My bad for the test failure, tests are passing now.
The PR is also rebased.

@sparrc sparrc closed this in 0be69b8 Jul 19, 2016
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.

Standard URI for haproxy Stats
3 participants