Transformation opt (#22)

* do more things as onetime init for more performance
* transform cleanup 2
* nicer errors and logs

Author: yuval-k

source/extensions/filters/http/transformation/BUILD

@@ -82,6 +82,7 @@ envoy_cc_library(
         "@envoy//include/envoy/buffer:buffer_interface",
         "@envoy//include/envoy/http:header_map_interface",
         "@envoy//source/common/common:macros",
+        "@envoy//source/common/common:utility_lib",
         "@envoy//source/common/protobuf",
         "@inja//:inja-lib",
         "@json//:json-lib",

source/extensions/filters/http/transformation/body_header_transformer.h

@@ -12,6 +12,7 @@ namespace Transformation {
 class BodyHeaderTransformer : public Transformer {
 public:
   void transform(Http::HeaderMap &map, Buffer::Instance &body) const override;
+  bool passthrough_body() const override { return false; };
 };
 
 } // namespace Transformation

source/extensions/filters/http/transformation/inja_transformer.cc

@@ -3,6 +3,7 @@
 #include <iterator>
 
 #include "common/common/macros.h"
+#include "common/common/utility.h"
 
 // For convenience
 using namespace inja;
@@ -34,24 +35,23 @@ const Http::HeaderEntry *getHeader(const Http::HeaderMap &header_map,
 
 } // namespace
 
-std::string ExtractorUtil::extract(
-    const envoy::api::v2::filter::http::Extraction &extractor,
-    const Http::HeaderMap &header_map) {
+Extractor::Extractor(const envoy::api::v2::filter::http::Extraction &extractor)
+    : headername_(extractor.header()), group_(extractor.subgroup()),
+      extract_regex_(RegexUtil::parseRegex(extractor.regex())) {}
+std::string Extractor::extract(const Http::HeaderMap &header_map) const {
   // TODO: should we lowercase them in the config?
-  const std::string &headername = extractor.header();
-  const Http::HeaderEntry *header_entry = getHeader(header_map, headername);
+  const Http::HeaderEntry *header_entry = getHeader(header_map, headername_);
   if (!header_entry) {
     return "";
   }
 
   std::string value(header_entry->value().getStringView());
-  unsigned int group = extractor.subgroup();
+
   // get and regex
-  std::regex extract_regex(extractor.regex());
   std::smatch regex_result;
-  if (std::regex_match(value, regex_result, extract_regex)) {
+  if (std::regex_match(value, regex_result, extract_regex_)) {
     std::smatch::iterator submatch_it = regex_result.begin();
-    for (unsigned i = 0; i < group; i++) {
+    for (unsigned i = 0; i < group_; i++) {
       std::advance(submatch_it, 1);
       if (submatch_it == regex_result.end()) {
         return "";
@@ -65,7 +65,8 @@ std::string ExtractorUtil::extract(
 
 TransformerInstance::TransformerInstance(
     const Http::HeaderMap &header_map,
-    const std::map<std::string, std::string> &extractions, const json &context)
+    const std::unordered_map<std::string, std::string> &extractions,
+    const json &context)
     : header_map_(header_map), extractions_(extractions), context_(context) {
   env_.add_callback("header", 1,
                     [this](Arguments args) { return header_callback(args); });
@@ -93,13 +94,59 @@ json TransformerInstance::extracted_callback(Arguments args) {
   return "";
 }
 
-std::string TransformerInstance::render(const std::string &input) {
+std::string TransformerInstance::render(const inja::Template &input) {
   return env_.render(input, context_);
 }
 
 InjaTransformer::InjaTransformer(
     const envoy::api::v2::filter::http::TransformationTemplate &transformation)
-    : transformation_(transformation) {}
+    : advanced_templates_(transformation.advanced_templates()),
+      passthrough_body_(transformation.has_passthrough()) {
+  inja::ParserConfig parser_config;
+  inja::LexerConfig lexer_config;
+  inja::TemplateStorage template_storage;
+  if (!advanced_templates_) {
+    parser_config.notation = inja::ElementNotation::Dot;
+  }
+
+  inja::Parser parser(parser_config, lexer_config, template_storage);
+
+  const auto &extractors = transformation.extractors();
+  for (auto it = extractors.begin(); it != extractors.end(); it++) {
+    extractors_.emplace_back(std::make_pair(it->first, it->second));
+  }
+  const auto &headers = transformation.headers();
+  for (auto it = headers.begin(); it != headers.end(); it++) {
+    Http::LowerCaseString header_name(it->first);
+    try {
+      headers_.emplace_back(std::make_pair(std::move(header_name),
+                                           parser.parse(it->second.text())));
+    } catch (const std::runtime_error &e) {
+      throw EnvoyException(fmt::format(
+          "Failed to parse header template '{}': {}", it->first, e.what()));
+    }
+  }
+
+  switch (transformation.body_transformation_case()) {
+  case envoy::api::v2::filter::http::TransformationTemplate::kBody: {
+    try {
+      body_template_.emplace(parser.parse(transformation.body().text()));
+    } catch (const std::runtime_error &e) {
+      throw EnvoyException(
+          fmt::format("Failed to parse body template {}", e.what()));
+    }
+  }
+  case envoy::api::v2::filter::http::TransformationTemplate::
+      kMergeExtractorsToBody: {
+    merged_extractors_to_body_ = true;
+  }
+  case envoy::api::v2::filter::http::TransformationTemplate::kPassthrough:
+  case envoy::api::v2::filter::http::TransformationTemplate::
+      BODY_TRANSFORMATION_NOT_SET: {
+    break;
+  }
+  }
+}
 
 InjaTransformer::~InjaTransformer() {}
 
@@ -113,15 +160,15 @@ void InjaTransformer::transform(Http::HeaderMap &header_map,
     json_body = json::parse(bodystring);
   }
   // get the extractions
-  std::map<std::string, std::string> extractions;
-
-  const auto &extractors = transformation_.extractors();
+  std::unordered_map<std::string, std::string> extractions;
+  if (advanced_templates_) {
+    extractions.reserve(extractors_.size());
+  }
 
-  for (auto it = extractors.begin(); it != extractors.end(); it++) {
-    const std::string &name = it->first;
-    const envoy::api::v2::filter::http::Extraction &extractor = it->second;
-    if (transformation_.advanced_templates()) {
-      extractions[name] = ExtractorUtil::extract(extractor, header_map);
+  for (const auto &named_extractor : extractors_) {
+    const std::string &name = named_extractor.first;
+    if (advanced_templates_) {
+      extractions[name] = named_extractor.second.extract(header_map);
     } else {
       std::string name_to_split = name;
       json *current = &json_body;
@@ -131,62 +178,40 @@ void InjaTransformer::transform(Http::HeaderMap &header_map,
         current = &(*current)[field_name];
         name_to_split.erase(0, pos + 1);
       }
-      (*current)[name_to_split] = ExtractorUtil::extract(extractor, header_map);
+      (*current)[name_to_split] = named_extractor.second.extract(header_map);
     }
   }
   // start transforming!
   TransformerInstance instance(header_map, extractions, json_body);
 
-  if (!transformation_.advanced_templates()) {
-    instance.useDotNotation();
-  }
-
-  switch (transformation_.body_transformation_case()) {
-  case envoy::api::v2::filter::http::TransformationTemplate::kBody: {
-    const std::string &input = transformation_.body().text();
-    auto output = instance.render(input);
-
+  // Body transform:
+  auto replace_body = [&](std::string &output) {
     // remove content length, as we have new body.
     header_map.removeContentLength();
     // replace body
     body.drain(body.length());
     body.add(output);
     header_map.insertContentLength().value(body.length());
-    break;
-  }
+  };
 
-  case envoy::api::v2::filter::http::TransformationTemplate::
-      kMergeExtractorsToBody: {
+  if (body_template_.has_value()) {
+    std::string output = instance.render(body_template_.value());
+    replace_body(output);
+  } else if (merged_extractors_to_body_) {
     std::string output = json_body.dump();
-
-    // remove content length, as we have new body.
-    header_map.removeContentLength();
-    // replace body
-    body.drain(body.length());
-    body.add(output);
-    header_map.insertContentLength().value(body.length());
-    break;
-  }
-  case envoy::api::v2::filter::http::TransformationTemplate::kPassthrough:
-  case envoy::api::v2::filter::http::TransformationTemplate::
-      BODY_TRANSFORMATION_NOT_SET: {
-    break;
+    replace_body(output);
   }
-  }
-
-  // add headers
-  const auto &headers = transformation_.headers();
 
-  for (auto it = headers.begin(); it != headers.end(); it++) {
-    std::string name = it->first;
-    auto lkname = Http::LowerCaseString(std::move(name));
-    const envoy::api::v2::filter::http::InjaTemplate &text = it->second;
-    std::string output = instance.render(text.text());
+  // Headers transform:
+  for (const auto &templated_header : headers_) {
+    std::string output = instance.render(templated_header.second);
     // remove existing header
-    header_map.remove(lkname);
+    header_map.remove(templated_header.first);
     // TODO(yuval-k): Do we need to support intentional empty headers?
     if (!output.empty()) {
-      header_map.addCopy(lkname, output);
+      // we can add the key as reference as the headers_ lifetime is as the
+      // route's
+      header_map.addReferenceKey(templated_header.first, output);
     }
   }
 }

source/extensions/filters/http/transformation/inja_transformer.h

@@ -21,33 +21,34 @@ namespace Transformation {
 
 class TransformerInstance {
 public:
-  TransformerInstance(const Http::HeaderMap &header_map,
-                      const std::map<std::string, std::string> &extractions,
-                      const nlohmann::json &context);
+  TransformerInstance(
+      const Http::HeaderMap &header_map,
+      const std::unordered_map<std::string, std::string> &extractions,
+      const nlohmann::json &context);
   // header_value(name)
   // extracted_value(name, index)
   nlohmann::json header_callback(inja::Arguments args);
 
   nlohmann::json extracted_callback(inja::Arguments args);
 
-  std::string render(const std::string &input);
-
-  void useDotNotation() {
-    env_.set_element_notation(inja::ElementNotation::Dot);
-  }
+  std::string render(const inja::Template &input);
 
 private:
   inja::Environment env_;
   const Http::HeaderMap &header_map_;
-  const std::map<std::string, std::string> &extractions_;
+  const std::unordered_map<std::string, std::string> &extractions_;
   const nlohmann::json &context_;
 };
 
-class ExtractorUtil {
+class Extractor {
 public:
-  static std::string
-  extract(const envoy::api::v2::filter::http::Extraction &extractor,
-          const Http::HeaderMap &header_map);
+  Extractor(const envoy::api::v2::filter::http::Extraction &extractor);
+  std::string extract(const Http::HeaderMap &header_map) const;
+
+private:
+  const Http::LowerCaseString headername_;
+  const unsigned int group_;
+  const std::regex extract_regex_;
 };
 
 class InjaTransformer : public Transformer {
@@ -57,6 +58,7 @@ class InjaTransformer : public Transformer {
   ~InjaTransformer();
 
   void transform(Http::HeaderMap &map, Buffer::Instance &body) const override;
+  bool passthrough_body() const override { return passthrough_body_; };
 
 private:
   /*
@@ -69,7 +71,13 @@ class InjaTransformer : public Transformer {
 
     std::aligned_storage<TransformerImplSize, TransformerImplAlign>::type impl_;
   */
-  const envoy::api::v2::filter::http::TransformationTemplate &transformation_;
+  bool advanced_templates_{};
+  bool passthrough_body_{};
+  std::vector<std::pair<std::string, Extractor>> extractors_;
+  std::vector<std::pair<Http::LowerCaseString, inja::Template>> headers_;
+
+  absl::optional<inja::Template> body_template_;
+  bool merged_extractors_to_body_{};
 };
 
 } // namespace Transformation