From 1c7064c066691c5794fac1f38d6243792cad0582 Mon Sep 17 00:00:00 2001 From: Zach <3724288+zedkipp@users.noreply.github.com> Date: Fri, 1 May 2026 09:16:42 -0600 Subject: [PATCH] fix: use atomic.Int64 for workspace traffic metrics (#24844) connMetrics.total was written to via atomic.AddInt64 and read as a plain int64, producing a data race. Fix by switching the field to atomic.Int64 and using its typed Add/Load methods. The testMetrics mock had a similar issue where mutex use was missing where GetTotalBytes read the total bytes, which is also fixed in this change. --- scaletest/workspacetraffic/metrics.go | 6 +-- scaletest/workspacetraffic/metrics_test.go | 48 ++++++++++++++++++++++ scaletest/workspacetraffic/run_test.go | 2 + 3 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 scaletest/workspacetraffic/metrics_test.go diff --git a/scaletest/workspacetraffic/metrics.go b/scaletest/workspacetraffic/metrics.go index c472258d47..b48876abec 100644 --- a/scaletest/workspacetraffic/metrics.go +++ b/scaletest/workspacetraffic/metrics.go @@ -86,7 +86,7 @@ type connMetrics struct { addError func(float64) observeLatency func(float64) addTotal func(float64) - total int64 + total atomic.Int64 } func (c *connMetrics) AddError(f float64) { @@ -98,10 +98,10 @@ func (c *connMetrics) ObserveLatency(f float64) { } func (c *connMetrics) AddTotal(f float64) { - atomic.AddInt64(&c.total, int64(f)) + c.total.Add(int64(f)) c.addTotal(f) } func (c *connMetrics) GetTotalBytes() int64 { - return c.total + return c.total.Load() } diff --git a/scaletest/workspacetraffic/metrics_test.go b/scaletest/workspacetraffic/metrics_test.go new file mode 100644 index 0000000000..a189367ef9 --- /dev/null +++ b/scaletest/workspacetraffic/metrics_test.go @@ -0,0 +1,48 @@ +package workspacetraffic_test + +import ( + "sync" + "testing" + + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/scaletest/workspacetraffic" +) + +func TestConnMetrics_Concurrent(t *testing.T) { + t.Parallel() + + reg := prometheus.NewRegistry() + m := workspacetraffic.NewMetrics(reg, "username", "workspace_name", "agent_name") + cm := m.ReadMetrics("username", "workspace_name", "agent_name") + + const ( + writers = 8 + readers = 8 + opsPerWriter = 1000 + bytesPerWrite = 1 + ) + + var wg sync.WaitGroup + wg.Add(writers + readers) + for i := 0; i < writers; i++ { + go func() { + defer wg.Done() + for j := 0; j < opsPerWriter; j++ { + cm.AddTotal(float64(bytesPerWrite)) + } + }() + } + for i := 0; i < readers; i++ { + go func() { + defer wg.Done() + for j := 0; j < opsPerWriter; j++ { + _ = cm.GetTotalBytes() + } + }() + } + wg.Wait() + + require.Equal(t, int64(writers*opsPerWriter*bytesPerWrite), cm.GetTotalBytes()) +} diff --git a/scaletest/workspacetraffic/run_test.go b/scaletest/workspacetraffic/run_test.go index beda847762..50e7ca3c2e 100644 --- a/scaletest/workspacetraffic/run_test.go +++ b/scaletest/workspacetraffic/run_test.go @@ -423,5 +423,7 @@ func (m *testMetrics) Latencies() []float64 { } func (m *testMetrics) GetTotalBytes() int64 { + m.Lock() + defer m.Unlock() return int64(m.total) }