Start of refactored filter configurations.

Signed-off-by: Nathan Perry <[email protected]>

api/server/BUILD

@@ -10,7 +10,11 @@ envoy_package()
 
 api_cc_py_proto_library(
     name = "response_options_proto",
-    srcs = ["response_options.proto"],
+    srcs = [
+        "dynamic_delay.proto",
+        "response_options.proto",
+        "time_tracking.proto",
+    ],
     deps = [
         "@envoy_api//envoy/api/v2/core:pkg",
         "@envoy_api//envoy/config/core/v3:pkg",

api/server/dynamic_delay.proto

@@ -0,0 +1,17 @@
+syntax = "proto3";
+
+import "api/server/response_options.proto";
+import "google/protobuf/duration.proto";
+import "validate/validate.proto";
+
+package nighthawk.server;
+
+// Configures the dynamic-delay filter.
+message DynamicDelayConfiguration {
+  oneof oneof_delay_options {
+    // Static delay duration.
+    google.protobuf.Duration static_delay = 4 [(validate.rules).duration.gte.nanos = 0];
+    // Concurrency based linear delay configuration.
+    ConcurrencyBasedLinearDelay concurrency_based_linear_delay = 5;
+  }
+}

api/server/response_options.proto

@@ -35,30 +35,19 @@ message ResponseOptions {
   // If true, then echo request headers in the response body.
   bool echo_request_headers = 3;
 
+  // DEPRECATED - please provide this in DynamicDelayConfiguration
   oneof oneof_delay_options {
     // Static delay duration.
-    google.protobuf.Duration static_delay = 4 [(validate.rules).duration.gte.nanos = 0];
+    google.protobuf.Duration static_delay = 4
+        [(validate.rules).duration.gte.nanos = 0, deprecated = true];
     // Concurrency based linear delay configuration.
-    ConcurrencyBasedLinearDelay concurrency_based_linear_delay = 5;
+    ConcurrencyBasedLinearDelay concurrency_based_linear_delay = 5 [deprecated = true];
   }
   // If set, makes the extension include timing data in the supplied response header name.
   // For example, when set to "x-abc", and 3 requests are performed, the test server will respond
   // with: Response 1: No x-abc header because there's no previous response. Response 2: Header
   // x-abc: <ns elapsed between responses 2 and 1>. Response 3: Header x-abc: <ns elapsed between
   // responses 3 and 2>.
-  string emit_previous_request_delta_in_response_header = 6;
-}
-
-// Configures the dynamic-delay test filter.
-message DynamicDelayConfiguration {
-  // This is a temporary solution to allow this functionality to continue, but will likely be
-  // reconfigured soon.
-  ResponseOptions experimental_response_options = 1;
-}
-
-// Configures the time-tracking test filter
-message TimeTrackingConfiguration {
-  // This is a temporary solution to allow this functionality to continue, but will likely be
-  // reconfigured soon.
-  ResponseOptions experimental_response_options = 1;
+  // DEPRECATED - please provide this in TimeTrackingConfiguration
+  string emit_previous_request_delta_in_response_header = 6 [deprecated = true];
 }

api/server/time_tracking.proto

@@ -0,0 +1,15 @@
+syntax = "proto3";
+
+import "api/server/response_options.proto";
+
+package nighthawk.server;
+
+// Configures the time-tracking filter.
+message TimeTrackingConfiguration {
+  // If set, makes the extension include timing data in the supplied response header name.
+  // For example, when set to "x-abc", and 3 requests are performed, the test server will respond
+  // with: Response 1: No x-abc header because there's no previous response. Response 2: Header
+  // x-abc: <ns elapsed between responses 2 and 1>. Response 3: Header x-abc: <ns elapsed between
+  // responses 3 and 2>.
+  string emit_previous_request_delta_in_response_header = 6;
+}

source/server/configuration.cc

@@ -16,21 +16,6 @@ namespace Nighthawk {
 namespace Server {
 namespace Configuration {
 
-bool mergeJsonConfig(absl::string_view json, nighthawk::server::ResponseOptions& config,
-                     std::string& error_message) {
-  error_message = "";
-  try {
-    nighthawk::server::ResponseOptions json_config;
-    auto& validation_visitor = Envoy::ProtobufMessage::getStrictValidationVisitor();
-    Envoy::MessageUtil::loadFromJson(std::string(json), json_config, validation_visitor);
-    config.MergeFrom(json_config);
-    Envoy::MessageUtil::validate(config, validation_visitor);
-  } catch (const Envoy::EnvoyException& exception) {
-    error_message = fmt::format("Error merging json config: {}", exception.what());
-  }
-  return error_message == "";
-}
-
 void applyConfigToResponseHeaders(Envoy::Http::ResponseHeaderMap& response_headers,
                                   const nighthawk::server::ResponseOptions& response_options) {
 

source/server/configuration.h

@@ -6,8 +6,14 @@
 #include "envoy/config/core/v3/base.pb.h"
 #include "envoy/http/header_map.h"
 
+#include "external/envoy/source/common/common/statusor.h"
+#include "external/envoy/source/common/protobuf/message_validator_impl.h"
+#include "external/envoy/source/common/protobuf/utility.h"
+
 #include "api/server/response_options.pb.h"
 
+#include "source/server/well_known_headers.h"
+
 namespace Nighthawk {
 namespace Server {
 namespace Configuration {
@@ -20,8 +26,20 @@ namespace Configuration {
  * @param error_message Set to an error message if one occurred, else set to an empty string.
  * @return bool false if an error occurred.
  */
-bool mergeJsonConfig(absl::string_view json, nighthawk::server::ResponseOptions& config,
-                     std::string& error_message);
+template <typename ProtoType>
+bool mergeJsonConfig(absl::string_view json, ProtoType& config, std::string& error_message) {
+  error_message = "";
+  try {
+    ProtoType json_config;
+    auto& validation_visitor = Envoy::ProtobufMessage::getStrictValidationVisitor();
+    Envoy::MessageUtil::loadFromJson(std::string(json), json_config, validation_visitor);
+    config.MergeFrom(json_config);
+    Envoy::MessageUtil::validate(config, validation_visitor);
+  } catch (const Envoy::EnvoyException& exception) {
+    error_message = fmt::format("Error merging json config: {}", exception.what());
+  }
+  return error_message == "";
+}
 
 /**
  * Applies ResponseOptions onto a HeaderMap containing response headers.
@@ -51,6 +69,41 @@ envoy::config::core::v3::HeaderValueOption upgradeDeprecatedEnvoyV2HeaderValueOp
  */
 void validateResponseOptions(const nighthawk::server::ResponseOptions& response_options);
 
+/**
+ * Compute the effective configuration, based on considering the static configuration as well as
+ * any configuration provided via request headers.
+ *
+ * @param base_filter_config Base configuration configured in the server, to be merged with the
+ * configuration in the headers
+ * @param request_headers Full set of request headers to be inspected for configuration.
+ * @return const absl::StatusOr<Envoy::Protobuf::Message> The effective configuration, a proto of
+ * the same type as the passed in parameter
+ */
+template <typename ProtoType>
+const absl::StatusOr<std::shared_ptr<const ProtoType>>
+computeEffectiveConfiguration(std::shared_ptr<const ProtoType> base_filter_config,
+                              const Envoy::Http::RequestHeaderMap& request_headers) {
+  const auto& request_config_header =
+      request_headers.get(TestServer::HeaderNames::get().TestServerConfig);
+  if (request_config_header.size() == 1) {
+    // We could be more flexible and look for the first request header that has a value,
+    // but without a proper understanding of a real use case for that, we are assuming that any
+    // existence of duplicate headers here is an error.
+    ProtoType modified_filter_config = *base_filter_config;
+    std::string error_message;
+    if (mergeJsonConfig(request_config_header[0]->value().getStringView(), modified_filter_config,
+                        error_message)) {
+      return std::make_shared<const ProtoType>(std::move(modified_filter_config));
+    } else {
+      return absl::InvalidArgumentError(error_message);
+    }
+  } else if (request_config_header.size() > 1) {
+    return absl::InvalidArgumentError(
+        "Received multiple configuration headers in the request, expected only one.");
+  }
+  return base_filter_config;
+}
+
 } // namespace Configuration
 } // namespace Server
 } // namespace Nighthawk

source/server/http_dynamic_delay_filter.cc

@@ -7,19 +7,26 @@
 #include "source/server/configuration.h"
 #include "source/server/well_known_headers.h"
 
+#include "api/server/dynamic_delay.pb.validate.h"
+
 #include "absl/strings/str_cat.h"
 
 namespace Nighthawk {
 namespace Server {
 
+using ::nighthawk::server::DynamicDelayConfiguration;
+
 HttpDynamicDelayDecoderFilterConfig::HttpDynamicDelayDecoderFilterConfig(
-    const nighthawk::server::DynamicDelayConfiguration& proto_config,
+    const DynamicDelayConfiguration& proto_config,
     Envoy::Runtime::Loader& runtime, const std::string& stats_prefix, Envoy::Stats::Scope& scope,
     Envoy::TimeSource& time_source)
-    : FilterConfigurationBase(proto_config.experimental_response_options(), "dynamic-delay"),
-      runtime_(runtime),
+    : FilterConfigurationBase("dynamic-delay"), runtime_(runtime),
       stats_prefix_(absl::StrCat(stats_prefix, fmt::format("{}.", filter_name()))), scope_(scope),
-      time_source_(time_source) {}
+      time_source_(time_source), server_config_(std::make_shared<DynamicDelayConfiguration>(proto_config)) {}
+
+std::shared_ptr<const DynamicDelayConfiguration> HttpDynamicDelayDecoderFilterConfig::getServerConfig() {
+  return server_config_;
+}
 
 HttpDynamicDelayDecoderFilter::HttpDynamicDelayDecoderFilter(
     HttpDynamicDelayDecoderFilterConfigSharedPtr config)
@@ -42,14 +49,14 @@ void HttpDynamicDelayDecoderFilter::onDestroy() {
 Envoy::Http::FilterHeadersStatus
 HttpDynamicDelayDecoderFilter::decodeHeaders(Envoy::Http::RequestHeaderMap& headers,
                                              bool end_stream) {
-  effective_config_ = config_->computeEffectiveConfiguration(headers);
+  effective_config_ = Configuration::computeEffectiveConfiguration<DynamicDelayConfiguration>(config_->getServerConfig(), headers);
   if (effective_config_.ok()) {
     const absl::optional<int64_t> delay_ms =
         computeDelayMs(*effective_config_.value(), config_->approximateFilterInstances());
     maybeRequestFaultFilterDelay(delay_ms, headers);
   } else {
     if (end_stream) {
-      config_->validateOrSendError(effective_config_, *decoder_callbacks_);
+      config_->validateOrSendError(effective_config_.status(), *decoder_callbacks_);
       return Envoy::Http::FilterHeadersStatus::StopIteration;
     }
     return Envoy::Http::FilterHeadersStatus::Continue;
@@ -61,7 +68,7 @@ Envoy::Http::FilterDataStatus
 HttpDynamicDelayDecoderFilter::decodeData(Envoy::Buffer::Instance& buffer, bool end_stream) {
   if (!effective_config_.ok()) {
     if (end_stream) {
-      config_->validateOrSendError(effective_config_, *decoder_callbacks_);
+      config_->validateOrSendError(effective_config_.status(), *decoder_callbacks_);
       return Envoy::Http::FilterDataStatus::StopIterationNoBuffer;
     }
     return Envoy::Http::FilterDataStatus::Continue;
@@ -70,14 +77,13 @@ HttpDynamicDelayDecoderFilter::decodeData(Envoy::Buffer::Instance& buffer, bool
 }
 
 absl::optional<int64_t> HttpDynamicDelayDecoderFilter::computeDelayMs(
-    const nighthawk::server::ResponseOptions& response_options, const uint64_t concurrency) {
+    const nighthawk::server::DynamicDelayConfiguration& config, const uint64_t concurrency) {
   absl::optional<int64_t> delay_ms;
-  if (response_options.has_static_delay()) {
-    delay_ms =
-        Envoy::Protobuf::util::TimeUtil::DurationToMilliseconds(response_options.static_delay());
-  } else if (response_options.has_concurrency_based_linear_delay()) {
+  if (config.has_static_delay()) {
+    delay_ms = Envoy::Protobuf::util::TimeUtil::DurationToMilliseconds(config.static_delay());
+  } else if (config.has_concurrency_based_linear_delay()) {
     const nighthawk::server::ConcurrencyBasedLinearDelay& concurrency_config =
-        response_options.concurrency_based_linear_delay();
+        config.concurrency_based_linear_delay();
     delay_ms = computeConcurrencyBasedLinearDelayMs(concurrency, concurrency_config.minimal_delay(),
                                                     concurrency_config.concurrency_delay_factor());
   }

source/server/http_dynamic_delay_filter.h

@@ -7,7 +7,7 @@
 
 #include "external/envoy/source/extensions/filters/http/fault/fault_filter.h"
 
-#include "api/server/response_options.pb.h"
+#include "api/server/dynamic_delay.pb.h"
 
 #include "source/server/http_filter_config_base.h"
 
@@ -73,6 +73,8 @@ class HttpDynamicDelayDecoderFilterConfig : public FilterConfigurationBase {
    */
   std::string stats_prefix() { return stats_prefix_; }
 
+  std::shared_ptr<const nighthawk::server::DynamicDelayConfiguration> getServerConfig();
+
 private:
   static std::atomic<uint64_t>& instances() {
     // We lazy-init the atomic to avoid static initialization / a fiasco.
@@ -83,6 +85,7 @@ class HttpDynamicDelayDecoderFilterConfig : public FilterConfigurationBase {
   const std::string stats_prefix_;
   Envoy::Stats::Scope& scope_;
   Envoy::TimeSource& time_source_;
+  std::shared_ptr<const nighthawk::server::DynamicDelayConfiguration> server_config_;
 };
 
 using HttpDynamicDelayDecoderFilterConfigSharedPtr =
@@ -127,15 +130,15 @@ class HttpDynamicDelayDecoderFilter : public Envoy::Extensions::HttpFilters::Fau
   }
 
   /**
-   * Compute the delay in milliseconds, based on provided response options and number of active
-   * requests.
+   * Compute the delay in milliseconds, based on provided dynamic delay configuration and
+   * number of active requests.
    *
-   * @param response_options Response options configuration.
+   * @param configuration Dynamic delay configuration.
    * @param concurrency The number of concurrenct active requests.
    * @return absl::optional<int64_t> The computed delay in milliseconds, if any.
    */
   static absl::optional<int64_t>
-  computeDelayMs(const nighthawk::server::ResponseOptions& response_options,
+  computeDelayMs(const nighthawk::server::DynamicDelayConfiguration& configuration,
                  const uint64_t concurrency);
 
   /**
@@ -160,7 +163,7 @@ class HttpDynamicDelayDecoderFilter : public Envoy::Extensions::HttpFilters::Fau
 
 private:
   const HttpDynamicDelayDecoderFilterConfigSharedPtr config_;
-  absl::StatusOr<EffectiveFilterConfigurationPtr> effective_config_;
+  absl::StatusOr<nighthawk::server::DynamicDelayConfiguration> effective_config_;
   Envoy::Http::StreamDecoderFilterCallbacks* decoder_callbacks_;
   bool destroyed_{false};
 };

source/server/http_dynamic_delay_filter_config.cc

@@ -4,8 +4,8 @@
 
 #include "external/envoy/source/common/protobuf/message_validator_impl.h"
 
-#include "api/server/response_options.pb.h"
-#include "api/server/response_options.pb.validate.h"
+#include "api/server/dynamic_delay.pb.h"
+#include "api/server/dynamic_delay.pb.validate.h"
 
 #include "source/server/configuration.h"
 #include "source/server/http_dynamic_delay_filter.h"
@@ -26,7 +26,6 @@ class HttpDynamicDelayDecoderFilterConfigFactory
     const nighthawk::server::DynamicDelayConfiguration& dynamic_delay_configuration =
         Envoy::MessageUtil::downcastAndValidate<
             const nighthawk::server::DynamicDelayConfiguration&>(proto_config, validation_visitor);
-    validateResponseOptions(dynamic_delay_configuration.experimental_response_options());
     return createFilter(dynamic_delay_configuration, context);
   }
 

source/server/http_filter_config_base.cc

@@ -5,42 +5,16 @@
 namespace Nighthawk {
 namespace Server {
 
-FilterConfigurationBase::FilterConfigurationBase(
-    const nighthawk::server::ResponseOptions& proto_config, absl::string_view filter_name)
-    : filter_name_(filter_name),
-      server_config_(std::make_shared<nighthawk::server::ResponseOptions>(proto_config)) {}
-
-const absl::StatusOr<EffectiveFilterConfigurationPtr>
-FilterConfigurationBase::computeEffectiveConfiguration(
-    const Envoy::Http::RequestHeaderMap& headers) {
-  const auto& request_config_header = headers.get(TestServer::HeaderNames::get().TestServerConfig);
-  if (request_config_header.size() == 1) {
-    // We could be more flexible and look for the first request header that has a value,
-    // but without a proper understanding of a real use case for that, we are assuming that any
-    // existence of duplicate headers here is an error.
-    nighthawk::server::ResponseOptions response_options = *server_config_;
-    std::string error_message;
-    if (Configuration::mergeJsonConfig(request_config_header[0]->value().getStringView(),
-                                       response_options, error_message)) {
-      return std::make_shared<const nighthawk::server::ResponseOptions>(
-          std::move(response_options));
-    } else {
-      return absl::InvalidArgumentError(error_message);
-    }
-  } else if (request_config_header.size() > 1) {
-    return absl::InvalidArgumentError(
-        "Received multiple configuration headers in the request, expected only one.");
-  }
-  return server_config_;
-}
+FilterConfigurationBase::FilterConfigurationBase(absl::string_view filter_name)
+    : filter_name_(filter_name) {}
 
 bool FilterConfigurationBase::validateOrSendError(
-    absl::StatusOr<EffectiveFilterConfigurationPtr>& effective_config,
+    const absl::Status& effective_config,
     Envoy::Http::StreamDecoderFilterCallbacks& decoder_callbacks) const {
   if (!effective_config.ok()) {
     decoder_callbacks.sendLocalReply(static_cast<Envoy::Http::Code>(500),
                                      fmt::format("{} didn't understand the request: {}",
-                                                 filter_name_, effective_config.status().message()),
+                                                 filter_name_, effective_config.message()),
                                      nullptr, absl::nullopt, "");
     return true;
   }