Skip to content

Commit

Permalink
Factory plugin implementation (#566)
Browse files Browse the repository at this point in the history
Adding implementation in the factory_impl class for loading request source plugins. This also introduces python tests for the previous PR and undoes the temporary hack to reduce test_integration_coverage threshold in issue #564.

Signed-off-by: William Juan <66322422+wjuan-AFK@users.noreply.github.com>
  • Loading branch information
wjuan-AFK committed Nov 11, 2020
1 parent 350c2d3 commit d63ade7
Show file tree
Hide file tree
Showing 8 changed files with 231 additions and 20 deletions.
31 changes: 28 additions & 3 deletions source/client/factories_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -168,18 +170,41 @@ 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
// will shoot for maintaining an amount of headers of at least one second.
return std::make_unique<RemoteRequestSourceImpl>(cluster_manager, dispatcher, scope,
service_cluster_name, std::move(header),
options_.requestsPerSecond());
} else if (options_.requestSourcePluginConfig().has_value()) {
absl::StatusOr<RequestSourcePtr> 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<StaticRequestSourceImpl>(std::move(header));
}
}
absl::StatusOr<RequestSourcePtr> 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<RequestSourcePluginConfigFactory>(
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) {}
Expand Down
22 changes: 21 additions & 1 deletion source/client/factories_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<RequestSourcePtr> Initialized plugin or error status due to missing
* plugin or config proto validation error.
*/
absl::StatusOr<RequestSourcePtr>
LoadRequestSourcePlugin(const envoy::config::core::v3::TypedExtensionConfig& config,
Envoy::Api::Api& api, Envoy::Http::RequestHeaderMapPtr header) const;
};

class TerminationPredicateFactoryImpl : public OptionBasedFactoryImpl,
Expand Down
3 changes: 2 additions & 1 deletion source/client/process_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
Expand Down
1 change: 1 addition & 0 deletions test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
114 changes: 110 additions & 4 deletions test/factories_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -52,20 +53,125 @@ TEST_F(FactoriesTest, CreateBenchmarkClient) {
EXPECT_NE(nullptr, benchmark_client.get());
}

TEST_F(FactoriesTest, CreateRequestSourcePluginWithWorkingJsonReturnsWorkingRequestSource) {
absl::optional<envoy::config::core::v3::TypedExtensionConfig> 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<nighthawk::client::CommandLineOptions>();
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<envoy::config::core::v3::TypedExtensionConfig> 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<nighthawk::client::CommandLineOptions>();
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<envoy::config::core::v3::TypedExtensionConfig> 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<nighthawk::client::CommandLineOptions>();
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<envoy::config::core::v3::TypedExtensionConfig> 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<nighthawk::client::CommandLineOptions>();
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());
}

Expand Down
59 changes: 59 additions & 0 deletions test/integration/test_request_source_plugin.py
Original file line number Diff line number Diff line change
@@ -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)
18 changes: 8 additions & 10 deletions test/request_source/test_data/test-config.yaml
Original file line number Diff line number Diff line change
@@ -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" } }
- { header: { key: ":path", value: "/b" } }
- { header: { key: "x-nighthawk-test-server-config", value: "{response_body_size:17}" } }
3 changes: 2 additions & 1 deletion tools/format_python_tools.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d63ade7

Please sign in to comment.