From 7a0082c3d2a285a1b651a01551755d5b1edd8f24 Mon Sep 17 00:00:00 2001 From: Thomas Kowalski Date: Mon, 18 May 2026 10:36:04 +0200 Subject: [PATCH 1/4] test: make tests independent of containerID ordering --- statsd/cleanup_go114_test.go | 10 ++++++++++ statsd/cleanup_pre114_test.go | 17 +++++++++++++++++ statsd/container.go | 12 +++++++----- statsd/container_linux.go | 16 +++++++++------- statsd/container_stub.go | 3 +-- statsd/container_test.go | 6 +++--- statsd/service_check_test.go | 3 +++ statsd/statsd_test.go | 15 ++++++++++++++- statsd/telemetry_test.go | 6 ++++++ statsd/test_helpers_test.go | 25 +++++++++++++++++++++++-- 10 files changed, 93 insertions(+), 20 deletions(-) create mode 100644 statsd/cleanup_go114_test.go create mode 100644 statsd/cleanup_pre114_test.go diff --git a/statsd/cleanup_go114_test.go b/statsd/cleanup_go114_test.go new file mode 100644 index 00000000..830e3c3c --- /dev/null +++ b/statsd/cleanup_go114_test.go @@ -0,0 +1,10 @@ +//go:build go1.14 +// +build go1.14 + +package statsd + +import "testing" + +func registerCleanup(t *testing.T, f func()) { + t.Cleanup(f) +} diff --git a/statsd/cleanup_pre114_test.go b/statsd/cleanup_pre114_test.go new file mode 100644 index 00000000..58898e62 --- /dev/null +++ b/statsd/cleanup_pre114_test.go @@ -0,0 +1,17 @@ +//go:build !go1.14 +// +build !go1.14 + +package statsd + +import "testing" + +// t.Cleanup is not available before Go 1.14, so this is a no-op on Go +// 1.13. Tests that assert payloads without a container-ID suffix must +// therefore call withoutOriginGlobals at their start to reset the +// package globals explicitly. +// +// Tests in format_test.go are safe without that call: they run before +// any client-constructing test (alphabetical file order), and the only +// earlier file that touches containerID (container_test.go, Linux-only) +// restores it with a defer in each test body. +func registerCleanup(t *testing.T, f func()) {} diff --git a/statsd/container.go b/statsd/container.go index 20d69ef6..9b9b9d6c 100644 --- a/statsd/container.go +++ b/statsd/container.go @@ -2,18 +2,20 @@ package statsd import ( "sync" + "sync/atomic" ) var ( - // containerID holds the container ID. - containerID = "" - - initOnce sync.Once + containerID atomic.Value + initOnce sync.Once ) // getContainerID returns the container ID configured at the client creation // It can either be auto-discovered with origin detection or provided by the user. // User-defined container ID is prioritized. func getContainerID() string { - return containerID + if v := containerID.Load(); v != nil { + return v.(string) + } + return "" } diff --git a/statsd/container_linux.go b/statsd/container_linux.go index 12513234..4e259c32 100644 --- a/statsd/container_linux.go +++ b/statsd/container_linux.go @@ -193,27 +193,29 @@ func internalInitContainerID(userProvidedID string, cgroupFallback, isHostCgroup // readCIDOrInode reads the container ID from the user provided ID, cgroups or mountinfo. func readCIDOrInode(userProvidedID, cgroupPath, selfMountInfoPath, defaultCgroupMountPath string, cgroupFallback, isHostCgroupNs bool) { if userProvidedID != "" { - containerID = userProvidedID + containerID.Store(userProvidedID) return } if cgroupFallback { - containerID = readContainerID(cgroupPath) - if containerID != "" { + if cid := readContainerID(cgroupPath); cid != "" { + containerID.Store(cid) return } - containerID = readMountinfo(selfMountInfoPath) - if containerID != "" { + if cid := readMountinfo(selfMountInfoPath); cid != "" { + containerID.Store(cid) return } // If we're in the host cgroup namespace, the cid should be retrievable in /proc/self/cgroup // In private cgroup namespace, we can retrieve the cgroup controller inode. - if containerID == "" && isHostCgroupNs { + if isHostCgroupNs { return } - containerID = getCgroupInode(defaultCgroupMountPath, cgroupPath) + if cid := getCgroupInode(defaultCgroupMountPath, cgroupPath); cid != "" { + containerID.Store(cid) + } } } diff --git a/statsd/container_stub.go b/statsd/container_stub.go index 29ab7f2c..00790809 100644 --- a/statsd/container_stub.go +++ b/statsd/container_stub.go @@ -10,8 +10,7 @@ func isHostCgroupNamespace() bool { var initContainerID = func(userProvidedID string, _, _ bool) { initOnce.Do(func() { if userProvidedID != "" { - containerID = userProvidedID - return + containerID.Store(userProvidedID) } }) } diff --git a/statsd/container_test.go b/statsd/container_test.go index a2aa0896..60845446 100644 --- a/statsd/container_test.go +++ b/statsd/container_test.go @@ -448,9 +448,9 @@ func TestReadCIDOrInode(t *testing.T) { for _, tc := range tests { t.Run(tc.description, func(t *testing.T) { - prevContainerID := containerID + prevContainerID := getContainerID() defer func() { - containerID = prevContainerID + patchContainerID(prevContainerID) }() sysFsCgroupPath := path.Join(os.TempDir(), "sysfscgroup") @@ -483,7 +483,7 @@ func TestReadCIDOrInode(t *testing.T) { require.NoError(t, err) readCIDOrInode("", procSelfCgroup.Name(), mountInfo.Name(), sysFsCgroupPath, true, tc.isHostCgroupNs) - require.Equal(t, expectedResult, containerID) + require.Equal(t, expectedResult, getContainerID()) }) } } diff --git a/statsd/service_check_test.go b/statsd/service_check_test.go index e51230f4..c19b0ca5 100644 --- a/statsd/service_check_test.go +++ b/statsd/service_check_test.go @@ -18,6 +18,7 @@ func encodeSC(sc *ServiceCheck) (string, error) { } func TestServiceChecks(t *testing.T) { + withoutOriginGlobals(t) matrix := []struct { serviceCheck *ServiceCheck expectedEncode string @@ -78,6 +79,7 @@ func TestUnknownStatus(t *testing.T) { } func TestNewServiceCheckWithTags(t *testing.T) { + withoutOriginGlobals(t) sc := NewServiceCheck("hello", Warn) sc.Tags = []string{"tag1", "tag2"} s, err := encodeSC(sc) @@ -87,6 +89,7 @@ func TestNewServiceCheckWithTags(t *testing.T) { } func TestNewServiceCheckWithTagsAppend(t *testing.T) { + withoutOriginGlobals(t) sc := NewServiceCheck("hello", Warn) sc.Tags = append(sc.Tags, "tag1", "tag2") s, err := encodeSC(sc) diff --git a/statsd/statsd_test.go b/statsd/statsd_test.go index 485db025..5212b89f 100644 --- a/statsd/statsd_test.go +++ b/statsd/statsd_test.go @@ -110,9 +110,17 @@ func (s *statsdWriterWrapper) Write(p []byte) (n int, err error) { } func TestNewWithWriter(t *testing.T) { + // The expected payloads built by ts.sendAllType do not include any + // container-ID / external-env suffix, so the test only passes when + // origin detection is off. Without WithoutOriginDetection the client + // would populate the package-level containerID from /proc/self/cgroup + // and ClientEx.send would tag every metric with |c:, breaking the + // assertion on hosts that have a non-empty cgroup container id. + withoutOriginGlobals(t) w := statsdWriterWrapper{} - client, err := NewWithWriter(&w, WithoutTelemetry()) + client, err := NewWithWriter(&w, WithoutTelemetry(), WithoutOriginDetection()) require.Nil(t, err) + defer resetContainerID() ts := &testServer{} expected := ts.sendAllType(client) @@ -321,11 +329,16 @@ func TestResolveAddressFromEnvironment(t *testing.T) { } func TestGetTelemetry(t *testing.T) { + // TotalBytesSent below was computed before the client emitted + // |c: on every payload. Disable origin detection so the byte + // count is independent of the host's cgroup / container id. + withoutOriginGlobals(t) ts, client := newClientAndTestServer(t, "udp", "localhost:8765", nil, WithExtendedClientSideAggregation(), + WithoutOriginDetection(), ) ts.sendAllAndAssert(t, client) diff --git a/statsd/telemetry_test.go b/statsd/telemetry_test.go index abecd6f8..f6451aa2 100644 --- a/statsd/telemetry_test.go +++ b/statsd/telemetry_test.go @@ -18,6 +18,11 @@ import ( // func TestTelemetryCustomAddr(t *testing.T) { + // The expected strings below were written for an empty container ID; + // disable origin detection so the test is hermetic and does not depend + // on /proc/self/cgroup or on test ordering populating the package + // global. + withoutOriginGlobals(t) telAddr := "localhost:8764" ts, client := newClientAndTestServer(t, "udp", @@ -25,6 +30,7 @@ func TestTelemetryCustomAddr(t *testing.T) { nil, WithTelemetryAddr(telAddr), WithNamespace("test_namespace"), + WithoutOriginDetection(), ) udpAddr, err := net.ResolveUDPAddr("udp", telAddr) diff --git a/statsd/test_helpers_test.go b/statsd/test_helpers_test.go index cce790ae..729a3451 100644 --- a/statsd/test_helpers_test.go +++ b/statsd/test_helpers_test.go @@ -122,6 +122,12 @@ func newClientAndTestServer(t *testing.T, proto string, addr string, tags []stri client, err := New(addr, options...) require.NoError(t, err) + // Constructing a real Client populates the package-level containerID + // global via initContainerID. Reset it after the test so subsequent + // tests (notably the format_test.go and service_check_test.go ones, + // which build expected payloads assuming an empty container ID) are + // not affected by test ordering. + registerCleanup(t, resetContainerID) startTestServer(ts) return ts, client @@ -133,6 +139,7 @@ func newClientDirectAndTestServer(t *testing.T, proto string, addr string, tags client, err := NewDirect(addr, options...) require.NoError(t, err) + registerCleanup(t, resetContainerID) startTestServer(ts) return ts, client @@ -644,13 +651,27 @@ func (ts *testServer) sendExtendedBasicAggregationMetricsWithPreAggregatedSample return append(expectedMetrics, ts.namespace+"distro2:5:6|d|@0.5"+finalTags) } -func patchContainerID(id string) { containerID = id } +func patchContainerID(id string) { + containerID.Store(id) +} func resetContainerID() { - containerID = "" + containerID.Store("") initOnce = sync.Once{} } +// withoutOriginGlobals clears the package-level containerID and externalEnv +// before the test runs and restores them after, so the test is independent +// of any other test in the package that may have populated them through a +// real client construction. Tests that build clients and assert wire +// payloads which do not include container-id / external-env tags should +// call this at the very top of the test body. +func withoutOriginGlobals(t *testing.T) { + t.Helper() + resetContainerID() + resetExternalEnv() +} + func patchExternalEnv(env string) { os.Setenv(ddExternalEnvVarName, env) initExternalEnv() From faa9bd6e15e7b27ea4ceeecd6d6c08e1b592d79f Mon Sep 17 00:00:00 2001 From: Thomas Kowalski Date: Tue, 19 May 2026 10:51:58 +0200 Subject: [PATCH 2/4] test: don't use an atomic --- statsd/cleanup_go114_test.go | 10 ---------- statsd/cleanup_pre114_test.go | 17 ---------------- statsd/container.go | 8 ++------ statsd/container_linux.go | 16 +++++++-------- statsd/container_stub.go | 2 +- statsd/format_test.go | 37 +++++++++++++++++++++++++++++++++++ statsd/test_helpers_test.go | 11 ++--------- 7 files changed, 49 insertions(+), 52 deletions(-) delete mode 100644 statsd/cleanup_go114_test.go delete mode 100644 statsd/cleanup_pre114_test.go diff --git a/statsd/cleanup_go114_test.go b/statsd/cleanup_go114_test.go deleted file mode 100644 index 830e3c3c..00000000 --- a/statsd/cleanup_go114_test.go +++ /dev/null @@ -1,10 +0,0 @@ -//go:build go1.14 -// +build go1.14 - -package statsd - -import "testing" - -func registerCleanup(t *testing.T, f func()) { - t.Cleanup(f) -} diff --git a/statsd/cleanup_pre114_test.go b/statsd/cleanup_pre114_test.go deleted file mode 100644 index 58898e62..00000000 --- a/statsd/cleanup_pre114_test.go +++ /dev/null @@ -1,17 +0,0 @@ -//go:build !go1.14 -// +build !go1.14 - -package statsd - -import "testing" - -// t.Cleanup is not available before Go 1.14, so this is a no-op on Go -// 1.13. Tests that assert payloads without a container-ID suffix must -// therefore call withoutOriginGlobals at their start to reset the -// package globals explicitly. -// -// Tests in format_test.go are safe without that call: they run before -// any client-constructing test (alphabetical file order), and the only -// earlier file that touches containerID (container_test.go, Linux-only) -// restores it with a defer in each test body. -func registerCleanup(t *testing.T, f func()) {} diff --git a/statsd/container.go b/statsd/container.go index 9b9b9d6c..c442cf21 100644 --- a/statsd/container.go +++ b/statsd/container.go @@ -2,11 +2,10 @@ package statsd import ( "sync" - "sync/atomic" ) var ( - containerID atomic.Value + containerID string initOnce sync.Once ) @@ -14,8 +13,5 @@ var ( // It can either be auto-discovered with origin detection or provided by the user. // User-defined container ID is prioritized. func getContainerID() string { - if v := containerID.Load(); v != nil { - return v.(string) - } - return "" + return containerID } diff --git a/statsd/container_linux.go b/statsd/container_linux.go index 4e259c32..12513234 100644 --- a/statsd/container_linux.go +++ b/statsd/container_linux.go @@ -193,29 +193,27 @@ func internalInitContainerID(userProvidedID string, cgroupFallback, isHostCgroup // readCIDOrInode reads the container ID from the user provided ID, cgroups or mountinfo. func readCIDOrInode(userProvidedID, cgroupPath, selfMountInfoPath, defaultCgroupMountPath string, cgroupFallback, isHostCgroupNs bool) { if userProvidedID != "" { - containerID.Store(userProvidedID) + containerID = userProvidedID return } if cgroupFallback { - if cid := readContainerID(cgroupPath); cid != "" { - containerID.Store(cid) + containerID = readContainerID(cgroupPath) + if containerID != "" { return } - if cid := readMountinfo(selfMountInfoPath); cid != "" { - containerID.Store(cid) + containerID = readMountinfo(selfMountInfoPath) + if containerID != "" { return } // If we're in the host cgroup namespace, the cid should be retrievable in /proc/self/cgroup // In private cgroup namespace, we can retrieve the cgroup controller inode. - if isHostCgroupNs { + if containerID == "" && isHostCgroupNs { return } - if cid := getCgroupInode(defaultCgroupMountPath, cgroupPath); cid != "" { - containerID.Store(cid) - } + containerID = getCgroupInode(defaultCgroupMountPath, cgroupPath) } } diff --git a/statsd/container_stub.go b/statsd/container_stub.go index 00790809..0ea90f9b 100644 --- a/statsd/container_stub.go +++ b/statsd/container_stub.go @@ -10,7 +10,7 @@ func isHostCgroupNamespace() bool { var initContainerID = func(userProvidedID string, _, _ bool) { initOnce.Do(func() { if userProvidedID != "" { - containerID.Store(userProvidedID) + containerID = userProvidedID } }) } diff --git a/statsd/format_test.go b/statsd/format_test.go index 5a4af34c..97af85ab 100644 --- a/statsd/format_test.go +++ b/statsd/format_test.go @@ -8,6 +8,7 @@ import ( ) func TestFormatAppendTags(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendTags(buffer, []string{"global:tag"}, []string{"tag:tag", "tag2:tag2"}) assert.Equal(t, `|#global:tag,tag:tag,tag2:tag2`, string(buffer)) @@ -26,6 +27,7 @@ func TestFormatAppendTags(t *testing.T) { } func TestFormatAppendTagsAggregated(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendTagsAggregated(buffer, []string{"global:tag"}, "tag:tag,tag2:tag2") assert.Equal(t, `|#global:tag,tag:tag,tag2:tag2`, string(buffer)) @@ -44,84 +46,98 @@ func TestFormatAppendTagsAggregated(t *testing.T) { } func TestFormatAppendGauge(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendGauge(buffer, "namespace.", []string{"global:tag"}, "gauge", 1., []string{"tag:tag"}, 1, true) assert.Equal(t, `namespace.gauge:1|g|#global:tag,tag:tag`, string(buffer)) } func TestFormatAppendCount(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendCount(buffer, "namespace.", []string{"global:tag"}, "count", 2, []string{"tag:tag"}, 1, true) assert.Equal(t, `namespace.count:2|c|#global:tag,tag:tag`, string(buffer)) } func TestFormatAppendHistogram(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendHistogram(buffer, "namespace.", []string{"global:tag"}, "histogram", 3., []string{"tag:tag"}, 1, true) assert.Equal(t, `namespace.histogram:3|h|#global:tag,tag:tag`, string(buffer)) } func TestFormatAppendDistribution(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendDistribution(buffer, "namespace.", []string{"global:tag"}, "distribution", 4., []string{"tag:tag"}, 1, true) assert.Equal(t, `namespace.distribution:4|d|#global:tag,tag:tag`, string(buffer)) } func TestFormatAppendSet(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendSet(buffer, "namespace.", []string{"global:tag"}, "set", "five", []string{"tag:tag"}, 1, true) assert.Equal(t, `namespace.set:five|s|#global:tag,tag:tag`, string(buffer)) } func TestFormatAppendTiming(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendTiming(buffer, "namespace.", []string{"global:tag"}, "timing", 6., []string{"tag:tag"}, 1, true) assert.Equal(t, `namespace.timing:6.000000|ms|#global:tag,tag:tag`, string(buffer)) } func TestFormatNoTag(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendGauge(buffer, "", []string{}, "gauge", 1., []string{}, 1, true) assert.Equal(t, `gauge:1|g`, string(buffer)) } func TestFormatOneTag(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendGauge(buffer, "", []string{}, "gauge", 1., []string{"tag1:tag1"}, 1, true) assert.Equal(t, `gauge:1|g|#tag1:tag1`, string(buffer)) } func TestFormatTwoTag(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendGauge(buffer, "", []string{}, "metric", 1., []string{"tag1:tag1", "tag2:tag2"}, 1, true) assert.Equal(t, `metric:1|g|#tag1:tag1,tag2:tag2`, string(buffer)) } func TestFormatRate(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendGauge(buffer, "", []string{}, "metric", 1., []string{}, 0.1, true) assert.Equal(t, `metric:1|g|@0.1`, string(buffer)) } func TestFormatRateAndTag(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendGauge(buffer, "", []string{}, "metric", 1., []string{"tag1:tag1"}, 0.1, true) assert.Equal(t, `metric:1|g|@0.1|#tag1:tag1`, string(buffer)) } func TestFormatNil(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendGauge(buffer, "", nil, "metric", 1., nil, 1, true) assert.Equal(t, `metric:1|g`, string(buffer)) } func TestFormatTagRemoveNewLines(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendGauge(buffer, "", []string{"tag\n:d\nog\n"}, "metric", 1., []string{"\ntag\n:d\nog2\n"}, 0.1, true) assert.Equal(t, `metric:1|g|@0.1|#tag:dog,tag:dog2`, string(buffer)) } func TestFormatEvent(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendEvent(buffer, &Event{ Title: "EvenTitle", @@ -131,6 +147,7 @@ func TestFormatEvent(t *testing.T) { } func TestFormatEventEscapeText(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendEvent(buffer, &Event{ Title: "EvenTitle", @@ -140,6 +157,7 @@ func TestFormatEventEscapeText(t *testing.T) { } func TestFormatEventTimeStamp(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendEvent(buffer, &Event{ Title: "EvenTitle", @@ -150,6 +168,7 @@ func TestFormatEventTimeStamp(t *testing.T) { } func TestFormatEventHostname(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendEvent(buffer, &Event{ Title: "EvenTitle", @@ -160,6 +179,7 @@ func TestFormatEventHostname(t *testing.T) { } func TestFormatEventAggregationKey(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendEvent(buffer, &Event{ Title: "EvenTitle", @@ -170,6 +190,7 @@ func TestFormatEventAggregationKey(t *testing.T) { } func TestFormatEventPriority(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendEvent(buffer, &Event{ Title: "EvenTitle", @@ -180,6 +201,7 @@ func TestFormatEventPriority(t *testing.T) { } func TestFormatEventSourceTypeName(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendEvent(buffer, &Event{ Title: "EvenTitle", @@ -190,6 +212,7 @@ func TestFormatEventSourceTypeName(t *testing.T) { } func TestFormatEventAlertType(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendEvent(buffer, &Event{ Title: "EvenTitle", @@ -200,6 +223,7 @@ func TestFormatEventAlertType(t *testing.T) { } func TestFormatEventOneTag(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendEvent(buffer, &Event{ Title: "EvenTitle", @@ -209,6 +233,7 @@ func TestFormatEventOneTag(t *testing.T) { } func TestFormatEventTwoTag(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendEvent(buffer, &Event{ Title: "EvenTitle", @@ -219,6 +244,7 @@ func TestFormatEventTwoTag(t *testing.T) { } func TestFormatEventAllOptions(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendEvent(buffer, &Event{ Title: "EvenTitle", @@ -235,12 +261,14 @@ func TestFormatEventAllOptions(t *testing.T) { } func TestFormatEventNil(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendEvent(buffer, &Event{}, []string{}, true) assert.Equal(t, `_e{0,0}:|`, string(buffer)) } func TestFormatServiceCheck(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendServiceCheck(buffer, &ServiceCheck{ Name: "service.check", @@ -250,6 +278,7 @@ func TestFormatServiceCheck(t *testing.T) { } func TestFormatServiceCheckEscape(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendServiceCheck(buffer, &ServiceCheck{ Name: "service.check", @@ -260,6 +289,7 @@ func TestFormatServiceCheckEscape(t *testing.T) { } func TestFormatServiceCheckTimestamp(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendServiceCheck(buffer, &ServiceCheck{ Name: "service.check", @@ -270,6 +300,7 @@ func TestFormatServiceCheckTimestamp(t *testing.T) { } func TestFormatServiceCheckHostname(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendServiceCheck(buffer, &ServiceCheck{ Name: "service.check", @@ -280,6 +311,7 @@ func TestFormatServiceCheckHostname(t *testing.T) { } func TestFormatServiceCheckMessage(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendServiceCheck(buffer, &ServiceCheck{ Name: "service.check", @@ -290,6 +322,7 @@ func TestFormatServiceCheckMessage(t *testing.T) { } func TestFormatServiceCheckOneTag(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendServiceCheck(buffer, &ServiceCheck{ Name: "service.check", @@ -300,6 +333,7 @@ func TestFormatServiceCheckOneTag(t *testing.T) { } func TestFormatServiceCheckTwoTag(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendServiceCheck(buffer, &ServiceCheck{ Name: "service.check", @@ -310,6 +344,7 @@ func TestFormatServiceCheckTwoTag(t *testing.T) { } func TestFormatServiceCheckAllOptions(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendServiceCheck(buffer, &ServiceCheck{ Name: "service.check", @@ -323,12 +358,14 @@ func TestFormatServiceCheckAllOptions(t *testing.T) { } func TestFormatServiceCheckNil(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendServiceCheck(buffer, &ServiceCheck{}, nil, true) assert.Equal(t, `_sc||0`, string(buffer)) } func TestFormatSeparator(t *testing.T) { + withoutOriginGlobals(t) var buffer []byte buffer = appendSeparator(buffer) assert.Equal(t, "\n", string(buffer)) diff --git a/statsd/test_helpers_test.go b/statsd/test_helpers_test.go index 729a3451..2bb96e90 100644 --- a/statsd/test_helpers_test.go +++ b/statsd/test_helpers_test.go @@ -122,12 +122,6 @@ func newClientAndTestServer(t *testing.T, proto string, addr string, tags []stri client, err := New(addr, options...) require.NoError(t, err) - // Constructing a real Client populates the package-level containerID - // global via initContainerID. Reset it after the test so subsequent - // tests (notably the format_test.go and service_check_test.go ones, - // which build expected payloads assuming an empty container ID) are - // not affected by test ordering. - registerCleanup(t, resetContainerID) startTestServer(ts) return ts, client @@ -139,7 +133,6 @@ func newClientDirectAndTestServer(t *testing.T, proto string, addr string, tags client, err := NewDirect(addr, options...) require.NoError(t, err) - registerCleanup(t, resetContainerID) startTestServer(ts) return ts, client @@ -652,11 +645,11 @@ func (ts *testServer) sendExtendedBasicAggregationMetricsWithPreAggregatedSample } func patchContainerID(id string) { - containerID.Store(id) + containerID = id } func resetContainerID() { - containerID.Store("") + containerID = "" initOnce = sync.Once{} } From 22e2d80e9e999a6efff7282d7ba18e2a5118cb14 Mon Sep 17 00:00:00 2001 From: Thomas Kowalski Date: Fri, 5 Jun 2026 14:47:46 +0200 Subject: [PATCH 3/4] fix: make containerID an atomic --- statsd/container.go | 21 ++++++++++++++++++--- statsd/container_linux.go | 14 +++++++------- statsd/container_stub.go | 2 +- statsd/test_helpers_test.go | 4 ++-- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/statsd/container.go b/statsd/container.go index c442cf21..181a3fb9 100644 --- a/statsd/container.go +++ b/statsd/container.go @@ -2,16 +2,31 @@ package statsd import ( "sync" + "sync/atomic" ) var ( - containerID string + containerID atomic.Value initOnce sync.Once ) -// getContainerID returns the container ID configured at the client creation +func init() { + // Pre-seed with an empty string so Load never returns nil and the + // type-assertion in the read path is unconditional. + containerID.Store("") +} + +// getContainerID returns the container ID configured at the client creation. // It can either be auto-discovered with origin detection or provided by the user. // User-defined container ID is prioritized. func getContainerID() string { - return containerID + return containerID.Load().(string) +} + +func storeContainerID(id string) { + containerID.Store(id) +} + +func setContainerIDForTest(id string) { + containerID.Store(id) } diff --git a/statsd/container_linux.go b/statsd/container_linux.go index 12513234..2b2c7f42 100644 --- a/statsd/container_linux.go +++ b/statsd/container_linux.go @@ -193,27 +193,27 @@ func internalInitContainerID(userProvidedID string, cgroupFallback, isHostCgroup // readCIDOrInode reads the container ID from the user provided ID, cgroups or mountinfo. func readCIDOrInode(userProvidedID, cgroupPath, selfMountInfoPath, defaultCgroupMountPath string, cgroupFallback, isHostCgroupNs bool) { if userProvidedID != "" { - containerID = userProvidedID + storeContainerID(userProvidedID) return } if cgroupFallback { - containerID = readContainerID(cgroupPath) - if containerID != "" { + if id := readContainerID(cgroupPath); id != "" { + storeContainerID(id) return } - containerID = readMountinfo(selfMountInfoPath) - if containerID != "" { + if id := readMountinfo(selfMountInfoPath); id != "" { + storeContainerID(id) return } // If we're in the host cgroup namespace, the cid should be retrievable in /proc/self/cgroup // In private cgroup namespace, we can retrieve the cgroup controller inode. - if containerID == "" && isHostCgroupNs { + if isHostCgroupNs { return } - containerID = getCgroupInode(defaultCgroupMountPath, cgroupPath) + storeContainerID(getCgroupInode(defaultCgroupMountPath, cgroupPath)) } } diff --git a/statsd/container_stub.go b/statsd/container_stub.go index 0ea90f9b..6c62e5ab 100644 --- a/statsd/container_stub.go +++ b/statsd/container_stub.go @@ -10,7 +10,7 @@ func isHostCgroupNamespace() bool { var initContainerID = func(userProvidedID string, _, _ bool) { initOnce.Do(func() { if userProvidedID != "" { - containerID = userProvidedID + storeContainerID(userProvidedID) } }) } diff --git a/statsd/test_helpers_test.go b/statsd/test_helpers_test.go index 2bb96e90..00b6a8cc 100644 --- a/statsd/test_helpers_test.go +++ b/statsd/test_helpers_test.go @@ -645,11 +645,11 @@ func (ts *testServer) sendExtendedBasicAggregationMetricsWithPreAggregatedSample } func patchContainerID(id string) { - containerID = id + setContainerIDForTest(id) } func resetContainerID() { - containerID = "" + setContainerIDForTest("") initOnce = sync.Once{} } From 7a02a3c10dd30ac48ad722f27909122473980d39 Mon Sep 17 00:00:00 2001 From: Thomas Kowalski Date: Mon, 8 Jun 2026 10:44:05 +0200 Subject: [PATCH 4/4] test: add withoutOriginGlobals calls where relevant --- statsd/buffer_test.go | 11 +++++++++++ statsd/test_helpers_test.go | 9 ++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/statsd/buffer_test.go b/statsd/buffer_test.go index 4191d269..fb1d97f1 100644 --- a/statsd/buffer_test.go +++ b/statsd/buffer_test.go @@ -7,6 +7,7 @@ import ( ) func TestBufferGauge(t *testing.T) { + withoutOriginGlobals(t) buffer := newStatsdBuffer(1024, 1) err := buffer.writeGauge("namespace.", []string{"tag:tag"}, "metric", 1, []string{}, 1, noTimestamp, true, CardinalityNotSet) assert.Nil(t, err) @@ -44,6 +45,7 @@ func TestBufferGauge(t *testing.T) { } func TestBufferCount(t *testing.T) { + withoutOriginGlobals(t) buffer := newStatsdBuffer(1024, 1) err := buffer.writeCount("namespace.", []string{"tag:tag"}, "metric", 1, []string{}, 1, noTimestamp, true, CardinalityNotSet) assert.Nil(t, err) @@ -81,6 +83,7 @@ func TestBufferCount(t *testing.T) { } func TestBufferHistogram(t *testing.T) { + withoutOriginGlobals(t) buffer := newStatsdBuffer(1024, 1) err := buffer.writeHistogram("namespace.", []string{"tag:tag"}, "metric", 1, []string{}, 1, true, CardinalityNotSet) assert.Nil(t, err) @@ -112,6 +115,7 @@ func TestBufferHistogram(t *testing.T) { } func TestBufferDistribution(t *testing.T) { + withoutOriginGlobals(t) buffer := newStatsdBuffer(1024, 1) err := buffer.writeDistribution("namespace.", []string{"tag:tag"}, "metric", 1, []string{}, 1, true, CardinalityNotSet) assert.Nil(t, err) @@ -142,6 +146,7 @@ func TestBufferDistribution(t *testing.T) { assert.Equal(t, "namespace.metric:1|d|#tag:tag|c:container-id|e:external-env|card:low\n", string(buffer.bytes())) } func TestBufferSet(t *testing.T) { + withoutOriginGlobals(t) buffer := newStatsdBuffer(1024, 1) err := buffer.writeSet("namespace.", []string{"tag:tag"}, "metric", "value", []string{}, 1, true, CardinalityNotSet) assert.Nil(t, err) @@ -173,6 +178,7 @@ func TestBufferSet(t *testing.T) { } func TestBufferTiming(t *testing.T) { + withoutOriginGlobals(t) buffer := newStatsdBuffer(1024, 1) err := buffer.writeTiming("namespace.", []string{"tag:tag"}, "metric", 1, []string{}, 1, true, CardinalityNotSet) assert.Nil(t, err) @@ -204,6 +210,7 @@ func TestBufferTiming(t *testing.T) { } func TestBufferEvent(t *testing.T) { + withoutOriginGlobals(t) buffer := newStatsdBuffer(1024, 1) err := buffer.writeEvent(&Event{Title: "title", Text: "text"}, []string{"tag:tag"}, true, CardinalityNotSet) assert.Nil(t, err) @@ -235,6 +242,7 @@ func TestBufferEvent(t *testing.T) { } func TestBufferServiceCheck(t *testing.T) { + withoutOriginGlobals(t) buffer := newStatsdBuffer(1024, 1) err := buffer.writeServiceCheck(&ServiceCheck{Name: "name", Status: Ok}, []string{"tag:tag"}, true, CardinalityNotSet) assert.Nil(t, err) @@ -266,6 +274,7 @@ func TestBufferServiceCheck(t *testing.T) { } func TestBufferFullSize(t *testing.T) { + withoutOriginGlobals(t) buffer := newStatsdBuffer(30, 10) err := buffer.writeGauge("namespace.", []string{"tag:tag"}, "metric", 1, []string{}, 1, noTimestamp, true, CardinalityNotSet) assert.Nil(t, err) @@ -275,6 +284,7 @@ func TestBufferFullSize(t *testing.T) { } func TestBufferSeparator(t *testing.T) { + withoutOriginGlobals(t) buffer := newStatsdBuffer(1024, 10) err := buffer.writeGauge("namespace.", []string{"tag:tag"}, "metric", 1, []string{}, 1, noTimestamp, true, CardinalityNotSet) assert.Nil(t, err) @@ -284,6 +294,7 @@ func TestBufferSeparator(t *testing.T) { } func TestBufferAggregated(t *testing.T) { + withoutOriginGlobals(t) buffer := newStatsdBuffer(1024, 1) pos, err := buffer.writeAggregated([]byte("h"), "namespace.", []string{"tag:tag"}, "metric", []float64{1}, "", 12, -1, 1, true, CardinalityNotSet) assert.Nil(t, err) diff --git a/statsd/test_helpers_test.go b/statsd/test_helpers_test.go index 00b6a8cc..47088476 100644 --- a/statsd/test_helpers_test.go +++ b/statsd/test_helpers_test.go @@ -654,11 +654,10 @@ func resetContainerID() { } // withoutOriginGlobals clears the package-level containerID and externalEnv -// before the test runs and restores them after, so the test is independent -// of any other test in the package that may have populated them through a -// real client construction. Tests that build clients and assert wire -// payloads which do not include container-id / external-env tags should -// call this at the very top of the test body. +// so the test is independent of any other test in the package that may have +// populated them through a real client construction. Tests that build clients +// and assert wire payloads which do not include container id/external env +// tags should call this at the very top of the test body. func withoutOriginGlobals(t *testing.T) { t.Helper() resetContainerID()