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

Proxy from environment not working #190

Closed
Fra-nk opened this issue Jul 12, 2017 · 4 comments
Closed

Proxy from environment not working #190

Fra-nk opened this issue Jul 12, 2017 · 4 comments

Comments

@Fra-nk
Copy link

Fra-nk commented Jul 12, 2017

#92 added support for using proxy variables (HTTP_PROXY, HTTPS_PROXY & NO_PROXY) from the environment.
#173 removed this feature

This is not documented in the 0.6.0 release notes, so I assume it is a bug?

@brian-brazil
Copy link
Contributor

As a general Prometheus principle, we don't technically support configuration from anything other than the config file. I will accept a PR to fix that over in common though.

@brian-brazil
Copy link
Contributor

Actually the current behaviour matches Prometheus, so it'd be best to keep in sync.

@Fra-nk
Copy link
Author

Fra-nk commented Jul 13, 2017

Well I'll happily submit a pull request, if there is a reasonable chance of getting it implemented.
So after the second comment I want to know if it's worth sending the pull request at all?

Just tested it successfully with directly modifying the vendored dependencies, but it's just copy, paste and a pull request to submit it to common:

diff --git vendor/github.com/prometheus/common/config/http_config.go vendor/github.com/prometheus/common/config/http_config.go
index ff5837f..5ff5668 100644
--- vendor/github.com/prometheus/common/config/http_config.go
+++ vendor/github.com/prometheus/common/config/http_config.go
@@ -121,9 +121,16 @@ func NewHTTPClientFromConfig(cfg *HTTPClientConfig) (*http.Client, error) {
                return nil, err
        }

+       var proxyFunc func(*http.Request) (*url.URL, error)
+       if cfg.ProxyURL.URL != nil {
+               proxyFunc = http.ProxyURL(cfg.ProxyURL.URL)
+       } else {
+               proxyFunc = http.ProxyFromEnvironment
+       }
+
        // It's the caller's job to handle timeouts
        var rt http.RoundTripper = &http.Transport{
-               Proxy:             http.ProxyURL(cfg.ProxyURL.URL),
+               Proxy:             proxyFunc,
                DisableKeepAlives: true,
                TLSClientConfig:   tlsConfig,
        }

Imho there are some more complex setups where the option of setting the values as environment variables may be beneficial, especially because of the presence/evaluation of NO_PROXY.
In general, explicitly defining the proxy in the module/configuration file is the better approach, though.

@brian-brazil
Copy link
Contributor

We're keeping in sync with Prometheus, so the place to try to get this changed this is in Prometheus.

In general though, the config file is how you're meant to specify things.

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

No branches or pull requests

2 participants