Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
8ea442d
Merge pull request #5 from envoyproxy/master
eric846 Jun 1, 2020
5ac755a
Merge pull request #6 from envoyproxy/master
eric846 Jun 28, 2020
b8c25a5
Merge pull request #7 from envoyproxy/master
eric846 Jul 7, 2020
1c19c68
initial commit
eric846 Jul 9, 2020
7050686
fix comments
eric846 Jul 9, 2020
0776563
fix format
eric846 Jul 9, 2020
16fd8f6
rename adaptive_rps to adaptive_load
eric846 Jul 10, 2020
c383010
add field_selector in example
eric846 Jul 10, 2020
6e1a483
fix example comment
eric846 Jul 10, 2020
4ef1140
fix format
eric846 Jul 10, 2020
4111bf4
add support for fault injection headers
eric846 Jul 10, 2020
871a959
replace linear and binary search with exponential search
eric846 Jul 10, 2020
1fd77c1
add InputVariableSetter mechanism
eric846 Jul 11, 2020
edc36b2
add input variable setter to build file
eric846 Jul 11, 2020
4d0364e
fix syntax errors
eric846 Jul 11, 2020
aed6d94
rename samples/adaptive_rps
eric846 Jul 11, 2020
d9ae87d
improve comments, change step controller initial value from int64 to …
eric846 Jul 12, 2020
a05a6f5
add proto validation rules, fix comments, make rps the default input_…
eric846 Jul 13, 2020
8cd4d21
fix comment wording
eric846 Jul 13, 2020
d814a96
simplify protos, add defaults, specify required or optional
eric846 Jul 14, 2020
5f5a885
add missing newline
eric846 Jul 14, 2020
7e20a78
Kick CI
eric846 Jul 14, 2020
9048267
simplify protos
eric846 Jul 15, 2020
306c0ec
fix format
eric846 Jul 15, 2020
d33f543
fix some optional field comments and rules
eric846 Jul 15, 2020
442cca9
Merge pull request #10 from envoyproxy/master
eric846 Jul 16, 2020
677b783
add Nighthawk status field in BenchmarkResult as nested nighthawk.cli…
eric846 Jul 19, 2020
cefb366
switch to standard Envoy plugin config proto, add prefix to internal …
eric846 Jul 22, 2020
f3684df
Merge remote-tracking branch 'upstream/master' into adaptive-rps-protos2
eric846 Jul 22, 2020
5463051
create headers
eric846 Jul 22, 2020
46e0e25
fix format
eric846 Jul 22, 2020
f634642
use docstring format
eric846 Jul 22, 2020
3c39faa
fix typos in comments
eric846 Jul 23, 2020
b9c8f2b
split build target, get rid of ostream, change InputValueSetter to us…
eric846 Jul 24, 2020
5fc4db4
remove nested namespace, remove redundant _include in target names
eric846 Jul 26, 2020
64e7852
merge from upstream
eric846 Jul 29, 2020
12807f1
Merge remote-tracking branch 'upstream/master' into adaptive-rps-headers
eric846 Jul 29, 2020
e8e960f
merge from upstream
eric846 Aug 27, 2020
94fbcbe
nighthawk service client library
eric846 Aug 27, 2020
6142700
clean up imports
eric846 Aug 27, 2020
afbc049
clean up test names, fix format, fix assertion text
eric846 Aug 27, 2020
48e7121
further clean up includes and imports, fix typos
eric846 Aug 27, 2020
de747eb
tighten mock call expectations
eric846 Aug 27, 2020
0b76542
move PerformNighthawkBenchmark into an interface to allow mocking
eric846 Aug 27, 2020
fd8d03d
add class comment
eric846 Aug 27, 2020
7adbabb
fix include deps
eric846 Aug 27, 2020
6306b4e
Merge remote-tracking branch 'upstream/master' into master2
eric846 Aug 27, 2020
1ece783
Merge remote-tracking branch 'upstream/master' into master2
eric846 Aug 28, 2020
4616e6f
Merge branch 'master2' into adaptive-rps-nighthawk-service
eric846 Aug 28, 2020
70705e9
Merge remote-tracking branch 'upstream/master' into master2
eric846 Aug 31, 2020
e576bc1
Merge remote-tracking branch 'upstream/master' into master2
eric846 Sep 1, 2020
1fca528
Merge remote-tracking branch 'upstream/master' into master2
eric846 Sep 3, 2020
cd6f6da
move nighthawk service client to common, remove separate duration arg
eric846 Sep 3, 2020
8d642bb
Merge branch 'master2' into adaptive-rps-nighthawk-service
eric846 Sep 3, 2020
b1284f5
fix comments, remove bazelrc
eric846 Sep 3, 2020
5cf83e4
comment gRPC mocks, change gRPC error statuses from unknown to internal
eric846 Sep 3, 2020
24cf230
change gRPC write failure status to Unavailable
eric846 Sep 3, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/client/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ cc_grpc_library(
srcs = [
":base",
],
generate_mocks = True,
grpc_only = True,
proto_only = False,
use_external = False,
Expand Down
16 changes: 16 additions & 0 deletions include/nighthawk/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,22 @@ envoy_basic_cc_library(
],
)

