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

Use preferred protocol first when resolving hostname #728

Merged
merged 5 commits into from Jan 5, 2021

Conversation

yegle
Copy link
Contributor

@yegle yegle commented Jan 3, 2021

Prior to this change, LookupIPAddr() is used to do the DNS query,
which sends two DNS queries for A/AAAA records, even if the config has
preferred protocol set to ip4 and does not allow fallback protocol.

This change will change to use LookupIP() with the preferred protocol,
and only try fallback protocol if it's set to true. In most cases doing
this will save one RTT of DNS query.

Fixes #724

@yegle
Copy link
Contributor Author

yegle commented Jan 3, 2021

cc @brian-brazil

Prior to this change, LookupIPAddr() is used to do the DNS query,
which sends two DNS queries for A/AAAA records, even if the config has
preferred protocol set to ip4 and does not allow fallback protocol.

This change will change to use LookupIP() with the preferred protocol,
and only try fallback protocol if it's set to true. In most cases doing
this will save one RTT of DNS query.

Signed-off-by: Yuchen Ying <github.com@yegle.net>
prober/utils_test.go Outdated Show resolved Hide resolved
prober/utils.go Outdated Show resolved Hide resolved
prober/utils.go Outdated Show resolved Hide resolved
Signed-off-by: Yuchen Ying <github.com@yegle.net>
Signed-off-by: Yuchen Ying <github.com@yegle.net>
Signed-off-by: Yuchen Ying <github.com@yegle.net>
Signed-off-by: Yuchen Ying <github.com@yegle.net>
@brian-brazil brian-brazil merged commit 847b668 into prometheus:master Jan 5, 2021
@brian-brazil
Copy link
Contributor

Thanks!

@yegle
Copy link
Contributor Author

yegle commented Feb 2, 2021

I wonder if a new version can cut with this PR included?

Or alternatively can you publish a nightly docker image?

@crockk
Copy link

crockk commented Apr 27, 2021

Does the docker image at prom/blackbox-exporter:master include this merge?

@hferreira23
Copy link

@crockk It should be even though I'm actually using the master tag image and I see no change in the behavior.

Either way this PR will be on 0.19 which for the looks of it it's just around the corner.

roidelapluie added a commit to roidelapluie/blackbox_exporter that referenced this pull request May 28, 2021
…eus#728)"

This reverts commit 847b668.

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
@james58899
Copy link

Also related to golang/go#45024, need wait golang 1.17 release and upgrade.

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.

icmp: Both A and AAAA records are queried regardless of preferred_ip_protocol/ip_protocol_fallback
5 participants