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

Add use_proxy_dns to bypass local DNS resolution #554

Closed
wants to merge 2 commits into from

Conversation

Scapal
Copy link

@Scapal Scapal commented Nov 27, 2019

Addressing issue #553 and possibly #264 .

@brian-brazil
Copy link
Contributor

As indicated this breaks other features, so doesn't make sense. As I've previously stated I'd consider a feature to better test HTTP proxies, but DNS resolution of whatever is passed in with the URL needs to stay inside the blackbox exporter.

@Scapal
Copy link
Author

Scapal commented Nov 27, 2019

I really don't get it why you states it breaks other features.
The purpose of black box monitoring is end to end testing: "does it works from here".
Even now when using the existing proxy option you actually don't control the IP protocol used to connect to the proxy, only the target host.
If you use the option, you know it bypasses the local DNS resolution and IP protocol selection, what is wrong with that?

Please reconsider accepting this PR.

@brian-brazil
Copy link
Contributor

The purpose of the blackbox exporter is to test many many replicas of a service, including knowing what it was that you actually tested.

If a proxy does resolution we don't know what was tested, and as mentioned before that http proxies are supported at all is because it happens to come from the http library we use - it was never an intended feature of the blackbox exporter. If you need to test things at the far end of a proxy, you can have Prometheus use the proxy_url and put the blackbox exporter on the other side of the proxy.

@Scapal
Copy link
Author

Scapal commented Nov 27, 2019

We are not authorised to install an exporter on the far end of the proxy and the purpose is to test wether the service is working from here.
For example, one of my customer HQ moved servers to another network segment. It was transparent for users on the headquarter network but stopped working for branch offices forced to use the proxy.
Blackbox_exporter should imho stay a Swiss army knife. Monitoring many many replicas of a services being just one possible usage.
Even in your use case, using a Docker service IP address would also go through load-balancers...

@brian-brazil
Copy link
Contributor

We can't cover every possible deployment model to work in the easiest way possible I'm afraid, and we end up presuming that there is only one DNS view. Monitoring many many replicas of a service is explicitly the design goal, in this case the idea would be to test that each of multiple proxy servers work for getting to some HTTP fixed endpoint - which isn't supported as noone has proposed a PR/design yet.

I believe what I proposed would have caught that particular issue, as Prometheus wouldn't have been able to get to the blackbox exporter.

We are not authorised to install an exporter on the far end of the proxy and the purpose is to test wether the service is working from here.

Unfortunately we can't help with problems arising due to internal policies/politics, as otherwise that becomes a general argument for everything.

@Scapal
Copy link
Author

Scapal commented Nov 27, 2019

Could we have additional opinions about this PR or are you the sole contributor to have a voice for this repository ?

@brian-brazil
Copy link
Contributor

I'm the sole maintainer of this repo, per our governance you can bring it up on the developers list.

@mblaschke
Copy link

Hm.. that would maybe also solve our issue within the corporate network.

Something similar I already tried to address with #543 and #545

@pbeaumontQc
Copy link

We also need this and have been monitoring answer since issue #303 ... We have a list of third party providers that we need to monitor, as a breakdown from one of them directly breaks our ability to function. Our instances are hosted on AWS EC2 and all go through a proxy to the 3rd party providers... Any update on this much desired feature?

@brian-brazil
Copy link
Contributor

As I've already indicated this feature as-stated will not be accepted. I'd suggest looking at moving where the exporter runs, or fixing your proxy server to work with IP addresses.

@lyz-code
Copy link

Although it's stated that it won't be implemented, I'd also need this feature.

The ugly workaround I'm implementing is creating "fake" DNS records in my internal DNS server so the probe sees they exist and then the probe works as expected

@riuvshyn
Copy link

This is frustrating... I also need this feature.

@riuvshyn
Copy link

As indicated this breaks other features

@brian-brazil can you please elaborate on what exact features will be impacted by this change?

Most proxy setups do ACLs based on DNS names because IPs are not constants.
The proposed change is optional and will not affect users who are not using HTTP proxy but those who using proxy can use Blackbox exporter without this at all...

@brian-brazil
Copy link
Contributor

As indicated above, it breaks the ability to see what you talked to.

I'll be closing this now, as there's no plans to accept any feature like this. As previously stated I am willing to consider features to allow probing of http proxies.

riuvshyn added a commit to transferwise/blackbox_exporter that referenced this pull request Apr 17, 2020
Changes picked up from original PR prometheus#554 which was rejected by maintainer.
riuvshyn added a commit to transferwise/blackbox_exporter that referenced this pull request Apr 21, 2020
Changes picked up from original PR prometheus#554 which was rejected by maintainer.
@dbluxo
Copy link

dbluxo commented Oct 29, 2020

We would also be happy about this feature 😞

@riuvshyn
Copy link

@dbluxo that is why there are 552 forks ;)

@dbluxo
Copy link

dbluxo commented Oct 30, 2020

Like this PR, PR-338 was also rejected. Also with the hint to discuss it on the prometheus developers list. Has anyone tried it yet?

@riuvshyn
Copy link

riuvshyn commented Nov 4, 2020

@dbluxo changes in this PR works perfectly fine.

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.

7 participants