From ec1305f687afe000e0f0f24294cc5ea8f81306b0 Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Thu, 18 Sep 2025 17:51:31 +0200 Subject: [PATCH 1/4] [chore][testbed] Use os.MkdirTemp on Windows --- testbed/testbed/child_process_collector.go | 16 +++++++++++++++- testbed/testbed/in_process_collector.go | 15 ++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/testbed/testbed/child_process_collector.go b/testbed/testbed/child_process_collector.go index 69cf64f56313e..1e81acf1a01dd 100644 --- a/testbed/testbed/child_process_collector.go +++ b/testbed/testbed/child_process_collector.go @@ -117,8 +117,22 @@ func (cp *childProcessCollector) PrepareConfig(t *testing.T, configStr string) ( configCleanup = func() { // NoOp } + + var dir string + if runtime.GOOS == "windows" { + // On Windows, use os.MkdirTemp to create directory since t.TempDir results in an error during cleanup in scoped-tests. + // See https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/42639 + var err error + dir, err = os.MkdirTemp("", t.Name()) //nolint:usetesting + if err != nil { + return configCleanup, fmt.Errorf("failed to create temporary directory: %w", err) + } + } else { + dir = t.TempDir() + } + var file *os.File - file, err = os.CreateTemp(t.TempDir(), "agent*.yaml") + file, err = os.CreateTemp(dir, "agent*.yaml") if err != nil { log.Printf("%s", err) return configCleanup, err diff --git a/testbed/testbed/in_process_collector.go b/testbed/testbed/in_process_collector.go index 058882b60847e..84aae0dc07723 100644 --- a/testbed/testbed/in_process_collector.go +++ b/testbed/testbed/in_process_collector.go @@ -52,7 +52,20 @@ func (ipp *inProcessCollector) PrepareConfig(t *testing.T, configStr string) (co func (ipp *inProcessCollector) Start(StartParams) error { var err error - confFile, err := os.CreateTemp(ipp.t.TempDir(), "conf-") + var dir string + if runtime.GOOS == "windows" { + // On Windows, use os.MkdirTemp to create directory since t.TempDir results in an error during cleanup in scoped-tests. + // See https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/42639 + var err error + dir, err = os.MkdirTemp("", ipp.t.Name()) //nolint:usetesting + if err != nil { + return fmt.Errorf("failed to create temporary directory: %w", err) + } + } else { + dir = ipp.t.TempDir() + } + + confFile, err := os.CreateTemp(dir, "conf-") if err != nil { return err } From 00b1f06d844f9202354214e3f15e389c697a8e2f Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Thu, 18 Sep 2025 18:01:56 +0200 Subject: [PATCH 2/4] Fix linting --- testbed/testbed/child_process_collector.go | 1 - testbed/testbed/in_process_collector.go | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/testbed/testbed/child_process_collector.go b/testbed/testbed/child_process_collector.go index 1e81acf1a01dd..03dcb5ef6f566 100644 --- a/testbed/testbed/child_process_collector.go +++ b/testbed/testbed/child_process_collector.go @@ -122,7 +122,6 @@ func (cp *childProcessCollector) PrepareConfig(t *testing.T, configStr string) ( if runtime.GOOS == "windows" { // On Windows, use os.MkdirTemp to create directory since t.TempDir results in an error during cleanup in scoped-tests. // See https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/42639 - var err error dir, err = os.MkdirTemp("", t.Name()) //nolint:usetesting if err != nil { return configCleanup, fmt.Errorf("failed to create temporary directory: %w", err) diff --git a/testbed/testbed/in_process_collector.go b/testbed/testbed/in_process_collector.go index 84aae0dc07723..4dcbcb2c5120d 100644 --- a/testbed/testbed/in_process_collector.go +++ b/testbed/testbed/in_process_collector.go @@ -56,8 +56,7 @@ func (ipp *inProcessCollector) Start(StartParams) error { if runtime.GOOS == "windows" { // On Windows, use os.MkdirTemp to create directory since t.TempDir results in an error during cleanup in scoped-tests. // See https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/42639 - var err error - dir, err = os.MkdirTemp("", ipp.t.Name()) //nolint:usetesting + dir, err = os.MkdirTemp("", ipp.t.Name()) if err != nil { return fmt.Errorf("failed to create temporary directory: %w", err) } From 425226008346ac2a969969ce385bd34cf13f4205 Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Fri, 19 Sep 2025 10:37:45 +0200 Subject: [PATCH 3/4] [chore] Move to internal/common/testutil and sanitize pattern like the testing package does --- cmd/opampsupervisor/go.mod | 1 + .../exporter_test.go | 14 +---- internal/common/testutil/testutil.go | 53 +++++++++++++++++++ testbed/testbed/child_process_collector.go | 15 +----- testbed/testbed/in_process_collector.go | 16 +----- 5 files changed, 59 insertions(+), 40 deletions(-) diff --git a/cmd/opampsupervisor/go.mod b/cmd/opampsupervisor/go.mod index 24628524f45c3..79cba2b82b91a 100644 --- a/cmd/opampsupervisor/go.mod +++ b/cmd/opampsupervisor/go.mod @@ -97,6 +97,7 @@ require ( github.com/open-telemetry/opentelemetry-collector-contrib/exporter/stefexporter v0.135.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/exporter/syslogexporter v0.135.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/exporter/zipkinexporter v0.135.0 // indirect + github.com/open-telemetry/opentelemetry-collector-contrib/internal/common v0.135.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal v0.135.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil v0.135.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/pkg/core/xidutils v0.135.0 // indirect diff --git a/exporter/prometheusremotewriteexporter/exporter_test.go b/exporter/prometheusremotewriteexporter/exporter_test.go index c948056150367..079b921cdd2ba 100644 --- a/exporter/prometheusremotewriteexporter/exporter_test.go +++ b/exporter/prometheusremotewriteexporter/exporter_test.go @@ -11,8 +11,6 @@ import ( "net/http" "net/http/httptest" "net/url" - "os" - "runtime" "strconv" "strings" "sync" @@ -759,18 +757,8 @@ func Test_PushMetrics(t *testing.T) { } if useWAL { - var dir string - if runtime.GOOS == "windows" { - // On Windows, use os.MkdirTemp to create directory since t.TempDir results in an error during cleanup in scoped-tests. - // See https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/42639 - var err error - dir, err = os.MkdirTemp("", tt.name) //nolint:usetesting - require.NoError(t, err) - } else { - dir = t.TempDir() - } cfg.WAL = configoptional.Some(WALConfig{ - Directory: dir, + Directory: testutil.TempDir(t), }) } diff --git a/internal/common/testutil/testutil.go b/internal/common/testutil/testutil.go index 823815fbb6c94..8358fda63adc9 100644 --- a/internal/common/testutil/testutil.go +++ b/internal/common/testutil/testutil.go @@ -6,11 +6,14 @@ package testutil // import "github.com/open-telemetry/opentelemetry-collector-co import ( "fmt" "net" + "os" "os/exec" "runtime" "strconv" "strings" "testing" + "unicode" + "unicode/utf8" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -151,3 +154,53 @@ func GetAvailablePort(tb testing.TB) int { func EndpointForPort(port int) string { return fmt.Sprintf("localhost:%d", port) } + +// TempDir creates a temporary directory in a safe way. On Linux, it just calls tb.TempDir. +// +// On Windows, it uses os.MkdirTemp to create directory since t.TempDir results in an error during cleanup in scoped-tests, +// possibly due to an interaction with the -count argument in `go test`. It does not do any cleanup (i.e. the directory is left on the filesystem). +// Prefer using tb.TempDir directly if possible. +// +// See https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/42639 +func TempDir(tb testing.TB) string { + if runtime.GOOS == "windows" { + name := sanitizePattern(tb.Name()) + dir, err := os.MkdirTemp("", name) + require.NoError(tb, err) + return dir + } + + return tb.TempDir() +} + +// sanitizePattern so that it works correctly with os.MkdirTemp. +// +// The following code is taken from the t.TempDir implementation. +// Copyright 2009 The Go Authors. All rights reserved. +// Taken from https://cs.opensource.google/go/go/+/refs/tags/go1.25.1:src/testing/testing.go;l=1340-1364;drc=49cdf0c42e320dfed044baa551610f081eafb781 +func sanitizePattern(pattern string) string { + // Limit length of file names on disk. + // Invalid runes from slicing are dropped by strings.Map below. + pattern = pattern[:min(len(pattern), 64)] + + // Drop unusual characters (such as path separators or + // characters interacting with globs) from the directory name to + // avoid surprising os.MkdirTemp behavior. + mapper := func(r rune) rune { + if r < utf8.RuneSelf { + const allowed = "!#$%&()+,-.=@^_{}~ " + if '0' <= r && r <= '9' || + 'a' <= r && r <= 'z' || + 'A' <= r && r <= 'Z' { + return r + } + if strings.ContainsRune(allowed, r) { + return r + } + } else if unicode.IsLetter(r) || unicode.IsNumber(r) { + return r + } + return -1 + } + return strings.Map(mapper, pattern) +} diff --git a/testbed/testbed/child_process_collector.go b/testbed/testbed/child_process_collector.go index 03dcb5ef6f566..5fd14c5cdede4 100644 --- a/testbed/testbed/child_process_collector.go +++ b/testbed/testbed/child_process_collector.go @@ -22,6 +22,7 @@ import ( "text/template" "time" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil" "github.com/shirou/gopsutil/v4/cpu" "github.com/shirou/gopsutil/v4/process" ) @@ -118,20 +119,8 @@ func (cp *childProcessCollector) PrepareConfig(t *testing.T, configStr string) ( // NoOp } - var dir string - if runtime.GOOS == "windows" { - // On Windows, use os.MkdirTemp to create directory since t.TempDir results in an error during cleanup in scoped-tests. - // See https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/42639 - dir, err = os.MkdirTemp("", t.Name()) //nolint:usetesting - if err != nil { - return configCleanup, fmt.Errorf("failed to create temporary directory: %w", err) - } - } else { - dir = t.TempDir() - } - var file *os.File - file, err = os.CreateTemp(dir, "agent*.yaml") + file, err = os.CreateTemp(testutil.TempDir(t), "agent*.yaml") if err != nil { log.Printf("%s", err) return configCleanup, err diff --git a/testbed/testbed/in_process_collector.go b/testbed/testbed/in_process_collector.go index 4dcbcb2c5120d..5e4b5df848176 100644 --- a/testbed/testbed/in_process_collector.go +++ b/testbed/testbed/in_process_collector.go @@ -13,6 +13,7 @@ import ( "testing" "time" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil" "github.com/shirou/gopsutil/v4/process" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" @@ -51,20 +52,7 @@ func (ipp *inProcessCollector) PrepareConfig(t *testing.T, configStr string) (co func (ipp *inProcessCollector) Start(StartParams) error { var err error - - var dir string - if runtime.GOOS == "windows" { - // On Windows, use os.MkdirTemp to create directory since t.TempDir results in an error during cleanup in scoped-tests. - // See https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/42639 - dir, err = os.MkdirTemp("", ipp.t.Name()) - if err != nil { - return fmt.Errorf("failed to create temporary directory: %w", err) - } - } else { - dir = ipp.t.TempDir() - } - - confFile, err := os.CreateTemp(dir, "conf-") + confFile, err := os.CreateTemp(testutil.TempDir(ipp.t), "conf-") if err != nil { return err } From 9c1c73e0fc98190bf4c0036985853aed32fd1c4a Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Fri, 19 Sep 2025 10:51:27 +0200 Subject: [PATCH 4/4] make generate and add nolint directive --- internal/common/testutil/testutil.go | 2 +- testbed/testbed/child_process_collector.go | 3 ++- testbed/testbed/in_process_collector.go | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/common/testutil/testutil.go b/internal/common/testutil/testutil.go index 8358fda63adc9..347a6f9718cb9 100644 --- a/internal/common/testutil/testutil.go +++ b/internal/common/testutil/testutil.go @@ -165,7 +165,7 @@ func EndpointForPort(port int) string { func TempDir(tb testing.TB) string { if runtime.GOOS == "windows" { name := sanitizePattern(tb.Name()) - dir, err := os.MkdirTemp("", name) + dir, err := os.MkdirTemp("", name) //nolint:usetesting require.NoError(tb, err) return dir } diff --git a/testbed/testbed/child_process_collector.go b/testbed/testbed/child_process_collector.go index 5fd14c5cdede4..90b02cfce2b04 100644 --- a/testbed/testbed/child_process_collector.go +++ b/testbed/testbed/child_process_collector.go @@ -22,9 +22,10 @@ import ( "text/template" "time" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil" "github.com/shirou/gopsutil/v4/cpu" "github.com/shirou/gopsutil/v4/process" + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil" ) // childProcessCollector implements the OtelcolRunner interface as a child process on the same machine executing diff --git a/testbed/testbed/in_process_collector.go b/testbed/testbed/in_process_collector.go index 5e4b5df848176..5aa22a23bfd29 100644 --- a/testbed/testbed/in_process_collector.go +++ b/testbed/testbed/in_process_collector.go @@ -13,13 +13,14 @@ import ( "testing" "time" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil" "github.com/shirou/gopsutil/v4/process" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/confmap/provider/fileprovider" "go.opentelemetry.io/collector/otelcol" + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil" ) // inProcessCollector implements the OtelcolRunner interfaces running a single otelcol as a go routine within the