diff --git a/source/client/factories_impl.cc b/source/client/factories_impl.cc index 63ae22da6..01ae6c50d 100644 --- a/source/client/factories_impl.cc +++ b/source/client/factories_impl.cc @@ -17,6 +17,8 @@ #include "client/output_collector_impl.h" #include "client/output_formatter_impl.h" +#include "request_source/request_options_list_plugin_impl.h" + using namespace std::chrono_literals; namespace Nighthawk { @@ -117,8 +119,8 @@ OutputFormatterPtr OutputFormatterFactoryImpl::create( } } -RequestSourceFactoryImpl::RequestSourceFactoryImpl(const Options& options) - : OptionBasedFactoryImpl(options) {} +RequestSourceFactoryImpl::RequestSourceFactoryImpl(const Options& options, Envoy::Api::Api& api) + : OptionBasedFactoryImpl(options), api_(api) {} void RequestSourceFactoryImpl::setRequestHeader(Envoy::Http::RequestHeaderMap& header, absl::string_view key, @@ -168,7 +170,6 @@ RequestSourceFactoryImpl::create(const Envoy::Upstream::ClusterManagerPtr& clust for (const auto& option_header : request_options.request_headers()) { setRequestHeader(*header, option_header.header().key(), option_header.header().value()); } - if (!options_.requestSource().empty()) { RELEASE_ASSERT(!service_cluster_name.empty(), "expected cluster name to be set"); // We pass in options_.requestsPerSecond() as the header buffer length so the grpc client @@ -176,10 +177,34 @@ RequestSourceFactoryImpl::create(const Envoy::Upstream::ClusterManagerPtr& clust return std::make_unique(cluster_manager, dispatcher, scope, service_cluster_name, std::move(header), options_.requestsPerSecond()); + } else if (options_.requestSourcePluginConfig().has_value()) { + absl::StatusOr plugin_or = LoadRequestSourcePlugin( + options_.requestSourcePluginConfig().value(), api_, std::move(header)); + if (!plugin_or.ok()) { + throw NighthawkException( + absl::StrCat("Request Source plugin loading error should have been caught " + "during input validation: ", + plugin_or.status().message())); + } + RequestSourcePtr request_source = std::move(plugin_or.value()); + return request_source; } else { return std::make_unique(std::move(header)); } } +absl::StatusOr RequestSourceFactoryImpl::LoadRequestSourcePlugin( + const envoy::config::core::v3::TypedExtensionConfig& config, Envoy::Api::Api& api, + Envoy::Http::RequestHeaderMapPtr header) const { + try { + auto& config_factory = + Envoy::Config::Utility::getAndCheckFactoryByName( + config.name()); + return config_factory.createRequestSourcePlugin(config.typed_config(), api, std::move(header)); + } catch (const Envoy::EnvoyException& e) { + return absl::InvalidArgumentError( + absl::StrCat("Could not load plugin: ", config.name(), ": ", e.what())); + } +} TerminationPredicateFactoryImpl::TerminationPredicateFactoryImpl(const Options& options) : OptionBasedFactoryImpl(options) {} diff --git a/source/client/factories_impl.h b/source/client/factories_impl.h index fa7e6c3cd..2c8abfd6d 100644 --- a/source/client/factories_impl.h +++ b/source/client/factories_impl.h @@ -9,6 +9,9 @@ #include "nighthawk/common/termination_predicate.h" #include "nighthawk/common/uri.h" +#include "external/envoy/source/common/common/statusor.h" +#include "external/envoy/source/common/config/utility.h" + #include "common/platform_util_impl.h" namespace Nighthawk { @@ -58,14 +61,31 @@ class OutputFormatterFactoryImpl : public OutputFormatterFactory { class RequestSourceFactoryImpl : public OptionBasedFactoryImpl, public RequestSourceFactory { public: - RequestSourceFactoryImpl(const Options& options); + RequestSourceFactoryImpl(const Options& options, Envoy::Api::Api& api); RequestSourcePtr create(const Envoy::Upstream::ClusterManagerPtr& cluster_manager, Envoy::Event::Dispatcher& dispatcher, Envoy::Stats::Scope& scope, absl::string_view service_cluster_name) const override; private: + Envoy::Api::Api& api_; void setRequestHeader(Envoy::Http::RequestHeaderMap& header, absl::string_view key, absl::string_view value) const; + /** + * Instantiates a RequestSource using a RequestSourcePluginFactory based on the plugin name in + * |config|, unpacking the plugin-specific config proto within |config|. Validates the config + * proto. + * + * @param config Proto containing plugin name and plugin-specific config proto. + * @param api Api parameter that contains timesystem, filesystem, and threadfactory. + * @param header Any headers in request specifiers yielded by the request + * source plugin will override what is specified here. + + * @return absl::StatusOr Initialized plugin or error status due to missing + * plugin or config proto validation error. + */ + absl::StatusOr + LoadRequestSourcePlugin(const envoy::config::core::v3::TypedExtensionConfig& config, + Envoy::Api::Api& api, Envoy::Http::RequestHeaderMapPtr header) const; }; class TerminationPredicateFactoryImpl : public OptionBasedFactoryImpl, diff --git a/source/client/process_impl.cc b/source/client/process_impl.cc index 516a76f92..08cbaefe5 100644 --- a/source/client/process_impl.cc +++ b/source/client/process_impl.cc @@ -104,7 +104,8 @@ ProcessImpl::ProcessImpl(const Options& options, Envoy::Event::TimeSystem& time_ generator_)), dispatcher_(api_->allocateDispatcher("main_thread")), benchmark_client_factory_(options), termination_predicate_factory_(options), sequencer_factory_(options), - request_generator_factory_(options), options_(options), init_manager_("nh_init_manager"), + request_generator_factory_(options, *api_), options_(options), + init_manager_("nh_init_manager"), local_info_(new Envoy::LocalInfo::LocalInfoImpl( {}, Envoy::Network::Utility::getLocalAddress(Envoy::Network::Address::IpVersion::v4), "nighthawk_service_zone", "nighthawk_service_cluster", "nighthawk_service_node")), diff --git a/test/BUILD b/test/BUILD index ecb8d5039..326d971ee 100644 --- a/test/BUILD +++ b/test/BUILD @@ -87,6 +87,7 @@ envoy_cc_test( "//test/mocks/client:mock_benchmark_client", "//test/mocks/client:mock_options", "//test/mocks/common:mock_termination_predicate", + "//test/test_common:environment_lib", "@envoy//test/mocks/event:event_mocks", "@envoy//test/mocks/tracing:tracing_mocks", "@envoy//test/test_common:simulated_time_system_lib", diff --git a/test/factories_test.cc b/test/factories_test.cc index 1a410496a..21cf49c9e 100644 --- a/test/factories_test.cc +++ b/test/factories_test.cc @@ -11,6 +11,7 @@ #include "test/mocks/client/mock_benchmark_client.h" #include "test/mocks/client/mock_options.h" #include "test/mocks/common/mock_termination_predicate.h" +#include "test/test_common/environment.h" #include "gtest/gtest.h" @@ -52,20 +53,125 @@ TEST_F(FactoriesTest, CreateBenchmarkClient) { EXPECT_NE(nullptr, benchmark_client.get()); } +TEST_F(FactoriesTest, CreateRequestSourcePluginWithWorkingJsonReturnsWorkingRequestSource) { + absl::optional request_source_plugin_config; + std::string request_source_plugin_config_json = + "{" + "name:\"nighthawk.in-line-options-list-request-source-plugin\"," + "typed_config:{" + "\"@type\":\"type.googleapis.com/" + "nighthawk.request_source.InLineOptionsListRequestSourceConfig\"," + "options_list:{" + "options:[{request_method:\"1\",request_headers:[{header:{key:\":path\",value:\"inlinepath\"}" + "}]}]" + "}," + "}" + "}"; + request_source_plugin_config.emplace(envoy::config::core::v3::TypedExtensionConfig()); + Envoy::MessageUtil::loadFromJson(request_source_plugin_config_json, + request_source_plugin_config.value(), + Envoy::ProtobufMessage::getStrictValidationVisitor()); + EXPECT_CALL(options_, requestMethod()).Times(1); + EXPECT_CALL(options_, requestBodySize()).Times(1); + EXPECT_CALL(options_, uri()).Times(2).WillRepeatedly(Return("http://foo/")); + EXPECT_CALL(options_, requestSource()).Times(1); + EXPECT_CALL(options_, requestSourcePluginConfig()) + .Times(2) + .WillRepeatedly(ReturnRef(request_source_plugin_config)); + auto cmd = std::make_unique(); + envoy::config::core::v3::HeaderValueOption* request_headers = + cmd->mutable_request_options()->add_request_headers(); + request_headers->mutable_header()->set_key("foo"); + request_headers->mutable_header()->set_value("bar"); + EXPECT_CALL(options_, toCommandLineOptions()).Times(1).WillOnce(Return(ByMove(std::move(cmd)))); + RequestSourceFactoryImpl factory(options_, *api_); + Envoy::Upstream::ClusterManagerPtr cluster_manager; + Nighthawk::RequestSourcePtr request_source = factory.create( + cluster_manager, dispatcher_, *stats_store_.createScope("foo."), "requestsource"); + EXPECT_NE(nullptr, request_source.get()); + Nighthawk::RequestGenerator generator = request_source->get(); + Nighthawk::RequestPtr request = generator(); + EXPECT_EQ("inlinepath", request->header()->getPathValue()); +} + +TEST_F(FactoriesTest, CreateRequestSourcePluginWithNonWorkingJsonThrowsError) { + absl::optional request_source_plugin_config; + std::string request_source_plugin_config_json = + "{" + R"(name:"nighthawk.file-based-request-source-plugin",)" + "typed_config:{" + R"("@type":"type.googleapis.com/)" + R"(nighthawk.request_source.FileBasedOptionsListRequestSourceConfig",)" + R"(file_path:")" + + TestEnvironment::runfilesPath("test/request_source/test_data/NotARealFile.yaml") + + "\"," + "}" + "}"; + request_source_plugin_config.emplace(envoy::config::core::v3::TypedExtensionConfig()); + Envoy::MessageUtil::loadFromJson(request_source_plugin_config_json, + request_source_plugin_config.value(), + Envoy::ProtobufMessage::getStrictValidationVisitor()); + EXPECT_CALL(options_, requestMethod()).Times(1); + EXPECT_CALL(options_, requestBodySize()).Times(1); + EXPECT_CALL(options_, uri()).Times(2).WillRepeatedly(Return("http://foo/")); + EXPECT_CALL(options_, requestSource()).Times(1); + EXPECT_CALL(options_, requestSourcePluginConfig()) + .Times(2) + .WillRepeatedly(ReturnRef(request_source_plugin_config)); + auto cmd = std::make_unique(); + envoy::config::core::v3::HeaderValueOption* request_headers = + cmd->mutable_request_options()->add_request_headers(); + request_headers->mutable_header()->set_key("foo"); + request_headers->mutable_header()->set_value("bar"); + EXPECT_CALL(options_, toCommandLineOptions()).Times(1).WillOnce(Return(ByMove(std::move(cmd)))); + RequestSourceFactoryImpl factory(options_, *api_); + Envoy::Upstream::ClusterManagerPtr cluster_manager; + EXPECT_THROW_WITH_REGEX( + factory.create(cluster_manager, dispatcher_, *stats_store_.createScope("foo."), + "requestsource"), + NighthawkException, + "Request Source plugin loading error should have been caught during input validation"); +} + TEST_F(FactoriesTest, CreateRequestSource) { + absl::optional request_source_plugin_config; EXPECT_CALL(options_, requestMethod()).Times(1); EXPECT_CALL(options_, requestBodySize()).Times(1); EXPECT_CALL(options_, uri()).Times(2).WillRepeatedly(Return("http://foo/")); EXPECT_CALL(options_, requestSource()).Times(1); + EXPECT_CALL(options_, requestSourcePluginConfig()) + .Times(1) + .WillRepeatedly(ReturnRef(request_source_plugin_config)); + auto cmd = std::make_unique(); + envoy::config::core::v3::HeaderValueOption* request_headers = + cmd->mutable_request_options()->add_request_headers(); + request_headers->mutable_header()->set_key("foo"); + request_headers->mutable_header()->set_value("bar"); + EXPECT_CALL(options_, toCommandLineOptions()).Times(1).WillOnce(Return(ByMove(std::move(cmd)))); + RequestSourceFactoryImpl factory(options_, *api_); + Envoy::Upstream::ClusterManagerPtr cluster_manager; + RequestSourcePtr request_generator = factory.create( + cluster_manager, dispatcher_, *stats_store_.createScope("foo."), "requestsource"); + EXPECT_NE(nullptr, request_generator.get()); +} + +TEST_F(FactoriesTest, CreateRemoteRequestSource) { + absl::optional request_source_plugin_config; + EXPECT_CALL(options_, requestMethod()).Times(1); + EXPECT_CALL(options_, requestBodySize()).Times(1); + EXPECT_CALL(options_, uri()).Times(2).WillRepeatedly(Return("http://foo/")); + EXPECT_CALL(options_, requestSource()).Times(1).WillRepeatedly(Return("http://bar/")); + EXPECT_CALL(options_, requestsPerSecond()).Times(1).WillRepeatedly(Return(5)); auto cmd = std::make_unique(); - auto request_headers = cmd->mutable_request_options()->add_request_headers(); + envoy::config::core::v3::HeaderValueOption* request_headers = + cmd->mutable_request_options()->add_request_headers(); request_headers->mutable_header()->set_key("foo"); request_headers->mutable_header()->set_value("bar"); EXPECT_CALL(options_, toCommandLineOptions()).Times(1).WillOnce(Return(ByMove(std::move(cmd)))); - RequestSourceFactoryImpl factory(options_); + RequestSourceFactoryImpl factory(options_, *api_); Envoy::Upstream::ClusterManagerPtr cluster_manager; - auto request_generator = factory.create(cluster_manager, dispatcher_, - *stats_store_.createScope("foo."), "requestsource"); + RequestSourcePtr request_generator = factory.create( + cluster_manager, dispatcher_, *stats_store_.createScope("foo."), "requestsource"); EXPECT_NE(nullptr, request_generator.get()); } diff --git a/test/integration/test_request_source_plugin.py b/test/integration/test_request_source_plugin.py new file mode 100644 index 000000000..7609841c2 --- /dev/null +++ b/test/integration/test_request_source_plugin.py @@ -0,0 +1,59 @@ +"""Tests for the nighthawk_service binary.""" + +import pytest +import os + +from test.integration.integration_test_fixtures import (http_test_server_fixture, server_config) +from test.integration import utility +from test.integration import asserts + + +@pytest.mark.parametrize( + "request_source_config,expected_min,expected_max", + [ + pytest.param(""" + { + name:"nighthawk.in-line-options-list-request-source-plugin", + typed_config:{ + "@type":"type.googleapis.com/nighthawk.request_source.InLineOptionsListRequestSourceConfig", + options_list:{ + options:[ + {request_method:"1",request_body_size:"1",request_headers:[{header:{"key":"x-nighthawk-test-server-config","value":"{response_body_size:13}"}}]}, + {request_method:"1",request_body_size:"2",request_headers:[{header:{"key":"x-nighthawk-test-server-config","value":"{response_body_size:17}"}}]}, + ] + }, + } + }""", + 13, + 17, + id="in-line"), + pytest.param(""" + { + name:"nighthawk.file-based-request-source-plugin", + typed_config:{ + "@type":"type.googleapis.com/nighthawk.request_source.FileBasedOptionsListRequestSourceConfig", + file_path:"%s", + } + }""" % (os.path.dirname(os.path.abspath(os.path.dirname(__file__))) + + "/request_source/test_data/test-config.yaml"), + 13, + 17, + id="file-based"), + ], +) +def test_request_source_plugin_happy_flow_parametrized(http_test_server_fixture, + request_source_config, expected_min, + expected_max): + """Test that the nighthawkClient can run with request-source-plugin option.""" + parsed_json, _ = http_test_server_fixture.runNighthawkClient([ + "--termination-predicate", "benchmark.http_2xx:5", "--rps 10", + "--request-source-plugin-config %s" % request_source_config, + http_test_server_fixture.getTestServerRootUri(), "--request-header", "host: sni.com" + ]) + counters = http_test_server_fixture.getNighthawkCounterMapFromJson(parsed_json) + global_histograms = http_test_server_fixture.getNighthawkGlobalHistogramsbyIdFromJson(parsed_json) + asserts.assertGreaterEqual(counters["benchmark.http_2xx"], 5) + asserts.assertEqual(int(global_histograms["benchmark_http_client.response_body_size"]["raw_max"]), + expected_max) + asserts.assertEqual(int(global_histograms["benchmark_http_client.response_body_size"]["raw_min"]), + expected_min) diff --git a/test/request_source/test_data/test-config.yaml b/test/request_source/test_data/test-config.yaml index e8b4cdaac..b78bcba64 100644 --- a/test/request_source/test_data/test-config.yaml +++ b/test/request_source/test_data/test-config.yaml @@ -1,13 +1,11 @@ options: - - request_body_size: 10 + - request_method: 1 + request_body_size: 10 request_headers: - - { header: { key: ":path", value: "/a" } } - - { header: { key: "foo", value: "bar" } } - - { header: { key: "foo", value: "bar2" } } - - { header: { key: "x-nh", value: "1" } } - - request_body_size: 10 + - { header: { key: ":path", value: "/a" } } + - { header: { key: "x-nighthawk-test-server-config", value: "{response_body_size:13}" } } + - request_method: 1 + request_body_size: 10 request_headers: - - { header: { key: ":path", value: "/b" } } - - { header: { key: "bar", value: "foo" } } - - { header: { key: "bar", value: "foo2" } } - - { header: { key: "x-nh", value: "2" } } \ No newline at end of file + - { header: { key: ":path", value: "/b" } } + - { header: { key: "x-nighthawk-test-server-config", value: "{response_body_size:17}" } } \ No newline at end of file diff --git a/tools/format_python_tools.sh b/tools/format_python_tools.sh index 2523042d5..e1657df41 100755 --- a/tools/format_python_tools.sh +++ b/tools/format_python_tools.sh @@ -26,11 +26,12 @@ EXCLUDE="--exclude=benchmarks/tmp/*,.cache/*,*/venv/*,tools/format_python_tools. # E124 Closing bracket does not match visual indentation # E125 Continuation line with same indent as next logical line # E126 Continuation line over-indented for hanging indent +# W504 line break after binary operator # We ignore false positives because of what look like pytest peculiarities # F401 Module imported but unused # F811 Redefinition of unused name from line n -flake8 . ${EXCLUDE} --ignore=E114,E111,E501,F401,F811,E124,E125,E126,D --count --show-source --statistics +flake8 . ${EXCLUDE} --ignore=E114,E111,E501,F401,F811,E124,E125,E126,W504,D --count --show-source --statistics # D = Doc comment related checks (We check both p257 AND google conventions). flake8 . ${EXCLUDE} --docstring-convention pep257 --select=D --count --show-source --statistics flake8 . ${EXCLUDE} --docstring-convention google --select=D --count --show-source --statistics