Merge branch 'master' into AGDNS-3568-imp-fswatcher

Author: EugeneOne1

CHANGELOG.md

@@ -20,6 +20,8 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 ### Added
 
+- New query parameter `recent` in `GET /control/stats/` defines statistics lookback period in millieseconds.  See `openapi/openapi.yaml` for details.
+
 - New field `"ignored_enabled"` in `GetStatsConfigResponse` or `GetQueryLogConfigResponse`.  See `openapi/openapi.yaml` for details.
 
 ### Changed

internal/stats/http.go

@@ -4,15 +4,25 @@ package stats
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
+	"strconv"
 	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
 	"github.com/AdguardTeam/golibs/timeutil"
+	"github.com/AdguardTeam/golibs/validate"
 )
 
+// queryKeyRecent is the key of the query parameter that contains the lookback
+// interval for statistics.
+const queryKeyRecent = "recent"
+
+// millisecondsInHour contains number of milliseconds in one hour.
+const millisecondsInHour = int64(time.Hour / time.Millisecond)
+
 // topAddrs is an alias for the types of the TopFoo fields of statsResponse.
 // The key is either a client's address or a requested address.
 type topAddrs = map[string]uint64
@@ -53,17 +63,25 @@ func (s *StatsCtx) handleStats(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	l := s.logger
 
-	var (
-		resp *StatsResp
-		ok   bool
-	)
+	var limit time.Duration
 	func() {
 		s.confMu.RLock()
 		defer s.confMu.RUnlock()
 
-		resp, ok = s.getData(uint32(s.limit.Hours()))
+		limit = s.limit
 	}()
 
+	recent := r.URL.Query().Get(queryKeyRecent)
+
+	limit, err := parseRecent(recent, limit)
+	if err != nil {
+		aghhttp.ErrorAndLog(ctx, l, r, w, http.StatusBadRequest, "%s", err)
+
+		return
+	}
+
+	resp, ok := s.getData(uint32(limit.Hours()))
+
 	l.DebugContext(ctx, "prepared data", "elapsed", time.Since(start))
 
 	if !ok {
@@ -78,6 +96,31 @@ func (s *StatsCtx) handleStats(w http.ResponseWriter, r *http.Request) {
 	aghhttp.WriteJSONResponseOK(ctx, l, w, r, resp)
 }
 
+// parseRecent parses and validates the value of the recent URL parameter.  If
+// the parameter is empty, the original limit is returned.
+func parseRecent(recent string, limit time.Duration) (parsedLimit time.Duration, err error) {
+	if recent == "" {
+		return limit, nil
+	}
+
+	recentMs, err := strconv.ParseInt(recent, 10, 64)
+	if err != nil {
+		return 0, fmt.Errorf("%s: parsing interval: %s", queryKeyRecent, err)
+	}
+
+	err = validate.InRange(queryKeyRecent, recentMs, millisecondsInHour, limit.Milliseconds())
+	if err != nil {
+		// Don't wrap the error since it's already informative enough as is.
+		return 0, err
+	}
+
+	if recentMs%millisecondsInHour != 0 {
+		return 0, fmt.Errorf("%s: must be a multiple of 1 hour", queryKeyRecent)
+	}
+
+	return time.Duration(recentMs) * time.Millisecond, nil
+}
+
 // configResp is the response to the GET /control/stats_info.
 type configResp struct {
 	IntervalDays uint32 `json:"interval"`

internal/stats/http_internal_test.go

@@ -3,40 +3,33 @@ package stats
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
-	"path/filepath"
 	"testing"
 	"time"
 
-	"github.com/AdguardTeam/AdGuardHome/internal/agh"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
-	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
-	"github.com/AdguardTeam/golibs/logutil/slogutil"
+	"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"
 )
 
+// Common domain values for tests.
+const (
+	TestDomain1 = "example.com"
+	TestDomain2 = "example.org"
+)
+
 func TestHandleStatsConfig(t *testing.T) {
 	const (
 		smallIvl = 1 * time.Minute
 		minIvl   = 1 * time.Hour
 		maxIvl   = 365 * timeutil.Day
 	)
 
-	conf := Config{
-		Logger:            slogutil.NewDiscardLogger(),
-		UnitID:            func() (id uint32) { return 0 },
-		ConfigModifier:    agh.EmptyConfigModifier{},
-		ShouldCountClient: func([]string) bool { return true },
-		HTTPReg:           aghhttp.EmptyRegistrar{},
-		Filename:          filepath.Join(t.TempDir(), "stats.db"),
-		Limit:             time.Hour * 24,
-		Enabled:           true,
-	}
-
 	testCases := []struct {
 		name     string
 		wantErr  string
@@ -98,8 +91,7 @@ func TestHandleStatsConfig(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			s, err := New(conf)
-			require.NoError(t, err)
+			s := newTestStatsCtx(t, Config{Enabled: true})
 
 			s.Start()
 			testutil.CleanupAndRequireSuccess(t, s.Close)
@@ -138,3 +130,112 @@ func TestHandleStatsConfig(t *testing.T) {
 		})
 	}
 }
+
+// populateTestData is a helper that creates test entries in db.  s must not be
+// nil.
+func populateTestData(tb testing.TB, s *StatsCtx) {
+	tb.Helper()
+
+	oldUnitID := newUnitID() - 1
+	oldUnit := &unitDB{
+		NResult: make([]uint64, resultLast),
+		Domains: []countPair{{Name: TestDomain1, Count: 1}},
+		NTotal:  1,
+	}
+
+	db := s.db.Load()
+	tx, err := db.Begin(true)
+	require.NoError(tb, err)
+
+	err = s.flushUnitToDB(oldUnit, tx, uint32(oldUnitID))
+	require.NoError(tb, err)
+
+	err = finishTxn(tx, true)
+	require.NoError(tb, err)
+
+	s.Update(&Entry{
+		Client:         netutil.IPv4Localhost().String(),
+		Domain:         TestDomain2,
+		ProcessingTime: 3 * time.Minute,
+		Result:         RNotFiltered,
+	})
+}
+
+func TestStatsCtx_handleStats(t *testing.T) {
+	testCases := []struct {
+		name                  string
+		wantErr               string
+		wantTopQueriedDomains []topAddrs
+		wantDNSQueries        uint64
+		wantCode              int
+		recent                int64
+	}{{
+		name:     "short_interval",
+		wantErr:  "recent: out of range: must be no less than 3600000, got 240000\n",
+		wantCode: http.StatusBadRequest,
+		recent:   4 * time.Minute.Milliseconds(),
+	}, {
+		name:     "long_interval",
+		wantErr:  "recent: out of range: must be no greater than 86400000, got 259200000\n",
+		wantCode: http.StatusBadRequest,
+		recent:   72 * time.Hour.Milliseconds(),
+	}, {
+		name:     "interval_is_not_multiple_of_hour",
+		wantErr:  "recent: must be a multiple of 1 hour\n",
+		wantCode: http.StatusBadRequest,
+		recent:   time.Hour.Milliseconds() + 1,
+	}, {
+		name:           "no_interval",
+		wantCode:       http.StatusOK,
+		wantDNSQueries: 2,
+		wantTopQueriedDomains: []topAddrs{{
+			TestDomain1: 1,
+		}, {
+			TestDomain2: 1,
+		}},
+	}, {
+		name:           "valid_interval",
+		wantCode:       http.StatusOK,
+		wantDNSQueries: 1,
+		wantTopQueriedDomains: []topAddrs{{
+			TestDomain2: 1,
+		}},
+		recent: time.Hour.Milliseconds(),
+	}}
+
+	s := newTestStatsCtx(t, Config{
+		Enabled: true,
+	})
+
+	s.Start()
+	defer testutil.CleanupAndRequireSuccess(t, s.Close)
+
+	populateTestData(t, s)
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			url := "/control/stats"
+			if tc.recent != 0 {
+				url += fmt.Sprintf("?recent=%d", tc.recent)
+			}
+
+			req := httptest.NewRequest(http.MethodGet, url, nil)
+			rw := httptest.NewRecorder()
+
+			s.handleStats(rw, req)
+			require.Equal(t, tc.wantCode, rw.Code)
+
+			if rw.Code != http.StatusOK {
+				require.Equal(t, tc.wantErr, rw.Body.String())
+
+				return
+			}
+
+			ans := StatsResp{}
+			err := json.Unmarshal(rw.Body.Bytes(), &ans)
+			require.NoError(t, err)
+
+			assert.Equal(t, tc.wantDNSQueries, ans.NumDNSQueries)
+			assert.ElementsMatch(t, tc.wantTopQueriedDomains, ans.TopQueried)
+		})
+	}
+}

