-
Notifications
You must be signed in to change notification settings - Fork 89
Sink service client & unit tests #639
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
Merged
mum4k
merged 9 commits into
envoyproxy:main
from
oschaaf:horizontal-scaling-sink-client
Mar 13, 2021
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
bdef102
Sink service client & unit tests
d09ea56
Tweak comments
ac07d68
Merge remote-tracking branch 'upstream/main' into horizontal-scaling-…
fe2eac3
Review: move sink api into its own space for clarity
d89620e
Review: Clarify that execution_id is required. Also, add validation.
f64a229
Review: consistently pass stub interface by ref
3734e54
Review: add comment to clarify we panic on >1 response messages.
78dad24
Review: remove stray code comment
5c49c60
Review: don't use a separate .sink namespace/package
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| load("@com_github_grpc_grpc//bazel:cc_grpc_library.bzl", "cc_grpc_library") | ||
| load("@envoy_api//bazel:api_build_system.bzl", "api_cc_py_proto_library") | ||
|
|
||
| licenses(["notice"]) # Apache 2 | ||
|
|
||
| api_cc_py_proto_library( | ||
| name = "sink", | ||
| srcs = [ | ||
| "sink.proto", | ||
| ], | ||
| visibility = ["//visibility:public"], | ||
| deps = [ | ||
| "//api/client:base", | ||
| "@envoy_api//envoy/config/core/v3:pkg", | ||
| ], | ||
| ) | ||
|
|
||
| cc_grpc_library( | ||
| name = "sink_grpc_lib", | ||
| srcs = [ | ||
| ":sink", | ||
| ], | ||
| generate_mocks = True, | ||
| grpc_only = True, | ||
| proto_only = False, | ||
| use_external = False, | ||
| visibility = ["//visibility:public"], | ||
| well_known_protos = True, | ||
| deps = [ | ||
| ":sink_cc_proto", | ||
| ], | ||
| ) |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| syntax = "proto3"; | ||
|
|
||
| package nighthawk; | ||
|
|
||
| import "api/client/service.proto"; | ||
| import "validate/validate.proto"; | ||
|
|
||
| // Encapsulates an ExecutionResponse. | ||
| message StoreExecutionRequest { | ||
| // Response contains the effective execution id, which will serve as the lookup key. | ||
| nighthawk.client.ExecutionResponse execution_response = 1; | ||
| } | ||
|
|
||
| // Empty return value message, that serves as a future extension point. | ||
| message StoreExecutionResponse { | ||
| } | ||
|
|
||
| message SinkRequest { | ||
| // Unique id for lookup purposes. Required. | ||
| string execution_id = 1 [(validate.rules).string.min_len = 1]; | ||
| } | ||
|
|
||
| message SinkResponse { | ||
| // Response associated to the requested execution id. | ||
| nighthawk.client.ExecutionResponse execution_response = 1; | ||
| } | ||
|
|
||
| service NighthawkSink { | ||
| // Accepts a stream of execution responses, which is the return value of | ||
| // NighthawkService.ExecutionStream. Workers can forward their results using this method. | ||
| rpc StoreExecutionResponseStream(stream StoreExecutionRequest) returns (StoreExecutionResponse) { | ||
| } | ||
|
|
||
| // Gets the stored responses associated to an execution, keyed by execution id. | ||
| rpc SinkRequestStream(stream SinkRequest) returns (stream SinkResponse) { | ||
| } | ||
| } |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| #pragma once | ||
| #include "envoy/common/pure.h" | ||
|
|
||
| #include "external/envoy/source/common/common/statusor.h" | ||
| #include "external/envoy/source/common/protobuf/protobuf.h" | ||
|
|
||
| #include "api/sink/sink.grpc.pb.h" | ||
|
|
||
| namespace Nighthawk { | ||
|
|
||
| /** | ||
| * Interface of a gRPC sink service client. | ||
| */ | ||
| class NighthawkSinkClient { | ||
| public: | ||
| virtual ~NighthawkSinkClient() = default; | ||
|
|
||
| /** | ||
| * @brief Store an execution response. | ||
| * | ||
| * @param nighthawk_sink_stub Used to open a channel to the sink service. | ||
| * @param store_execution_request Provide the message that the sink should store. | ||
| * @return absl::StatusOr<nighthawk::StoreExecutionResponse> | ||
| */ | ||
| virtual absl::StatusOr<nighthawk::StoreExecutionResponse> StoreExecutionResponseStream( | ||
| nighthawk::NighthawkSink::StubInterface& nighthawk_sink_stub, | ||
| const nighthawk::StoreExecutionRequest& store_execution_request) const PURE; | ||
|
|
||
| /** | ||
| * Look up ExecutionResponse messages in the sink. | ||
| * | ||
| * @param nighthawk_sink_stub Used to open a channel to the sink service. | ||
| * @param sink_request Provide the message that the sink should handle. | ||
| * @return absl::StatusOr<nighthawk::SinkResponse> Either a status indicating failure, or | ||
| * a SinkResponse upon success. | ||
| */ | ||
| virtual absl::StatusOr<nighthawk::SinkResponse> | ||
| SinkRequestStream(nighthawk::NighthawkSink::StubInterface& nighthawk_sink_stub, | ||
| const nighthawk::SinkRequest& sink_request) const PURE; | ||
| }; | ||
|
|
||
| } // namespace Nighthawk |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| #include "common/nighthawk_sink_client_impl.h" | ||
|
|
||
| #include "external/envoy/source/common/common/assert.h" | ||
|
|
||
| namespace Nighthawk { | ||
|
|
||
| absl::StatusOr<nighthawk::StoreExecutionResponse> | ||
| NighthawkSinkClientImpl::StoreExecutionResponseStream( | ||
| nighthawk::NighthawkSink::StubInterface& nighthawk_sink_stub, | ||
| const nighthawk::StoreExecutionRequest& store_execution_request) const { | ||
| ::grpc::ClientContext context; | ||
| ::nighthawk::StoreExecutionResponse store_execution_response; | ||
| std::shared_ptr<::grpc::ClientWriterInterface<::nighthawk::StoreExecutionRequest>> stream( | ||
| nighthawk_sink_stub.StoreExecutionResponseStream(&context, &store_execution_response)); | ||
| if (!stream->Write(store_execution_request)) { | ||
| return absl::UnavailableError("Failed to write request to the Nighthawk Sink gRPC channel."); | ||
| } else if (!stream->WritesDone()) { | ||
| return absl::InternalError("WritesDone() failed on the Nighthawk Sink gRPC channel."); | ||
| } | ||
| ::grpc::Status status = stream->Finish(); | ||
| if (!status.ok()) { | ||
| return absl::Status(static_cast<absl::StatusCode>(status.error_code()), status.error_message()); | ||
| } | ||
| return store_execution_response; | ||
| } | ||
|
|
||
| absl::StatusOr<nighthawk::SinkResponse> NighthawkSinkClientImpl::SinkRequestStream( | ||
| nighthawk::NighthawkSink::StubInterface& nighthawk_sink_stub, | ||
| const nighthawk::SinkRequest& sink_request) const { | ||
| nighthawk::SinkResponse response; | ||
|
|
||
| ::grpc::ClientContext context; | ||
| std::shared_ptr< | ||
| ::grpc::ClientReaderWriterInterface<nighthawk::SinkRequest, nighthawk::SinkResponse>> | ||
| stream(nighthawk_sink_stub.SinkRequestStream(&context)); | ||
|
|
||
| if (!stream->Write(sink_request)) { | ||
| return absl::UnavailableError("Failed to write request to the Nighthawk Sink gRPC channel."); | ||
| } else if (!stream->WritesDone()) { | ||
| return absl::InternalError("WritesDone() failed on the Nighthawk Sink gRPC channel."); | ||
| } | ||
|
|
||
| bool got_response = false; | ||
| while (stream->Read(&response)) { | ||
| /* | ||
| At the proto api level we support returning a stream of results. The sink service proto api | ||
| reflects this, and accepts what NighthawkService. ExecutionStream returns as a parameter | ||
| (though we wrap it in StoreExecutionRequest messages for extensibility purposes). So this | ||
| implies a stream, and not a single message. | ||
|
|
||
| Having said that, today we constrain what we return to a single message in the implementations | ||
| where this is relevant. That's why we assert here, to make sure that stays put until an | ||
| explicit choice is made otherwise. | ||
|
|
||
| Why do this? The intent of NighthawkService. ExecutionStream was to be able to stream | ||
| intermediate updates some day. So having streams in the api's keeps the door open on streaming | ||
| intermediary updates, without forcing a change the proto api. | ||
| */ | ||
| RELEASE_ASSERT(!got_response, | ||
mum4k marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "Sink Service has started responding with more than one message."); | ||
| got_response = true; | ||
| } | ||
| ::grpc::Status status = stream->Finish(); | ||
| if (!status.ok()) { | ||
| return absl::Status(static_cast<absl::StatusCode>(status.error_code()), status.error_message()); | ||
| } | ||
| return response; | ||
| } | ||
|
|
||
| } // namespace Nighthawk | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| #include "nighthawk/common/nighthawk_sink_client.h" | ||
|
|
||
| #include "external/envoy/source/common/common/statusor.h" | ||
| #include "external/envoy/source/common/protobuf/protobuf.h" | ||
|
|
||
| namespace Nighthawk { | ||
|
|
||
| /** | ||
| * Implements a the gRPC sink client interface. | ||
| * | ||
| * This class is stateless and may be called from multiple threads. Furthermore, the same gRPC stub | ||
| * is safe to use from multiple threads simultaneously. | ||
| */ | ||
| class NighthawkSinkClientImpl : public NighthawkSinkClient { | ||
| public: | ||
| absl::StatusOr<nighthawk::StoreExecutionResponse> StoreExecutionResponseStream( | ||
| nighthawk::NighthawkSink::StubInterface& nighthawk_sink_stub, | ||
| const nighthawk::StoreExecutionRequest& store_execution_request) const override; | ||
|
|
||
| absl::StatusOr<nighthawk::SinkResponse> | ||
| SinkRequestStream(nighthawk::NighthawkSink::StubInterface& nighthawk_sink_stub, | ||
| const nighthawk::SinkRequest& sink_request) const override; | ||
| }; | ||
|
|
||
| } // namespace Nighthawk |
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.