Pull request 1727: 4299-querylog-ignore

Merge in DNS/adguard-home from 4299-querylog-ignore to master

Squashed commit of the following:

commit 06f32fef860d63dc2af9aad8d4251918c5babd00
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Tue Feb 7 17:14:26 2023 +0300

    add debug msg

commit 48fc9cf90bcb5baec4b9a7949b5be00055ad0955
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Tue Feb 7 16:30:06 2023 +0300

    add line break

commit a96fe712b6e5c6a190a92b2f83ed031a85658e58
Merge: d1035219 b8d55eaf
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Tue Feb 7 16:12:32 2023 +0300

    Merge branch 'master' into 4299-querylog-ignore

commit d1035219e15e5b5639b2fc39e0b17cfc05904722
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Tue Feb 7 16:11:03 2023 +0300

    fix docs

commit caea5dcdf3e2ca8fe2d54cb1463226bb791470f8
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Tue Feb 7 15:30:16 2023 +0300

    fix issue link

commit f6e3d122404e1363dd6dd7fa0221e8ce321354e9
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Tue Feb 7 14:34:11 2023 +0300

    all: add issues links

commit 52f77188f1c3a93494585ca8a3ea16e373a8b5c6
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Tue Feb 7 13:11:58 2023 +0300

    all: add log message

commit a40a0c87937abb778e4e632a5403543371b6d2e0
Merge: 87fd71ba b31bab59
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Tue Feb 7 12:16:08 2023 +0300

    Merge branch 'master' into 4299-querylog-ignore

commit 87fd71ba01588f798ba944a75e6585ebdc4aa1f7
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Tue Feb 7 12:13:44 2023 +0300

    all: add todo

commit c0c2ea08d36f25003c709eb2c190a147c47c2e0c
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Mon Feb 6 13:27:24 2023 +0300

    all: add changelog

commit 8d227b684794e306e314d8cb848fe354d4578607
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Feb 3 16:06:43 2023 +0300

    all: querylog ignore

CHANGELOG.md

@@ -23,10 +23,43 @@ See also the [v0.107.24 GitHub milestone][ms-v0.107.24].
 NOTE: Add new changes BELOW THIS COMMENT.
 -->
 
