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.

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

MAIN B-20627 Fix circleci tests and bump dependency #13913

Merged

Conversation

msaki-caci
Copy link
Contributor

@msaki-caci msaki-caci commented Oct 15, 2024

B-20627

INTEGRATION PR-#13866

Summary

The package otelhttp:v.0.43.0 is outdated and needs to be bumped up to the latest version otelhttp:v.0.55.0.

Normally this is managed through dependabot automatically but each time dependabot attempted the bump there were test failures in circlci. You can find the trail of failed PRs by going here. The latest results show that the dependency bump was failing one of the automated tests run by circleci, server_test.

It appears that the test is failing due to the use of deprecated methods http.server.request_content_length and http.server.response_content_length. OpenTelemetry-Go Contrib removed these in otelhttp:v0.49.0 release. It appears that the new methods that need to be used are http.server.request.size and http.server.response.size.

After these tests have been fixed, the package should be able to upgraded in the go.mod file to the latest version otelhttp:v.0.55.0 with all circleci tests passing.

Verification Steps for the Author

These are to be checked by the author.

  • Have the Agility acceptance criteria been met for this change?

Verification Steps for Reviewers

These are to be checked by a reviewer.

  • Has the branch been pulled in and checked out?
  • Have the BL acceptance criteria been met for this change?
  • Was the CircleCI build successful?
  • Has the code been reviewed from a standards and best practices point of view?

Setup to Run the Code

How to test

  1. Login to GitHub.
  2. Open up a DRAFT pull request for MAIN-B-20627-Fix-circleci-tests-and-upgrade-dependency to be merged into main.
  3. Scroll down to the automated tests and wait for the server_test to finish.
  4. Verify the test has passed.

Note

Make sure to make the pull request a draft.

Screenshots

image

Updated method calls for http.server.request_content_length and
http.server.response_content_length. These are deprecated and we
should be using http.server.request.size and http.server.response.size.
Also updated the package dependency for otelhttp:v0.43.0 to
otelhttp:v0.55.0.
@msaki-caci msaki-caci added dependencies Pull requests that update a dependency file security Pull requests that address a security vulnerability MAIN Go-Rillaz Go-Rillaz labels Oct 15, 2024
@msaki-caci msaki-caci self-assigned this Oct 15, 2024
@msaki-caci msaki-caci requested a review from a team as a code owner October 15, 2024 16:38
Copy link
Contributor

@ryan-mchugh ryan-mchugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good from my end. Not sure if there is a separate checklist/requirement check for MAIN merges

@msaki-caci
Copy link
Contributor Author

Good from my end. Not sure if there is a separate checklist/requirement check for MAIN merges

Didn't see anything on my end kind of just seemed like a copy and paste according to https://transcom.github.io/mymove-docs/docs/getting-started/development/how-to-create-and-submit-PR

@msaki-caci
Copy link
Contributor Author

@deandreJones I think everything should be good to go for main.

@deandreJones deandreJones merged commit bb30a1f into main Oct 17, 2024
43 checks passed
@deandreJones deandreJones deleted the MAIN-B-20627-Fix-circleci-tests-and-upgrade-dependency branch October 17, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Go-Rillaz Go-Rillaz MAIN security Pull requests that address a security vulnerability
Development

Successfully merging this pull request may close these issues.

6 participants