Added logging for dns resolution error in the Query component as well

CHANGELOG.md

@@ -18,6 +18,7 @@ We use *breaking* word for marking changes that are not backward compatible (rel
 - [#2568](https://github.com/thanos-io/thanos/pull/2568) Query: Does not close the connection of strict, static nodes if establishing a connection had succeeded but Info() call failed
 - [#2615](https://github.com/thanos-io/thanos/pull/2615) Rule: Fix bugs where rules were out of sync.
 - [#2548](https://github.com/thanos-io/thanos/pull/2548) Query: Fixed rare cases of double counter reset accounting when querying `rate` with deduplication enabled.
+- [#2525](https://github.com/thanos-io/thanos/pull/2525) Query: Fixed logging for dns resolution error in the `Query` component.
 
 ### Added
 

cmd/thanos/query.go

@@ -304,7 +304,9 @@ func runQuery(
 					}
 					fileSDCache.Update(update)
 					stores.Update(ctxUpdate)
-					dnsProvider.Resolve(ctxUpdate, append(fileSDCache.Addresses(), storeAddrs...))
+					if err := dnsProvider.Resolve(ctxUpdate, append(fileSDCache.Addresses(), storeAddrs...)); err != nil {
+						level.Error(logger).Log("msg", "failed to resolve addresses for storeAPIs", "err", err)
+					}
 				case <-ctxUpdate.Done():
 					return nil
 				}
@@ -319,7 +321,9 @@ func runQuery(
 		ctx, cancel := context.WithCancel(context.Background())
 		g.Add(func() error {
 			return runutil.Repeat(dnsSDInterval, ctx.Done(), func() error {
-				dnsProvider.Resolve(ctx, append(fileSDCache.Addresses(), storeAddrs...))
+				if err := dnsProvider.Resolve(ctx, append(fileSDCache.Addresses(), storeAddrs...)); err != nil {
+					level.Error(logger).Log("msg", "failed to resolve addresses for storeAPIs", "err", err)
+				}
 				return nil
 			})
 		}, func(error) {

cmd/thanos/rule.go

@@ -748,8 +748,7 @@ func addDiscoveryGroups(g *run.Group, c *http_util.Client, interval time.Duratio
 
 	g.Add(func() error {
 		return runutil.Repeat(interval, ctx.Done(), func() error {
-			c.Resolve(ctx)
-			return nil
+			return c.Resolve(ctx)
 		})
 	}, func(error) {
 		cancel()

pkg/cacheutil/memcached_client.go

@@ -5,6 +5,7 @@ package cacheutil
 
 import (
 	"context"
+	"strings"
 	"sync"
 	"time"
 
@@ -462,8 +463,10 @@ func (c *memcachedClient) resolveAddrs() error {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	c.dnsProvider.Resolve(ctx, c.config.Addresses)
-
+	// If some of the dns resolution fails, log the error.
+	if err := c.dnsProvider.Resolve(ctx, c.config.Addresses); err != nil {
+		level.Error(c.logger).Log("msg", "failed to resolve addresses for storeAPIs", "addresses", strings.Join(c.config.Addresses, ","), "err", err)
+	}
 	// Fail in case no server address is resolved.
 	servers := c.dnsProvider.Addresses()
 	if len(servers) == 0 {

pkg/discovery/dns/provider.go

@@ -13,6 +13,7 @@ import (
 	"github.com/go-kit/kit/log/level"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
+	tsdberrors "github.com/prometheus/prometheus/tsdb/errors"
 	"github.com/thanos-io/thanos/pkg/discovery/dns/miekgdns"
 	"github.com/thanos-io/thanos/pkg/extprom"
 )
@@ -107,8 +108,10 @@ func GetQTypeName(addr string) (qtype string, name string) {
 // Resolve stores a list of provided addresses or their DNS records if requested.
 // Addresses prefixed with `dns+` or `dnssrv+` will be resolved through respective DNS lookup (A/AAAA or SRV).
 // defaultPort is used for non-SRV records when a port is not supplied.
-func (p *Provider) Resolve(ctx context.Context, addrs []string) {
+func (p *Provider) Resolve(ctx context.Context, addrs []string) error {
 	resolvedAddrs := map[string][]string{}
+	errs := tsdberrors.MultiError{}
+
 	for _, addr := range addrs {
 		var resolved []string
 		qtype, name := GetQTypeName(addr)
@@ -120,9 +123,10 @@ func (p *Provider) Resolve(ctx context.Context, addrs []string) {
 		resolved, err := p.resolver.Resolve(ctx, name, QType(qtype))
 		p.resolverLookupsCount.Inc()
 		if err != nil {
+			// Append all the failed dns resolution in the error list.
+			errs.Add(err)
 			// The DNS resolution failed. Continue without modifying the old records.
 			p.resolverFailuresCount.Inc()
-			level.Error(p.logger).Log("msg", "dns resolution failed", "addr", addr, "err", err)
 			// Use cached values.
 			p.RLock()
 			resolved = p.resolved[addr]
@@ -143,6 +147,8 @@ func (p *Provider) Resolve(ctx context.Context, addrs []string) {
 	p.resolverAddrs.Submit()
 
 	p.resolved = resolvedAddrs
+
+	return errs.Err()
 }
 
 // Addresses returns the latest addresses present in the Provider.

pkg/discovery/dns/provider_test.go

@@ -32,14 +32,16 @@ func TestProvider(t *testing.T) {
 	}
 	ctx := context.TODO()
 
-	prv.Resolve(ctx, []string{"any+x"})
+	err := prv.Resolve(ctx, []string{"any+x"})
+	testutil.Ok(t, err)
 	result := prv.Addresses()
 	sort.Strings(result)
 	testutil.Equals(t, []string(nil), result)
 	testutil.Equals(t, 1, promtestutil.CollectAndCount(prv.resolverAddrs))
 	testutil.Equals(t, float64(0), promtestutil.ToFloat64(prv.resolverAddrs.WithLabelValues("any+x")))
 
-	prv.Resolve(ctx, []string{"any+a", "any+b", "any+c"})
+	err = prv.Resolve(ctx, []string{"any+a", "any+b", "any+c"})
+	testutil.Ok(t, err)
 	result = prv.Addresses()
 	sort.Strings(result)
 	testutil.Equals(t, ips, result)
@@ -48,22 +50,25 @@ func TestProvider(t *testing.T) {
 	testutil.Equals(t, float64(2), promtestutil.ToFloat64(prv.resolverAddrs.WithLabelValues("any+b")))
 	testutil.Equals(t, float64(1), promtestutil.ToFloat64(prv.resolverAddrs.WithLabelValues("any+c")))
 
-	prv.Resolve(ctx, []string{"any+b", "any+c"})
+	err = prv.Resolve(ctx, []string{"any+b", "any+c"})
+	testutil.Ok(t, err)
 	result = prv.Addresses()
 	sort.Strings(result)
 	testutil.Equals(t, ips[2:], result)
 	testutil.Equals(t, 2, promtestutil.CollectAndCount(prv.resolverAddrs))
 	testutil.Equals(t, float64(2), promtestutil.ToFloat64(prv.resolverAddrs.WithLabelValues("any+b")))
 	testutil.Equals(t, float64(1), promtestutil.ToFloat64(prv.resolverAddrs.WithLabelValues("any+c")))
 
-	prv.Resolve(ctx, []string{"any+x"})
+	err = prv.Resolve(ctx, []string{"any+x"})
+	testutil.Ok(t, err)
 	result = prv.Addresses()
 	sort.Strings(result)
 	testutil.Equals(t, []string(nil), result)
 	testutil.Equals(t, 1, promtestutil.CollectAndCount(prv.resolverAddrs))
 	testutil.Equals(t, float64(0), promtestutil.ToFloat64(prv.resolverAddrs.WithLabelValues("any+x")))
 
-	prv.Resolve(ctx, []string{"any+a", "any+b", "any+c"})
+	err = prv.Resolve(ctx, []string{"any+a", "any+b", "any+c"})
+	testutil.Ok(t, err)
 	result = prv.Addresses()
 	sort.Strings(result)
 	testutil.Equals(t, ips, result)
@@ -72,7 +77,8 @@ func TestProvider(t *testing.T) {
 	testutil.Equals(t, float64(2), promtestutil.ToFloat64(prv.resolverAddrs.WithLabelValues("any+b")))
 	testutil.Equals(t, float64(1), promtestutil.ToFloat64(prv.resolverAddrs.WithLabelValues("any+c")))
 
-	prv.Resolve(ctx, []string{"any+b", "example.com:90", "any+c"})
+	err = prv.Resolve(ctx, []string{"any+b", "example.com:90", "any+c"})
+	testutil.Ok(t, err)
 	result = prv.Addresses()
 	sort.Strings(result)
 	testutil.Equals(t, append(ips[2:], "example.com:90"), result)
@@ -81,7 +87,8 @@ func TestProvider(t *testing.T) {
 	testutil.Equals(t, float64(1), promtestutil.ToFloat64(prv.resolverAddrs.WithLabelValues("example.com:90")))
 	testutil.Equals(t, float64(1), promtestutil.ToFloat64(prv.resolverAddrs.WithLabelValues("any+c")))
 
-	prv.Resolve(ctx, []string{"any+b", "any+c"})
+	err = prv.Resolve(ctx, []string{"any+b", "any+c"})
+	testutil.Ok(t, err)
 	result = prv.Addresses()
 	sort.Strings(result)
 	testutil.Equals(t, ips[2:], result)

pkg/http/http.go

@@ -163,7 +163,7 @@ func (c FileSDConfig) convert() (file.SDConfig, error) {
 }
 
 type AddressProvider interface {
-	Resolve(context.Context, []string)
+	Resolve(context.Context, []string) error
 	Addresses() []string
 }
 
@@ -259,6 +259,6 @@ func (c *Client) Discover(ctx context.Context) {
 }
 
 // Resolve refreshes and resolves the list of targets.
-func (c *Client) Resolve(ctx context.Context) {
-	c.provider.Resolve(ctx, append(c.fileSDCache.Addresses(), c.staticAddresses...))
+func (c *Client) Resolve(ctx context.Context) error {
+	return c.provider.Resolve(ctx, append(c.fileSDCache.Addresses(), c.staticAddresses...))
 }