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

Updated grpc-go to v1.29.1 #2445

Merged
merged 1 commit into from
Sep 2, 2020
Merged

Updated grpc-go to v1.29.1 #2445

merged 1 commit into from
Sep 2, 2020

Conversation

jpkrohling
Copy link
Contributor

Closes #2443

Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de

@jpkrohling jpkrohling requested a review from a team as a code owner September 1, 2020 15:29
Copy link
Member

@annanay25 annanay25 left a comment

Choose a reason for hiding this comment

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

This upgrade seems to cover a bunch of breaking changes to client side load balancing in grpc, but since we don't use any of that in the agent, the one change that might affect us is probably this - grpc/grpc#23086

DNS resolutions will now be sorted.

lgtm, modulo the green CI!

@yurishkuro
Copy link
Member

test failed

--- FAIL: TestTraceSpanIDMarshalProto (0.00s)
    --- FAIL: TestTraceSpanIDMarshalProto/protobuf (0.00s)
panic: invalid Go type model.TraceID for field jaeger.api_v2.SpanRef.trace_id [recovered]
	panic: invalid Go type model.TraceID for field jaeger.api_v2.SpanRef.trace_id

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@jpkrohling jpkrohling changed the title Updated grpc-go to v1.31.1 Updated grpc-go to v1.29.1 Sep 2, 2020
@jpkrohling
Copy link
Contributor Author

This update sent be to a rabbit hole, and I think it requires more effort than I'm willing to invest right now. So, I'm updating this PR to bump it to 1.29.1 instead, which does fix the original bug I was trying to solve.

That said, I'll open another issue to track the work for updating gRPC to latest versions and will try to allocate some time to work on it in case we have no volunteers.

@jpkrohling
Copy link
Contributor Author

Created #2449 to track the upgrade to 1.31.x

@codecov
Copy link

codecov bot commented Sep 2, 2020

Codecov Report

Merging #2445 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2445      +/-   ##
==========================================
- Coverage   95.58%   95.56%   -0.02%     
==========================================
  Files         208      208              
  Lines       10679    10679              
==========================================
- Hits        10207    10205       -2     
- Misses        399      400       +1     
- Partials       73       74       +1     
Impacted Files Coverage Δ
cmd/query/app/server.go 91.11% <0.00%> (-2.23%) ⬇️
...lugin/sampling/strategystore/adaptive/processor.go 99.20% <0.00%> (-0.80%) ⬇️
plugin/storage/integration/integration.go 80.86% <0.00%> (+0.47%) ⬆️
pkg/discovery/grpcresolver/grpc_resolver.go 100.00% <0.00%> (+1.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 319e2fb...c8fe8ac. Read the comment docs.

@jpkrohling jpkrohling changed the title Updated grpc-go to v1.29.1 WIP - Updated grpc-go to v1.29.1 Sep 2, 2020
@jpkrohling jpkrohling changed the title WIP - Updated grpc-go to v1.29.1 Updated grpc-go to v1.29.1 Sep 2, 2020
@yurishkuro yurishkuro merged commit 21de269 into jaegertracing:master Sep 2, 2020
albertteoh pushed a commit to albertteoh/jaeger that referenced this pull request Sep 3, 2020
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
Signed-off-by: albertteoh <albert.teoh@logz.io>
@jpkrohling jpkrohling deleted the jpkrohling/issue2443 branch July 28, 2021 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump gRPC dependency to v1.29.x
3 participants