-
Notifications
You must be signed in to change notification settings - Fork 5.9k
chore(migrate): migrate otelgrpc pkg interceptor to stats handler(#18258) #18366
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
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #18366 +/- ##
==========================================
- Coverage 55.79% 55.76% -0.04%
==========================================
Files 342 341 -1
Lines 57213 57196 -17
==========================================
- Hits 31920 31893 -27
- Misses 22655 22656 +1
- Partials 2638 2647 +9 ☔ View full report in Codecov by Sentry. |
464c408
to
37c0f77
Compare
[previous CI integratiion tests e2e log]
Modified the code based on the logs from the previous E2E test. |
37c0f77
to
c1e018a
Compare
@blakepettersson Can you check on that PR? |
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.
This LGTM @Jack-R-lantern
c1e018a
to
9e03229
Compare
@blakepettersson |
@blakepettersson |
@Jack-R-lantern looks like there's some duplicate work here: #22098 Would y'all mind cross-reviewing and making sure everything looks right? |
9e03229
to
8ad4295
Compare
1a9862c
to
3bb5dc4
Compare
Of course. |
3bb5dc4
to
b6f81d5
Compare
@Jack-R-lantern could you rebase? |
771d7d2
to
5a2cc73
Compare
@crenshaw-dev |
5a2cc73
to
da29bb0
Compare
Hm. Those test failures look very unrelated to these changes. |
I'll check about that part. |
What some failed tests have in common is
I wonder if the cmp server takes more to initialize with the new changes. Can you try bumping the sleep time to 1 second or more and see if the issue still persists, please? |
223c1d9
to
de20c41
Compare
migrate cmdserver, server, pkg/apiclient, reposerver - server - migrate ServerInterceptor -> NewServerHandler - client - migrate ClientInterceptor -> NewClientHandler fix server/application - add ClientStream GenerateManfiestWithFiles retry.Disable() remove util/grpc/trace - remvoe util/grpc/trace.go Signed-off-by: Jack-R-lantern <[email protected]>
de20c41
to
419981e
Compare
This PR will migrate the otelgrpc pkg.
It changes server/clientinterceptor in otelgrpc to NewServer/ClientHandler
I think that otelgrpc should be updated to 0.52.0 or higher in the future, in which case grpc should also be updated to 1.64.
Closes #18258
Checklist: