Pull request 1736: 4299-querylog-stats-api

Merge in DNS/adguard-home from 4299-querylog-stats-api to master Updates #1717. Updates #4299. Squashed commit of the following: commit 5b706b7997a536bc4fd2c532fb89ca5ab3536848 Merge: 48b62b0f 306c1983 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Mar 22 13:53:09 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit 48b62b0f1882f1ad120c6cdd90cd7dd8cb8a7738 Author: Vladislav Abdulmyanov <v.abdulmyanov@adguard.com> Date: Wed Mar 22 12:25:04 2023 +0200 client: fix styles, add titles and descrs commit 97e31cff70d05b51bd0e5ea2d20e8e7a251a7e41 Author: Vladislav Abdulmyanov <v.abdulmyanov@adguard.com> Date: Tue Mar 21 18:38:12 2023 +0200 client: add ignored domains for querylog commit 24d75c4376382205ae6b8f731b1cd23d517772c9 Author: Vladislav Abdulmyanov <v.abdulmyanov@adguard.com> Date: Tue Mar 21 18:21:13 2023 +0200 client: add ignore domains for stats commit eefc3891d01f90af79fdac9ba8eea06d4d54a0bc Merge: 978675ea 1daabb97 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Tue Mar 21 10:53:35 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit 978675ea2c07bf248b4c8f26ebdf78cf59a12ef5 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Tue Mar 21 10:53:11 2023 +0300 openapi: fix chlog commit 2ed33007aade115d38b0ca582206cc10678b084c Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Mar 20 17:49:07 2023 +0300 home: fix tests commit 6af11520c164553ee9fce8f214ea169672188d7e Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Mar 20 17:40:16 2023 +0300 home: fix typo commit 56acdfde5b1ee8d16b232c1293b91affbe319ad1 Merge: 319da34d 48431f8b Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Mar 20 17:32:58 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit 319da34de41ec84310b23bba2ad79c8a3a4c14ff Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Fri Mar 3 17:34:38 2023 +0300 querylog: fix docs commit d5a8f24d5b336e7bdbbca18069f6ede8c96bcc2c Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Fri Mar 3 11:42:00 2023 +0300 stats: fix docs commit e0cbfc1c4078180a05835ce7587e9f45484adc81 Merge: 4743c810 012e5beb Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Mar 1 18:45:17 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit 4743c81038052b9e0ca29ae5f1565021d36ca1ef Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Mar 1 18:14:16 2023 +0300 all: imp code; fix time conversion commit 34310cffd7e331d098c535590245387051674fa8 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Mar 1 12:34:11 2023 +0300 chlog: restore order commit cadd864a66655242948f1cb16e6d4945c0235d7e Merge: 2f3e25be bb226434 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Mar 1 12:26:06 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit 2f3e25bee56d2c6ddcf4aa2fc6a1dc51ed9b06e1 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Mar 1 12:25:14 2023 +0300 all: fix fmt commit d54022baa6c8a3d0d3c308a9b6b1a6a9dc6ac7b6 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Tue Feb 28 16:16:40 2023 +0300 all: imp code; fix chlog commit df22de91f59a51194c55e7bcbe5bc3fcc60cb8e3 Merge: e1ea4797 a772212d Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Feb 27 17:24:09 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit e1ea4797af974c36f06683ffc6eaaae917921a43 Merge: d7db0a5a bb80a7c2 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Feb 27 17:23:20 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit d7db0a5af1e1f49f6174c1c42e6d9306f2381d16 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Feb 27 17:12:20 2023 +0300 all: imp docs ... and 15 more commits
2023-03-23 13:46:57 +03:00
parent 306c1983a2
commit 143616ca6e
29 changed files with 995 additions and 157 deletions
--- a/internal/stats/http.go
+++ b/internal/stats/http.go
@@ -7,8 +7,12 @@ import (
 	"net/http"
 	"time"

+	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
+	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
 	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/timeutil"
+	"golang.org/x/exp/slices"
 )

 // topAddrs is an alias for the types of the TopFoo fields of statsResponse.
@@ -44,7 +48,7 @@ func (s *StatsCtx) handleStats(w http.ResponseWriter, r *http.Request) {
 	defer s.lock.Unlock()

 	start := time.Now()
-	resp, ok := s.getData(s.limitHours)
+	resp, ok := s.getData(uint32(s.limit.Hours()))
 	log.Debug("stats: prepared data in %v", time.Since(start))

 	if !ok {
@@ -63,20 +67,62 @@ type configResp struct {
 	IntervalDays uint32 `json:"interval"`
 }

+// getConfigResp is the response to the GET /control/stats_info.
+type getConfigResp struct {
+	// Ignored is the list of host names, which should not be counted.
+	Ignored []string `json:"ignored"`
+
+	// Interval is the statistics rotation interval in milliseconds.
+	Interval float64 `json:"interval"`
+
+	// Enabled shows if statistics are enabled.  It is an aghalg.NullBool to be
+	// able to tell when it's set without using pointers.
+	Enabled aghalg.NullBool `json:"enabled"`
+}
+
 // handleStatsInfo handles requests to the GET /control/stats_info endpoint.
+//
+// Deprecated:  Remove it when migration to the new API is over.
 func (s *StatsCtx) handleStatsInfo(w http.ResponseWriter, r *http.Request) {
 	s.lock.Lock()
 	defer s.lock.Unlock()

-	resp := configResp{IntervalDays: s.limitHours / 24}
+	days := uint32(s.limit / timeutil.Day)
+	ok := checkInterval(days)
+	if !ok || (s.enabled && days == 0) {
+		// NOTE: If interval is custom we set it to 90 days for compatibility
+		// with old API.
+		days = 90
+	}
+
+	resp := configResp{IntervalDays: days}
 	if !s.enabled {
 		resp.IntervalDays = 0
 	}
 	_ = aghhttp.WriteJSONResponse(w, r, resp)
 }

+// handleGetStatsConfig handles requests to the GET /control/stats/config
+// endpoint.
+func (s *StatsCtx) handleGetStatsConfig(w http.ResponseWriter, r *http.Request) {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	ignored := s.ignored.Values()
+	slices.Sort(ignored)
+
+	resp := getConfigResp{
+		Ignored:  ignored,
+		Interval: float64(s.limit.Milliseconds()),
+		Enabled:  aghalg.BoolToNullBool(s.enabled),
+	}
+	_ = aghhttp.WriteJSONResponse(w, r, resp)
+}
+
 // handleStatsConfig handles requests to the POST /control/stats_config
 // endpoint.
+//
+// Deprecated:  Remove it when migration to the new API is over.
 func (s *StatsCtx) handleStatsConfig(w http.ResponseWriter, r *http.Request) {
 	reqData := configResp{}
 	err := json.NewDecoder(r.Body).Decode(&reqData)
@@ -92,8 +138,55 @@ func (s *StatsCtx) handleStatsConfig(w http.ResponseWriter, r *http.Request) {
 		return
 	}

-	s.setLimit(int(reqData.IntervalDays))
-	s.configModified()
+	defer s.configModified()
+
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	limit := time.Duration(reqData.IntervalDays) * timeutil.Day
+	s.setLimit(limit)
+}
+
+// handlePutStatsConfig handles requests to the PUT /control/stats/config/update
+// endpoint.
+func (s *StatsCtx) handlePutStatsConfig(w http.ResponseWriter, r *http.Request) {
+	reqData := getConfigResp{}
+	err := json.NewDecoder(r.Body).Decode(&reqData)
+	if err != nil {
+		aghhttp.Error(r, w, http.StatusBadRequest, "json decode: %s", err)
+
+		return
+	}
+
+	set, err := aghnet.NewDomainNameSet(reqData.Ignored)
+	if err != nil {
+		aghhttp.Error(r, w, http.StatusUnprocessableEntity, "ignored: %s", err)
+
+		return
+	}
+
+	ivl := time.Duration(reqData.Interval) * time.Millisecond
+	err = validateIvl(ivl)
+	if err != nil {
+		aghhttp.Error(r, w, http.StatusUnprocessableEntity, "unsupported interval: %s", err)
+
+		return
+	}
+
+	if reqData.Enabled == aghalg.NBNull {
+		aghhttp.Error(r, w, http.StatusUnprocessableEntity, "enabled is null")
+
+		return
+	}
+
+	defer s.configModified()
+
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	s.ignored = set
+	s.limit = ivl
+	s.enabled = reqData.Enabled == aghalg.NBTrue
 }

 // handleStatsReset handles requests to the POST /control/stats_reset endpoint.
@@ -114,4 +207,7 @@ func (s *StatsCtx) initWeb() {
 	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)
+
+	s.httpRegister(http.MethodGet, "/control/stats/config", s.handleGetStatsConfig)
+	s.httpRegister(http.MethodPut, "/control/stats/config/update", s.handlePutStatsConfig)
 }
--- a/internal/stats/http_test.go
+++ b/internal/stats/http_test.go
@@ -0,0 +1,152 @@
+package stats
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
+	"github.com/AdguardTeam/golibs/testutil"
+	"github.com/AdguardTeam/golibs/timeutil"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestHandleStatsConfig(t *testing.T) {
+	const (
+		smallIvl = 1 * time.Minute
+		minIvl   = 1 * time.Hour
+		maxIvl   = 365 * timeutil.Day
+	)
+
+	conf := Config{
+		Filename:       filepath.Join(t.TempDir(), "stats.db"),
+		Limit:          time.Hour * 24,
+		Enabled:        true,
+		UnitID:         func() (id uint32) { return 0 },
+		ConfigModified: func() {},
+	}
+
+	testCases := []struct {
+		name     string
+		body     getConfigResp
+		wantCode int
+		wantErr  string
+	}{{
+		name: "set_ivl_1_minIvl",
+		body: getConfigResp{
+			Enabled:  aghalg.NBTrue,
+			Interval: float64(minIvl.Milliseconds()),
+			Ignored:  []string{},
+		},
+		wantCode: http.StatusOK,
+		wantErr:  "",
+	}, {
+		name: "small_interval",
+		body: getConfigResp{
+			Enabled:  aghalg.NBTrue,
+			Interval: float64(smallIvl.Milliseconds()),
+			Ignored:  []string{},
+		},
+		wantCode: http.StatusUnprocessableEntity,
+		wantErr:  "unsupported interval: less than an hour\n",
+	}, {
+		name: "big_interval",
+		body: getConfigResp{
+			Enabled:  aghalg.NBTrue,
+			Interval: float64(maxIvl.Milliseconds() + minIvl.Milliseconds()),
+			Ignored:  []string{},
+		},
+		wantCode: http.StatusUnprocessableEntity,
+		wantErr:  "unsupported interval: more than a year\n",
+	}, {
+		name: "set_ignored_ivl_1_maxIvl",
+		body: getConfigResp{
+			Enabled:  aghalg.NBTrue,
+			Interval: float64(maxIvl.Milliseconds()),
+			Ignored: []string{
+				"ignor.ed",
+			},
+		},
+		wantCode: http.StatusOK,
+		wantErr:  "",
+	}, {
+		name: "ignored_duplicate",
+		body: getConfigResp{
+			Enabled:  aghalg.NBTrue,
+			Interval: float64(minIvl.Milliseconds()),
+			Ignored: []string{
+				"ignor.ed",
+				"ignor.ed",
+			},
+		},
+		wantCode: http.StatusUnprocessableEntity,
+		wantErr:  "ignored: duplicate host name \"ignor.ed\" at index 1\n",
+	}, {
+		name: "ignored_empty",
+		body: getConfigResp{
+			Enabled:  aghalg.NBTrue,
+			Interval: float64(minIvl.Milliseconds()),
+			Ignored: []string{
+				"",
+			},
+		},
+		wantCode: http.StatusUnprocessableEntity,
+		wantErr:  "ignored: host name is empty\n",
+	}, {
+		name: "enabled_is_null",
+		body: getConfigResp{
+			Enabled:  aghalg.NBNull,
+			Interval: float64(minIvl.Milliseconds()),
+			Ignored:  []string{},
+		},
+		wantCode: http.StatusUnprocessableEntity,
+		wantErr:  "enabled is null\n",
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			s, err := New(conf)
+			require.NoError(t, err)
+
+			s.Start()
+			testutil.CleanupAndRequireSuccess(t, s.Close)
+
+			buf, err := json.Marshal(tc.body)
+			require.NoError(t, err)
+
+			const (
+				configGet = "/control/stats/config"
+				configPut = "/control/stats/config/update"
+			)
+
+			req := httptest.NewRequest(http.MethodPut, configPut, bytes.NewReader(buf))
+			rw := httptest.NewRecorder()
+
+			s.handlePutStatsConfig(rw, req)
+			require.Equal(t, tc.wantCode, rw.Code)
+
+			if tc.wantCode != http.StatusOK {
+				assert.Equal(t, tc.wantErr, rw.Body.String())
+
+				return
+			}
+
+			resp := httptest.NewRequest(http.MethodGet, configGet, nil)
+			rw = httptest.NewRecorder()
+
+			s.handleGetStatsConfig(rw, resp)
+			require.Equal(t, http.StatusOK, rw.Code)
+
+			ans := getConfigResp{}
+			err = json.Unmarshal(rw.Body.Bytes(), &ans)
+			require.NoError(t, err)
+
+			assert.Equal(t, tc.body, ans)
+		})
+	}
+}
--- a/internal/stats/stats.go
+++ b/internal/stats/stats.go
@@ -16,6 +16,7 @@ import (
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/AdguardTeam/golibs/stringutil"
+	"github.com/AdguardTeam/golibs/timeutil"
 	"go.etcd.io/bbolt"
 )

