proxy: slog

AdguardTeam · Jun 18, 2024 · 48f8830 · 48f8830
1 parent 22e1bb2
commit 48f8830
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 24 deletions.
diff --git a/main.go b/main.go
@@ -526,7 +526,7 @@ func (opts *Options) initUpstreams(
 	}
 	upstreams := loadServersList(opts.Upstreams)
 
-	config.UpstreamConfig, err = proxy.ParseUpstreamsConfig(upstreams, upsOpts)
+	config.UpstreamConfig, err = proxy.ParseUpstreamsConfig(upstreams, upsOpts, l)
 	if err != nil {
 		return fmt.Errorf("parsing upstreams configuration: %w", err)
 	}
@@ -538,7 +538,7 @@ func (opts *Options) initUpstreams(
 	}
 	privateUpstreams := loadServersList(opts.PrivateRDNSUpstreams)
 
-	private, err := proxy.ParseUpstreamsConfig(privateUpstreams, privateUpsOpts)
+	private, err := proxy.ParseUpstreamsConfig(privateUpstreams, privateUpsOpts, l)
 	if err != nil {
 		return fmt.Errorf("parsing private rdns upstreams configuration: %w", err)
 	}
@@ -548,7 +548,7 @@ func (opts *Options) initUpstreams(
 	}
 
 	fallbackUpstreams := loadServersList(opts.Fallbacks)
-	fallbacks, err := proxy.ParseUpstreamsConfig(fallbackUpstreams, upsOpts)
+	fallbacks, err := proxy.ParseUpstreamsConfig(fallbackUpstreams, upsOpts, l)
 	if err != nil {
 		return fmt.Errorf("parsing fallback upstreams configuration: %w", err)
 	}

diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go
@@ -229,10 +229,13 @@ func newTestUpstreamConfigWithBoot(
 	)
 	require.NoError(t, err)
 
-	upsConf, err := ParseUpstreamsConfig(addrs, &upstream.Options{
-		Timeout:   timeout,
-		Bootstrap: upstream.NewCachingResolver(googleRslv),
-	})
+	upsConf, err := ParseUpstreamsConfig(
+		addrs,
+		&upstream.Options{
+			Timeout:   timeout,
+			Bootstrap: upstream.NewCachingResolver(googleRslv),
+		},
+		slogutil.NewDiscardLogger())
 	require.NoError(t, err)
 
 	return upsConf
