proxy: imp docs

proxy/stats.go

@@ -2,7 +2,6 @@ package proxy
 
 import (
 	"fmt"
-	"sync"
 	"time"
 
 	"github.com/AdguardTeam/dnsproxy/upstream"
@@ -15,9 +14,6 @@ type upstreamWithStats struct {
 	// upstream is the upstream DNS resolver.
 	upstream upstream.Upstream
 
-	// mu protects err and queryDuration.
-	mu *sync.Mutex
-
 	// err is the DNS lookup error, if any.
 	err error
 
@@ -32,33 +28,19 @@ type upstreamWithStats struct {
 func newUpstreamWithStats(upstream upstream.Upstream, isFallback bool) (u *upstreamWithStats) {
 	return &upstreamWithStats{
 		upstream:   upstream,
-		mu:         &sync.Mutex{},
 		isFallback: isFallback,
 	}
 }
 
-// stats returns the stored statistics.
-func (u *upstreamWithStats) stats() (dur time.Duration, err error) {
-	u.mu.Lock()
-	defer u.mu.Unlock()
-
-	return u.queryDuration, u.err
-}
-
 // type check
 var _ upstream.Upstream = (*upstreamWithStats)(nil)
 
 // Exchange implements the [upstream.Upstream] for *upstreamWithStats.
 func (u *upstreamWithStats) Exchange(req *dns.Msg) (resp *dns.Msg, err error) {
 	start := time.Now()
 	resp, err = u.upstream.Exchange(req)
-	dur := time.Since(start)
-
-	u.mu.Lock()
-	defer u.mu.Unlock()
-
 	u.err = err
-	u.queryDuration = dur
+	u.queryDuration = time.Since(start)
 
 	return resp, err
 }
@@ -115,15 +97,27 @@ func (s *QueryStatistics) Fallback() (us []*UpstreamStatistics) {
 	return s.fallback
 }
 
-// collectQueryStats gathers the statistics from the wrapped upstreams,
-// considering the upstream mode.  resolver is an upstream DNS resolver that
-// successfully resolved the request, and it will be unwrapped.  If resolver is
-// nil (i.e. the DNS query was not resolved) or upstream mode is
-// [UpstreamModeFastestAddr], the function returns the gathered statistics for
-// both the upstream and fallback DNS servers.  If resolver is fallback, it also
-// gathers the statistics for the upstreams.  Otherwise, it returns the query
-// statistics specifically for upstream resolver.  Provided upstreams must be of
-// type *upstreamWithStats.
+// collectQueryStats gathers the statistics from the wrapped upstreams.
+// resolver is an upstream DNS resolver that successfully resolved the request,
+// if any.  Provided upstreams must be of type [*upstreamWithStats].  unwrapped
+// is the unwrapped resolver, see [upstreamWithStats.upstream].
+//
+// Depending on whether the DNS request was successfully resolved and the
+// upstream mode, the returned statistics consist of:
+//
+//   - If the query was successfully resolved, the statistics contain the DNS
+//     lookup duration for the main resolver.
+//
+//   - If the upstream mode is [UpstreamModeFastestAddr] and the query was
+//     successfully resolved, the statistics contain the DNS lookup durations or
+//     errors for each main upstream.
+//
+//   - If the query was resolved by the fallback resolver, the statistics
+//     contain the DNS lookup errors for each main upstream and the query
+//     duration for the fallback resolver.
+//
+//   - If the query was not resolved at all, the statistics contain the DNS
+//     lookup errors for each main and fallback resolvers.
 func collectQueryStats(
 	mode UpstreamMode,
 	resolver upstream.Upstream,
@@ -143,13 +137,19 @@ func collectQueryStats(
 		unwrapped = wrapped.upstream
 	}
 
-	if wrapped == nil || mode == UpstreamModeFastestAddr {
+	if wrapped == nil {
 		return unwrapped, &QueryStatistics{
 			main:     collectUpstreamStats(upstreams),
 			fallback: collectUpstreamStats(fallbacks),
 		}
 	}
 
+	if mode == UpstreamModeFastestAddr && !wrapped.isFallback {
+		return unwrapped, &QueryStatistics{
+			main: collectUpstreamStats(upstreams),
+		}
+	}
+
 	return unwrapped, collectResolverQueryStats(upstreams, wrapped)
 }
 
@@ -161,7 +161,7 @@ func collectResolverQueryStats(
 	upstreams []upstream.Upstream,
 	resolver *upstreamWithStats,
 ) (stats *QueryStatistics) {
-	dur, err := resolver.stats()
+	dur, err := resolver.queryDuration, resolver.err
 	s := &UpstreamStatistics{
 		Address:       resolver.upstream.Address(),
 		Error:         err,
@@ -211,7 +211,11 @@ func collectUpstreamStats(upstreams []upstream.Upstream) (stats []*UpstreamStati
 			panic(err)
 		}
 
-		dur, err := w.stats()
+		dur, err := w.queryDuration, w.err
+		if dur == 0 {
+			continue
+		}
+
 		stats = append(stats, &UpstreamStatistics{
 			Error:         err,
 			Address:       w.Address(),

proxy/stats_test.go

@@ -254,13 +254,13 @@ func assertQueryStats(
 	t.Helper()
 
 	main := stats.Main()
-	assert.Equal(t, mainCount, len(main), "main stats count")
+	assert.Equalf(t, mainCount, len(main), "main stats count")
 
 	fallback := stats.Fallback()
-	assert.Equal(t, fallbackCount, len(fallback), "fallback stats count")
+	assert.Equalf(t, fallbackCount, len(fallback), "fallback stats count")
 
-	assert.Equal(t, isMainErr, isErrorInStats(main), "main err")
-	assert.Equal(t, isFallbackErr, isErrorInStats(fallback), "fallback err")
+	assert.Equalf(t, isMainErr, isErrorInStats(main), "main err")
+	assert.Equalf(t, isFallbackErr, isErrorInStats(fallback), "fallback err")
 }
 
 // isErrorInStats is a helper function for tests that returns true if the