From c855d73a74156fe52511dced9d9af3020140bb26 Mon Sep 17 00:00:00 2001 From: Zixin Zhou Date: Sun, 5 Jan 2025 21:29:17 +0800 Subject: [PATCH 1/4] [chore] add more Windows versions for testing --- .github/workflows/build-and-test-windows.yml | 3 ++- .github/workflows/e2e-tests-windows.yml | 18 +++++++++++++++--- .../exporter_concurrency_test.go | 10 ++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-and-test-windows.yml b/.github/workflows/build-and-test-windows.yml index dafbfe3838a8..b4d56ec35f48 100644 --- a/.github/workflows/build-and-test-windows.yml +++ b/.github/workflows/build-and-test-windows.yml @@ -45,7 +45,8 @@ jobs: - cmd-0 - cmd-1 - other - runs-on: windows-latest + os: [windows-2022, windows-2025] + runs-on: ${{ matrix.os }} if: ${{ github.actor != 'dependabot[bot]' && (contains(github.event.pull_request.labels.*.name, 'Run Windows') || github.event_name == 'push' || github.event_name == 'merge_group') }} env: # Limit memory usage via GC environment variables to avoid OOM on GH runners, especially for `cmd/otelcontribcol`, diff --git a/.github/workflows/e2e-tests-windows.yml b/.github/workflows/e2e-tests-windows.yml index bcfb4f38d865..6c1afe9c7f9a 100644 --- a/.github/workflows/e2e-tests-windows.yml +++ b/.github/workflows/e2e-tests-windows.yml @@ -32,7 +32,11 @@ jobs: - run: echo $(./.github/workflows/scripts/is_changed_file_windows.sh ${{ github.event.pull_request.base.sha }} ${{ github.sha }} ) collector-build: - runs-on: windows-latest + strategy: + fail-fast: false + matrix: + os: [windows-2022, windows-2025] + runs-on: ${{ matrix.os }} needs: [windows-file-changed] if: ${{ github.actor != 'dependabot[bot]' && ((contains(github.event.pull_request.labels.*.name, 'Run Windows') || github.event_name == 'push' || github.event_name == 'merge_group') || needs.windows-file-changed.outputs.changed == 'true') }} steps: @@ -65,7 +69,11 @@ jobs: path: ./bin/* supervisor-test: - runs-on: windows-latest + strategy: + fail-fast: false + matrix: + os: [windows-2022, windows-2025] + runs-on: ${{ matrix.os }} if: ${{ github.actor != 'dependabot[bot]' && (contains(github.event.pull_request.labels.*.name, 'Run Windows') || github.event_name == 'push' || github.event_name == 'merge_group') }} needs: [collector-build] steps: @@ -97,7 +105,11 @@ jobs: go test -v --tags=e2e windows-supervisor-service-test: - runs-on: windows-latest + strategy: + fail-fast: false + matrix: + os: [windows-2022, windows-2025] + runs-on: ${{ matrix.os }} if: ${{ github.actor != 'dependabot[bot]' && (contains(github.event.pull_request.labels.*.name, 'Run Windows') || github.event_name == 'push' || github.event_name == 'merge_group') }} needs: [collector-build] steps: diff --git a/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go b/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go index 689cbcf7b9fc..2a2b4f00184e 100644 --- a/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go +++ b/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go @@ -52,6 +52,10 @@ func Test_PushMetricsConcurrent(t *testing.T) { t.Fatal(err) } assert.NotNil(t, body) + if len(body) == 0 { + // No content, nothing to do. The request is just checking that the server is up. + return + } // Receives the http requests and unzip, unmarshalls, and extracts TimeSeries assert.Equal(t, "0.1.0", r.Header.Get("X-Prometheus-Remote-Write-Version")) assert.Equal(t, "snappy", r.Header.Get("Content-Encoding")) @@ -124,6 +128,12 @@ func Test_PushMetricsConcurrent(t *testing.T) { require.NoError(t, prwe.Shutdown(ctx)) }() + // Ensure that the test server is up before making the requests + assert.EventuallyWithT(t, func(c *assert.CollectT) { + _, checkRequestErr := http.Get(server.URL) + assert.NoError(c, checkRequestErr) + }, 5*time.Second, 100*time.Millisecond) + var wg sync.WaitGroup wg.Add(n) for _, m := range ms { From 9d2111a29661ff1fe59edbb9398d0e0e42c5e827 Mon Sep 17 00:00:00 2001 From: Zixin Zhou Date: Wed, 8 Jan 2025 13:15:18 +0800 Subject: [PATCH 2/4] [fix] adjust version and try to fix ci --- .github/workflows/e2e-tests-windows.yml | 2 +- .../prometheusremotewriteexporter/exporter_concurrency_test.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/e2e-tests-windows.yml b/.github/workflows/e2e-tests-windows.yml index 6c1afe9c7f9a..69b3f8ac8027 100644 --- a/.github/workflows/e2e-tests-windows.yml +++ b/.github/workflows/e2e-tests-windows.yml @@ -35,7 +35,7 @@ jobs: strategy: fail-fast: false matrix: - os: [windows-2022, windows-2025] + os: [windows-latest] runs-on: ${{ matrix.os }} needs: [windows-file-changed] if: ${{ github.actor != 'dependabot[bot]' && ((contains(github.event.pull_request.labels.*.name, 'Run Windows') || github.event_name == 'push' || github.event_name == 'merge_group') || needs.windows-file-changed.outputs.changed == 'true') }} diff --git a/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go b/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go index 2a2b4f00184e..5964fbf11b61 100644 --- a/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go +++ b/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go @@ -130,8 +130,9 @@ func Test_PushMetricsConcurrent(t *testing.T) { // Ensure that the test server is up before making the requests assert.EventuallyWithT(t, func(c *assert.CollectT) { - _, checkRequestErr := http.Get(server.URL) + resp, checkRequestErr := http.Get(server.URL) assert.NoError(c, checkRequestErr) + assert.NoError(c, resp.Body.Close()) }, 5*time.Second, 100*time.Millisecond) var wg sync.WaitGroup From 2ca8aa6f39bfad61c2c134e6f32b4927a03e6ac2 Mon Sep 17 00:00:00 2001 From: Zixin Zhou Date: Wed, 8 Jan 2025 13:30:43 +0800 Subject: [PATCH 3/4] [fix] change assert to require --- .../prometheusremotewriteexporter/exporter_concurrency_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go b/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go index 5964fbf11b61..13c3e5d9a38f 100644 --- a/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go +++ b/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go @@ -131,7 +131,7 @@ func Test_PushMetricsConcurrent(t *testing.T) { // Ensure that the test server is up before making the requests assert.EventuallyWithT(t, func(c *assert.CollectT) { resp, checkRequestErr := http.Get(server.URL) - assert.NoError(c, checkRequestErr) + require.NoError(c, checkRequestErr) assert.NoError(c, resp.Body.Close()) }, 5*time.Second, 100*time.Millisecond) From 6e3438d2ac0a8b9c665a4a3acab1fae1703d3642 Mon Sep 17 00:00:00 2001 From: Zixin Zhou Date: Thu, 9 Jan 2025 09:57:15 +0800 Subject: [PATCH 4/4] [fix] skip Test_PushMetricsConcurrent when win25 and merge upstream/main --- .../exporter_concurrency_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go b/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go index 13c3e5d9a38f..bf9fcbd968cc 100644 --- a/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go +++ b/exporter/prometheusremotewriteexporter/exporter_concurrency_test.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "net/http/httptest" + "os" "strconv" "sync" "testing" @@ -31,6 +32,9 @@ import ( // Test everything works when there is more than one goroutine calling PushMetrics. // Today we only use 1 worker per exporter, but the intention of this test is to future-proof in case it changes. func Test_PushMetricsConcurrent(t *testing.T) { + if os.Getenv("ImageOs") == "win25" && os.Getenv("GITHUB_ACTIONS") == "true" { + t.Skip("Skipping test on Windows 2025 GH runners, see https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/37104") + } n := 1000 ms := make([]pmetric.Metrics, n) testIDKey := "test_id"