Make agent grpc-gateway work with grpc 1.18+#501
Make agent grpc-gateway work with grpc 1.18+#501songy23 merged 2 commits intocensus-instrumentation:masterfrom
Conversation
|
cc/ @songy23 |
Codecov Report
@@ Coverage Diff @@
## master #501 +/- ##
=======================================
Coverage 58.81% 58.81%
=======================================
Files 69 69
Lines 4507 4507
=======================================
Hits 2651 2651
Misses 1692 1692
Partials 164 164
Continue to review full report at Codecov.
|
|
@draffensperger we reverted grpc to 1.17.0 since there were other things broken. It looks like that we may skip v1.18.0 will this PR still be needed in this case? |
|
I'd suggest to put this PR on hold and merge it later once we roll forward the proto (and gRPC) updates. |
…-service into fix-grpc-gateway
|
The main thing I would like is for the agent grpc-gateway functionality to continue to work as future changes are made, since I'm using that for the opencensus-web project to write spans via HTTP. I'm open to waiting until the grpc dependency is updated but then I'd like this change to be done in the same PR as the upgrade to preserve the functionality. Given that, I actually think it would be simpler to just merge this now so that there will be one less thing to think about when upgrading to grpc 1.18 or 1.19. I updated the PR title and description to talk about this as a preparatory change for the upgrade rather than a fix. |
|
Makes sense @draffensperger. |
songy23
left a comment
There was a problem hiding this comment.
Given that, I actually think it would be simpler to just merge this now so that there will be one less thing to think about when upgrading to grpc 1.18 or 1.19. I updated the PR title and description to talk about this as a preparatory change for the upgrade rather than a fix.
SGTM
The
google.golang.org/grpcv1.18.0 release includes a change that made handshakes required by default. That when combined with thecmuxmultiplexer caused an "all SubConns are in TransientFailure" error message for HTTP calls made via grpc-gateway (see this issue comment for more details). The fix is to use a setting that cmux offers that handles the handshake correctly.Our grpc 1.18 upgrade was rolled back in #503 so this is not immediately needed, but having it in place will make the grpc-gateway functionality continue to work once the grpc dependency is upgraded past 1.18.
See #492 - this will enable the grpc-gateway test to keep passing (and the functionality to keep working) after the future grpc upgrade.