-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
go.mod: upgrade opentelemetry deps #13888
Conversation
ad000c8
to
aeb7b1b
Compare
Is there a way to run the workflow locally? |
Error in Static analysis:
|
Thanks! I'll take a look tomorrow and correct that |
If I'm reading that correctly, I just need to update |
For future people: you can run static analysis yourself. For the above failure (inconsistent dependencies), for example, I can use this command:
This isn't guaranteed to catch all failures as there could be architecture-specific failures, but it will likely catch most of them. |
I see - the main intent of these failures is that:
|
c7fcde7
to
dfcd6f8
Compare
Ok - new failure this time, so making progress. Here is the failure: https://gist.github.com/willbeason/c774057148da7dc69fb1bf0fbc5d166c This failure can be recreated with the command:
This seems to ensure that various files that list dependencies all contain the appropriate records |
The solution here was to run this script which automatically fixes these issues:
|
That's odd - the only change in the code is running |
Ah, I see from the failure that it is a flake:
|
Yep, it's flaky. #13448 |
Hooray, static analysis passed! |
@willbeason appreciate any feedback on how we could make contributions easier :P. |
@serathius What would have made things faster for me would be:
|
Created #13896, feel free to add more. |
Thanks! I'm really hoping that this PR doesn't run into the same issues that the previous PRs ran into. The deps here are higher versions than the others, but the related issues haven't been closed, so fingers crossed. |
Drat! Integration tests failing with timeouts like previous PRs. |
Heads up: I'll be intermittently available tomorrow, and I'm OOO Friday. I'll be back Monday. I'll still be on for a few more hours today. Looking at first failure: https://gist.github.com/willbeason/e6b372803acbf58d16bc27a533d338ac So for the TestV3StorageQuotaApply, there's a goroutine leak before the test even starts. That should help narrow down when the goroutine is leaked. |
That's odd - running the test in isolation locally passes:
(note that since I ran the test 10x, the runtime is 10x longer than usual) So this means that a test which runs before TestV3StorageQuotaApply causes the problem |
@serathius I see that while many tests in Do I have this right? |
Codecov Report
@@ Coverage Diff @@
## main #13888 +/- ##
==========================================
- Coverage 72.39% 72.30% -0.10%
==========================================
Files 469 469
Lines 38402 38402
==========================================
- Hits 27802 27765 -37
- Misses 8813 8855 +42
+ Partials 1787 1782 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@willbeason yes, we have a lot of flaky tests and have effort to track and fix them. Please ignore the test failures if they are flakes. I will rerun the test if needed. Please fix the conflict and rebase the PR. |
@serathius Done! |
|
1f060a9
to
8dd55bf
Compare
Updated, and manually verified locally that "dep" and "bom" pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a nit
@ahrtr Fixed! I don't think I have permissions to merge. |
Please squash the commits and I will merge as soon as the tests are green. |
Downstream users of etcd experience build issues when using dependencies which require more recent (incompatible) versions of opentelemetry. This commit upgrades the dependencies so that downstream users stop experiencing these issues.
aabf426
to
eab1e0c
Compare
@ptabor Ok - squashed commits |
Downstream users of etcd experience build issues when using dependencies
which require more recent (incompatible) versions of opentelemetry. This
commit upgrades the dependencies so that downstream users stop
experiencing these issues.
This will resolve: