Fix conditionals that trigger Artifact upload by ArthurSens · Pull Request #38236 · open-telemetry/opentelemetry-collector-contrib

ArthurSens · 2025-02-26T19:51:19Z

Description

After the revert done in #38230 and #38231, I'm looking yet again on what went wrong there.

I'm solving this in small parts, and this PR focuses on ensuring JUnit Artifacts are uploaded correctly.

Link to tracking issue

Still related to #36761

Testing

I plan to test this by looking at the artifacts produced in this PR. The JUnit files need to be there. See https://github.com/actions/upload-artifact?tab=readme-ov-file#where-does-the-upload-go

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

ArthurSens · 2025-02-26T20:05:07Z

Ok, I noticed that running tests twice (one without JUnit and another with JUnit) takes a lot of time; I think we want:

Tests without JUnit artifacts running on branch != main
Tests with JUnit artifacts running on branch == main

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

ArthurSens · 2025-02-26T21:31:18Z

-        if: startsWith( matrix.go-version, '1.23' ) # only run junit/coverage on one version
+        if: startsWith( matrix.go-version, '~1.23' ) # only run junit/coverage on one version
        run: make gotest-with-junit-and-cover GROUP=${{ matrix.group }}
      - uses: actions/upload-artifact@v4
-        if: startsWith( matrix.go-version, '1.23' ) # only upload artifact for one version
+        if: startsWith( matrix.go-version, '~1.23' ) # only upload artifact for one version
        with:
          name: coverage-artifacts-${{ matrix.go-version }}-${{ matrix.runner }}-${{ matrix.group }}
          path: ${{ matrix.group }}-coverage.txt
      - uses: actions/upload-artifact@v4
-        if: startsWith( matrix.go-version, '1.23' ) # only upload artifact for one version
+        if: startsWith( matrix.go-version, '~1.23' ) # only upload artifact for one version


@mx-psi @songy2, the problem was that the tilde was missing in the previous PRs. You can check this workflow that ran from the first commit: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/13552199813/job/37878313056?pr=38236

There's one step called Debug startWith Condition that shows what's expected

Ah, nice catch!

btw you tagged the wrong person :)))

ArthurSens · 2025-02-26T21:32:35Z

@@ -289,15 +289,15 @@ jobs:
        if: startsWith( matrix.go-version, '1.23' ) != true


Related to the comment above, what do we want to do with this part specifically? Currently we're running tests in both go versions, is that the intended behavior? Should we be testing only with go 1.23? go 1.24?

We should be testing both versions, that looks a bit weird yep https://github.com/open-telemetry/opentelemetry-collector?tab=readme-ov-file#compatibility

Ok, I'll just remove that conditional then... Just creates confusion

ArthurSens · 2025-02-26T21:37:21Z

Artifacts were uploaded correctly :)

https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/13552199813

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

ArthurSens · 2025-02-27T13:15:39Z

I still want to make the two steps running tests mutually exclusive. I'm not sure if it's worth running a 1h long test execution on every PR if we're not using the JUnit artifacts on every PR

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

mx-psi · 2025-02-27T13:17:54Z

I still want to make the two steps running tests mutually exclusive. I'm not sure if it's worth running a 1h long test execution on every PR if we're not using the JUnit artifacts on every PR

That makes sense to me 👍 will wait to merge this until Yang has reviewed and you feel ready for it

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

ArthurSens · 2025-02-27T13:28:52Z

I still want to make the two steps running tests mutually exclusive. I'm not sure if it's worth running a 1h long test execution on every PR if we're not using the JUnit artifacts on every PR

That makes sense to me 👍 will wait to merge this until Yang has reviewed and you feel ready for it

Should be ok from my side

songy23

Thanks for catching this!

songy23 · 2025-02-27T14:43:17Z

All good from my end, let me know when you would like to merge 🚀

#### Description Uploads to codecov have been broken since #37875 was merged. Efforts have been made to address the reasoning behind why it's not working (eg #38236), but it's still failing. [Example recent failure:](https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/14409354807/job/40413708600) ``` Warning: No files were found with the provided path: receiver-0-coverage.txt. No artifacts will be uploaded. ``` This failure is hit [here](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/2e35cae16745dab737e1d0bf636119b4c669c9cd/.github/workflows/build-and-test.yml#L305). This PR aims to correct the naming and location of code coverage files to enable the successful upload to codecov.

