Merge remote-tracking branch 'origin/master' into perfilterconfig

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

include/envoy/http/filter.h

@@ -261,7 +261,7 @@ class StreamDecoderFilterCallbacks : public virtual StreamFilterCallbacks {
   /**
    * This routine may be called to change the buffer limit for decoder filters.
    *
-   * @param boolean supplies the desired buffer limit.
+   * @param limit supplies the desired buffer limit.
    */
   virtual void setDecoderBufferLimit(uint32_t limit) PURE;
 
@@ -393,7 +393,7 @@ class StreamEncoderFilterCallbacks : public virtual StreamFilterCallbacks {
   /**
    * This routine may be called to change the buffer limit for encoder filters.
    *
-   * @limit settings supplies the desired buffer limit.
+   * @param limit supplies the desired buffer limit.
    */
   virtual void setEncoderBufferLimit(uint32_t limit) PURE;
 

include/envoy/router/router.h

@@ -232,6 +232,17 @@ class VirtualCluster {
 class RateLimitPolicy;
 class Config;
 
+/**
+ * All route specific config returned by the method at
+ *   NamedHttpFilterConfigFactory::createRouteSpecificFilterConfig
+ * should be derived from this class.
+ */
+class RouteSpecificFilterConfig {
+public:
+  virtual ~RouteSpecificFilterConfig() {}
+};
+typedef std::shared_ptr<const RouteSpecificFilterConfig> RouteSpecificFilterConfigConstSharedPtr;
+
 /**
  * Virtual host defintion.
  */
@@ -260,11 +271,19 @@ class VirtualHost {
   virtual const Config& routeConfig() const PURE;
 
   /**
-   * @return const Protobuf::Message* the per-filter config for the given
-   * filter name configured for this virtual host. If none is present,
-   * nullptr is returned.
+   * @return const RouteSpecificFilterConfig* the per-filter config pre-processed object for
+   *  the given filter name. If there is not per-filter config, or the filter factory returns
+   *  nullptr, nullptr is returned.
+   */
+  virtual const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const PURE;
+
+  /**
+   * This is a helper on top of perFilterConfig() that casts the return object to the specified
+   * type.
    */
-  virtual const Protobuf::Message* perFilterConfig(const std::string& name) const PURE;
+  template <class Derived> const Derived* perFilterConfigTyped(const std::string& name) const {
+    return dynamic_cast<const Derived*>(perFilterConfig(name));
+  }
 };
 
 /**
@@ -472,12 +491,19 @@ class RouteEntry : public ResponseEntry {
   virtual const PathMatchCriterion& pathMatchCriterion() const PURE;
 
   /**
-   * @return const Protobuf::Message* the per-filter config for the given
-   * filter name configured for this route entry. Only weighted cluster entries
-   * will potentially have these values available. If none is present,
-   * nullptr is returned.
+   * @return const RouteSpecificFilterConfig* the per-filter config pre-processed object for
+   *  the given filter name. If there is not per-filter config, or the filter factory returns
+   *  nullptr, nullptr is returned.
+   */
+  virtual const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const PURE;
+
+  /**
+   * This is a helper on top of perFilterConfig() that casts the return object to the specified
+   * type.
    */
-  virtual const Protobuf::Message* perFilterConfig(const std::string& name) const PURE;
+  template <class Derived> const Derived* perFilterConfigTyped(const std::string& name) const {
+    return dynamic_cast<const Derived*>(perFilterConfig(name));
+  }
 };
 
 /**
@@ -525,11 +551,19 @@ class Route {
   virtual const Decorator* decorator() const PURE;
 
   /**
-   * @return const Protobuf::Message* the per-filter config for the given
-   * filter name configured for this route. If none is present, nullptr is
-   * returned.
+   * @return const RouteSpecificFilterConfig* the per-filter config pre-processed object for
+   *  the given filter name. If there is not per-filter config, or the filter factory returns
+   *  nullptr, nullptr is returned.
+   */
+  virtual const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const PURE;
+
+  /**
+   * This is a helper on top of perFilterConfig() that casts the return object to the specified
+   * type.
    */
-  virtual const Protobuf::Message* perFilterConfig(const std::string& name) const PURE;
+  template <class Derived> const Derived* perFilterConfigTyped(const std::string& name) const {
+    return dynamic_cast<const Derived*>(perFilterConfig(name));
+  }
 };
 
 typedef std::shared_ptr<const Route> RouteConstSharedPtr;

include/envoy/server/filter_config.h

@@ -296,14 +296,12 @@ class NamedHttpFilterConfigFactory {
   virtual ProtobufTypes::MessagePtr createEmptyConfigProto() { return nullptr; }
 
   /**
-   * @return ProtobufTypes::MessagePtr create an empty virtual host, route, or weight cluster-local
-   *         config proto message for v2. The filter config, which arrives in an opaque
-   *         google.protobuf.Struct message, will be converted to JSON and then parsed into this
-   *         empty proto. By default, this method returns the same value as createEmptyConfigProto,
-   *         and can be optionally overridden in implementations.
+   * @return RouteSpecificFilterConfigConstSharedPtr allow the filter to pre-process per route
+   * config. Returned object will be stored in the loaded route configuration.
    */
-  virtual ProtobufTypes::MessagePtr createEmptyRouteConfigProto() {
-    return createEmptyConfigProto();
+  virtual Router::RouteSpecificFilterConfigConstSharedPtr
+  createRouteSpecificFilterConfig(const ProtobufWkt::Struct&) {
+    return nullptr;
   }
 
   /**

source/common/config/utility.h

@@ -236,28 +236,6 @@ class Utility {
     return config;
   }
 
-  /**
-   * Translate a nested config into a route-specific proto message provided by
-   * the implementation factory.
-   * @param source a message (typically Struct) that contains the config for
-   *        the given factory's route-local configuration (as returned by
-   *        createEmptyRouteConfigProto).
-   * @param factory implementation factory with the method
-   *        'createEmptyRouteConfigProto' to produce a proto to be filled with
-   *        the translated configuration.
-   */
-  template <class Factory>
-  static ProtobufTypes::MessagePtr translateToFactoryRouteConfig(const Protobuf::Message& source,
-                                                                 Factory& factory) {
-    ProtobufTypes::MessagePtr config = factory.createEmptyRouteConfigProto();
-
-    // Fail in an obvious way if a plugin does not return a proto.
-    RELEASE_ASSERT(config != nullptr);
-
-    MessageUtil::jsonConvert(source, *config);
-    return config;
-  }
-
   /**
    * Create TagProducer instance. Check all tag names for conflicts to avoid
    * unexpected tag name overwriting.

source/common/http/async_client_impl.h

@@ -148,7 +148,9 @@ class AsyncStreamImpl : public AsyncClient::Stream,
     const Router::RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; }
     const Router::CorsPolicy* corsPolicy() const override { return nullptr; }
     const Router::Config& routeConfig() const override { return route_configuration_; }
-    const Protobuf::Message* perFilterConfig(const std::string&) const override { return nullptr; }
+    const Router::RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override {
+      return nullptr;
+    }
 
     static const NullRateLimitPolicy rate_limit_policy_;
     static const NullConfig route_configuration_;
@@ -203,7 +205,9 @@ class AsyncStreamImpl : public AsyncClient::Stream,
       return path_match_criterion_;
     }
 
-    const Protobuf::Message* perFilterConfig(const std::string&) const override { return nullptr; }
+    const Router::RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override {
+      return nullptr;
+    }
 
     static const NullRateLimitPolicy rate_limit_policy_;
     static const NullRetryPolicy retry_policy_;
@@ -226,7 +230,9 @@ class AsyncStreamImpl : public AsyncClient::Stream,
     const Router::DirectResponseEntry* directResponseEntry() const override { return nullptr; }
     const Router::RouteEntry* routeEntry() const override { return &route_entry_; }
     const Router::Decorator* decorator() const override { return nullptr; }
-    const Protobuf::Message* perFilterConfig(const std::string&) const override { return nullptr; }
+    const Router::RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override {
+      return nullptr;
+    }
 
     RouteEntryImpl route_entry_;
   };

source/common/router/BUILD

@@ -31,7 +31,6 @@ envoy_cc_library(
         "//source/common/common:utility_lib",
         "//source/common/config:metadata_lib",
         "//source/common/config:rds_json_lib",
-        "//source/common/config:utility_lib",
         "//source/common/config:well_known_names",
         "//source/common/http:headers_lib",
         "//source/common/http:utility_lib",

source/common/router/config_impl.cc

@@ -524,7 +524,8 @@ void RouteEntryImplBase::validateClusters(Upstream::ClusterManager& cm) const {
   }
 }
 
-const Protobuf::Message* RouteEntryImplBase::perFilterConfig(const std::string& name) const {
+const RouteSpecificFilterConfig*
+RouteEntryImplBase::perFilterConfig(const std::string& name) const {
   return per_filter_configs_.get(name);
 }
 
@@ -551,13 +552,10 @@ RouteEntryImplBase::WeightedClusterEntry::WeightedClusterEntry(
   }
 }
 
-const Protobuf::Message*
+const RouteSpecificFilterConfig*
 RouteEntryImplBase::WeightedClusterEntry::perFilterConfig(const std::string& name) const {
-  const Protobuf::Message* cfg = per_filter_configs_.get(name);
-  if (cfg != nullptr) {
-    return cfg;
-  }
-  return DynamicRouteEntry::perFilterConfig(name);
+  const auto cfg = per_filter_configs_.get(name);
+  return cfg != nullptr ? cfg : DynamicRouteEntry::perFilterConfig(name);
 }
 
 PrefixRouteEntryImpl::PrefixRouteEntryImpl(const VirtualHostImpl& vhost,
@@ -735,7 +733,7 @@ VirtualHostImpl::VirtualClusterEntry::VirtualClusterEntry(
 
 const Config& VirtualHostImpl::routeConfig() const { return global_route_config_; }
 
-const Protobuf::Message* VirtualHostImpl::perFilterConfig(const std::string& name) const {
+const RouteSpecificFilterConfig* VirtualHostImpl::perFilterConfig(const std::string& name) const {
   return per_filter_configs_.get(name);
 }
 
@@ -886,13 +884,16 @@ PerFilterConfigs::PerFilterConfigs(const Protobuf::Map<std::string, ProtobufWkt:
     auto& factory = Envoy::Config::Utility::getAndCheckFactory<
         Server::Configuration::NamedHttpFilterConfigFactory>(name);
 
-    configs_[name] = Envoy::Config::Utility::translateToFactoryRouteConfig(struct_config, factory);
+    auto object = factory.createRouteSpecificFilterConfig(struct_config);
+    if (object) {
+      configs_[name] = object;
+    }
   }
 }
 
-const Protobuf::Message* PerFilterConfigs::get(const std::string& name) const {
-  auto cfg = configs_.find(name);
-  return cfg == configs_.end() ? nullptr : cfg->second.get();
+const RouteSpecificFilterConfig* PerFilterConfigs::get(const std::string& name) const {
+  auto it = configs_.find(name);
+  return it == configs_.end() ? nullptr : it->second.get();
 }
 
 } // namespace Router

source/common/router/config_impl.h

@@ -48,10 +48,10 @@ class PerFilterConfigs {
 public:
   PerFilterConfigs(const Protobuf::Map<std::string, ProtobufWkt::Struct>& configs);
 
-  const Protobuf::Message* get(const std::string& name) const;
+  const RouteSpecificFilterConfig* get(const std::string& name) const;
 
 private:
-  std::unordered_map<std::string, ProtobufTypes::MessagePtr> configs_;
+  std::unordered_map<std::string, RouteSpecificFilterConfigConstSharedPtr> configs_;
 };
 
 class RouteEntryImplBase;
@@ -76,7 +76,9 @@ class SslRedirectRoute : public Route {
   const DirectResponseEntry* directResponseEntry() const override { return &SSL_REDIRECTOR; }
   const RouteEntry* routeEntry() const override { return nullptr; }
   const Decorator* decorator() const override { return nullptr; }
-  const Protobuf::Message* perFilterConfig(const std::string&) const override { return nullptr; }
+  const RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override {
+    return nullptr;
+  }
 
 private:
   static const SslRedirector SSL_REDIRECTOR;
@@ -130,7 +132,7 @@ class VirtualHostImpl : public VirtualHost {
   const std::string& name() const override { return name_; }
   const RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; }
   const Config& routeConfig() const override;
-  const Protobuf::Message* perFilterConfig(const std::string& name) const override;
+  const RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override;
 
 private:
   enum class SslRequirements { NONE, EXTERNAL_ONLY, ALL };
@@ -370,7 +372,7 @@ class RouteEntryImplBase : public RouteEntry,
   const DirectResponseEntry* directResponseEntry() const override;
   const RouteEntry* routeEntry() const override;
   const Decorator* decorator() const override { return decorator_.get(); }
-  const Protobuf::Message* perFilterConfig(const std::string& name) const override;
+  const RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override;
 
 protected:
   const bool case_sensitive_;
@@ -444,7 +446,7 @@ class RouteEntryImplBase : public RouteEntry,
     const RouteEntry* routeEntry() const override { return this; }
     const Decorator* decorator() const override { return parent_->decorator(); }
 
-    const Protobuf::Message* perFilterConfig(const std::string& name) const override {
+    const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const override {
       return parent_->perFilterConfig(name);
     };
 
@@ -487,7 +489,7 @@ class RouteEntryImplBase : public RouteEntry,
       DynamicRouteEntry::finalizeResponseHeaders(headers, request_info);
     }
 
-    const Protobuf::Message* perFilterConfig(const std::string& name) const override;
+    const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const override;
 
   private:
     const std::string runtime_key_;

source/server/options_impl.cc

@@ -38,7 +38,6 @@ OptionsImpl::OptionsImpl(int argc, char** argv, const HotRestartVersionCb& hot_r
   }
   log_levels_string +=
       fmt::format("\nDefault is [{}]", spdlog::level::level_names[default_log_level]);
-  log_levels_string += "\n[trace] and [debug] are only available on debug builds";
 
   const std::string log_format_string =
       fmt::format("Log message format in spdlog syntax "

test/BUILD

@@ -18,6 +18,7 @@ envoy_cc_test_library(
         "//source/common/common:thread_lib",
         "//source/common/event:libevent_lib",
         "//test/test_common:environment_lib",
+        "//test/mocks/access_log:access_log_mocks",
         "//test/test_common:printers_lib",
     ] + select({
         "//bazel:disable_signal_trace": [],

test/common/access_log/BUILD

@@ -2,12 +2,20 @@ licenses(["notice"])  # Apache 2
 
 load(
     "//bazel:envoy_build_system.bzl",
+    "envoy_cc_binary",
+    "envoy_cc_library",
     "envoy_cc_test",
     "envoy_package",
 )
 
 envoy_package()
 
+envoy_cc_library(
+    name = "test_util",
+    hdrs = ["test_util.h"],
+    deps = [],
+)
+
 envoy_cc_test(
     name = "access_log_formatter_test",
     srcs = ["access_log_formatter_test.cc"],
@@ -26,6 +34,7 @@ envoy_cc_test(
     name = "access_log_impl_test",
     srcs = ["access_log_impl_test.cc"],
     deps = [
+        ":test_util",
         "//source/common/access_log:access_log_lib",
         "//source/common/config:filter_json_lib",
         "//source/extensions/access_loggers/file:config",
@@ -52,3 +61,21 @@ envoy_cc_test(
         "//test/mocks/filesystem:filesystem_mocks",
     ],
 )
+
+envoy_cc_binary(
+    name = "access_log_formatter_speed_test",
+    testonly = 1,
+    srcs = ["access_log_formatter_speed_test.cc"],
+    external_deps = [
+        "benchmark",
+    ],
+    deps = [
+        ":test_util",
+        "//source/common/access_log:access_log_formatter_lib",
+        "//source/common/http:header_map_lib",
+        "//source/common/network:address_lib",
+        "//test/mocks/http:http_mocks",
+        "//test/mocks/request_info:request_info_mocks",
+        "//test/test_common:printers_lib",
+    ],
+)