+### Added
+
+- The ability to exclude domain names from the query log by using the new
+  `querylog.ignored` field ([#1717], [#4299].
+
+#### Configuration Changes
+
+In this release, the schema version has changed from 14 to 15.
+
+- The fields `dns.…` have been moved to the new `querylog` object.
+
+  ```yaml
+  # BEFORE:
+  'dns':
+    'querylog_enabled': true
+    'querylog_file_enabled': true
+    'querylog_interval': '2160h'
+    'querylog_size_memory': 1000
+
+  # AFTER:
+  'querylog':
+    'enabled': true
+    'file_enabled': true
+    'interval': '2160h'
+    'size_memory': 1000
+  ```
+
+  To rollback this change, rename and move properties back into the `dns`
+  object, remove `querylog` object and `querylog.ignored` property, and change
+  the `schema_version` back to `14`.
+
 ### Fixed
 
 - The icon for League Of Legends on the Blocked services page ([#5433]).
 
+[#1717]: https://github.com/AdguardTeam/AdGuardHome/issues/1717
+[#4299]: https://github.com/AdguardTeam/AdGuardHome/issues/4299
 [#5433]: https://github.com/AdguardTeam/AdGuardHome/issues/5433
 
 <!--

internal/dnsforward/dns.go

@@ -19,6 +19,9 @@ import (
 )
 
 // To transfer information between modules
+//
+// TODO(s.chzhen):  Add lowercased, non-FQDN version of the hostname from the
+// question of the request.
 type dnsContext struct {
 	proxyCtx *proxy.DNSContext
 

internal/dnsforward/stats.go

@@ -22,9 +22,11 @@ func (s *Server) processQueryLogsAndStats(dctx *dnsContext) (rc resultCode) {
 
 	shouldLog := true
 	msg := pctx.Req
+	q := msg.Question[0]
+	host := strings.ToLower(strings.TrimSuffix(q.Name, "."))
 
 	// don't log ANY request if refuseAny is enabled
-	if len(msg.Question) >= 1 && msg.Question[0].Qtype == dns.TypeANY && s.conf.RefuseAny {
+	if q.Qtype == dns.TypeANY && s.conf.RefuseAny {
 		shouldLog = false
 	}
 
@@ -41,8 +43,12 @@ func (s *Server) processQueryLogsAndStats(dctx *dnsContext) (rc resultCode) {
 	// Synchronize access to s.queryLog and s.stats so they won't be suddenly
 	// uninitialized while in use.  This can happen after proxy server has been
 	// stopped, but its workers haven't yet exited.
-	if shouldLog && s.queryLog != nil {
+	if shouldLog &&
+		s.queryLog != nil &&
+		s.queryLog.ShouldLog(host, q.Qtype, q.Qclass) {
 		s.logQuery(dctx, pctx, elapsed, ip)
+	} else {
+		log.Debug("request for %s from %s ignored; not logging", host, ip)
 	}
 
 	if s.stats != nil {

internal/dnsforward/stats_test.go

@@ -16,20 +16,25 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// testQueryLog is a simple querylog.QueryLog implementation for tests.
+// testQueryLog is a simple [querylog.QueryLog] implementation for tests.
 type testQueryLog struct {
 	// QueryLog is embedded here simply to make testQueryLog
-	// a querylog.QueryLog without actually implementing all methods.
+	// a [querylog.QueryLog] without actually implementing all methods.
 	querylog.QueryLog
 
 	lastParams *querylog.AddParams
 }
 
-// Add implements the querylog.QueryLog interface for *testQueryLog.
+// Add implements the [querylog.QueryLog] interface for *testQueryLog.
 func (l *testQueryLog) Add(p *querylog.AddParams) {
 	l.lastParams = p
 }
 
+// ShouldLog implements the [querylog.QueryLog] interface for *testQueryLog.
+func (l *testQueryLog) ShouldLog(string, uint16, uint16) bool {
+	return true
+}
+
 // testStats is a simple stats.Stats implementation for tests.
 type testStats struct {
 	// Stats is embedded here simply to make testStats a stats.Stats without

internal/home/config.go

@@ -6,6 +6,7 @@ import (
 	"net/netip"
 	"os"
 	"path/filepath"
+	"sort"
 	"sync"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
@@ -114,6 +115,7 @@ type configuration struct {
 
 	DNS      dnsConfig         `yaml:"dns"`
 	TLS      tlsConfigSettings `yaml:"tls"`
+	QueryLog queryLogConfig    `yaml:"querylog"`
 
 	// Filters reflects the filters from [filtering.Config].  It's cloned to the
 	// config used in the filtering module at the startup.  Afterwards it's
@@ -151,16 +153,6 @@ type dnsConfig struct {
 	// days.
 	StatsInterval uint32 `yaml:"statistics_interval"`
 
-	// QueryLogEnabled defines if the query log is enabled.
-	QueryLogEnabled bool `yaml:"querylog_enabled"`
-	// QueryLogFileEnabled defines, if the query log is written to the file.
-	QueryLogFileEnabled bool `yaml:"querylog_file_enabled"`
-	// QueryLogInterval is the interval for query log's files rotation.
-	QueryLogInterval timeutil.Duration `yaml:"querylog_interval"`
-	// QueryLogMemSize is the number of entries kept in memory before they are
-	// flushed to disk.
-	QueryLogMemSize uint32 `yaml:"querylog_size_memory"`
-
 	// AnonymizeClientIP defines if clients' IP addresses should be anonymized
 	// in query log and statistics.
 	AnonymizeClientIP bool `yaml:"anonymize_client_ip"`
@@ -228,6 +220,25 @@ type tlsConfigSettings struct {
 	dnsforward.TLSConfig `yaml:",inline" json:",inline"`
 }
 
+type queryLogConfig struct {
+	// Enabled defines if the query log is enabled.
+	Enabled bool `yaml:"enabled"`
+
+	// FileEnabled defines, if the query log is written to the file.
+	FileEnabled bool `yaml:"file_enabled"`
+
+	// Interval is the interval for query log's files rotation.
+	Interval timeutil.Duration `yaml:"interval"`
+
+	// MemSize is the number of entries kept in memory before they are
+	// flushed to disk.
+	MemSize uint32 `yaml:"size_memory"`
+
+	// Ignored is the list of host names, which are should not be written
+	// to log.
+	Ignored []string `yaml:"ignored"`
+}
+
 // config is the global configuration structure.
 //
 // TODO(a.garipov, e.burkov): This global is awful and must be removed.
@@ -241,10 +252,6 @@ var config = &configuration{
 		BindHosts:     []netip.Addr{netip.IPv4Unspecified()},
 		Port:          defaultPortDNS,
 		StatsInterval: 1,
-		QueryLogEnabled:     true,
-		QueryLogFileEnabled: true,
-		QueryLogInterval:    timeutil.Duration{Duration: 90 * timeutil.Day},
-		QueryLogMemSize:     1000,
 		FilteringConfig: dnsforward.FilteringConfig{
 			ProtectionEnabled:  true, // whether or not use any of filtering features
 			BlockingMode:       dnsforward.BlockingModeDefault,
@@ -282,6 +289,13 @@ var config = &configuration{
 		PortDNSOverTLS:  defaultPortTLS, // needs to be passed through to dnsproxy
 		PortDNSOverQUIC: defaultPortQUIC,
 	},
+	QueryLog: queryLogConfig{
+		Enabled:     true,
+		FileEnabled: true,
+		Interval:    timeutil.Duration{Duration: 90 * timeutil.Day},
+		MemSize:     1000,
+		Ignored:     []string{},
+	},
 	// NOTE: Keep these parameters in sync with the one put into
 	// client/src/helpers/filters/filters.js by scripts/vetted-filters.
 	//
@@ -466,11 +480,13 @@ func (c *configuration) write() (err error) {
 	if Context.queryLog != nil {
 		dc := querylog.Config{}
 		Context.queryLog.WriteDiskConfig(&dc)
-		config.DNS.QueryLogEnabled = dc.Enabled
-		config.DNS.QueryLogFileEnabled = dc.FileEnabled
-		config.DNS.QueryLogInterval = timeutil.Duration{Duration: dc.RotationIvl}
-		config.DNS.QueryLogMemSize = dc.MemSize
 		config.DNS.AnonymizeClientIP = dc.AnonymizeClientIP
+		config.QueryLog.Enabled = dc.Enabled
+		config.QueryLog.FileEnabled = dc.FileEnabled
+		config.QueryLog.Interval = timeutil.Duration{Duration: dc.RotationIvl}
+		config.QueryLog.MemSize = dc.MemSize
+		config.QueryLog.Ignored = dc.Ignored.Values()
+		sort.Strings(config.QueryLog.Ignored)
 	}
 
 	if Context.filters != nil {

internal/home/dns.go

@@ -7,6 +7,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
@@ -20,6 +21,7 @@ import (
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/AdguardTeam/golibs/netutil"
+	"github.com/AdguardTeam/golibs/stringutil"
 	"github.com/ameshkov/dnscrypt/v2"
 	yaml "gopkg.in/yaml.v3"
 )
@@ -66,11 +68,20 @@ func initDNS() (err error) {
 		HTTPRegister:      httpRegister,
 		FindClient:        Context.clients.findMultiple,
 		BaseDir:           baseDir,
-		RotationIvl:       config.DNS.QueryLogInterval.Duration,
-		MemSize:           config.DNS.QueryLogMemSize,
-		Enabled:           config.DNS.QueryLogEnabled,
-		FileEnabled:       config.DNS.QueryLogFileEnabled,
 		AnonymizeClientIP: config.DNS.AnonymizeClientIP,
+		RotationIvl:       config.QueryLog.Interval.Duration,
+		MemSize:           config.QueryLog.MemSize,
+		Enabled:           config.QueryLog.Enabled,
+		FileEnabled:       config.QueryLog.FileEnabled,
+		Ignored:           stringutil.NewSet(),
+	}
+	for _, v := range config.QueryLog.Ignored {
+		host := strings.ToLower(strings.TrimSuffix(v, "."))
+		if conf.Ignored.Has(host) {
+			return fmt.Errorf("duplicate ignored host %s", host)
+		}
+
+		conf.Ignored.Add(host)
 	}
 	Context.queryLog = querylog.New(conf)
 

internal/home/upgrade.go

@@ -22,7 +22,7 @@ import (
 )
 
 // currentSchemaVersion is the current schema version.
-const currentSchemaVersion = 14
+const currentSchemaVersion = 15
 
 // These aliases are provided for convenience.
 type (
@@ -87,6 +87,7 @@ func upgradeConfigSchema(oldVersion int, diskConf yobj) (err error) {
 		upgradeSchema11to12,
 		upgradeSchema12to13,
 		upgradeSchema13to14,
+		upgradeSchema14to15,
 	}
 
 	n := 0
@@ -802,6 +803,59 @@ func upgradeSchema13to14(diskConf yobj) (err error) {
 	return nil
 }
 
+// upgradeSchema14to15 performs the following changes:
+//
+//	# BEFORE:
+//	'dns':
+//	  'querylog_enabled': true
+//	  'querylog_file_enabled': true
+//	  'querylog_interval': '2160h'
+//	  'querylog_size_memory': 1000
+//
+//	# AFTER:
+//	'querylog':
+//	  'enabled': true
+//	  'file_enabled': true
+//	  'interval': '2160h'
+//	  'size_memory': 1000
+//	  'ignored': []
+func upgradeSchema14to15(diskConf yobj) (err error) {
+	log.Printf("Upgrade yaml: 14 to 15")
+	diskConf["schema_version"] = 15
+
+	dnsVal := diskConf["dns"]
+	dns, ok := dnsVal.(map[string]any)
+	if !ok {
+		return fmt.Errorf("unexpected type of dns: %T", dnsVal)
+	}
+
+	type temp struct {
+		from string
+		to   string
+		val  any
+	}
+	replaces := []temp{
+		{from: "querylog_enabled", to: "enabled", val: true},
+		{from: "querylog_file_enabled", to: "file_enabled", val: true},
+		{from: "querylog_interval", to: "interval", val: "2160h"},
+		{from: "querylog_size_memory", to: "size_memory", val: 1000},
+	}
+	qlog := map[string]any{
+		"ignored": []any{},
+	}
+	for _, r := range replaces {
+		v, has := dns[r.from]
+		if !has {
+			v = r.val
+		}
+		delete(dns, r.from)
+		qlog[r.to] = v
+	}
+	diskConf["querylog"] = qlog
+
+	return nil
+}
+
 // TODO(a.garipov): Replace with log.Output when we port it to our logging
 // package.
 func funcName() string {

internal/home/upgrade_test.go

@@ -640,3 +640,51 @@ func TestUpgradeSchema13to14(t *testing.T) {
 		})
 	}
 }
+
+func TestUpgradeSchema14to15(t *testing.T) {
+	const newSchemaVer = 15
+
+	defaultWantObj := yobj{
+		"querylog": map[string]any{
+			"enabled":      true,
+			"file_enabled": true,
+			"interval":     "2160h",
+			"size_memory":  1000,
+			"ignored":      []any{},
+		},
+		"dns":            map[string]any{},
+		"schema_version": newSchemaVer,
+	}
+
+	testCases := []struct {
+		in   yobj
+		want yobj
+		name string
+	}{{
+		in: yobj{
+			"dns": map[string]any{
+				"querylog_enabled":      true,
+				"querylog_file_enabled": true,
+				"querylog_interval":     "2160h",
+				"querylog_size_memory":  1000,
+			},
+		},
+		want: defaultWantObj,
+		name: "basic",
+	}, {
+		in: yobj{
+			"dns": map[string]any{},
+		},
+		want: defaultWantObj,
+		name: "default_values",
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := upgradeSchema14to15(tc.in)
+			require.NoError(t, err)
+
+			assert.Equal(t, tc.want, tc.in)
+		})
+	}
+}

internal/querylog/qlog.go

@@ -247,3 +247,13 @@ func (l *queryLog) Add(params *AddParams) {
 		}()
 	}
 }
+
+// ShouldLog returns true if request for the host should be logged.
+func (l *queryLog) ShouldLog(host string, _, _ uint16) bool {
+	return !l.isIgnored(host)
+}
+
+// isIgnored returns true if the host is in the Ignored list.
+func (l *queryLog) isIgnored(host string) bool {
+	return l.conf.Ignored.Has(host)
+}

internal/querylog/qlog_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
 	"github.com/AdguardTeam/dnsproxy/proxyutil"
+	"github.com/AdguardTeam/golibs/stringutil"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/AdguardTeam/golibs/timeutil"
 	"github.com/miekg/dns"
@@ -249,6 +250,48 @@ func TestQueryLogFileDisabled(t *testing.T) {
 	assert.Equal(t, "example2.org", ll[1].QHost)
 }
 
+func TestQueryLogShouldLog(t *testing.T) {
+	const (
+		ignored1 = "ignor.ed"
+		ignored2 = "ignored.to"
+	)
+	set := stringutil.NewSet(ignored1, ignored2)
+
+	l := newQueryLog(Config{
+		Enabled:     true,
+		RotationIvl: timeutil.Day,
+		MemSize:     100,
+		BaseDir:     t.TempDir(),
+		Ignored:     set,
+	})
+
+	testCases := []struct {
+		name    string
+		host    string
+		wantLog bool
+	}{{
+		name:    "log",
+		host:    "example.com",
+		wantLog: true,
+	}, {
+		name:    "no_log_ignored_1",
+		host:    ignored1,
+		wantLog: false,
+	}, {
+		name:    "no_log_ignored_2",
+		host:    ignored2,
+		wantLog: false,
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			res := l.ShouldLog(tc.host, dns.TypeA, dns.ClassINET)
+
+			assert.Equal(t, tc.wantLog, res)
+		})
+	}
+}
+
 func addEntry(l *queryLog, host string, answerStr, client net.IP) {
 	q := dns.Msg{
 		Question: []dns.Question{{

internal/querylog/querylog.go

@@ -10,6 +10,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/stringutil"
 	"github.com/AdguardTeam/golibs/timeutil"
 	"github.com/miekg/dns"
 )
@@ -26,6 +27,9 @@ type QueryLog interface {
 
 	// WriteDiskConfig - write configuration
 	WriteDiskConfig(c *Config)
+
+	// ShouldLog returns true if request for the host should be logged.
+	ShouldLog(host string, qType, qClass uint16) bool
 }
 
 // Config is the query log configuration structure.
@@ -71,6 +75,10 @@ type Config struct {
 	// AnonymizeClientIP tells if the query log should anonymize clients' IP
 	// addresses.
 	AnonymizeClientIP bool
+
+	// Ignored is the list of host names, which are should not be written
+	// to log.
+	Ignored *stringutil.Set
 }
 
 // AddParams is the parameters for adding an entry.

internal/querylog/search.go

@@ -263,6 +263,10 @@ func (l *queryLog) readNextEntry(
 	e = &logEntry{}
 	decodeLogEntry(e, line)
 
+	if l.isIgnored(e.QHost) {
+		return nil, ts, nil
+	}
+
 	e.client, err = l.client(e.ClientID, e.IP.String(), cache)
 	if err != nil {
 		log.Error(