cherry-pick: 4358 fix stats

Merge in DNS/adguard-home from 4358-fix-stats to master Updates #4358. Updates #4342. Squashed commit of the following: commit 5683cb3 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Thu Aug 4 18:20:54 2022 +0300 stats: rm races test commit 63dd676 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Thu Aug 4 17:13:36 2022 +0300 stats: try to imp test commit 59a0f24 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Thu Aug 4 16:38:57 2022 +0300 stats: fix nil ptr deref commit 7fc3ff1 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Thu Apr 7 16:02:51 2022 +0300 stats: fix races finally, imp tests commit c63f5f4 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Thu Aug 4 00:56:49 2022 +0300 aghhttp: add register func commit 61adc7f Merge: edbdb2d 9b3adac Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Thu Aug 4 00:36:01 2022 +0300 Merge branch 'master' into 4358-fix-stats commit edbdb2d Merge: a91e4d7 a481ff4 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Wed Aug 3 21:00:42 2022 +0300 Merge branch 'master' into 4358-fix-stats commit a91e4d7 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Wed Aug 3 18:46:19 2022 +0300 stats: imp code, docs commit c5f3814 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Wed Aug 3 18:16:13 2022 +0300 all: log changes commit 5e6caaf Merge: 091ba75 eb8e816 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Wed Aug 3 18:09:10 2022 +0300 Merge branch 'master' into 4358-fix-stats commit 091ba75 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Wed Aug 3 18:07:39 2022 +0300 stats: imp docs, code commit f2b2de7 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Tue Aug 2 17:09:30 2022 +0300 all: refactor stats & add mutexes commit b3f11c4 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Wed Apr 27 15:30:09 2022 +0300 WIP
AdguardTeam · Aug 17, 2022 · 39b404b · 39b404b
1 parent 56dc3ea
commit 39b404b
Show file tree

Hide file tree

Showing 15 changed files with 434 additions and 300 deletions.
diff --git a/internal/aghhttp/aghhttp.go b/internal/aghhttp/aghhttp.go
@@ -9,6 +9,12 @@ import (
 	"github.com/AdguardTeam/golibs/log"
 )
 
