From 27383833033216ab938b0896dbc39e5af8d4dde9 Mon Sep 17 00:00:00 2001
From: Stanislav Chzhen <s.chzhen@adguard.com>
Date: Mon, 26 Aug 2024 19:11:10 +0300
Subject: [PATCH] all: slog arpdb

---
 internal/arpdb/arpdb.go                     | 46 +++++++++---
 internal/arpdb/arpdb_bsd.go                 | 31 +++-----
 internal/arpdb/arpdb_internal_test.go       |  8 +-
 internal/arpdb/arpdb_linux.go               | 82 +++++++--------------
 internal/arpdb/arpdb_linux_internal_test.go | 15 ++--
 internal/arpdb/arpdb_openbsd.go             | 31 +++-----
 internal/arpdb/arpdb_windows.go             | 28 +++----
 internal/home/home.go                       |  7 +-
 8 files changed, 106 insertions(+), 142 deletions(-)

diff --git a/internal/arpdb/arpdb.go b/internal/arpdb/arpdb.go
index 8405e4ba..2d1ffd46 100644
--- a/internal/arpdb/arpdb.go
+++ b/internal/arpdb/arpdb.go
@@ -5,6 +5,7 @@ import (
 	"bufio"
 	"bytes"
 	"fmt"
+	"log/slog"
 	"net"
 	"net/netip"
 	"slices"
@@ -12,7 +13,7 @@ import (
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
 	"github.com/AdguardTeam/golibs/errors"
-	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/osutil"
 )
@@ -38,8 +39,8 @@ type Interface interface {
 }
 
 // New returns the [Interface] properly initialized for the OS.
-func New() (arp Interface) {
-	return newARPDB()
+func New(logger *slog.Logger) (arp Interface) {
+	return newARPDB(logger)
 }
 
 // Empty is the [Interface] implementation that does nothing.
@@ -69,6 +70,28 @@ type Neighbor struct {
 	MAC net.HardwareAddr
 }
 
+// newNeighbor returns the new initialized [Neighbor] by parsing string
+// representations of IP and MAC addresses.
+func newNeighbor(host, ipStr, macStr string) (n *Neighbor, err error) {
+	defer func() { err = errors.Annotate(err, "getting arp neighbor: %w") }()
+
+	ip, err := netip.ParseAddr(ipStr)
+	if err != nil {
+		return nil, err
+	}
+
+	mac, err := net.ParseMAC(macStr)
+	if err != nil {
+		return nil, err
+	}
+
+	return &Neighbor{
+		Name: host,
+		IP:   ip,
+		MAC:  mac,
+	}, nil
+}
+
 // Clone returns the deep copy of n.
 func (n Neighbor) Clone() (clone Neighbor) {
 	return Neighbor{
@@ -80,10 +103,10 @@ func (n Neighbor) Clone() (clone Neighbor) {
 
 // validatedHostname returns h if it's a valid hostname, or an empty string
 // otherwise, logging the validation error.
-func validatedHostname(h string) (host string) {
+func validatedHostname(logger *slog.Logger, h string) (host string) {
 	err := netutil.ValidateHostname(h)
 	if err != nil {
-		log.Debug("arpdb: parsing arp output: host: %s", err)
+		logger.Debug("parsing host of arp output", slogutil.KeyError, err)
 
 		return ""
 	}
@@ -132,15 +155,16 @@ func (ns *neighs) reset(with []Neighbor) {
 // parseNeighsFunc parses the text from sc as if it'd be an output of some
 // ARP-related command.  lenHint is a hint for the size of the allocated slice
 // of Neighbors.
-type parseNeighsFunc func(sc *bufio.Scanner, lenHint int) (ns []Neighbor)
+type parseNeighsFunc func(logger *slog.Logger, sc *bufio.Scanner, lenHint int) (ns []Neighbor)
 
 // cmdARPDB is the implementation of the [Interface] that uses command line to
 // retrieve data.
 type cmdARPDB struct {
-	parse parseNeighsFunc
-	ns    *neighs
-	cmd   string
-	args  []string
+	logger *slog.Logger
+	parse  parseNeighsFunc
+	ns     *neighs
+	cmd    string
+	args   []string
 }
 
 // type check
@@ -158,7 +182,7 @@ func (arp *cmdARPDB) Refresh() (err error) {
 	}
 
 	sc := bufio.NewScanner(bytes.NewReader(out))
-	ns := arp.parse(sc, arp.ns.len())
+	ns := arp.parse(arp.logger, sc, arp.ns.len())
 	if err = sc.Err(); err != nil {
 		// TODO(e.burkov):  This error seems unreachable.  Investigate.
 		return fmt.Errorf("scanning the output: %w", err)
diff --git a/internal/arpdb/arpdb_bsd.go b/internal/arpdb/arpdb_bsd.go
index c9658dbb..e7357603 100644
--- a/internal/arpdb/arpdb_bsd.go
+++ b/internal/arpdb/arpdb_bsd.go
@@ -4,17 +4,17 @@ package arpdb
 
 import (
 	"bufio"
-	"net"
-	"net/netip"
+	"log/slog"
 	"strings"
 	"sync"
 
-	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 )
 
-func newARPDB() (arp *cmdARPDB) {
+func newARPDB(logger *slog.Logger) (arp *cmdARPDB) {
 	return &cmdARPDB{
-		parse: parseArpA,
+		logger: logger,
+		parse:  parseArpA,
 		ns: &neighs{
 			mu: &sync.RWMutex{},
 			ns: make([]Neighbor, 0),
@@ -33,7 +33,7 @@ func newARPDB() (arp *cmdARPDB) {
 // The expected input format:
 //
 //	host.name (192.168.0.1) at ff:ff:ff:ff:ff:ff on en0 ifscope [ethernet]
-func parseArpA(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
+func parseArpA(logger *slog.Logger, sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 	ns = make([]Neighbor, 0, lenHint)
 	for sc.Scan() {
 		ln := sc.Text()
@@ -48,26 +48,15 @@ func parseArpA(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 			continue
 		}
 
-		ip, err := netip.ParseAddr(ipStr[1 : len(ipStr)-1])
+		host := validatedHostname(logger, fields[0])
+		n, err := newNeighbor(host, ipStr[1:len(ipStr)-1], fields[3])
 		if err != nil {
-			log.Debug("arpdb: parsing arp output: ip: %s", err)
+			logger.Debug("parsing arp output", "line", ln, slogutil.KeyError, err)
 
 			continue
 		}
 
-		hwStr := fields[3]
-		mac, err := net.ParseMAC(hwStr)
-		if err != nil {
-			log.Debug("arpdb: parsing arp output: mac: %s", err)
-
-			continue
-		}
-
-		ns = append(ns, Neighbor{
-			IP:   ip,
-			MAC:  mac,
-			Name: validatedHostname(fields[0]),
-		})
+		ns = append(ns, *n)
 	}
 
 	return ns
diff --git a/internal/arpdb/arpdb_internal_test.go b/internal/arpdb/arpdb_internal_test.go
index dfd57614..50cd6661 100644
--- a/internal/arpdb/arpdb_internal_test.go
+++ b/internal/arpdb/arpdb_internal_test.go
@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -61,7 +62,7 @@ func (s mapShell) RunCmd(cmd string, args ...string) (code int, out []byte, err
 
 func Test_New(t *testing.T) {
 	var a Interface
-	require.NotPanics(t, func() { a = New() })
+	require.NotPanics(t, func() { a = New(slogutil.NewDiscardLogger()) })
 
 	assert.NotNil(t, a)
 }
@@ -201,8 +202,9 @@ func Test_NewARPDBs(t *testing.T) {
 
 func TestCmdARPDB_arpa(t *testing.T) {
 	a := &cmdARPDB{
-		cmd:   "cmd",
-		parse: parseArpA,
+		logger: slogutil.NewDiscardLogger(),
+		cmd:    "cmd",
+		parse:  parseArpA,
 		ns: &neighs{
 			mu: &sync.RWMutex{},
 			ns: make([]Neighbor, 0),
diff --git a/internal/arpdb/arpdb_linux.go b/internal/arpdb/arpdb_linux.go
index 0cf7f0ef..364d2ce9 100644
--- a/internal/arpdb/arpdb_linux.go
+++ b/internal/arpdb/arpdb_linux.go
@@ -6,17 +6,18 @@ import (
 	"bufio"
 	"fmt"
 	"io/fs"
+	"log/slog"
 	"net"
 	"net/netip"
 	"strings"
 	"sync"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
-	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/stringutil"
 )
 
-func newARPDB() (arp *arpdbs) {
+func newARPDB(logger *slog.Logger) (arp *arpdbs) {
 	// Use the common storage among the implementations.
 	ns := &neighs{
 		mu: &sync.RWMutex{},
@@ -39,9 +40,10 @@ func newARPDB() (arp *arpdbs) {
 		},
 		// Then, try "arp -a -n".
 		&cmdARPDB{
-			parse: parseF,
-			ns:    ns,
-			cmd:   "arp",
+			logger: logger,
+			parse:  parseF,
+			ns:     ns,
+			cmd:    "arp",
 			// Use -n flag to avoid resolving the hostnames of the neighbors.
 			// By default ARP attempts to resolve the hostnames via DNS.  See
 			// man 8 arp.
@@ -51,10 +53,11 @@ func newARPDB() (arp *arpdbs) {
 		},
 		// Finally, try "ip neigh".
 		&cmdARPDB{
-			parse: parseIPNeigh,
-			ns:    ns,
-			cmd:   "ip",
-			args:  []string{"neigh"},
+			logger: logger,
+			parse:  parseIPNeigh,
+			ns:     ns,
+			cmd:    "ip",
+			args:   []string{"neigh"},
 		},
 	)
 }
@@ -131,7 +134,7 @@ func (arp *fsysARPDB) Neighbors() (ns []Neighbor) {
 //
 //	IP address     HW type  Flags  HW address         Mask  Device
 //	192.168.11.98  0x1      0x2    5a:92:df:a9:7e:28  *     wan
-func parseArpAWrt(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
+func parseArpAWrt(logger *slog.Logger, sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 	if !sc.Scan() {
 		// Skip the header.
 		return
@@ -146,25 +149,14 @@ func parseArpAWrt(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 			continue
 		}
 
-		ip, err := netip.ParseAddr(fields[0])
+		n, err := newNeighbor("", fields[0], fields[3])
 		if err != nil {
-			log.Debug("arpdb: parsing arp output: ip: %s", err)
+			logger.Debug("parsing arp output", "line", ln, slogutil.KeyError, err)
 
 			continue
 		}
 
-		hwStr := fields[3]
-		mac, err := net.ParseMAC(hwStr)
-		if err != nil {
-			log.Debug("arpdb: parsing arp output: mac: %s", err)
-
-			continue
-		}
-
-		ns = append(ns, Neighbor{
-			IP:  ip,
-			MAC: mac,
-		})
+		ns = append(ns, *n)
 	}
 
 	return ns
@@ -174,7 +166,7 @@ func parseArpAWrt(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 // expected input format:
 //
 //	hostname (192.168.1.1) at ab:cd:ef:ab:cd:ef [ether] on enp0s3
-func parseArpA(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
+func parseArpA(logger *slog.Logger, sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 	ns = make([]Neighbor, 0, lenHint)
 	for sc.Scan() {
 		ln := sc.Text()
@@ -189,26 +181,15 @@ func parseArpA(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 			continue
 		}
 
-		ip, err := netip.ParseAddr(ipStr[1 : len(ipStr)-1])
+		host := validatedHostname(logger, fields[0])
+		n, err := newNeighbor(host, ipStr[1:len(ipStr)-1], fields[3])
 		if err != nil {
-			log.Debug("arpdb: parsing arp output: ip: %s", err)
+			logger.Debug("parsing arp output", "line", ln, slogutil.KeyError, err)
 
 			continue
 		}
 
-		hwStr := fields[3]
-		mac, err := net.ParseMAC(hwStr)
-		if err != nil {
-			log.Debug("arpdb: parsing arp output: mac: %s", err)
-
-			continue
-		}
-
-		ns = append(ns, Neighbor{
-			IP:   ip,
-			MAC:  mac,
-			Name: validatedHostname(fields[0]),
-		})
+		ns = append(ns, *n)
 	}
 
 	return ns
@@ -218,7 +199,7 @@ func parseArpA(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 // expected input format:
 //
 //	192.168.1.1 dev enp0s3 lladdr ab:cd:ef:ab:cd:ef REACHABLE
-func parseIPNeigh(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
+func parseIPNeigh(logger *slog.Logger, sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 	ns = make([]Neighbor, 0, lenHint)
 	for sc.Scan() {
 		ln := sc.Text()
@@ -228,27 +209,14 @@ func parseIPNeigh(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 			continue
 		}
 
-		n := Neighbor{}
-
-		ip, err := netip.ParseAddr(fields[0])
+		n, err := newNeighbor("", fields[0], fields[4])
 		if err != nil {
-			log.Debug("arpdb: parsing arp output: ip: %s", err)
+			logger.Debug("parsing arp output", "line", ln, slogutil.KeyError, err)
 
 			continue
-		} else {
-			n.IP = ip
 		}
 
-		mac, err := net.ParseMAC(fields[4])
-		if err != nil {
-			log.Debug("arpdb: parsing arp output: mac: %s", err)
-
-			continue
-		} else {
-			n.MAC = mac
-		}
-
-		ns = append(ns, n)
+		ns = append(ns, *n)
 	}
 
 	return ns
diff --git a/internal/arpdb/arpdb_linux_internal_test.go b/internal/arpdb/arpdb_linux_internal_test.go
index 44c76843..cb0cb1d3 100644
--- a/internal/arpdb/arpdb_linux_internal_test.go
+++ b/internal/arpdb/arpdb_linux_internal_test.go
@@ -9,6 +9,7 @@ import (
 	"testing"
 	"testing/fstest"
 
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -69,9 +70,10 @@ func TestCmdARPDB_linux(t *testing.T) {
 
 	t.Run("wrt", func(t *testing.T) {
 		a := &cmdARPDB{
-			parse: parseArpAWrt,
-			cmd:   "arp",
-			args:  []string{"-a"},
+			logger: slogutil.NewDiscardLogger(),
+			parse:  parseArpAWrt,
+			cmd:    "arp",
+			args:   []string{"-a"},
 			ns: &neighs{
 				mu: &sync.RWMutex{},
 				ns: make([]Neighbor, 0),
@@ -86,9 +88,10 @@ func TestCmdARPDB_linux(t *testing.T) {
 
 	t.Run("ip_neigh", func(t *testing.T) {
 		a := &cmdARPDB{
-			parse: parseIPNeigh,
-			cmd:   "ip",
-			args:  []string{"neigh"},
+			logger: slogutil.NewDiscardLogger(),
+			parse:  parseIPNeigh,
+			cmd:    "ip",
+			args:   []string{"neigh"},
 			ns: &neighs{
 				mu: &sync.RWMutex{},
 				ns: make([]Neighbor, 0),
diff --git a/internal/arpdb/arpdb_openbsd.go b/internal/arpdb/arpdb_openbsd.go
index 7b60737b..8d6ee657 100644
--- a/internal/arpdb/arpdb_openbsd.go
+++ b/internal/arpdb/arpdb_openbsd.go
@@ -4,17 +4,17 @@ package arpdb
 
 import (
 	"bufio"
-	"net"
-	"net/netip"
+	"log/slog"
 	"strings"
 	"sync"
 
-	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 )
 
-func newARPDB() (arp *cmdARPDB) {
+func newARPDB(logger *slog.Logger) (arp *cmdARPDB) {
 	return &cmdARPDB{
-		parse: parseArpA,
+		logger: logger,
+		parse:  parseArpA,
 		ns: &neighs{
 			mu: &sync.RWMutex{},
 			ns: make([]Neighbor, 0),
@@ -34,7 +34,7 @@ func newARPDB() (arp *cmdARPDB) {
 //
 //	Host        Ethernet Address  Netif Expire    Flags
 //	192.168.1.1 ab:cd:ef:ab:cd:ef   em0 19m59s
-func parseArpA(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
+func parseArpA(logger *slog.Logger, sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 	// Skip the header.
 	if !sc.Scan() {
 		return nil
@@ -49,27 +49,14 @@ func parseArpA(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 			continue
 		}
 
-		n := Neighbor{}
-
-		ip, err := netip.ParseAddr(fields[0])
+		n, err := newNeighbor("", fields[0], fields[1])
 		if err != nil {
-			log.Debug("arpdb: parsing arp output: ip: %s", err)
+			logger.Debug("parsing arp output", "line", ln, slogutil.KeyError, err)
 
 			continue
-		} else {
-			n.IP = ip
 		}
 
-		mac, err := net.ParseMAC(fields[1])
-		if err != nil {
-			log.Debug("arpdb: parsing arp output: mac: %s", err)
-
-			continue
-		} else {
-			n.MAC = mac
-		}
-
-		ns = append(ns, n)
+		ns = append(ns, *n)
 	}
 
 	return ns
diff --git a/internal/arpdb/arpdb_windows.go b/internal/arpdb/arpdb_windows.go
index 3b4bb725..878d1d31 100644
--- a/internal/arpdb/arpdb_windows.go
+++ b/internal/arpdb/arpdb_windows.go
@@ -4,17 +4,17 @@ package arpdb
 
 import (
 	"bufio"
-	"net"
-	"net/netip"
+	"log/slog"
 	"strings"
 	"sync"
 
-	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 )
 
-func newARPDB() (arp *cmdARPDB) {
+func newARPDB(logger *slog.Logger) (arp *cmdARPDB) {
 	return &cmdARPDB{
-		parse: parseArpA,
+		logger: logger,
+		parse:  parseArpA,
 		ns: &neighs{
 			mu: &sync.RWMutex{},
 			ns: make([]Neighbor, 0),
@@ -31,7 +31,7 @@ func newARPDB() (arp *cmdARPDB) {
 //	  Internet Address      Physical Address      Type
 //	  192.168.56.1          0a-00-27-00-00-00     dynamic
 //	  192.168.56.255        ff-ff-ff-ff-ff-ff     static
-func parseArpA(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
+func parseArpA(logger *slog.Logger, sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 	ns = make([]Neighbor, 0, lenHint)
 	for sc.Scan() {
 		ln := sc.Text()
@@ -44,24 +44,14 @@ func parseArpA(sc *bufio.Scanner, lenHint int) (ns []Neighbor) {
 			continue
 		}
 
-		ip, err := netip.ParseAddr(fields[0])
+		n, err := newNeighbor("", fields[0], fields[1])
 		if err != nil {
-			log.Debug("arpdb: parsing arp output: ip: %s", err)
+			logger.Debug("parsing arp output", "line", ln, slogutil.KeyError, err)
 
 			continue
 		}
 
-		mac, err := net.ParseMAC(fields[1])
-		if err != nil {
-			log.Debug("arpdb: parsing arp output: mac: %s", err)
-
-			continue
-		}
-
-		ns = append(ns, Neighbor{
-			IP:  ip,
-			MAC: mac,
-		})
+		ns = append(ns, *n)
 	}
 
 	return ns
diff --git a/internal/home/home.go b/internal/home/home.go
index bd15f114..8c343dbc 100644
--- a/internal/home/home.go
+++ b/internal/home/home.go
@@ -39,6 +39,7 @@ import (
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/hostsfile"
 	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/osutil"
 )
@@ -276,7 +277,7 @@ func setupOpts(opts options) (err error) {
 }
 
 // initContextClients initializes Context clients and related fields.
-func initContextClients() (err error) {
+func initContextClients(logger *slog.Logger) (err error) {
 	err = setupDNSFilteringConf(config.Filtering)
 	if err != nil {
 		// Don't wrap the error, because it's informative enough as is.
@@ -300,7 +301,7 @@ func initContextClients() (err error) {
 
 	var arpDB arpdb.Interface
 	if config.Clients.Sources.ARP {
-		arpDB = arpdb.New()
+		arpDB = arpdb.New(logger.With(slogutil.KeyError, "arpdb"))
 	}
 
 	return Context.clients.Init(
@@ -582,7 +583,7 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}) {
 	// data first, but also to avoid relying on automatic Go init() function.
 	filtering.InitModule()
 
-	err = initContextClients()
+	err = initContextClients(slogLogger)
 	fatalOnError(err)
 
 	err = setupOpts(opts)