From 904a847684d6e6d3aa55dfc478d5b6c4dc8767db Mon Sep 17 00:00:00 2001 From: Dimitry Kolyshev Date: Fri, 27 Jun 2025 12:57:14 +0400 Subject: [PATCH] filtering: rewrite slog --- internal/filtering/rewrite/storage.go | 35 +++++++++++++----- .../rewrite/storage_internal_test.go | 37 ++++++++++++++++--- 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/internal/filtering/rewrite/storage.go b/internal/filtering/rewrite/storage.go index 42b36273..0bf2d4d6 100644 --- a/internal/filtering/rewrite/storage.go +++ b/internal/filtering/rewrite/storage.go @@ -3,12 +3,12 @@ package rewrite import ( "fmt" + "log/slog" "slices" "strings" "sync" "github.com/AdguardTeam/golibs/container" - "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/urlfilter" "github.com/AdguardTeam/urlfilter/filterlist" "github.com/AdguardTeam/urlfilter/rules" @@ -30,8 +30,23 @@ type Storage interface { List() (items []*Item) } +// Config is the configuration for DefaultStorage. +type Config struct { + // logger is used for logging storage processes. It must not be nil. + Logger *slog.Logger + + // Rewrites stores the rewrite entries. It must not be nil. + Rewrites []*Item + + // ListID is used as an identifier of the underlying rules list. + ListID int +} + // DefaultStorage is the default storage for rewrite rules. type DefaultStorage struct { + // logger is used for logging storage processes. It must not be nil. + logger *slog.Logger + // mu protects items. mu *sync.RWMutex @@ -51,13 +66,13 @@ type DefaultStorage struct { urlFilterID int } -// NewDefaultStorage returns new rewrites storage. listID is used as an -// identifier of the underlying rules list. rewrites must not be nil. -func NewDefaultStorage(listID int, rewrites []*Item) (s *DefaultStorage, err error) { +// NewDefaultStorage returns new rewrites storage. conf must not be nil. +func NewDefaultStorage(conf *Config) (s *DefaultStorage, err error) { s = &DefaultStorage{ + logger: conf.Logger, mu: &sync.RWMutex{}, - urlFilterID: listID, - rewrites: rewrites, + urlFilterID: conf.ListID, + rewrites: conf.Rewrites, } s.mu.Lock() @@ -91,7 +106,7 @@ func (s *DefaultStorage) MatchRequest(dReq *urlfilter.DNSRequest) (rws []*rules. rule := rrules[0] rwAns := rule.DNSRewrite.NewCNAME - log.Debug("rewrite: cname for %s is %s", host, rwAns) + s.logger.Debug("cname found", "host", host, "cname", rwAns) if dReq.Hostname == rwAns { // A request for the hostname itself is an exception rule. @@ -109,7 +124,7 @@ func (s *DefaultStorage) MatchRequest(dReq *urlfilter.DNSRequest) (rws []*rules. } if cnames.Has(rwAns) { - log.Info("rewrite: cname loop for %q on %q", dReq.Hostname, rwAns) + s.logger.Info("rewrite cname loop", "host", dReq.Hostname, "rewrite", rwAns) return nil } @@ -173,7 +188,7 @@ func (s *DefaultStorage) Remove(item *Item) (err error) { // TODO(d.kolyshev): Use slices.IndexFunc + slices.Delete? for _, ent := range s.rewrites { if ent.equal(item) { - log.Debug("rewrite: removed element: %s -> %s", ent.Domain, ent.Answer) + s.logger.Debug("removed element", "domain", ent.Domain, "ans", ent.Answer) continue } @@ -215,7 +230,7 @@ func (s *DefaultStorage) resetRules() (err error) { s.ruleList = strList s.engine = urlfilter.NewDNSEngine(rs) - log.Info("rewrite: filter %d: reset %d rules", s.urlFilterID, s.engine.RulesCount) + s.logger.Info("reset rules", "filter", s.urlFilterID, "count", s.engine.RulesCount) return nil } diff --git a/internal/filtering/rewrite/storage_internal_test.go b/internal/filtering/rewrite/storage_internal_test.go index 502c20b9..10df670c 100644 --- a/internal/filtering/rewrite/storage_internal_test.go +++ b/internal/filtering/rewrite/storage_internal_test.go @@ -4,6 +4,7 @@ import ( "net/netip" "testing" + "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/urlfilter" "github.com/AdguardTeam/urlfilter/rules" @@ -18,7 +19,11 @@ func TestNewDefaultStorage(t *testing.T) { Answer: "answer.com", }} - s, err := NewDefaultStorage(-1, items) + s, err := NewDefaultStorage(&Config{ + Logger: slogutil.NewDiscardLogger(), + Rewrites: items, + ListID: -1, + }) require.NoError(t, err) require.Len(t, s.List(), 1) @@ -27,7 +32,11 @@ func TestNewDefaultStorage(t *testing.T) { func TestDefaultStorage_CRUD(t *testing.T) { var items []*Item - s, err := NewDefaultStorage(-1, items) + s, err := NewDefaultStorage(&Config{ + Logger: slogutil.NewDiscardLogger(), + Rewrites: items, + ListID: -1, + }) require.NoError(t, err) require.Len(t, s.List(), 0) @@ -112,7 +121,11 @@ func TestDefaultStorage_MatchRequest(t *testing.T) { Answer: "sub.issue4016.com", }} - s, err := NewDefaultStorage(-1, items) + s, err := NewDefaultStorage(&Config{ + Logger: slogutil.NewDiscardLogger(), + Rewrites: items, + ListID: -1, + }) require.NoError(t, err) testCases := []struct { @@ -284,7 +297,11 @@ func TestDefaultStorage_MatchRequest_Levels(t *testing.T) { Answer: addr3.String(), }} - s, err := NewDefaultStorage(-1, items) + s, err := NewDefaultStorage(&Config{ + Logger: slogutil.NewDiscardLogger(), + Rewrites: items, + ListID: -1, + }) require.NoError(t, err) testCases := []struct { @@ -352,7 +369,11 @@ func TestDefaultStorage_MatchRequest_ExceptionCNAME(t *testing.T) { Answer: "*.sub.host.com", }} - s, err := NewDefaultStorage(-1, items) + s, err := NewDefaultStorage(&Config{ + Logger: slogutil.NewDiscardLogger(), + Rewrites: items, + ListID: -1, + }) require.NoError(t, err) testCases := []struct { @@ -416,7 +437,11 @@ func TestDefaultStorage_MatchRequest_ExceptionIP(t *testing.T) { Answer: "A", }} - s, err := NewDefaultStorage(-1, items) + s, err := NewDefaultStorage(&Config{ + Logger: slogutil.NewDiscardLogger(), + Rewrites: items, + ListID: -1, + }) require.NoError(t, err) testCases := []struct {