@@ -247,7 +250,11 @@ func newTestUpstreamConfig(
 ) (u *UpstreamConfig) {
 	t.Helper()
 
-	upsConf, err := ParseUpstreamsConfig(addrs, &upstream.Options{Timeout: timeout})
+	upsConf, err := ParseUpstreamsConfig(
+		addrs,
+		&upstream.Options{Timeout: timeout},
+		slogutil.NewDiscardLogger(),
+	)
 	require.NoError(t, err)
 
 	return upsConf
@@ -835,6 +842,7 @@ func TestFallbackFromInvalidBootstrap(t *testing.T) {
 	upsConf, err := ParseUpstreamsConfig(
 		[]string{"tls://dns.adguard.com"},
 		&upstream.Options{Bootstrap: invalidRslv, Timeout: testTimeout},
+		slogutil.NewDiscardLogger(),
 	)
 	require.NoError(t, err)
 

diff --git a/proxy/upstreams.go b/proxy/upstreams.go
@@ -1,15 +1,16 @@
 package proxy
 
 import (
+	"cmp"
 	"fmt"
 	"io"
+	"log/slog"
 	"slices"
 	"strings"
 
 	"github.com/AdguardTeam/dnsproxy/upstream"
 	"github.com/AdguardTeam/golibs/container"
 	"github.com/AdguardTeam/golibs/errors"
-	"github.com/AdguardTeam/golibs/log"
 	"github.com/AdguardTeam/golibs/mapsutil"
 	"github.com/AdguardTeam/golibs/netutil"
 )
@@ -92,13 +93,15 @@ var _ io.Closer = (*UpstreamConfig)(nil)
 func ParseUpstreamsConfig(
 	lines []string,
 	opts *upstream.Options,
+	logger *slog.Logger,
 ) (conf *UpstreamConfig, err error) {
 	if opts == nil {
 		opts = &upstream.Options{}
 	}
 
 	p := &configParser{
 		options:                  opts,
+		logger:                   cmp.Or(logger, slog.Default()),
 		upstreamsIndex:           map[string]upstream.Upstream{},
 		domainReservedUpstreams:  map[string][]upstream.Upstream{},
 		specifiedDomainUpstreams: map[string][]upstream.Upstream{},
@@ -138,6 +141,9 @@ type configParser struct {
 	// options contains upstream properties.
 	options *upstream.Options
 
+	// logger is used for logging during parsing.  It's never nil.
+	logger *slog.Logger
+
 	// upstreamsIndex is used to avoid creating duplicates of upstreams.
 	upstreamsIndex map[string]upstream.Upstream
 
@@ -246,7 +252,11 @@ func splitConfigLine(confLine string) (upstreams, domains []string, err error) {
 }
 
 // specifyUpstream specifies the upstream for domains.
-func (p *configParser) specifyUpstream(domains []string, u string, idx int) (err error) {
+func (p *configParser) specifyUpstream(
+	domains []string,
+	u string,
+	idx int,
+) (err error) {
 	dnsUpstream, ok := p.upstreamsIndex[u]
 	// TODO(e.burkov):  Improve identifying duplicate upstreams.
 	if !ok {
@@ -266,15 +276,11 @@ func (p *configParser) specifyUpstream(domains []string, u string, idx int) (err
 		p.upstreams = append(p.upstreams, dnsUpstream)
 
 		// TODO(s.chzhen):  Logs without index.
-		log.Debug("dnsproxy: upstream at index %d: %s", idx, addr)
+		p.logger.Debug("set upstream", "idx", idx, "addr", addr)
 	} else {
 		p.includeToReserved(dnsUpstream, domains)
 
-		log.Debug("dnsproxy: upstream at index %d: %s is reserved for %d domains",
-			idx,
-			addr,
-			len(domains),
-		)
+		p.logger.Debug("upstream is reserved", "idx", idx, "addr", addr, "domains_len", len(domains))
 	}
 
 	return nil
@@ -303,7 +309,7 @@ func (p *configParser) includeToReserved(dnsUpstream upstream.Upstream, domains
 			host = host[len("*."):]
 
 			p.subdomainsOnlyExclusions.Add(host)
-			log.Debug("domain %q is added to exclusions list", host)
+			p.logger.Debug("domain is added to exclusions list", "domain", host)
 
 			p.subdomainsOnlyUpstreams[host] = append(p.subdomainsOnlyUpstreams[host], dnsUpstream)
 		} else {

diff --git a/proxy/upstreams_internal_test.go b/proxy/upstreams_internal_test.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/AdguardTeam/dnsproxy/upstream"
 	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/stretchr/testify/assert"
@@ -61,7 +62,7 @@ var testUpstreamConfigLines = []string{
 func TestUpstreamConfig_GetUpstreamsForDomain(t *testing.T) {
 	t.Parallel()
 
-	config, err := ParseUpstreamsConfig(testUpstreamConfigLines, nil)
+	config, err := ParseUpstreamsConfig(testUpstreamConfigLines, nil, slogutil.NewDiscardLogger())
 	require.NoError(t, err)
 
 	testCases := []struct {
@@ -115,7 +116,7 @@ func TestUpstreamConfig_GetUpstreamsForDomain(t *testing.T) {
 func TestUpstreamConfig_GetUpstreamsForDS(t *testing.T) {
 	t.Parallel()
 
-	config, err := ParseUpstreamsConfig(testUpstreamConfigLines, nil)
+	config, err := ParseUpstreamsConfig(testUpstreamConfigLines, nil, slogutil.NewDiscardLogger())
 	require.NoError(t, err)
 
 	testCases := []struct {
@@ -196,7 +197,7 @@ func TestUpstreamConfig_Validate(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			c, err := ParseUpstreamsConfig(tc.in, nil)
+			c, err := ParseUpstreamsConfig(tc.in, nil, slogutil.NewDiscardLogger())
 			require.NoError(t, err)
 
 			assert.ErrorIs(t, c.validate(), tc.wantErr)
@@ -262,7 +263,7 @@ func TestValidatePrivateConfig(t *testing.T) {
 		set := []string{"192.168.0.1", tc.u}
 
 		t.Run(tc.name, func(t *testing.T) {
-			upsConf, err := ParseUpstreamsConfig(set, nil)
+			upsConf, err := ParseUpstreamsConfig(set, nil, slogutil.NewDiscardLogger())
 			require.NoError(t, err)
 
 			testutil.AssertErrorMsg(t, tc.wantErr, ValidatePrivateConfig(upsConf, ss))
@@ -279,6 +280,7 @@ func TestGetUpstreamsForDomainWithoutDuplicates(t *testing.T) {
 			Bootstrap:          nil,
 			Timeout:            testTimeout,
 		},
+		slogutil.NewDiscardLogger(),
 	)
 	assert.NoError(t, err)
 	assert.Len(t, config.Upstreams, 0)
@@ -302,7 +304,7 @@ func TestGetUpstreamsForDomain_wildcards(t *testing.T) {
 		"[/c.b.a.x/]#",
 	}
 
-	uconf, err := ParseUpstreamsConfig(conf, nil)
+	uconf, err := ParseUpstreamsConfig(conf, nil, slogutil.NewDiscardLogger())
 	require.NoError(t, err)
 
 	testCases := []struct {
@@ -363,7 +365,7 @@ func TestGetUpstreamsForDomain_sub_wildcards(t *testing.T) {
 		"[/*.b.a.x/]0.0.0.5",
 	}
 
-	uconf, err := ParseUpstreamsConfig(conf, nil)
+	uconf, err := ParseUpstreamsConfig(conf, nil, slogutil.NewDiscardLogger())
 	require.NoError(t, err)
 
 	testCases := []struct {
@@ -405,7 +407,7 @@ func TestGetUpstreamsForDomain_default_wildcards(t *testing.T) {
 		"[/*.www.example.org/]#",
 	}
 
-	uconf, err := ParseUpstreamsConfig(conf, nil)
+	uconf, err := ParseUpstreamsConfig(conf, nil, slogutil.NewDiscardLogger())
 	require.NoError(t, err)
 
 	testCases := []struct {
@@ -456,6 +458,7 @@ func BenchmarkGetUpstreamsForDomain(b *testing.B) {
 			Bootstrap:          nil,
 			Timeout:            testTimeout,
 		},
+		slogutil.NewDiscardLogger(),
 	)
 
 	domains := []string{