revamp tests + thread-safety

pkg/trace/metrics/client.go

@@ -6,6 +6,7 @@
 package metrics
 
 import (
+	"sync"
 	"time"
 
 	"github.com/DataDog/datadog-go/v5/statsd"
@@ -20,12 +21,23 @@ type StatsClient interface {
 	Flush() error
 }
 
+var m sync.Mutex
+
+// SetClient sets the global Statsd client.
+func SetClient(client StatsClient) {
+	m.Lock()
+	defer m.Unlock()
+	Client = client
+}
+
 // Client is a global Statsd client. When a client is configured via Configure,
 // that becomes the new global Statsd client in the package.
 var Client StatsClient = (*statsd.Client)(nil)
 
 // Gauge calls Gauge on the global Client, if set.
 func Gauge(name string, value float64, tags []string, rate float64) error {
+	m.Lock()
+	defer m.Unlock()
 	if Client == nil {
 		return nil // no-op
 	}
@@ -34,6 +46,8 @@ func Gauge(name string, value float64, tags []string, rate float64) error {
 
 // Count calls Count on the global Client, if set.
 func Count(name string, value int64, tags []string, rate float64) error {
+	m.Lock()
+	defer m.Unlock()
 	if Client == nil {
 		return nil // no-op
 	}
@@ -42,6 +56,8 @@ func Count(name string, value int64, tags []string, rate float64) error {
 
 // Histogram calls Histogram on the global Client, if set.
 func Histogram(name string, value float64, tags []string, rate float64) error {
+	m.Lock()
+	defer m.Unlock()
 	if Client == nil {
 		return nil // no-op
 	}
@@ -50,6 +66,8 @@ func Histogram(name string, value float64, tags []string, rate float64) error {
 
 // Timing calls Timing on the global Client, if set.
 func Timing(name string, value time.Duration, tags []string, rate float64) error {
+	m.Lock()
+	defer m.Unlock()
 	if Client == nil {
 		return nil // no-op
 	}
@@ -58,6 +76,8 @@ func Timing(name string, value time.Duration, tags []string, rate float64) error
 
 // Flush flushes any pending metrics to the agent.
 func Flush() error {
+	m.Lock()
+	defer m.Unlock()
 	if Client == nil {
 		return nil // no-op
 	}

pkg/trace/metrics/timing/timing.go

@@ -20,7 +20,7 @@ import (
 )
 
 // autoreportInterval specifies the interval at which the default set reports.
-const autoreportInterval = 10 * time.Second
+var autoreportInterval = 10 * time.Second
 
 var (
 	defaultSet *set
@@ -29,15 +29,21 @@ var (
 // Since records the duration for the given metric name as time passed since start.
 // It uses the default set which is reported at 10 second intervals.
 func Since(name string, start time.Time) {
+	m.Lock()
+	defer m.Unlock()
 	if defaultSet == nil {
 		log.Error("Timing hasn't been initialized, trace-agent metrics will be missing")
 		return
 	}
 	defaultSet.Since(name, start)
 }
 
+var m = sync.Mutex{}
+
 // Start initializes autoreporting of timing metrics.
 func Start() {
+	m.Lock()
+	defer m.Unlock()
 	defaultSet = newSet()
 	defaultSet.autoreport(autoreportInterval)
 }
@@ -46,6 +52,8 @@ func Start() {
 // metrics. It can be useful to call when the program exits to ensure everything is
 // submitted.
 func Stop() {
+	m.Lock()
+	defer m.Unlock()
 	if defaultSet == nil {
 		return
 	}

pkg/trace/metrics/timing/timing_test.go

@@ -21,52 +21,50 @@ import (
 
 func TestTiming(t *testing.T) {
 	assert := assert.New(t)
-	stats := &teststatsd.Client{}
-
 	Stop() // https://github.com/DataDog/datadog-agent/issues/13934
-	defer func(old metrics.StatsClient) { metrics.Client = old }(metrics.Client)
-	metrics.Client = stats
-	Start()
+	defer func(old metrics.StatsClient) { metrics.SetClient(old) }(metrics.Client)
 
 	t.Run("report", func(t *testing.T) {
-		stats.Reset()
+		stats := &teststatsd.Client{}
+		metrics.SetClient(stats)
 		set := newSet()
 		set.Since("counter1", time.Now().Add(-2*time.Second))
 		set.Since("counter1", time.Now().Add(-3*time.Second))
 		set.report()
 
-		calls := stats.CountCalls
-		assert.Equal(1, len(calls))
-		assert.Equal(2., findCall(assert, calls, "counter1.count").Value)
+		counts := stats.GetCountSummaries()
+		assert.Equal(1, len(counts))
+		assert.Contains(counts, "counter1.count")
 
-		calls = stats.GaugeCalls
-		assert.Equal(2, len(calls))
-		assert.Equal(2500., findCall(assert, calls, "counter1.avg").Value, "avg")
-		assert.Equal(3000., findCall(assert, calls, "counter1.max").Value, "max")
+		gauges := stats.GetGaugeSummaries()
+		assert.Equal(2, len(gauges))
+		assert.Contains(gauges, "counter1.avg")
+		assert.Contains(gauges, "counter1.max")
 	})
 
 	t.Run("autoreport", func(t *testing.T) {
-		stats.Reset()
+		stats := &teststatsd.Client{}
+		metrics.SetClient(stats)
 		set := newSet()
 		set.Since("counter1", time.Now().Add(-1*time.Second))
 		set.autoreport(time.Millisecond)
 		if runtime.GOOS == "windows" {
 			time.Sleep(5 * time.Second)
 		}
-		time.Sleep(100 * time.Millisecond)
+		time.Sleep(10 * time.Millisecond)
 		Stop()
-		assert.True(len(stats.CountCalls) > 1)
+		assert.Contains(stats.GetCountSummaries(), "counter1.count")
 	})
 
 	t.Run("panic", func(t *testing.T) {
-		set := newSet()
-		set.autoreport(time.Millisecond)
+		Start()
 		Stop()
 		Stop()
 	})
 
 	t.Run("race", func(t *testing.T) {
-		stats.Reset()
+		stats := &teststatsd.Client{}
+		metrics.SetClient(stats)
 		set := newSet()
 		var wg sync.WaitGroup
 		for i := 0; i < 150; i++ {
@@ -87,13 +85,3 @@ func TestTiming(t *testing.T) {
 		wg.Wait()
 	})
 }
-
-func findCall(assert *assert.Assertions, calls []teststatsd.MetricsArgs, name string) teststatsd.MetricsArgs {
-	for _, c := range calls {
-		if c.Name == name {
-			return c
-		}
-	}
-	assert.Failf("call not found", "key %q missing", name)
-	return teststatsd.MetricsArgs{}
-}