Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CI] Contrib tests should also test updated mdatagen #11167

Closed
mx-psi opened this issue Sep 13, 2024 · 2 comments · Fixed by #11223
Closed

[CI] Contrib tests should also test updated mdatagen #11167

mx-psi opened this issue Sep 13, 2024 · 2 comments · Fixed by #11223
Assignees
@PrateekKumar1709
Labels
ci-cd CI, CD, testing, build issues cmd/mdatagen good first issue Good for newcomers priority:p2 Medium

Comments

@mx-psi
Copy link
Member

mx-psi commented Sep 13, 2024

When we run make update-otel we update the mdatagen version as well. To avoid issues in contrib, we have the contrib tests but they are not testing this.

I believe in order to do this it would be as simple as adding a -replace option on check-contrib here https://github.com/open-telemetry/opentelemetry-collector/blob/5fc39ba6334f396c3b5497bf6d29db12823cb50c/Makefile#L260C1-L260C14 to also replace the version of mdatagen
@mx-psi mx-psi added good first issue Good for newcomers priority:p2 Medium ci-cd CI, CD, testing, build issues cmd/mdatagen labels Sep 13, 2024
@jade-guiton-dd
Copy link
Contributor

For fully testing the implications of updating mdatagen, I think it will also be necessary to forcefully rebuild .tools/mdatagen and run make generate in contrib. Adding a matching -dropreplace in the restore-contrib target will also be necessary.

@PrateekKumar1709
Copy link
Contributor

PrateekKumar1709 commented Sep 17, 2024 •
edited
Loading

Hi @mx-psi, I would like to work on this issue. Can you please assign this to me?

@TylerHelmuth TylerHelmuth mentioned this issue Sep 18, 2024
Merged
@PrateekKumar1709 PrateekKumar1709 mentioned this issue Sep 19, 2024
Merged
jackgopack4 pushed a commit to jackgopack4/opentelemetry-collector that referenced this issue Oct 8, 2024
@PrateekKumar1709 @jackgopack4
Add mdatagen replace option to check-contrib target (open-telemetry#1…
daa63d7 
…1223)

#### Description
This PR addresses the issue of contrib tests not testing the updated
mdatagen version. It modifies the `check-contrib` and `restore-contrib`
targets in the Makefile to ensure that the contrib tests use the local
version of the `mdatagen` package.

#### Link to tracking issue
Fixes open-telemetry#11167 

#### Testing
All existing tests pass with `make test`

Signed-off-by: Prateek Kumar <85689959+PrateekKumar1709@users.noreply.github.com>
@jade-guiton-dd jade-guiton-dd mentioned this issue Oct 14, 2024
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@PrateekKumar1709 PrateekKumar1709

Labels
ci-cd CI, CD, testing, build issues cmd/mdatagen good first issue Good for newcomers priority:p2 Medium
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add mdatagen replace option to check-contrib target PrateekKumar1709/opentelemetry-collector
3 participants
@mx-psi @PrateekKumar1709 @jade-guiton-dd