#### Description Uploads to codecov have been broken since open-telemetry#37875 was merged. Efforts have been made to address the reasoning behind why it's not working (eg open-telemetry#38236), but it's still failing. [Example recent failure:](https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/14409354807/job/40413708600) ``` Warning: No files were found with the provided path: receiver-0-coverage.txt. No artifacts will be uploaded. ``` This failure is hit [here](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/2e35cae16745dab737e1d0bf636119b4c669c9cd/.github/workflows/build-and-test.yml#L305). This PR aims to correct the naming and location of code coverage files to enable the successful upload to codecov.

#### Description Now that we're uploading files correctly (#38236), this PR extends the current unit test workflow to download the artifacts and calling issuegenerator to create the issues.  #### Link to tracking issue Related to #36761 Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

#### Description Uploads to codecov have been broken since open-telemetry#37875 was merged. Efforts have been made to address the reasoning behind why it's not working (eg open-telemetry#38236), but it's still failing. [Example recent failure:](https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/14409354807/job/40413708600) ``` Warning: No files were found with the provided path: receiver-0-coverage.txt. No artifacts will be uploaded. ``` This failure is hit [here](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/2e35cae16745dab737e1d0bf636119b4c669c9cd/.github/workflows/build-and-test.yml#L305). This PR aims to correct the naming and location of code coverage files to enable the successful upload to codecov.

sincejune · 2025-05-01T06:56:32Z

CodeCov report on PR seems unavailable after this change. Is this expected behavior?

Fix conditionals that trigger Artifact upload

284a552

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

ArthurSens requested a review from a team as a code owner February 26, 2025 19:51

ArthurSens requested a review from TylerHelmuth February 26, 2025 19:51

ArthurSens marked this pull request as draft February 26, 2025 19:51

github-actions Bot assigned crobert-1 Feb 26, 2025

Remove step used for debugging

6288e04

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

ArthurSens commented Feb 26, 2025

View reviewed changes

Dont't stop workflow when tests fail, allowing artifact upload

26e86e0

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

ArthurSens force-pushed the fix-upload-artifacts branch from ce755e3 to 26e86e0 Compare February 26, 2025 22:18

ArthurSens marked this pull request as ready for review February 26, 2025 22:18

github-actions Bot assigned mx-psi Feb 26, 2025

Only fail workflow if outcome == failure

591324a

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

mx-psi added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Feb 27, 2025

mx-psi approved these changes Feb 27, 2025

View reviewed changes

mx-psi requested a review from songy23 February 27, 2025 12:53

Remove unnecessary conditional for regular test execution

1c0d68f

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

Make regular test and JUnit tests mutually exclusive

36b3fd8

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>

songy23 approved these changes Feb 27, 2025

View reviewed changes

songy23 added the ci-cd CI, CD, testing, build issues label Feb 27, 2025

mx-psi merged commit 6b0a752 into open-telemetry:main Feb 27, 2025

github-actions Bot added this to the next release milestone Feb 27, 2025

ArthurSens deleted the fix-upload-artifacts branch February 27, 2025 15:00

ArthurSens mentioned this pull request Feb 28, 2025

Add step that generate issues based on JUnit test results #38270

Merged

crobert-1 mentioned this pull request Apr 11, 2025

[chore][codecov] Fix coverage file naming #39356

Merged

		@@ -289,15 +289,15 @@ jobs:
		if: startsWith( matrix.go-version, '1.23' ) != true

Conversation

ArthurSens commented Feb 26, 2025

Description

Link to tracking issue

Testing

Uh oh!

ArthurSens commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArthurSens Feb 26, 2025

Choose a reason for hiding this comment

Uh oh!

mx-psi Feb 27, 2025

Choose a reason for hiding this comment

Uh oh!

songy23 Feb 27, 2025

Choose a reason for hiding this comment

Uh oh!

ArthurSens Feb 26, 2025

Choose a reason for hiding this comment

Uh oh!

mx-psi Feb 27, 2025

Choose a reason for hiding this comment

Uh oh!

ArthurSens Feb 27, 2025

Choose a reason for hiding this comment

Uh oh!

ArthurSens commented Feb 26, 2025

Uh oh!

ArthurSens commented Feb 27, 2025

Uh oh!

mx-psi commented Feb 27, 2025

Uh oh!

ArthurSens commented Feb 27, 2025

Uh oh!

songy23 left a comment

Choose a reason for hiding this comment

Uh oh!

songy23 commented Feb 27, 2025

Uh oh!

sincejune commented May 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ArthurSens commented Feb 26, 2025 •

edited

Loading