internal/stats/stats.go

@@ -72,6 +72,9 @@ type Config struct {
 	Ignored *aghnet.IgnoreEngine
 
 	// Filename is the name of the database file.
+	//
+	// TODO(f.setrakov): Move the work with DB into a separate entity with
+	// interface.
 	Filename string
 
 	// Limit is an upper limit for collecting statistics.

internal/stats/stats_internal_test.go

@@ -1,13 +1,15 @@
 package stats
 
 import (
+	"cmp"
 	"fmt"
 	"path/filepath"
 	"sync"
 	"sync/atomic"
 	"testing"
 	"time"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/agh"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
 	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/testutil"
@@ -16,20 +18,39 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// testLogger is the common logger for tests.
+var testLogger = slogutil.NewDiscardLogger()
+
+// newTestStatsCtx returns StatsCtx initialised with given values.  All empty
+// values from c will be replaced with defaults.
+func newTestStatsCtx(tb testing.TB, c Config) (s *StatsCtx) {
+	c.Logger = cmp.Or(c.Logger, testLogger)
+	c.ConfigModifier = cmp.Or[agh.ConfigModifier](c.ConfigModifier, agh.EmptyConfigModifier{})
+	c.HTTPReg = cmp.Or[aghhttp.Registrar](c.HTTPReg, aghhttp.EmptyRegistrar{})
+	c.Filename = cmp.Or(c.Filename, filepath.Join(tb.TempDir(), "./stats.db"))
+	c.Limit = cmp.Or(c.Limit, timeutil.Day)
+	if c.ShouldCountClient == nil {
+		c.ShouldCountClient = func([]string) bool { return true }
+	}
+
+	if c.UnitID == nil {
+		c.UnitID = newUnitID
+	}
+
+	var err error
+	s, err = New(c)
+	require.NoError(tb, err)
+
+	return s
+}
+
 func TestStats_races(t *testing.T) {
 	var r uint32
 	idGen := func() (id uint32) { return atomic.LoadUint32(&r) }
-	conf := Config{
-		Logger:            slogutil.NewDiscardLogger(),
-		ShouldCountClient: func([]string) bool { return true },
-		HTTPReg:           aghhttp.EmptyRegistrar{},
-		UnitID:            idGen,
-		Filename:          filepath.Join(t.TempDir(), "./stats.db"),
-		Limit:             timeutil.Day,
-	}
-
-	s, err := New(conf)
-	require.NoError(t, err)
+	s := newTestStatsCtx(t, Config{
+		UnitID:  idGen,
+		Enabled: true,
+	})
 
 	s.Start()
 	startTime := time.Now()
@@ -97,13 +118,10 @@ func TestStatsCtx_FillCollectedStats_daily(t *testing.T) {
 		timeUnits = "days"
 	)
 
-	s, err := New(Config{
-		Logger:            slogutil.NewDiscardLogger(),
-		ShouldCountClient: func([]string) bool { return true },
-		Filename:          filepath.Join(t.TempDir(), "./stats.db"),
-		Limit:             time.Hour,
+	s := newTestStatsCtx(t, Config{
+		Limit:   time.Hour,
+		Enabled: true,
 	})
-	require.NoError(t, err)
 
 	testutil.CleanupAndRequireSuccess(t, s.Close)
 
@@ -155,13 +173,10 @@ func TestStatsCtx_FillCollectedStats_daily(t *testing.T) {
 func TestStatsCtx_DataFromUnits_month(t *testing.T) {
 	const hoursInMonth = 720
 
-	s, err := New(Config{
-		Logger:            slogutil.NewDiscardLogger(),
-		ShouldCountClient: func([]string) bool { return true },
-		Filename:          filepath.Join(t.TempDir(), "./stats.db"),
-		Limit:             time.Hour,
+	s := newTestStatsCtx(t, Config{
+		Limit:   time.Hour,
+		Enabled: true,
 	})
-	require.NoError(t, err)
 
 	testutil.CleanupAndRequireSuccess(t, s.Close)
 

openapi/CHANGELOG.md

@@ -4,6 +4,10 @@
 
 ## v0.107.72: API changes
 
+## New `recent` query parameter in 'GET /control/stats/'
+
+- New query parameter `recent` defines the statistics lookback period in millieseconds.
+
 ### New `ignored_enabled` field in `GetStatsConfigResponse` and  `GetQueryLogConfigResponse`
 
 - The new field `ignored_enabled` indicates whether the host names in the ignored array should be ignored.  This field has been added for the following endpoints: