From 549b20bdea3c3215bb355ffc39546f7bb3260d80 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Wed, 25 May 2022 18:00:50 +0300 Subject: [PATCH] Pull request: querylog: fix oldest calc Updates #4591. Squashed commit of the following: commit 70b70c78c85311363535536c7ea12336b21accf8 Author: Ainar Garipov Date: Wed May 25 17:35:54 2022 +0300 querylog: fix oldest calc --- CHANGELOG.md | 2 ++ internal/dnsforward/stats.go | 2 +- internal/home/dns.go | 2 +- internal/querylog/http.go | 2 +- internal/querylog/qlog.go | 2 +- internal/querylog/qlog_test.go | 2 +- internal/querylog/querylog.go | 18 +++++++++--------- internal/querylog/search.go | 17 ++++++++++++----- 8 files changed, 28 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0c32ce7..4a72447c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -129,6 +129,7 @@ In this release, the schema version has changed from 12 to 14. ### Fixed +- Query log occasionally going into an infinite loop ([#4591]). - Service startup on boot on systems using SysV-init ([#4480]). - Detection of the stopped service status on macOS and Linux ([#4273]). - Case-sensitive ClientID ([#4542]). @@ -157,6 +158,7 @@ In this release, the schema version has changed from 12 to 14. [#4503]: https://github.com/AdguardTeam/AdGuardHome/issues/4503 [#4533]: https://github.com/AdguardTeam/AdGuardHome/issues/4533 [#4542]: https://github.com/AdguardTeam/AdGuardHome/issues/4542 +[#4591]: https://github.com/AdguardTeam/AdGuardHome/issues/4591 [#4592]: https://github.com/AdguardTeam/AdGuardHome/issues/4592 [rfc-9250]: https://datatracker.ietf.org/doc/html/rfc9250 diff --git a/internal/dnsforward/stats.go b/internal/dnsforward/stats.go index 56cc19c5..9a7b1ddb 100644 --- a/internal/dnsforward/stats.go +++ b/internal/dnsforward/stats.go @@ -64,9 +64,9 @@ func (s *Server) logQuery( Answer: pctx.Res, OrigAnswer: dctx.origResp, Result: dctx.result, - Elapsed: elapsed, ClientID: dctx.clientID, ClientIP: ip, + Elapsed: elapsed, AuthenticatedData: dctx.responseAD, } diff --git a/internal/home/dns.go b/internal/home/dns.go index 1c04c6c3..9eabfefa 100644 --- a/internal/home/dns.go +++ b/internal/home/dns.go @@ -58,6 +58,7 @@ func initDNSServer() (err error) { } conf := querylog.Config{ + Anonymizer: anonymizer, ConfigModified: onConfigModified, HTTPRegister: httpRegister, FindClient: Context.clients.findMultiple, @@ -67,7 +68,6 @@ func initDNSServer() (err error) { Enabled: config.DNS.QueryLogEnabled, FileEnabled: config.DNS.QueryLogFileEnabled, AnonymizeClientIP: config.DNS.AnonymizeClientIP, - Anonymizer: anonymizer, } Context.queryLog = querylog.New(conf) diff --git a/internal/querylog/http.go b/internal/querylog/http.go index 6a2bdcee..11f62d0d 100644 --- a/internal/querylog/http.go +++ b/internal/querylog/http.go @@ -19,10 +19,10 @@ import ( ) type qlogConfig struct { - Enabled bool `json:"enabled"` // Use float64 here to support fractional numbers and not mess the API // users by changing the units. Interval float64 `json:"interval"` + Enabled bool `json:"enabled"` AnonymizeClientIP bool `json:"anonymize_client_ip"` } diff --git a/internal/querylog/qlog.go b/internal/querylog/qlog.go index 8856fd9c..24eec40e 100644 --- a/internal/querylog/qlog.go +++ b/internal/querylog/qlog.go @@ -149,7 +149,7 @@ func (l *queryLog) clear() { log.Error("removing log file %q: %s", l.logFile, err) } - log.Debug("Query log: cleared") + log.Debug("querylog: cleared") } func (l *queryLog) Add(params *AddParams) { diff --git a/internal/querylog/qlog_test.go b/internal/querylog/qlog_test.go index fbfc459d..6beed1be 100644 --- a/internal/querylog/qlog_test.go +++ b/internal/querylog/qlog_test.go @@ -285,8 +285,8 @@ func addEntry(l *queryLog, host string, answerStr, client net.IP) { Answer: &a, OrigAnswer: &a, Result: &res, - ClientIP: client, Upstream: "upstream", + ClientIP: client, } l.Add(params) diff --git a/internal/querylog/querylog.go b/internal/querylog/querylog.go index bd6e1569..a854c2c4 100644 --- a/internal/querylog/querylog.go +++ b/internal/querylog/querylog.go @@ -28,8 +28,11 @@ type QueryLog interface { WriteDiskConfig(c *Config) } -// Config - configuration object +// Config is the query log configuration structure. type Config struct { + // Anonymizer processes the IP addresses to anonymize those if needed. + Anonymizer *aghnet.IPMut + // ConfigModified is called when the configuration is changed, for // example by HTTP requests. ConfigModified func() @@ -68,9 +71,6 @@ type Config struct { // AnonymizeClientIP tells if the query log should anonymize clients' IP // addresses. AnonymizeClientIP bool - - // Anonymizer processes the IP addresses to anonymize those if needed. - Anonymizer *aghnet.IPMut } // AddParams is the parameters for adding an entry. @@ -91,18 +91,18 @@ type AddParams struct { // Result is the filtering result (optional). Result *filtering.Result - // Elapsed is the time spent for processing the request. - Elapsed time.Duration - ClientID string - ClientIP net.IP - // Upstream is the URL of the upstream DNS server. Upstream string ClientProto ClientProto + ClientIP net.IP + + // Elapsed is the time spent for processing the request. + Elapsed time.Duration + // Cached indicates if the response is served from cache. Cached bool diff --git a/internal/querylog/search.go b/internal/querylog/search.go index 4a3de979..8fb32e60 100644 --- a/internal/querylog/search.go +++ b/internal/querylog/search.go @@ -73,7 +73,7 @@ func (l *queryLog) searchMemory(params *searchParams, cache clientCache) (entrie // search - searches log entries in the query log using specified parameters // returns the list of entries found + time of the oldest entry -func (l *queryLog) search(params *searchParams) ([]*logEntry, time.Time) { +func (l *queryLog) search(params *searchParams) (entries []*logEntry, oldest time.Time) { now := time.Now() if params.limit == 0 { @@ -88,7 +88,7 @@ func (l *queryLog) search(params *searchParams) ([]*logEntry, time.Time) { totalLimit := params.offset + params.limit // now let's get a unified collection - entries := append(memoryEntries, fileEntries...) + entries = append(memoryEntries, fileEntries...) if len(entries) > totalLimit { // remove extra records entries = entries[:totalLimit] @@ -111,13 +111,18 @@ func (l *queryLog) search(params *searchParams) ([]*logEntry, time.Time) { } } - if len(entries) > 0 && len(entries) <= totalLimit { + if len(entries) > 0 { // Update oldest after merging in the memory buffer. oldest = entries[len(entries)-1].Time } - log.Debug("QueryLog: prepared data (%d/%d) older than %s in %s", - len(entries), total, params.olderThan, time.Since(now)) + log.Debug( + "querylog: prepared data (%d/%d) older than %s in %s", + len(entries), + total, + params.olderThan, + time.Since(now), + ) return entries, oldest } @@ -180,6 +185,8 @@ func (l *queryLog) searchFiles( e, ts, err = l.readNextEntry(r, params, cache) if err != nil { if err == io.EOF { + oldestNano = 0 + break }