+// RegisterFunc is the function that sets the handler to handle the URL for the
+// method.
+//
+// TODO(e.burkov, a.garipov):  Get rid of it.
+type RegisterFunc func(method, url string, handler http.HandlerFunc)
+
 // OK responds with word OK.
 func OK(w http.ResponseWriter) {
 	if _, err := io.WriteString(w, "OK\n"); err != nil {

diff --git a/internal/dhcpd/dhcpd.go b/internal/dhcpd/dhcpd.go
@@ -5,11 +5,11 @@ import (
 	"encoding/json"
 	"fmt"
 	"net"
-	"net/http"
 	"path/filepath"
 	"runtime"
 	"time"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/AdguardTeam/golibs/netutil"
 )
@@ -126,7 +126,7 @@ type ServerConfig struct {
 	ConfigModified func() `yaml:"-"`
 
 	// Register an HTTP handler
-	HTTPRegister func(string, string, func(http.ResponseWriter, *http.Request)) `yaml:"-"`
+	HTTPRegister aghhttp.RegisterFunc `yaml:"-"`
 
 	Enabled       bool   `yaml:"enabled"`
 	InterfaceName string `yaml:"interface_name"`

diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go
@@ -5,12 +5,12 @@ import (
 	"crypto/x509"
 	"fmt"
 	"net"
-	"net/http"
 	"os"
 	"sort"
 	"strings"
 	"time"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
 	"github.com/AdguardTeam/dnsproxy/proxy"
 	"github.com/AdguardTeam/dnsproxy/upstream"
@@ -191,7 +191,7 @@ type ServerConfig struct {
 	ConfigModified func()
 
 	// Register an HTTP handler
-	HTTPRegister func(string, string, func(http.ResponseWriter, *http.Request))
+	HTTPRegister aghhttp.RegisterFunc
 
 	// ResolveClients signals if the RDNS should resolve clients' addresses.
 	ResolveClients bool

diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go
@@ -61,7 +61,7 @@ type Server struct {
 	dnsFilter  *filtering.DNSFilter  // DNS filter instance
 	dhcpServer dhcpd.ServerInterface // DHCP server instance (optional)
 	queryLog   querylog.QueryLog     // Query log instance
-	stats      stats.Stats
+	stats      stats.Interface
 	access     *accessCtx
 
 	// localDomainSuffix is the suffix used to detect internal hosts.  It
@@ -107,7 +107,7 @@ const defaultLocalDomainSuffix = "lan"
 // DNSCreateParams are parameters to create a new server.
 type DNSCreateParams struct {
 	DNSFilter   *filtering.DNSFilter
-	Stats       stats.Stats
+	Stats       stats.Interface
 	QueryLog    querylog.QueryLog
 	DHCPServer  dhcpd.ServerInterface
 	PrivateNets netutil.SubnetSet

diff --git a/internal/dnsforward/stats_test.go b/internal/dnsforward/stats_test.go
@@ -34,7 +34,7 @@ func (l *testQueryLog) Add(p *querylog.AddParams) {
 type testStats struct {
 	// Stats is embedded here simply to make testStats a stats.Stats without
 	// actually implementing all methods.
-	stats.Stats
+	stats.Interface
 
 	lastEntry stats.Entry
 }

diff --git a/internal/filtering/filtering.go b/internal/filtering/filtering.go
@@ -6,14 +6,14 @@ import (
 	"fmt"
 	"io/fs"
 	"net"
-	"net/http"
 	"os"
 	"runtime"
 	"runtime/debug"
 	"strings"
 	"sync"
 	"sync/atomic"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
 	"github.com/AdguardTeam/dnsproxy/upstream"
 	"github.com/AdguardTeam/golibs/cache"
@@ -94,7 +94,7 @@ type Config struct {
 	ConfigModified func() `yaml:"-"`
 
 	// Register an HTTP handler
-	HTTPRegister func(string, string, func(http.ResponseWriter, *http.Request)) `yaml:"-"`
+	HTTPRegister aghhttp.RegisterFunc `yaml:"-"`
 
 	// CustomResolver is the resolver used by DNSFilter.
 	CustomResolver Resolver `yaml:"-"`

diff --git a/internal/home/clients.go b/internal/home/clients.go
@@ -2,6 +2,7 @@ package home
 
 import (
 	"bytes"
+	"encoding"
 	"fmt"
 	"net"
 	"sort"
@@ -60,6 +61,33 @@ const (
 	ClientSourceHostsFile
 )
 
+var _ fmt.Stringer = clientSource(0)
+
+// String returns a human-readable name of cs.
+func (cs clientSource) String() (s string) {
+	switch cs {
+	case ClientSourceWHOIS:
+		return "WHOIS"
+	case ClientSourceARP:
+		return "ARP"
+	case ClientSourceRDNS:
+		return "rDNS"
+	case ClientSourceDHCP:
+		return "DHCP"
+	case ClientSourceHostsFile:
+		return "etc/hosts"
+	default:
+		return ""
+	}
+}
+
+var _ encoding.TextMarshaler = clientSource(0)
+
+// MarshalText implements encoding.TextMarshaler for the clientSource.
+func (cs clientSource) MarshalText() (text []byte, err error) {
+	return []byte(cs.String()), nil
+}
+
 // clientSourceConf is used to configure where the runtime clients will be
 // obtained from.
 type clientSourcesConf struct {
@@ -397,6 +425,7 @@ func (clients *clientsContainer) Find(id string) (c *Client, ok bool) {
 	c.Tags = stringutil.CloneSlice(c.Tags)
 	c.BlockedServices = stringutil.CloneSlice(c.BlockedServices)
 	c.Upstreams = stringutil.CloneSlice(c.Upstreams)
+
 	return c, true
 }
 

diff --git a/internal/home/clientshttp.go b/internal/home/clientshttp.go
@@ -47,9 +47,9 @@ type clientJSON struct {
 type runtimeClientJSON struct {
 	WHOISInfo *RuntimeClientWHOISInfo `json:"whois_info"`
 
-	Name   string `json:"name"`
-	Source string `json:"source"`
-	IP     net.IP `json:"ip"`
+	Name   string       `json:"name"`
+	Source clientSource `json:"source"`
+	IP     net.IP       `json:"ip"`
 }
 
 type clientListJSON struct {
@@ -81,20 +81,9 @@ func (clients *clientsContainer) handleGetClients(w http.ResponseWriter, r *http
 		cj := runtimeClientJSON{
 			WHOISInfo: rc.WHOISInfo,
 
-			Name: rc.Host,
-			IP:   ip,
-		}
-
-		cj.Source = "etc/hosts"
-		switch rc.Source {
-		case ClientSourceDHCP:
-			cj.Source = "DHCP"
-		case ClientSourceRDNS:
-			cj.Source = "rDNS"
-		case ClientSourceARP:
-			cj.Source = "ARP"
-		case ClientSourceWHOIS:
-			cj.Source = "WHOIS"
+			Name:   rc.Host,
+			Source: rc.Source,
+			IP:     ip,
 		}
 
 		data.RuntimeClients = append(data.RuntimeClients, cj)
@@ -107,13 +96,7 @@ func (clients *clientsContainer) handleGetClients(w http.ResponseWriter, r *http
 	w.Header().Set("Content-Type", "application/json")
 	e := json.NewEncoder(w).Encode(data)
 	if e != nil {
-		aghhttp.Error(
-			r,
-			w,
-			http.StatusInternalServerError,
-			"Failed to encode to json: %v",
-			e,
-		)
+		aghhttp.Error(r, w, http.StatusInternalServerError, "failed to encode to json: %v", e)
 
 		return
 	}
@@ -279,9 +262,9 @@ func (clients *clientsContainer) handleFindClient(w http.ResponseWriter, r *http
 func (clients *clientsContainer) findRuntime(ip net.IP, idStr string) (cj *clientJSON) {
 	rc, ok := clients.FindRuntimeClient(ip)
 	if !ok {
-		// It is still possible that the IP used to be in the runtime
-		// clients list, but then the server was reloaded.  So, check
-		// the DNS server's blocked IP list.
+		// It is still possible that the IP used to be in the runtime clients
+		// list, but then the server was reloaded.  So, check the DNS server's
+		// blocked IP list.
 		//
 		// See https://github.com/AdguardTeam/AdGuardHome/issues/2428.
 		disallowed, rule := clients.dnsServer.IsBlockedClient(ip, idStr)

diff --git a/internal/home/control.go b/internal/home/control.go
@@ -189,7 +189,7 @@ func registerControlHandlers() {
 	RegisterAuthHandlers()
 }
 
-func httpRegister(method, url string, handler func(http.ResponseWriter, *http.Request)) {
+func httpRegister(method, url string, handler http.HandlerFunc) {
 	if method == "" {
 		// "/dns-query" handler doesn't need auth, gzip and isn't restricted by 1 HTTP method
 		Context.mux.HandleFunc(url, postInstall(handler))

diff --git a/internal/home/home.go b/internal/home/home.go
@@ -46,7 +46,7 @@ type homeContext struct {
 	// --
 
 	clients    clientsContainer     // per-client-settings module
-	stats      stats.Stats          // statistics module
+	stats      stats.Interface      // statistics module
 	queryLog   querylog.QueryLog    // query log module
 	dnsServer  *dnsforward.Server   // DNS module
 	rdns       *RDNS                // rDNS module

diff --git a/internal/querylog/querylog.go b/internal/querylog/querylog.go
@@ -2,10 +2,10 @@ package querylog
 
 import (
 	"net"
-	"net/http"
 	"path/filepath"
 	"time"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
 	"github.com/AdguardTeam/golibs/errors"
@@ -38,7 +38,7 @@ type Config struct {
 	ConfigModified func()
 
 	// HTTPRegister registers an HTTP handler.
-	HTTPRegister func(string, string, func(http.ResponseWriter, *http.Request))
+	HTTPRegister aghhttp.RegisterFunc
 
 	// FindClient returns client information by their IDs.
 	FindClient func(ids []string) (c *Client, err error)

diff --git a/internal/stats/http.go b/internal/stats/http.go
@@ -39,34 +39,21 @@ type statsResponse struct {
 }
 
 // handleStats is a handler for getting statistics.
-func (s *statsCtx) handleStats(w http.ResponseWriter, r *http.Request) {
+func (s *StatsCtx) handleStats(w http.ResponseWriter, r *http.Request) {
 	start := time.Now()
 
 	var resp statsResponse
-	if s.conf.limit == 0 {
-		resp = statsResponse{
-			TimeUnits: "days",
-
-			TopBlocked: []topAddrs{},
-			TopClients: []topAddrs{},
-			TopQueried: []topAddrs{},
-
-			BlockedFiltering:     []uint64{},
-			DNSQueries:           []uint64{},
-			ReplacedParental:     []uint64{},
-			ReplacedSafebrowsing: []uint64{},
-		}
-	} else {
-		var ok bool
-		resp, ok = s.getData()
-
-		log.Debug("stats: prepared data in %v", time.Since(start))
-
-		if !ok {
-			aghhttp.Error(r, w, http.StatusInternalServerError, "Couldn't get statistics data")
-
-			return
-		}
+	var ok bool
+	resp, ok = s.getData()
+
+	log.Debug("stats: prepared data in %v", time.Since(start))
+
+	if !ok {
+		// Don't bring the message to the lower case since it's a part of UI
+		// text for the moment.
+		aghhttp.Error(r, w, http.StatusInternalServerError, "Couldn't get statistics data")
+
+		return
 	}
 
 	w.Header().Set("Content-Type", "application/json")
@@ -84,9 +71,9 @@ type config struct {
 }
 
 // Get configuration
-func (s *statsCtx) handleStatsInfo(w http.ResponseWriter, r *http.Request) {
+func (s *StatsCtx) handleStatsInfo(w http.ResponseWriter, r *http.Request) {
 	resp := config{}
-	resp.IntervalDays = s.conf.limit / 24
+	resp.IntervalDays = s.limitHours / 24
 
 	data, err := json.Marshal(resp)
 	if err != nil {
@@ -102,7 +89,7 @@ func (s *statsCtx) handleStatsInfo(w http.ResponseWriter, r *http.Request) {
 }
 
 // Set configuration
-func (s *statsCtx) handleStatsConfig(w http.ResponseWriter, r *http.Request) {
+func (s *StatsCtx) handleStatsConfig(w http.ResponseWriter, r *http.Request) {
 	reqData := config{}
 	err := json.NewDecoder(r.Body).Decode(&reqData)
 	if err != nil {
@@ -118,22 +105,22 @@ func (s *statsCtx) handleStatsConfig(w http.ResponseWriter, r *http.Request) {
 	}
 
 	s.setLimit(int(reqData.IntervalDays))
-	s.conf.ConfigModified()
+	s.configModified()
 }
 
 // Reset data
-func (s *statsCtx) handleStatsReset(w http.ResponseWriter, r *http.Request) {
+func (s *StatsCtx) handleStatsReset(w http.ResponseWriter, r *http.Request) {
 	s.clear()
 }
 
 // Register web handlers
-func (s *statsCtx) initWeb() {
-	if s.conf.HTTPRegister == nil {
+func (s *StatsCtx) initWeb() {
+	if s.httpRegister == nil {
 		return
 	}
 
-	s.conf.HTTPRegister(http.MethodGet, "/control/stats", s.handleStats)
-	s.conf.HTTPRegister(http.MethodPost, "/control/stats_reset", s.handleStatsReset)
-	s.conf.HTTPRegister(http.MethodPost, "/control/stats_config", s.handleStatsConfig)
-	s.conf.HTTPRegister(http.MethodGet, "/control/stats_info", s.handleStatsInfo)
+	s.httpRegister(http.MethodGet, "/control/stats", s.handleStats)
+	s.httpRegister(http.MethodPost, "/control/stats_reset", s.handleStatsReset)
+	s.httpRegister(http.MethodPost, "/control/stats_config", s.handleStatsConfig)
+	s.httpRegister(http.MethodGet, "/control/stats_info", s.handleStatsInfo)
 }