From 5487581e7b070b882d819f47b7bb53ca2ebe6f98 Mon Sep 17 00:00:00 2001 From: Yuchen Ying Date: Sat, 2 Jan 2021 23:09:21 -0800 Subject: [PATCH 1/5] Use preferred protocol first when resolving hostname 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 --- prober/utils.go | 90 ++++++++++++++++++++++---------------------- prober/utils_test.go | 14 ++++++- 2 files changed, 58 insertions(+), 46 deletions(-) diff --git a/prober/utils.go b/prober/utils.go index 98c9152e..365cb162 100644 --- a/prober/utils.go +++ b/prober/utils.go @@ -15,6 +15,7 @@ package prober import ( "context" + "errors" "fmt" "hash/fnv" "net" @@ -26,6 +27,28 @@ import ( "github.com/prometheus/client_golang/prometheus" ) +var protocolToGauge = map[string]float64{ + "ip4": 4, + "ip6": 6, +} + +type resolver struct { + net.Resolver +} + +// A simple wrapper around resolver.LookupIP. +func (r *resolver) resolve(ctx context.Context, target string, protocol string) (*net.IPAddr, error) { + ips, err := r.LookupIP(ctx, protocol, target) + if err != nil { + return nil, err + } + for _, ip := range ips { + return &net.IPAddr{IP: ip}, nil + } + // Go doc did not specify when this could happen, better be defensive. + return nil, errors.New("calling LookupIP returned empty list of addresses") +} + // Returns the IP for the IPProtocol and lookup time. func chooseProtocol(ctx context.Context, IPProtocol string, fallbackIPProtocol bool, target string, registry *prometheus.Registry, logger log.Logger) (ip *net.IPAddr, lookupTime float64, err error) { var fallbackProtocol string @@ -55,7 +78,6 @@ func chooseProtocol(ctx context.Context, IPProtocol string, fallbackIPProtocol b fallbackProtocol = "ip6" } - level.Info(logger).Log("msg", "Resolving target address", "ip_protocol", IPProtocol) resolveStart := time.Now() defer func() { @@ -63,55 +85,33 @@ func chooseProtocol(ctx context.Context, IPProtocol string, fallbackIPProtocol b probeDNSLookupTimeSeconds.Add(lookupTime) }() - resolver := &net.Resolver{} - ips, err := resolver.LookupIPAddr(ctx, target) - if err != nil { - level.Error(logger).Log("msg", "Resolution with IP protocol failed", "err", err) - return nil, 0.0, err - } - - // Return the IP in the requested protocol. - var fallback *net.IPAddr - for _, ip := range ips { - switch IPProtocol { - case "ip4": - if ip.IP.To4() != nil { - level.Info(logger).Log("msg", "Resolved target address", "ip", ip.String()) - probeIPProtocolGauge.Set(4) - probeIPAddrHash.Set(ipHash(ip.IP)) - return &ip, lookupTime, nil - } - - // ip4 as fallback - fallback = &ip - - case "ip6": - if ip.IP.To4() == nil { - level.Info(logger).Log("msg", "Resolved target address", "ip", ip.String()) - probeIPProtocolGauge.Set(6) - probeIPAddrHash.Set(ipHash(ip.IP)) - return &ip, lookupTime, nil - } - - // ip6 as fallback - fallback = &ip - } + r := &resolver{ + Resolver: net.Resolver{}, } - // Unable to find ip and no fallback set. - if fallback == nil || !fallbackIPProtocol { - return nil, 0.0, fmt.Errorf("unable to find ip; no fallback") + level.Info(logger).Log("msg", "Resolving target address", "ip_protocol", IPProtocol) + if ip, err := r.resolve(ctx, target, IPProtocol); err == nil { + level.Info(logger).Log("msg", "Resolved target address", "ip", ip.String()) + probeIPProtocolGauge.Set(protocolToGauge[IPProtocol]) + probeIPAddrHash.Set(ipHash(ip.IP)) + return ip, lookupTime, nil + } else if !fallbackIPProtocol { + level.Error(logger).Log("msg", "Resolution with IP protocol failed", "err", err) + return nil, 0.0, fmt.Errorf("unable to find ip; no fallback: %s", err) } - // Use fallback ip protocol. - if fallbackProtocol == "ip4" { - probeIPProtocolGauge.Set(4) - } else { - probeIPProtocolGauge.Set(6) + level.Info(logger).Log("msg", "Resolving target address", "ip_protocol", fallbackProtocol) + ip, err = r.resolve(ctx, target, fallbackProtocol) + if err != nil { + // This could happen when the domain don't have A and AAAA record (e.g. + // only have MX record). + level.Error(logger).Log("msg", "Resolution with IP protocol failed", "err", err) + return nil, 0.0, fmt.Errorf("unable to find ip; exhausted fallback: %s", err) } - probeIPAddrHash.Set(ipHash(fallback.IP)) - level.Info(logger).Log("msg", "Resolved target address", "ip", fallback.String()) - return fallback, lookupTime, nil + level.Info(logger).Log("msg", "Resolved target address", "ip", ip.String()) + probeIPProtocolGauge.Set(protocolToGauge[fallbackProtocol]) + probeIPAddrHash.Set(ipHash(ip.IP)) + return ip, lookupTime, nil } func ipHash(ip net.IP) float64 { diff --git a/prober/utils_test.go b/prober/utils_test.go index a53ee490..b7532b35 100644 --- a/prober/utils_test.go +++ b/prober/utils_test.go @@ -24,6 +24,7 @@ import ( "math/big" "net" "os" + "strings" "testing" "time" @@ -162,7 +163,7 @@ func TestChooseProtocol(t *testing.T) { registry = prometheus.NewPedanticRegistry() ip, _, err = chooseProtocol(ctx, "ip4", false, "ipv6.google.com", registry, logger) - if err != nil && err.Error() != "unable to find ip; no fallback" { + if err != nil && !strings.HasPrefix(err.Error(), "unable to find ip; no fallback") { t.Error(err) } else if err == nil { t.Error("should set error") @@ -170,6 +171,17 @@ func TestChooseProtocol(t *testing.T) { if ip != nil { t.Error("without fallback it should not answer") } + + registry = prometheus.NewPedanticRegistry() + ip, _, err = chooseProtocol(ctx, "ip4", true, "does-not-exist.google.com", registry, logger) + if err != nil && !strings.HasPrefix(err.Error(), "unable to find ip; exhausted fallback") { + t.Error(err) + } else if err == nil { + t.Error("should set error") + } + if ip != nil { + t.Error("with exhausted fallback it should not answer") + } } func checkMetrics(expected map[string]map[string]map[string]struct{}, mfs []*dto.MetricFamily, t *testing.T) { From 923274ab6d7a53891bb0497d28dbb4fd947e0eca Mon Sep 17 00:00:00 2001 From: Yuchen Ying Date: Mon, 4 Jan 2021 12:54:05 -0800 Subject: [PATCH 2/5] Use example.com hostname in test. Signed-off-by: Yuchen Ying --- prober/utils_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prober/utils_test.go b/prober/utils_test.go index b7532b35..bf21dc79 100644 --- a/prober/utils_test.go +++ b/prober/utils_test.go @@ -173,7 +173,7 @@ func TestChooseProtocol(t *testing.T) { } registry = prometheus.NewPedanticRegistry() - ip, _, err = chooseProtocol(ctx, "ip4", true, "does-not-exist.google.com", registry, logger) + ip, _, err = chooseProtocol(ctx, "ip4", true, "does-not-exist.example.com", registry, logger) if err != nil && !strings.HasPrefix(err.Error(), "unable to find ip; exhausted fallback") { t.Error(err) } else if err == nil { From d08e1b0774152eeb604a87c08ea3a162531ee22f Mon Sep 17 00:00:00 2001 From: Yuchen Ying Date: Mon, 4 Jan 2021 21:21:05 -0800 Subject: [PATCH 3/5] Inline LookupIP function call. Signed-off-by: Yuchen Ying --- prober/utils.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/prober/utils.go b/prober/utils.go index 365cb162..e5b3931f 100644 --- a/prober/utils.go +++ b/prober/utils.go @@ -85,33 +85,31 @@ func chooseProtocol(ctx context.Context, IPProtocol string, fallbackIPProtocol b probeDNSLookupTimeSeconds.Add(lookupTime) }() - r := &resolver{ - Resolver: net.Resolver{}, - } + resolver := &net.Resolver{} level.Info(logger).Log("msg", "Resolving target address", "ip_protocol", IPProtocol) - if ip, err := r.resolve(ctx, target, IPProtocol); err == nil { - level.Info(logger).Log("msg", "Resolved target address", "ip", ip.String()) + if ips, err := resolver.LookupIP(ctx, IPProtocol, target); err == nil { + level.Info(logger).Log("msg", "Resolved target address", "ip", ips[0].String()) probeIPProtocolGauge.Set(protocolToGauge[IPProtocol]) - probeIPAddrHash.Set(ipHash(ip.IP)) - return ip, lookupTime, nil + probeIPAddrHash.Set(ipHash(ips[0])) + return &net.IPAddr{IP: ips[0]}, lookupTime, nil } else if !fallbackIPProtocol { level.Error(logger).Log("msg", "Resolution with IP protocol failed", "err", err) return nil, 0.0, fmt.Errorf("unable to find ip; no fallback: %s", err) } level.Info(logger).Log("msg", "Resolving target address", "ip_protocol", fallbackProtocol) - ip, err = r.resolve(ctx, target, fallbackProtocol) + ips, err := resolver.LookupIP(ctx, fallbackProtocol, target) if err != nil { // This could happen when the domain don't have A and AAAA record (e.g. // only have MX record). level.Error(logger).Log("msg", "Resolution with IP protocol failed", "err", err) return nil, 0.0, fmt.Errorf("unable to find ip; exhausted fallback: %s", err) } - level.Info(logger).Log("msg", "Resolved target address", "ip", ip.String()) + level.Info(logger).Log("msg", "Resolved target address", "ip", ips[0].String()) probeIPProtocolGauge.Set(protocolToGauge[fallbackProtocol]) - probeIPAddrHash.Set(ipHash(ip.IP)) - return ip, lookupTime, nil + probeIPAddrHash.Set(ipHash(ips[0])) + return &net.IPAddr{IP: ips[0]}, lookupTime, nil } func ipHash(ip net.IP) float64 { From 5b106830b043c8c46af76fa8069b46a26bb80c9d Mon Sep 17 00:00:00 2001 From: Yuchen Ying Date: Mon, 4 Jan 2021 21:44:34 -0800 Subject: [PATCH 4/5] Remove unused struct. Signed-off-by: Yuchen Ying --- prober/utils.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/prober/utils.go b/prober/utils.go index e5b3931f..a834c76c 100644 --- a/prober/utils.go +++ b/prober/utils.go @@ -15,7 +15,6 @@ package prober import ( "context" - "errors" "fmt" "hash/fnv" "net" @@ -32,23 +31,6 @@ var protocolToGauge = map[string]float64{ "ip6": 6, } -type resolver struct { - net.Resolver -} - -// A simple wrapper around resolver.LookupIP. -func (r *resolver) resolve(ctx context.Context, target string, protocol string) (*net.IPAddr, error) { - ips, err := r.LookupIP(ctx, protocol, target) - if err != nil { - return nil, err - } - for _, ip := range ips { - return &net.IPAddr{IP: ip}, nil - } - // Go doc did not specify when this could happen, better be defensive. - return nil, errors.New("calling LookupIP returned empty list of addresses") -} - // Returns the IP for the IPProtocol and lookup time. func chooseProtocol(ctx context.Context, IPProtocol string, fallbackIPProtocol bool, target string, registry *prometheus.Registry, logger log.Logger) (ip *net.IPAddr, lookupTime float64, err error) { var fallbackProtocol string From f44254d4ef150187e825244065226079da4d6c10 Mon Sep 17 00:00:00 2001 From: Yuchen Ying Date: Mon, 4 Jan 2021 22:01:46 -0800 Subject: [PATCH 5/5] Make full use of named return variables and simplify the logic. Signed-off-by: Yuchen Ying --- prober/utils.go | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/prober/utils.go b/prober/utils.go index a834c76c..39aef270 100644 --- a/prober/utils.go +++ b/prober/utils.go @@ -32,7 +32,7 @@ var protocolToGauge = map[string]float64{ } // Returns the IP for the IPProtocol and lookup time. -func chooseProtocol(ctx context.Context, IPProtocol string, fallbackIPProtocol bool, target string, registry *prometheus.Registry, logger log.Logger) (ip *net.IPAddr, lookupTime float64, err error) { +func chooseProtocol(ctx context.Context, IPProtocol string, fallbackIPProtocol bool, target string, registry *prometheus.Registry, logger log.Logger) (ip *net.IPAddr, lookupTime float64, returnerr error) { var fallbackProtocol string probeDNSLookupTimeSeconds := prometheus.NewGauge(prometheus.GaugeOpts{ Name: "probe_dns_lookup_time_seconds", @@ -59,12 +59,19 @@ func chooseProtocol(ctx context.Context, IPProtocol string, fallbackIPProtocol b IPProtocol = "ip4" fallbackProtocol = "ip6" } + var usedProtocol string resolveStart := time.Now() defer func() { lookupTime = time.Since(resolveStart).Seconds() probeDNSLookupTimeSeconds.Add(lookupTime) + if usedProtocol != "" { + probeIPProtocolGauge.Set(protocolToGauge[usedProtocol]) + } + if ip != nil { + probeIPAddrHash.Set(ipHash(ip.IP)) + } }() resolver := &net.Resolver{} @@ -72,12 +79,13 @@ func chooseProtocol(ctx context.Context, IPProtocol string, fallbackIPProtocol b level.Info(logger).Log("msg", "Resolving target address", "ip_protocol", IPProtocol) if ips, err := resolver.LookupIP(ctx, IPProtocol, target); err == nil { level.Info(logger).Log("msg", "Resolved target address", "ip", ips[0].String()) - probeIPProtocolGauge.Set(protocolToGauge[IPProtocol]) - probeIPAddrHash.Set(ipHash(ips[0])) - return &net.IPAddr{IP: ips[0]}, lookupTime, nil + usedProtocol = IPProtocol + ip = &net.IPAddr{IP: ips[0]} + return } else if !fallbackIPProtocol { level.Error(logger).Log("msg", "Resolution with IP protocol failed", "err", err) - return nil, 0.0, fmt.Errorf("unable to find ip; no fallback: %s", err) + returnerr = fmt.Errorf("unable to find ip; no fallback: %s", err) + return } level.Info(logger).Log("msg", "Resolving target address", "ip_protocol", fallbackProtocol) @@ -86,12 +94,13 @@ func chooseProtocol(ctx context.Context, IPProtocol string, fallbackIPProtocol b // This could happen when the domain don't have A and AAAA record (e.g. // only have MX record). level.Error(logger).Log("msg", "Resolution with IP protocol failed", "err", err) - return nil, 0.0, fmt.Errorf("unable to find ip; exhausted fallback: %s", err) + returnerr = fmt.Errorf("unable to find ip; exhausted fallback: %s", err) + return } level.Info(logger).Log("msg", "Resolved target address", "ip", ips[0].String()) - probeIPProtocolGauge.Set(protocolToGauge[fallbackProtocol]) - probeIPAddrHash.Set(ipHash(ips[0])) - return &net.IPAddr{IP: ips[0]}, lookupTime, nil + usedProtocol = fallbackProtocol + ip = &net.IPAddr{IP: ips[0]} + return } func ipHash(ip net.IP) float64 {