@@ -25,6 +26,20 @@ func checkInterval(days uint32) (ok bool) {
 	return days == 0 || days == 1 || days == 7 || days == 30 || days == 90
 }

+// validateIvl returns an error if ivl is less than an hour or more than a
+// year.
+func validateIvl(ivl time.Duration) (err error) {
+	if ivl < time.Hour {
+		return errors.Error("less than an hour")
+	}
+
+	if ivl > timeutil.Day*365 {
+		return errors.Error("more than a year")
+	}
+
+	return nil
+}
+
 // Config is the configuration structure for the statistics collecting.
 type Config struct {
 	// UnitID is the function to generate the identifier for current unit.  If
@@ -42,9 +57,8 @@ type Config struct {
 	// Filename is the name of the database file.
 	Filename string

-	// LimitDays is the maximum number of days to collect statistics into the
-	// current unit.
-	LimitDays uint32
+	// Limit is an upper limit for collecting statistics.
+	Limit time.Duration

 	// Enabled tells if the statistics are enabled.
 	Enabled bool
@@ -105,11 +119,8 @@ type StatsCtx struct {
 	// enabled tells if the statistics are enabled.
 	enabled bool

-	// limitHours is the maximum number of hours to collect statistics into the
-	// current unit.
-	//
-	// TODO(s.chzhen):  Rewrite to use time.Duration.
-	limitHours uint32
+	// limit is an upper limit for collecting statistics.
+	limit time.Duration

 	// ignored is the list of host names, which should not be counted.
 	ignored *stringutil.Set
@@ -128,9 +139,14 @@ func New(conf Config) (s *StatsCtx, err error) {
 		httpRegister:   conf.HTTPRegister,
 		ignored:        conf.Ignored,
 	}
-	if s.limitHours = conf.LimitDays * 24; !checkInterval(conf.LimitDays) {
-		s.limitHours = 24
+
+	err = validateIvl(conf.Limit)
+	if err != nil {
+		return nil, fmt.Errorf("unsupported interval: %w", err)
 	}
+
+	s.limit = conf.Limit
+
 	if s.unitIDGen = newUnitID; conf.UnitID != nil {
 		s.unitIDGen = conf.UnitID
 	}
@@ -150,7 +166,7 @@ func New(conf Config) (s *StatsCtx, err error) {
 		return nil, fmt.Errorf("stats: opening a transaction: %w", err)
 	}

-	deleted := deleteOldUnits(tx, id-s.limitHours-1)
+	deleted := deleteOldUnits(tx, id-uint32(s.limit.Hours())-1)
 	udb = loadUnitFromDB(tx, id)

 	err = finishTxn(tx, deleted > 0)
@@ -231,7 +247,7 @@ func (s *StatsCtx) Update(e Entry) {
 	s.lock.Lock()
 	defer s.lock.Unlock()

-	if !s.enabled || s.limitHours == 0 {
+	if !s.enabled || s.limit == 0 {
 		return
 	}

@@ -263,7 +279,7 @@ func (s *StatsCtx) WriteDiskConfig(dc *Config) {
 	s.lock.Lock()
 	defer s.lock.Unlock()

-	dc.LimitDays = s.limitHours / 24
+	dc.Limit = s.limit
 	dc.Enabled = s.enabled
 	dc.Ignored = s.ignored
 }
@@ -273,7 +289,7 @@ func (s *StatsCtx) TopClientsIP(maxCount uint) (ips []netip.Addr) {
 	s.lock.Lock()
 	defer s.lock.Unlock()

-	limit := s.limitHours
+	limit := uint32(s.limit.Hours())
 	if !s.enabled || limit == 0 {
 		return nil
 	}
@@ -377,7 +393,7 @@ func (s *StatsCtx) flush() (cont bool, sleepFor time.Duration) {
 		return false, 0
 	}

-	limit := s.limitHours
+	limit := uint32(s.limit.Hours())
 	if limit == 0 || ptr.id == id {
 		return true, time.Second
 	}
@@ -436,14 +452,14 @@ func (s *StatsCtx) periodicFlush() {
 	log.Debug("periodic flushing finished")
 }

-func (s *StatsCtx) setLimit(limitDays int) {
-	s.lock.Lock()
-	defer s.lock.Unlock()
-
-	if limitDays != 0 {
+// setLimit sets the limit.  s.lock is expected to be locked.
+//
+// TODO(s.chzhen):  Remove it when migration to the new API is over.
+func (s *StatsCtx) setLimit(limit time.Duration) {
+	if limit != 0 {
 		s.enabled = true
-		s.limitHours = uint32(24 * limitDays)
-		log.Debug("stats: set limit: %d days", limitDays)
+		s.limit = limit
+		log.Debug("stats: set limit: %d days", limit/timeutil.Day)

 		return
 	}
--- a/internal/stats/stats_internal_test.go
+++ b/internal/stats/stats_internal_test.go
@@ -9,6 +9,7 @@ import (
 	"time"

 	"github.com/AdguardTeam/golibs/testutil"
+	"github.com/AdguardTeam/golibs/timeutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -35,9 +36,9 @@ func TestStats_races(t *testing.T) {
 	var r uint32
 	idGen := func() (id uint32) { return atomic.LoadUint32(&r) }
 	conf := Config{
-		UnitID:    idGen,
-		Filename:  filepath.Join(t.TempDir(), "./stats.db"),
-		LimitDays: 1,
+		UnitID:   idGen,
+		Filename: filepath.Join(t.TempDir(), "./stats.db"),
+		Limit:    timeutil.Day,
 	}

 	s, err := New(conf)
--- a/internal/stats/stats_test.go
+++ b/internal/stats/stats_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/stats"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/testutil"
+	"github.com/AdguardTeam/golibs/timeutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -51,10 +52,10 @@ func TestStats(t *testing.T) {

 	handlers := map[string]http.Handler{}
 	conf := stats.Config{
-		Filename:  filepath.Join(t.TempDir(), "stats.db"),
-		LimitDays: 1,
-		Enabled:   true,
-		UnitID:    constUnitID,
+		Filename: filepath.Join(t.TempDir(), "stats.db"),
+		Limit:    timeutil.Day,
+		Enabled:  true,
+		UnitID:   constUnitID,
 		HTTPRegister: func(_, url string, handler http.HandlerFunc) {
 			handlers[url] = handler
 		},
@@ -158,7 +159,7 @@ func TestLargeNumbers(t *testing.T) {

 	conf := stats.Config{
 		Filename:     filepath.Join(t.TempDir(), "stats.db"),
-		LimitDays:    1,
+		Limit:        timeutil.Day,
 		Enabled:      true,
 		UnitID:       func() (id uint32) { return atomic.LoadUint32(&curHour) },
 		HTTPRegister: func(_, url string, handler http.HandlerFunc) { handlers[url] = handler },