envoy_basic_cc_library(
name = "nighthawk_service_client",
hdrs = [
"nighthawk_service_client.h",
],
include_prefix = "nighthawk/common",
deps = [
"//api/client:base_cc_proto",
"//api/client:grpc_service_lib",
"@envoy//include/envoy/common:base_includes",
"@envoy//source/common/common:assert_lib_with_external_headers",
"@envoy//source/common/common:statusor_lib_with_external_headers",
"@envoy//source/common/protobuf:protobuf_with_external_headers",
],
)

envoy_basic_cc_library(
name = "request_lib",
hdrs = [
Expand Down
36 changes: 36 additions & 0 deletions include/nighthawk/common/nighthawk_service_client.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#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/client/options.pb.h"
#include "api/client/service.grpc.pb.h"

namespace Nighthawk {

/**
* An interface for interacting with a Nighthawk Service gRPC stub.
*/
class NighthawkServiceClient {
public:
virtual ~NighthawkServiceClient() = default;

/**
* Runs a single benchmark using a Nighthawk Service.
*
* @param nighthawk_service_stub Nighthawk Service gRPC stub.
* @param command_line_options Nighthawk Service benchmark request proto generated by the
* StepController, without the duration set.
*
* @return StatusOr<ExecutionResponse> If we reached the Nighthawk Service, this is the raw
* ExecutionResponse proto, containing the benchmark data or possibly an error message from
* Nighthawk Service; if we had trouble communicating with the Nighthawk Service, we return an
* error status.
*/
virtual absl::StatusOr<nighthawk::client::ExecutionResponse> PerformNighthawkBenchmark(
nighthawk::client::NighthawkService::StubInterface* nighthawk_service_stub,
const nighthawk::client::CommandLineOptions& command_line_options) PURE;
};

} // namespace Nighthawk
20 changes: 20 additions & 0 deletions source/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,26 @@ licenses(["notice"]) # Apache 2

envoy_package()

envoy_cc_library(
name = "nighthawk_service_client_impl",
srcs = [
"nighthawk_service_client_impl.cc",
],
hdrs = [
"nighthawk_service_client_impl.h",
],
repository = "@envoy",
visibility = ["//visibility:public"],
deps = [
"//api/client:base_cc_proto",
"//api/client:grpc_service_lib",
"//include/nighthawk/common:nighthawk_service_client",
"@envoy//source/common/common:assert_lib_with_external_headers",
"@envoy//source/common/common:statusor_lib_with_external_headers",
"@envoy//source/common/protobuf:protobuf_with_external_headers",
],
)

envoy_cc_library(
name = "request_impl_lib",
hdrs = [
Expand Down
42 changes: 42 additions & 0 deletions source/common/nighthawk_service_client_impl.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#include "common/nighthawk_service_client_impl.h"

#include "external/envoy/source/common/common/assert.h"

namespace Nighthawk {

absl::StatusOr<nighthawk::client::ExecutionResponse>
NighthawkServiceClientImpl::PerformNighthawkBenchmark(
nighthawk::client::NighthawkService::StubInterface* nighthawk_service_stub,
const nighthawk::client::CommandLineOptions& command_line_options) {
nighthawk::client::ExecutionRequest request;
nighthawk::client::ExecutionResponse response;
*request.mutable_start_request()->mutable_options() = command_line_options;

::grpc::ClientContext context;
std::shared_ptr<::grpc::ClientReaderWriterInterface<nighthawk::client::ExecutionRequest,
nighthawk::client::ExecutionResponse>>
stream(nighthawk_service_stub->ExecutionStream(&context));

if (!stream->Write(request)) {
return absl::UnavailableError("Failed to write request to the Nighthawk Service gRPC channel.");
} else if (!stream->WritesDone()) {
return absl::InternalError("WritesDone() failed on the Nighthawk Service gRPC channel.");
}

bool got_response = false;
while (stream->Read(&response)) {
RELEASE_ASSERT(!got_response,
"Nighthawk Service has started responding with more than one message.");
got_response = true;
}
if (!got_response) {
return absl::InternalError("Nighthawk Service did not send a gRPC response.");
}
::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
25 changes: 25 additions & 0 deletions source/common/nighthawk_service_client_impl.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#include "nighthawk/common/nighthawk_service_client.h"

#include "external/envoy/source/common/common/statusor.h"
#include "external/envoy/source/common/protobuf/protobuf.h"

#include "api/client/options.pb.h"
#include "api/client/service.grpc.pb.h"

namespace Nighthawk {

/**
* Real implementation of a helper that opens a channel with the gRPC stub, sends the input, and
* translates the output or errors into a StatusOr.
*
* 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 NighthawkServiceClientImpl : public NighthawkServiceClient {
public:
absl::StatusOr<nighthawk::client::ExecutionResponse> PerformNighthawkBenchmark(
nighthawk::client::NighthawkService::StubInterface* nighthawk_service_stub,
const nighthawk::client::CommandLineOptions& command_line_options) override;
};

} // namespace Nighthawk
10 changes: 10 additions & 0 deletions test/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,13 @@ envoy_cc_test(
":fake_time_source",
],
)

envoy_cc_test(
name = "nighthawk_service_client_test",
srcs = ["nighthawk_service_client_test.cc"],
repository = "@envoy",
deps = [
"//source/common:nighthawk_service_client_impl",
"@com_github_grpc_grpc//:grpc++_test",
],
)
178 changes: 178 additions & 0 deletions test/common/nighthawk_service_client_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
#include "external/envoy/source/common/protobuf/protobuf.h"

#include "api/client/options.pb.h"
#include "api/client/service.grpc.pb.h"
#include "api/client/service_mock.grpc.pb.h"

#include "common/nighthawk_service_client_impl.h"

#include "grpcpp/test/mock_stream.h"

#include "gmock/gmock.h"
#include "gtest/gtest.h"

namespace Nighthawk {

namespace {

using ::Envoy::Protobuf::util::MessageDifferencer;
using ::nighthawk::client::CommandLineOptions;
using ::nighthawk::client::ExecutionRequest;
using ::nighthawk::client::ExecutionResponse;
using ::testing::_;
using ::testing::DoAll;
using ::testing::HasSubstr;
using ::testing::Return;
using ::testing::SaveArg;
using ::testing::SetArgPointee;

TEST(PerformNighthawkBenchmark, UsesSpecifiedCommandLineOptions) {
const int kExpectedRps = 456;
ExecutionRequest request;
nighthawk::client::MockNighthawkServiceStub mock_nighthawk_service_stub;
// Configure the mock Nighthawk Service stub to return an inner mock channel when the code under
// test requests a channel. Set call expectations on the inner mock channel.
EXPECT_CALL(mock_nighthawk_service_stub, ExecutionStreamRaw)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two small asks, but on each of these tests.

  1. Can you add a linebreak between this EXPECT_CALL and the CommandLineOptions below (or in other tests, whatever is following it)? Just helps the eyes parse this a little as one block.

  2. Can we comment at the top of the EXPECT_CALL statement roughly what this block is mocking? Or if that seems like a bad way to document this, something else that helps clarify for future readers what is happening.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

.WillOnce([&request](grpc_impl::ClientContext*) {
auto* mock_reader_writer =
new grpc::testing::MockClientReaderWriter<ExecutionRequest, ExecutionResponse>();
// PerformNighthawkBenchmark currently expects Read to return true exactly once.
EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(true)).WillOnce(Return(false));
// Capture the Nighthawk request PerformNighthawkBenchmark sends on the channel.
EXPECT_CALL(*mock_reader_writer, Write(_, _))
.WillOnce(::testing::DoAll(::testing::SaveArg<0>(&request), Return(true)));
EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true));
EXPECT_CALL(*mock_reader_writer, Finish()).WillOnce(Return(::grpc::Status::OK));
return mock_reader_writer;
});

CommandLineOptions command_line_options;
command_line_options.mutable_requests_per_second()->set_value(kExpectedRps);
NighthawkServiceClientImpl client;
absl::StatusOr<ExecutionResponse> response_or =
client.PerformNighthawkBenchmark(&mock_nighthawk_service_stub, command_line_options);
EXPECT_TRUE(response_or.ok());
EXPECT_EQ(request.start_request().options().requests_per_second().value(), kExpectedRps);
}

TEST(PerformNighthawkBenchmark, ReturnsNighthawkResponseSuccessfully) {
ExecutionResponse expected_response;
nighthawk::client::MockNighthawkServiceStub mock_nighthawk_service_stub;
// Configure the mock Nighthawk Service stub to return an inner mock channel when the code under
// test requests a channel. Set call expectations on the inner mock channel.
EXPECT_CALL(mock_nighthawk_service_stub, ExecutionStreamRaw)
.WillOnce([&expected_response](grpc_impl::ClientContext*) {
auto* mock_reader_writer =
new grpc::testing::MockClientReaderWriter<ExecutionRequest, ExecutionResponse>();
// PerformNighthawkBenchmark currently expects Read to return true exactly once.
// Capture the gRPC response proto as it is written to the output parameter.
EXPECT_CALL(*mock_reader_writer, Read(_))
.WillOnce(DoAll(SetArgPointee<0>(expected_response), Return(true)))
.WillOnce(Return(false));
EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true));
EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true));
EXPECT_CALL(*mock_reader_writer, Finish()).WillOnce(Return(::grpc::Status::OK));
return mock_reader_writer;
});

NighthawkServiceClientImpl client;
absl::StatusOr<ExecutionResponse> response_or =
client.PerformNighthawkBenchmark(&mock_nighthawk_service_stub, CommandLineOptions());
EXPECT_TRUE(response_or.ok());
ExecutionResponse actual_response = response_or.value();
EXPECT_TRUE(MessageDifferencer::Equivalent(actual_response, expected_response));
EXPECT_EQ(actual_response.DebugString(), expected_response.DebugString());
}

TEST(PerformNighthawkBenchmark, ReturnsErrorIfNighthawkServiceDoesNotSendResponse) {
nighthawk::client::MockNighthawkServiceStub mock_nighthawk_service_stub;
// Configure the mock Nighthawk Service stub to return an inner mock channel when the code under
// test requests a channel. Set call expectations on the inner mock channel.
EXPECT_CALL(mock_nighthawk_service_stub, ExecutionStreamRaw)
.WillOnce([](grpc_impl::ClientContext*) {
auto* mock_reader_writer =
new grpc::testing::MockClientReaderWriter<ExecutionRequest, ExecutionResponse>();
EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(false));
EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true));
EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true));
return mock_reader_writer;
});

NighthawkServiceClientImpl client;
absl::StatusOr<ExecutionResponse> response_or =
client.PerformNighthawkBenchmark(&mock_nighthawk_service_stub, CommandLineOptions());
ASSERT_FALSE(response_or.ok());
EXPECT_EQ(response_or.status().code(), absl::StatusCode::kInternal);
EXPECT_THAT(response_or.status().message(),
HasSubstr("Nighthawk Service did not send a gRPC response."));
}

TEST(PerformNighthawkBenchmark, ReturnsErrorIfNighthawkServiceWriteFails) {
nighthawk::client::MockNighthawkServiceStub mock_nighthawk_service_stub;
// Configure the mock Nighthawk Service stub to return an inner mock channel when the code under
// test requests a channel. Set call expectations on the inner mock channel.
EXPECT_CALL(mock_nighthawk_service_stub, ExecutionStreamRaw)
.WillOnce([](grpc_impl::ClientContext*) {
auto* mock_reader_writer =
new grpc::testing::MockClientReaderWriter<ExecutionRequest, ExecutionResponse>();
EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(false));
return mock_reader_writer;
});

NighthawkServiceClientImpl client;
absl::StatusOr<ExecutionResponse> response_or =
client.PerformNighthawkBenchmark(&mock_nighthawk_service_stub, CommandLineOptions());
ASSERT_FALSE(response_or.ok());
EXPECT_EQ(response_or.status().code(), absl::StatusCode::kUnavailable);
EXPECT_THAT(response_or.status().message(), HasSubstr("Failed to write"));
}

TEST(PerformNighthawkBenchmark, ReturnsErrorIfNighthawkServiceWritesDoneFails) {
nighthawk::client::MockNighthawkServiceStub mock_nighthawk_service_stub;
// Configure the mock Nighthawk Service stub to return an inner mock channel when the code under
// test requests a channel. Set call expectations on the inner mock channel.
EXPECT_CALL(mock_nighthawk_service_stub, ExecutionStreamRaw)
.WillOnce([](grpc_impl::ClientContext*) {
auto* mock_reader_writer =
new grpc::testing::MockClientReaderWriter<ExecutionRequest, ExecutionResponse>();
EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true));
EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(false));
return mock_reader_writer;
});

NighthawkServiceClientImpl client;
absl::StatusOr<ExecutionResponse> response_or =
client.PerformNighthawkBenchmark(&mock_nighthawk_service_stub, CommandLineOptions());
ASSERT_FALSE(response_or.ok());
EXPECT_EQ(response_or.status().code(), absl::StatusCode::kInternal);
EXPECT_THAT(response_or.status().message(), HasSubstr("WritesDone() failed"));
}

TEST(PerformNighthawkBenchmark, PropagatesErrorIfNighthawkServiceGrpcStreamClosesAbnormally) {
nighthawk::client::MockNighthawkServiceStub mock_nighthawk_service_stub;
// Configure the mock Nighthawk Service stub to return an inner mock channel when the code under
// test requests a channel. Set call expectations on the inner mock channel.
EXPECT_CALL(mock_nighthawk_service_stub, ExecutionStreamRaw)
.WillOnce([](grpc_impl::ClientContext*) {
auto* mock_reader_writer =
new grpc::testing::MockClientReaderWriter<ExecutionRequest, ExecutionResponse>();
// PerformNighthawkBenchmark currently expects Read to return true exactly once.
EXPECT_CALL(*mock_reader_writer, Read(_)).WillOnce(Return(true)).WillOnce(Return(false));
EXPECT_CALL(*mock_reader_writer, Write(_, _)).WillOnce(Return(true));
EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true));
EXPECT_CALL(*mock_reader_writer, Finish())
.WillOnce(
Return(::grpc::Status(::grpc::PERMISSION_DENIED, "Finish failure status message")));
return mock_reader_writer;
});

NighthawkServiceClientImpl client;
absl::StatusOr<ExecutionResponse> response_or =
client.PerformNighthawkBenchmark(&mock_nighthawk_service_stub, CommandLineOptions());
ASSERT_FALSE(response_or.ok());
EXPECT_EQ(response_or.status().code(), absl::StatusCode::kPermissionDenied);
EXPECT_THAT(response_or.status().message(), HasSubstr("Finish failure status message"));
}

} // namespace
} // namespace Nighthawk
2 changes: 1 addition & 1 deletion tools/check_format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ TO_CHECK="${2:-$PWD}"
bazel run @envoy//tools:code_format/check_format.py -- \
--skip_envoy_build_rule_check --namespace_check Nighthawk \
--build_fixer_check_excluded_paths=$(realpath ".") \
--include_dir_order envoy,nighthawk,external/source/envoy,external,api,common,source,exe,server,client,test_common,test \
--include_dir_order envoy,nighthawk,external/source/envoy,external,api,common,source,exe,server,client,grpcpp,test_common,test \
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This grpcpp section is a simple way to prevent the script from rearranging the grpcpp, gtest, gmock headers into an order that is then considered invalid.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, that might have been a hassle to figure out, sorry about that. I wonder if we can improve there, ideally the "fix" doesn't produce something that the "check" objects to :-(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is definitely a quick fix. It seems to pass all the CI checks. I don't know how it's arriving at the order or how it's checking it. If we figure out how it works, this would no longer be necessary.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's the right tweak to address this. But I do find it's disappointing that check and fix don't line up when the order isn't explicitly specified. I'll file an issue to see if Envoy has progressed here, and if there are changes worth including in our version of this.

$1 $TO_CHECK

# The include checker doesn't support per-file checking, so we only
Expand Down