enable optional use of an HTTP proxy

[thomasgl-orange]

I need to run the haproxy_exporter from an environment which can only reach the target haproxy stats API by going through an HTTP proxy. The way haproxy_exporter currently executes its GET requests (with a customized http.Transport to allow disabling TLS validation) ignores the proxy settings defined in the traditional environment variables ($HTTP(S)_PROXY, $NO_PROXY). This PR fixes that, by adding the proxy from http.ProxyFromEnvironment to the http.Transport. If there is no proxy env vars, or the target haproxy domain is in $NO_PROXY, the proxy will be nil, and nothing changes in the way requests are executed.

It can be tested, for instance, by running a local mitmproxy service, and running haproxy_exporter with a HTTP_PROXY=http://localhost:8080 (or $HTTPS_PROXY) env var.

CC @grobie, @roidelapluie

[roidelapluie]

This is a breaking change and not how we usually do things in the Prometheus ecosystem. I think it would be preferable to add a proxy URL to the URL parameters, or to a specific file (to avoid leaking secrets).

[thomasgl-orange]

This is a breaking change

It is a breaking change for someone who runs haproxy_exporter with an $HTTP(S)_PROXY var set, but no (proper) $NO_PROXY var, and who relies on these proxy settings being ignored. I think it is a very unlikely situation, with straightforward solutions.

not how we usually do things in the Prometheus ecosystem

This PR simply restores what would otherwise be the default behaviour of http.Client, if the http.Transport had not been customized to allow disabling TLS check in #86. $HTTP_PROXY and friends were indeed properly taken into account before that (just tested with the 0.8.0 binary), so the current state looks more like an accident than a conscious decision (which is fine, I understand that having to go through an HTTP proxy from the exporter is not a common requirement, and that this "regression" was overlooked back then).

I think it would be preferable to add a proxy URL to the URL parameters, or to a specific file (to avoid leaking secrets).

I disagree, I don't like having every single binary invent its own way to get proxy settings. I like the standardized env-based way golang promotes in the http lib. Just my opinion of course.

====

On a side note, I realize that my PR could be much shorter:

diff --git a/haproxy_exporter.go b/haproxy_exporter.go
index 54b4cbb..7b44548 100644
--- a/haproxy_exporter.go
+++ b/haproxy_exporter.go
@@ -332,7 +332,10 @@ func (e *Exporter) Collect(ch chan<- prometheus.Metric) {
 }
 
 func fetchHTTP(uri string, sslVerify bool, timeout time.Duration) func() (io.ReadCloser, error) {
-	tr := &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: !sslVerify}}
+	tr := &http.Transport{
+		Proxy:           http.ProxyFromEnvironment,
+		TLSClientConfig: &tls.Config{InsecureSkipVerify: !sslVerify},
+	}
 	client := http.Client{
 		Timeout:   timeout,
 		Transport: tr,

I didn't want to push-force anything now that you've already viewed the initial version, but I'm happy to do it if you don't mind.

[roidelapluie]

Wheter or not it is a breaking change is not really subject of question. Someone could have a global proxy set but targetting haproxy locally which should not be proxied.

In the whole Prometheus ecosystem, we do not set proxy based on environment variables. I would avoid having one exporter behaving differently than others, but I do recognize that users should be able to set proxies. What about a boolean flag --http.proxy-from-env maybe?

[thomasgl-orange]

What about a boolean flag --http.proxy-from-env maybe?

Yes, it sounds good. I've pushed a new version with this flag.

[grobie]

Thanks for the review @roidelapluie and the quick turnaround @thomasgl-orange. The proposed solution looks good to me, I'll leave the final review to @roidelapluie.

[roidelapluie]

Thanks! There are linting issues.

Can we keep it http.proxy-from-env? that way we could reuse it for other exporters that might need this. It also clarifies that it does not concern HAProxy when used with unix sockets.

[thomasgl-orange]

Oops, sorry for haproxy.proxy-from-env vs http.proxy-from-env, I will change that tomorrow, and also have a look on the linting errors.

[thomasgl-orange]

The CLI flag is now renamed http.proxy-from-env.

Regarding linting error, I don't get any with golangci-lint latest version (1.45.2), which is what I had installed here (hence no error for me on make). Downgrading to 1.42.0, I've been able to reproduce (that's with go 1.18.1). It's not related to this PR, I get the same on master.
Anyway, I propose bumping golangci-lint to 1.45.2 (Makefile and GitHub actions). I've added a commit for that, so you can see it fixes the GitHub Action. But I can get rid of this commit if you prefer a separate PR.

[roidelapluie]

Thanks. golint will be updated by our bot, after which you will be able to rebase your PR on top of main.

[thomasgl-orange]

For what it's worth (just scratching an itch), a git bisect on golangci-lint from 1.43.0 to 1.44.0 (first version where linting errors disappear when running on haproxy_exporter) leads to golangci/golangci-lint#2373 (bump to gofumpt v0.2.0) as the commit which fixed these errors.

[thomasgl-orange]

Thanks for the review and suggestions; I've applied them all, but forgot Signed-off-by because I did it in GitHub web UI (todogroup/gh-issues#50), and so DCO not happy. I will rewrite that when rebasing on an updated master (with golangci-lint bump).

[roidelapluie]

done :)

[thomasgl-orange]

Thanks, rebased :)

[roidelapluie]

Thanks!