update all of linkerd2-proxy-http to std::future#541
Merged
hawkw merged 11 commits intomaster-tokio-0.2from May 29, 2020
Merged
update all of linkerd2-proxy-http to std::future#541hawkw merged 11 commits intomaster-tokio-0.2from
linkerd2-proxy-http to std::future#541hawkw merged 11 commits intomaster-tokio-0.2from
Conversation
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
i don't know why this broke everything but it did! Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Its purpose was to convert between `tower_grpc`'s `Body` and `hyper`'s `Payload`. Now, both crates use the `http_body` crate's `Body` trait, so, as far as I can tell, these conversions are no longer necessary. Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
olix0r
approved these changes
May 29, 2020
hawkw
added a commit
that referenced
this pull request
Jun 1, 2020
This branch updates the proxy handle time metric and puts the layer that records it back in the stack. Depends on #541 (for `insert`). Signed-off-by: Eliza Weisman <[email protected]>
hawkw
added a commit
that referenced
this pull request
Jun 1, 2020
This branch updates the proxy's OpenCensus tracing code and collector client to use Tonic and `std::future`, and puts the OpenCensus stack layers back in the inbound and outbound proxy. Of note: the previous implementation constructed a single client `Service` which was reused when restarting a new RPC if the stream terminated. Because Tonic's generated RPCs borrow the client service mutably for the duration of the RPC future, if we want these futures to be `'static`, we must move the service into the future. Rather than requiring the service to be `Clone`, I solved this by replacing the `Service` instance held by the client with a `NewService` instance. I don't *think* this should be an issue, let me know if I'm wrong? I'd like to do some additional testing with the OpenCensus integration tests but I'm opening this PR now to start getting reviews. Depends on #541. Signed-off-by: Eliza Weisman <[email protected]>
hawkw
added a commit
that referenced
this pull request
Jun 9, 2020
**Note**: this branch was already approved (as #543), but was accidentally merged into the dev branch for #541, which it depended on and was set as the base until that branch merged. It looks like, when merging #543, I forgot to change the base branch back to `master-tokio-0.2`, and these changes never made it to master. --- This branch updates the proxy's OpenCensus tracing code and collector client to use Tonic and `std::future`, and puts the OpenCensus stack layers back in the inbound and outbound proxy. Of note: the previous implementation constructed a single client `Service` which was reused when restarting a new RPC if the stream terminated. Because Tonic's generated RPCs borrow the client service mutably for the duration of the RPC future, if we want these futures to be `'static`, we must move the service into the future. Rather than requiring the service to be `Clone`, I solved this by replacing the `Service` instance held by the client with a `NewService` instance. I don't *think* this should be an issue, let me know if I'm wrong? I'd like to do some additional testing with the OpenCensus integration tests but I'm opening this PR now to start getting reviews. Signed-off-by: Eliza Weisman <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR updates everything remaining in the
linkerd2-proxy-httpcrateto use
std::future. In particular, this includes the followingmodules:
add_headerinsertoverride_authorityheader_from_targetThe updates for these modules was pretty mechanical and there are no
significant changes, just updates to use the new versions of the
FutureandServicetraits.In addition, I've put back the stack layers from these modules, where
possible (in some cases, these modules were used by other code that has
yet to be updated or is pending in a separate branch). I've also
re-enabled some of the the
discoveryintegration tests for thelinkerd2-dst-overrideheader. Some of these tests depend on serviceprofiles as well, so they will be enabled once #537 merges.
Furthermore, I've done a little cleanup now that everything in the crate
has been updated. I removed the
grpcmodule entirely. Its only purposewas converting between the
tower_grpc::Bodyandhyper::Payloadtraits, and
tonicand thestd::futureversion ofhyperboth usethe
http_body::Bodytrait, so this conversion is unnecessary. Finally,since all the code which depended on the old versions of
tokioandfutureshas now been updated, I've removed the dependencies on thelegacy versions, and renamed the
futures_03andtokio_02renamingimports to just
futuresandtokio.Signed-off-by: Eliza Weisman [email protected]