all: slog ipset

internal/dnsforward/config.go

@@ -457,24 +457,24 @@ func (s *Server) initDefaultSettings() {
 
 // prepareIpsetListSettings reads and prepares the ipset configuration either
 // from a file or from the data in the configuration file.
-func (s *Server) prepareIpsetListSettings() (err error) {
+func (s *Server) prepareIpsetListSettings() (ipsets []string, err error) {
 	fn := s.conf.IpsetListFileName
 	if fn == "" {
-		return s.ipset.init(s.conf.IpsetList)
+		return s.conf.IpsetList, nil
 	}
 
 	// #nosec G304 -- Trust the path explicitly given by the user.
 	data, err := os.ReadFile(fn)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	ipsets := stringutil.SplitTrimmed(string(data), "\n")
+	ipsets = stringutil.SplitTrimmed(string(data), "\n")
 	ipsets = stringutil.FilterOut(ipsets, IsCommentOrEmpty)
 
 	log.Debug("dns: using %d ipset rules from file %q", len(ipsets), fn)
 
-	return s.ipset.init(ipsets)
+	return ipsets, nil
 }
 
 // loadUpstreams parses upstream DNS servers from the configured file or from

internal/dnsforward/dnsforward.go

@@ -134,7 +134,7 @@ type Server struct {
 	localDomainSuffix string
 
 	// ipset processes DNS requests using ipset data.
-	ipset ipsetCtx
+	ipset *ipsetCtx
 
 	// privateNets is the configured set of IP networks considered private.
 	privateNets netutil.SubnetSet
@@ -609,11 +609,17 @@ func (s *Server) prepareLocalResolvers() (uc *proxy.UpstreamConfig, err error) {
 // the primary DNS proxy instance.  It assumes s.serverLock is locked or the
 // Server not running.
 func (s *Server) prepareInternalDNS() (err error) {
-	err = s.prepareIpsetListSettings()
+	ipsetConf, err := s.prepareIpsetListSettings()
 	if err != nil {
 		return fmt.Errorf("preparing ipset settings: %w", err)
 	}
 
+	s.ipset, err = newIPSetCtx(s.logger, ipsetConf)
+	if err != nil {
+		// Don't wrap the error, because it's informative enough as is.
+		return err
+	}
+
 	bootOpts := &upstream.Options{
 		Timeout:      DefaultTimeout,
 		HTTPVersions: UpstreamHTTPVersions(s.conf.UseHTTP3Upstreams),

internal/dnsforward/ipset.go

@@ -2,26 +2,31 @@ package dnsforward
 
 import (
 	"fmt"
+	"log/slog"
 	"net"
 	"os"
 	"strings"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/ipset"
 	"github.com/AdguardTeam/golibs/errors"
-	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/miekg/dns"
 )
 
 // ipsetCtx is the ipset context.  ipsetMgr can be nil.
 type ipsetCtx struct {
 	ipsetMgr ipset.Manager
+	logger   *slog.Logger
 }
 
-// init initializes the ipset context.  It is not safe for concurrent use.
+// newIPSetCtx returns a new initialized [ipsetCtx].  It is not safe for
-//
+// concurrent use.
-// TODO(a.garipov): Rewrite into a simple constructor?
+func newIPSetCtx(logger *slog.Logger, ipsetConf []string) (c *ipsetCtx, err error) {
-func (c *ipsetCtx) init(ipsetConf []string) (err error) {
+	c = &ipsetCtx{
-	c.ipsetMgr, err = ipset.NewManager(ipsetConf)
+		logger: logger,
+	}
+	ipsetLogger := logger.With(slogutil.KeyPrefix, "ipset")
+	c.ipsetMgr, err = ipset.NewManager(ipsetLogger, ipsetConf)
 	if errors.Is(err, os.ErrInvalid) || errors.Is(err, os.ErrPermission) {
 		// ipset cannot currently be initialized if the server was installed
 		// from Snap or when the user or the binary doesn't have the required
@@ -31,18 +36,18 @@ func (c *ipsetCtx) init(ipsetConf []string) (err error) {
 		//
 		// TODO(a.garipov): The Snap problem can probably be solved if we add
 		// the netlink-connector interface plug.
-		log.Info("ipset: warning: cannot initialize: %s", err)
+		logger.Warn("ipset: cannot initialize", slogutil.KeyError, err)
 
-		return nil
+		return c, nil
 	} else if errors.Is(err, errors.ErrUnsupported) {
-		log.Info("ipset: warning: %s", err)
+		logger.Warn("ipset: cannot initialize", slogutil.KeyError, err)
 
-		return nil
+		return c, nil
 	} else if err != nil {
-		return fmt.Errorf("initializing ipset: %w", err)
+		return nil, fmt.Errorf("initializing ipset: %w", err)
 	}
 
-	return nil
+	return c, nil
 }
 
 // close closes the Linux Netfilter connections.
@@ -109,15 +114,13 @@ func ipsFromAnswer(ans []dns.RR) (ip4s, ip6s []net.IP) {
 
 // process adds the resolved IP addresses to the domain's ipsets, if any.
 func (c *ipsetCtx) process(dctx *dnsContext) (rc resultCode) {
-	log.Debug("dnsforward: ipset: started processing")
+	c.logger.Debug("ipset: started processing")
-	defer log.Debug("dnsforward: ipset: finished processing")
+	defer c.logger.Debug("ipset: finished processing")
 
 	if c.skipIpsetProcessing(dctx) {
 		return resultCodeSuccess
 	}
 
-	log.Debug("ipset: starting processing")
-
 	req := dctx.proxyCtx.Req
 	host := req.Question[0].Name
 	host = strings.TrimSuffix(host, ".")
@@ -127,12 +130,12 @@ func (c *ipsetCtx) process(dctx *dnsContext) (rc resultCode) {
 	n, err := c.ipsetMgr.Add(host, ip4s, ip6s)
 	if err != nil {
 		// Consider ipset errors non-critical to the request.
-		log.Error("dnsforward: ipset: adding host ips: %s", err)
+		c.logger.Error("ipset: adding host ips", slogutil.KeyError, err)
 
 		return resultCodeSuccess
 	}
 
-	log.Debug("dnsforward: ipset: added %d new ipset entries", n)
+	c.logger.Debug("ipset: added new ipset entries", "num", n)
 
 	return resultCodeSuccess
 }

internal/dnsforward/ipset_internal_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/AdguardTeam/dnsproxy/proxy"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
 )
@@ -58,7 +59,9 @@ func TestIpsetCtx_process(t *testing.T) {
 			responseFromUpstream: true,
 		}
 
-		ictx := &ipsetCtx{}
+		ictx := &ipsetCtx{
+			logger: slogutil.NewDiscardLogger(),
+		}
 		rc := ictx.process(dctx)
 		assert.Equal(t, resultCodeSuccess, rc)
 
@@ -79,6 +82,7 @@ func TestIpsetCtx_process(t *testing.T) {
 		m := &fakeIpsetMgr{}
 		ictx := &ipsetCtx{
 			ipsetMgr: m,
+			logger:   slogutil.NewDiscardLogger(),
 		}
 
 		rc := ictx.process(dctx)
@@ -103,6 +107,7 @@ func TestIpsetCtx_process(t *testing.T) {
 		m := &fakeIpsetMgr{}
 		ictx := &ipsetCtx{
 			ipsetMgr: m,
+			logger:   slogutil.NewDiscardLogger(),
 		}
 
 		rc := ictx.process(dctx)
@@ -126,6 +131,7 @@ func TestIpsetCtx_SkipIpsetProcessing(t *testing.T) {
 	m := &fakeIpsetMgr{}
 	ictx := &ipsetCtx{
 		ipsetMgr: m,
+		logger:   slogutil.NewDiscardLogger(),
 	}
 
 	testCases := []struct {

internal/ipset/ipset.go

@@ -2,6 +2,7 @@
 package ipset
 
 import (
+	"log/slog"
 	"net"
 )
 
@@ -24,10 +25,10 @@ type Manager interface {
 //
 // If ipsetConf is empty, msg and err are nil.  The error's chain contains
 // [errors.ErrUnsupported] if current OS is not supported.
-func NewManager(ipsetConf []string) (mgr Manager, err error) {
+func NewManager(logger *slog.Logger, ipsetConf []string) (mgr Manager, err error) {
 	if len(ipsetConf) == 0 {
 		return nil, nil
 	}
 
-	return newManager(ipsetConf)
+	return newManager(logger, ipsetConf)
 }

internal/ipset/ipset_linux.go

@@ -5,13 +5,14 @@ package ipset
 import (
 	"bytes"
 	"fmt"
+	"log/slog"
 	"net"
 	"strings"
 	"sync"
 
 	"github.com/AdguardTeam/golibs/container"
 	"github.com/AdguardTeam/golibs/errors"
-	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/digineo/go-ipset/v2"
 	"github.com/mdlayher/netlink"
 	"github.com/ti-mo/netfilter"
@@ -34,8 +35,8 @@ import (
 //     resolved IP addresses.
 
 // newManager returns a new Linux ipset manager.
-func newManager(ipsetConf []string) (set Manager, err error) {
+func newManager(logger *slog.Logger, ipsetConf []string) (set Manager, err error) {
-	return newManagerWithDialer(ipsetConf, defaultDial)
+	return newManagerWithDialer(logger, ipsetConf, defaultDial)
 }
 
 // defaultDial is the default netfilter dialing function.
@@ -180,6 +181,8 @@ type manager struct {
 	nameToIpset    map[string]props
 	domainToIpsets map[string][]props
 
+	logger *slog.Logger
+
 	dial dialer
 
 	// mu protects all properties below.
@@ -336,10 +339,11 @@ func (m *manager) ipsets(names []string, currentlyKnown map[string]props) (sets
 		}
 
 		if p.family != netfilter.ProtoIPv4 && p.family != netfilter.ProtoIPv6 {
-			log.Debug("ipset: getting properties: %q %q unexpected ipset family %q",
+			m.logger.Debug("getting properties",
-				p.name,
+				slogutil.KeyError, "unexpected ipset family",
-				p.typeName,
+				"set_name", p.name,
-				p.family,
+				"set_type", p.typeName,
+				"set_family", p.family,
 			)
 
 			p, err = m.ipsetProps(n)
@@ -357,7 +361,11 @@ func (m *manager) ipsets(names []string, currentlyKnown map[string]props) (sets
 
 // newManagerWithDialer returns a new Linux ipset manager using the provided
 // dialer.
-func newManagerWithDialer(ipsetConf []string, dial dialer) (mgr Manager, err error) {
+func newManagerWithDialer(
+	logger *slog.Logger,
+	ipsetConf []string,
+	dial dialer,
+) (mgr Manager, err error) {
 	defer func() { err = errors.Annotate(err, "ipset: %w") }()
 
 	m := &manager{
@@ -366,6 +374,8 @@ func newManagerWithDialer(ipsetConf []string, dial dialer) (mgr Manager, err err
 		nameToIpset:    make(map[string]props),
 		domainToIpsets: make(map[string][]props),
 
+		logger: logger,
+
 		dial: dial,
 
 		addedIPs: container.NewMapSet[ipInIpsetEntry](),
@@ -376,7 +386,7 @@ func newManagerWithDialer(ipsetConf []string, dial dialer) (mgr Manager, err err
 		if errors.Is(err, unix.EPROTONOSUPPORT) {
 			// The implementation doesn't support this protocol version.  Just
 			// issue a warning.
-			log.Info("ipset: dialing netfilter: warning: %s", err)
+			logger.Warn("dialing netfilter", slogutil.KeyError, err)
 
 			return nil, nil
 		}
@@ -389,7 +399,7 @@ func newManagerWithDialer(ipsetConf []string, dial dialer) (mgr Manager, err err
 		return nil, fmt.Errorf("getting ipsets: %w", err)
 	}
 
-	log.Debug("ipset: initialized")
+	logger.Debug("initialized")
 
 	return m, nil
 }
@@ -498,7 +508,11 @@ func (m *manager) addToSets(
 			return n, fmt.Errorf("%q %q unexpected family %q", set.name, set.typeName, set.family)
 		}
 
-		log.Debug("ipset: added %d ips to set %q %q", nn, set.name, set.typeName)
+		m.logger.Debug("added ips to set",
+			"ips_num", nn,
+			"set_name", set.name,
+			"set_type", set.typeName,
+		)
 
 		n += nn
 	}
@@ -516,7 +530,7 @@ func (m *manager) Add(host string, ip4s, ip6s []net.IP) (n int, err error) {
 		return 0, nil
 	}
 
-	log.Debug("ipset: found %d sets", len(sets))
+	m.logger.Debug("found sets", "set_num", len(sets))
 
 	return m.addToSets(host, ip4s, ip6s, sets)
 }

internal/ipset/ipset_linux_internal_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/digineo/go-ipset/v2"
 	"github.com/mdlayher/netlink"
 	"github.com/stretchr/testify/assert"
@@ -89,7 +90,7 @@ func TestManager_Add(t *testing.T) {
 		}, nil
 	}
 
-	m, err := newManagerWithDialer(ipsetConf, fakeDial)
+	m, err := newManagerWithDialer(slogutil.NewDiscardLogger(), ipsetConf, fakeDial)
 	require.NoError(t, err)
 
 	ip4 := net.IP{1, 2, 3, 4}

internal/ipset/ipset_others.go

@@ -3,9 +3,11 @@
 package ipset
 
 import (
+	"log/slog"
+
 	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
 )
 
-func newManager(_ []string) (mgr Manager, err error) {
+func newManager(_ *slog.Logger, _ []string) (mgr Manager, err error) {
 	return nil, aghos.Unsupported("ipset")
 }