Avoid race conditions when checking logs in tests

cmd/grafana-agent-service/service_test.go

@@ -9,7 +9,6 @@ import (
 	"os/exec"
 	"path/filepath"
 	"runtime"
-	"sync"
 	"testing"
 
 	"github.com/go-kit/log"
@@ -84,7 +83,7 @@ func Test_serviceManager(t *testing.T) {
 	t.Run("can forward to stdout", func(t *testing.T) {
 		listenHost := getListenHost(t)
 
-		var buf syncBuffer
+		var buf util.SyncBuffer
 
 		mgr := newServiceManager(l, serviceManagerConfig{
 			Path:   serviceBinary,
@@ -112,7 +111,7 @@ func Test_serviceManager(t *testing.T) {
 	t.Run("can forward to stderr", func(t *testing.T) {
 		listenHost := getListenHost(t)
 
-		var buf syncBuffer
+		var buf util.SyncBuffer
 
 		mgr := newServiceManager(l, serviceManagerConfig{
 			Path:   serviceBinary,
@@ -186,24 +185,3 @@ func makeServiceRequest(host string, path string, body []byte) ([]byte, error) {
 	}
 	return io.ReadAll(resp.Body)
 }
-
-// syncBuffer wraps around a bytes.Buffer and makes it safe to use from
-// multiple goroutines.
-type syncBuffer struct {
-	mut sync.RWMutex
-	buf bytes.Buffer
-}
-
-func (sb *syncBuffer) Bytes() []byte {
-	sb.mut.RLock()
-	defer sb.mut.RUnlock()
-
-	return sb.buf.Bytes()
-}
-
-func (sb *syncBuffer) Write(p []byte) (n int, err error) {
-	sb.mut.Lock()
-	defer sb.mut.Unlock()
-
-	return sb.buf.Write(p)
-}

internal/util/syncbuffer.go

@@ -0,0 +1,34 @@
+package util
+
+import (
+	"bytes"
+	"sync"
+)
+
+// SyncBuffer wraps around a bytes.Buffer and makes it safe to use from
+// multiple goroutines.
+type SyncBuffer struct {
+	mut sync.RWMutex
+	buf bytes.Buffer
+}
+
+func (sb *SyncBuffer) Bytes() []byte {
+	sb.mut.RLock()
+	defer sb.mut.RUnlock()
+
+	return sb.buf.Bytes()
+}
+
+func (sb *SyncBuffer) String() string {
+	sb.mut.RLock()
+	defer sb.mut.RUnlock()
+
+	return sb.buf.String()
+}
+
+func (sb *SyncBuffer) Write(p []byte) (n int, err error) {
+	sb.mut.Lock()
+	defer sb.mut.Unlock()
+
+	return sb.buf.Write(p)
+}

static/metrics/agent_test.go

@@ -1,7 +1,6 @@
 package metrics
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -15,6 +14,7 @@ import (
 	"github.com/grafana/agent/internal/util"
 	"github.com/grafana/agent/static/metrics/instance"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/prometheus/prometheus/scrape"
 	"github.com/prometheus/prometheus/storage"
 	"github.com/stretchr/testify/require"
@@ -140,10 +140,12 @@ configs:
 
 	fact := newFakeInstanceFactory()
 
-	logBuffer := bytes.Buffer{}
-	logger := log.NewLogfmtLogger(&logBuffer)
+	logBuffer := util.SyncBuffer{}
+	logger := log.NewSyncLogger(log.NewLogfmtLogger(&logBuffer))
 
-	a, err := newAgent(prometheus.NewRegistry(), cfg, logger, fact.factory)
+	reg := prometheus.NewRegistry()
+
+	a, err := newAgent(reg, cfg, logger, fact.factory)
 	require.NoError(t, err)
 
 	util.Eventually(t, func(t require.TestingT) {
@@ -161,23 +163,45 @@ configs:
 		}
 	})
 
-	//
+	util.Eventually(t, func(t require.TestingT) {
+		if err := testutil.GatherAndCompare(reg,
+			strings.NewReader(`
+# HELP agent_metrics_configs_changed_total Total number of dynamically updated configs
+# TYPE agent_metrics_configs_changed_total counter
+agent_metrics_configs_changed_total{event="created"} 1
+`), "agent_metrics_configs_changed_total"); err != nil {
+			t.Errorf("mismatch metrics: %v", err)
+			t.FailNow()
+		}
+	})
+
 	// The config has changed (it used to be ""). The log line won't be printed.
-	//
 	checkConfigReloadLog(t, logBuffer.String(), 0)
 
-	var sameCfg Config
-
 	//
 	// Try the same config.
 	//
+	var sameCfg Config
+
 	err = yaml.Unmarshal([]byte(cfgText), &sameCfg)
 	require.NoError(t, err)
 	err = sameCfg.ApplyDefaults()
 	require.NoError(t, err)
 
 	a.ApplyConfig(sameCfg)
 
+	util.Eventually(t, func(t require.TestingT) {
+		if err := testutil.GatherAndCompare(reg,
+			strings.NewReader(`
+# HELP agent_metrics_configs_changed_total Total number of dynamically updated configs
+# TYPE agent_metrics_configs_changed_total counter
+agent_metrics_configs_changed_total{event="created"} 1
+`), "agent_metrics_configs_changed_total"); err != nil {
+			t.Errorf("mismatch metrics: %v", err)
+			t.FailNow()
+		}
+	})
+
 	// The config did not change. The log line should be printed.
 	checkConfigReloadLog(t, logBuffer.String(), 1)
 
@@ -202,8 +226,29 @@ configs:
 
 	a.ApplyConfig(differentCfg)
 
+	util.Eventually(t, func(t require.TestingT) {
+		if err := testutil.GatherAndCompare(reg,
+			strings.NewReader(`
+# HELP agent_metrics_configs_changed_total Total number of dynamically updated configs
+# TYPE agent_metrics_configs_changed_total counter
+agent_metrics_configs_changed_total{event="created"} 2
+agent_metrics_configs_changed_total{event="deleted"} 1
+`), "agent_metrics_configs_changed_total"); err != nil {
+			t.Errorf("mismatch metrics: %v", err)
+			t.FailNow()
+		}
+	})
+
 	// The config has changed. The log line won't be printed.
 	checkConfigReloadLog(t, logBuffer.String(), 1)
+
+	for _, mi := range fact.Mocks() {
+		util.Eventually(t, func(t require.TestingT) {
+			require.Equal(t, 1, int(mi.startedCount.Load()))
+		})
+	}
+
+	a.Stop()
 }
 
 func TestAgent(t *testing.T) {