formatter: add a formatter that returns a google::protobuf::Struct rather than a string

source/common/formatter/substitution_formatter.cc

@@ -49,8 +49,8 @@ const std::regex& getStartTimeNewlinePattern() {
 }
 const std::regex& getNewlinePattern() { CONSTRUCT_ON_FIRST_USE(std::regex, "\n"); }
 
-template <class... Ts> struct JsonFormatMapVisitor : Ts... { using Ts::operator()...; };
-template <class... Ts> JsonFormatMapVisitor(Ts...) -> JsonFormatMapVisitor<Ts...>;
+template <class... Ts> struct StructFormatMapVisitor : Ts... { using Ts::operator()...; };
+template <class... Ts> StructFormatMapVisitor(Ts...) -> StructFormatMapVisitor<Ts...>;
 
 } // namespace
 
@@ -135,17 +135,17 @@ std::string JsonFormatterImpl::format(const Http::RequestHeaderMap& request_head
                                       const Http::ResponseTrailerMap& response_trailers,
                                       const StreamInfo::StreamInfo& stream_info,
                                       absl::string_view local_reply_body) const {
-  const auto output_struct =
-      toStruct(request_headers, response_headers, response_trailers, stream_info, local_reply_body);
+  const auto output_struct = struct_formatter_.format(
+      request_headers, response_headers, response_trailers, stream_info, local_reply_body);
 
   const std::string log_line = MessageUtil::getJsonStringFromMessage(output_struct, false, true);
   return absl::StrCat(log_line, "\n");
 }
 
-JsonFormatterImpl::JsonFormatMapWrapper
-JsonFormatterImpl::toFormatMap(const ProtobufWkt::Struct& json_format) const {
-  auto output = std::make_unique<JsonFormatMap>();
-  for (const auto& pair : json_format.fields()) {
+StructFormatter::StructFormatMapWrapper
+StructFormatter::toFormatMap(const ProtobufWkt::Struct& struct_format) const {
+  auto output = std::make_unique<StructFormatMap>();
+  for (const auto& pair : struct_format.fields()) {
     switch (pair.second.kind_case()) {
     case ProtobufWkt::Value::kStringValue:
       output->emplace(pair.first, SubstitutionFormatParser::parse(pair.second.string_value()));
@@ -155,17 +155,17 @@ JsonFormatterImpl::toFormatMap(const ProtobufWkt::Struct& json_format) const {
       break;
     default:
       throw EnvoyException(
-          "Only string values or nested structs are supported in the JSON access log format.");
+          "Only string values or nested structs are supported in structured access log format.");
     }
   }
   return {std::move(output)};
 };
 
-ProtobufWkt::Struct JsonFormatterImpl::toStruct(const Http::RequestHeaderMap& request_headers,
-                                                const Http::ResponseHeaderMap& response_headers,
-                                                const Http::ResponseTrailerMap& response_trailers,
-                                                const StreamInfo::StreamInfo& stream_info,
-                                                absl::string_view local_reply_body) const {
+ProtobufWkt::Struct StructFormatter::format(const Http::RequestHeaderMap& request_headers,
+                                            const Http::ResponseHeaderMap& response_headers,
+                                            const Http::ResponseTrailerMap& response_trailers,
+                                            const StreamInfo::StreamInfo& stream_info,
+                                            absl::string_view local_reply_body) const {
   const std::string& empty_value =
       omit_empty_values_ ? EMPTY_STRING : DefaultUnspecifiedValueString;
   const std::function<ProtobufWkt::Value(const std::vector<FormatterProviderPtr>&)>
@@ -197,11 +197,11 @@ ProtobufWkt::Struct JsonFormatterImpl::toStruct(const Http::RequestHeaderMap& re
         }
         return ValueUtil::stringValue(str);
       };
-  const std::function<ProtobufWkt::Value(const JsonFormatterImpl::JsonFormatMapWrapper&)>
-      json_format_map_callback = [&](const JsonFormatterImpl::JsonFormatMapWrapper& format) {
+  const std::function<ProtobufWkt::Value(const StructFormatter::StructFormatMapWrapper&)>
+      struct_format_map_callback = [&](const StructFormatter::StructFormatMapWrapper& format) {
         ProtobufWkt::Struct output;
         auto* fields = output.mutable_fields();
-        JsonFormatMapVisitor visitor{json_format_map_callback, providers_callback};
+        StructFormatMapVisitor visitor{struct_format_map_callback, providers_callback};
         for (const auto& pair : *format.value_) {
           ProtobufWkt::Value value = absl::visit(visitor, pair.second);
           if (omit_empty_values_ && value.kind_case() == ProtobufWkt::Value::kNullValue) {
@@ -211,7 +211,7 @@ ProtobufWkt::Struct JsonFormatterImpl::toStruct(const Http::RequestHeaderMap& re
         }
         return ValueUtil::structValue(output);
       };
-  return json_format_map_callback(json_output_format_).struct_value();
+  return struct_format_map_callback(struct_output_format_).struct_value();
 }
 
 void SubstitutionFormatParser::parseCommandHeader(const std::string& token, const size_t start,

source/common/formatter/substitution_formatter.h

@@ -107,12 +107,47 @@ class FormatterImpl : public Formatter {
   std::vector<FormatterProviderPtr> providers_;
 };
 
+/**
+ * An formatter for structured log formats, which returns a Struct proto that
+ * can be converted easily into multiple formats.
+ */
+class StructFormatter {
+public:
+  StructFormatter(const ProtobufWkt::Struct& format_mapping, bool preserve_types,
+                  bool omit_empty_values)
+      : omit_empty_values_(omit_empty_values), preserve_types_(preserve_types),
+        struct_output_format_(toFormatMap(format_mapping)) {}
+
+  ProtobufWkt::Struct format(const Http::RequestHeaderMap& request_headers,
+                             const Http::ResponseHeaderMap& response_headers,
+                             const Http::ResponseTrailerMap& response_trailers,
+                             const StreamInfo::StreamInfo& stream_info,
+                             absl::string_view local_reply_body) const;
+
+private:
+  struct StructFormatMapWrapper;
+  using StructFormatMapValue =
+      absl::variant<const std::vector<FormatterProviderPtr>, const StructFormatMapWrapper>;
+  // Although not required for Struct/JSON, it is nice to have the order of
+  // properties preserved between the format and the log entry, thus std::map.
+  using StructFormatMap = std::map<std::string, StructFormatMapValue>;
+  using StructFormatMapPtr = std::unique_ptr<StructFormatMap>;
+  struct StructFormatMapWrapper {
+    StructFormatMapPtr value_;
+  };
+
+  StructFormatMapWrapper toFormatMap(const ProtobufWkt::Struct& struct_format) const;
+
+  const bool omit_empty_values_;
+  const bool preserve_types_;
+  const StructFormatMapWrapper struct_output_format_;
+};
+
 class JsonFormatterImpl : public Formatter {
 public:
   JsonFormatterImpl(const ProtobufWkt::Struct& format_mapping, bool preserve_types,
                     bool omit_empty_values)
-      : omit_empty_values_(omit_empty_values), preserve_types_(preserve_types),
-        json_output_format_(toFormatMap(format_mapping)) {}
+      : struct_formatter_(format_mapping, preserve_types, omit_empty_values) {}
 
   // Formatter::format
   std::string format(const Http::RequestHeaderMap& request_headers,
@@ -122,27 +157,7 @@ class JsonFormatterImpl : public Formatter {
                      absl::string_view local_reply_body) const override;
 
 private:
-  struct JsonFormatMapWrapper;
-  using JsonFormatMapValue =
-      absl::variant<const std::vector<FormatterProviderPtr>, const JsonFormatMapWrapper>;
-  // Although not required for JSON, it is nice to have the order of properties
-  // preserved between the format and the log entry, thus std::map.
-  using JsonFormatMap = std::map<std::string, JsonFormatMapValue>;
-  using JsonFormatMapPtr = std::unique_ptr<JsonFormatMap>;
-  struct JsonFormatMapWrapper {
-    JsonFormatMapPtr value_;
-  };
-
-  bool omit_empty_values_;
-  bool preserve_types_;
-  const JsonFormatMapWrapper json_output_format_;
-
-  ProtobufWkt::Struct toStruct(const Http::RequestHeaderMap& request_headers,
-                               const Http::ResponseHeaderMap& response_headers,
-                               const Http::ResponseTrailerMap& response_trailers,
-                               const StreamInfo::StreamInfo& stream_info,
-                               absl::string_view local_reply_body) const;
-  JsonFormatMapWrapper toFormatMap(const ProtobufWkt::Struct& json_format) const;
+  const StructFormatter struct_formatter_;
 };
 
 /**

test/common/formatter/substitution_format_string_test.cc

@@ -93,7 +93,7 @@ TEST_F(SubstitutionFormatStringUtilsTest, TestInvalidConfigs) {
     TestUtility::loadFromYaml(yaml, config_);
     EXPECT_THROW_WITH_MESSAGE(
         SubstitutionFormatStringUtils::fromProtoConfig(config_, context_.api()), EnvoyException,
-        "Only string values or nested structs are supported in the JSON access log format.");
+        "Only string values or nested structs are supported in structured access log format.");
   }
 }
 

test/common/formatter/substitution_formatter_speed_test.cc

@@ -28,6 +28,24 @@ std::unique_ptr<Envoy::Formatter::JsonFormatterImpl> makeJsonFormatter(bool type
   return std::make_unique<Envoy::Formatter::JsonFormatterImpl>(JsonLogFormat, typed, false);
 }
 
+std::unique_ptr<Envoy::Formatter::StructFormatter> makeStructFormatter(bool typed) {
+  ProtobufWkt::Struct StructLogFormat;
+  const std::string format_yaml = R"EOF(
+    remote_address: '%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%'
+    start_time: '%START_TIME(%Y/%m/%dT%H:%M:%S%z %s)%'
+    method: '%REQ(:METHOD)%'
+    url: '%REQ(X-FORWARDED-PROTO)%://%REQ(:AUTHORITY)%%REQ(X-ENVOY-ORIGINAL-PATH?:PATH)%'
+    protocol: '%PROTOCOL%'
+    respoinse_code: '%RESPONSE_CODE%'
+    bytes_sent: '%BYTES_SENT%'
+    duration: '%DURATION%'
+    referer: '%REQ(REFERER)%'
+    user-agent: '%REQ(USER-AGENT)%'
+  )EOF";
+  TestUtility::loadFromYaml(format_yaml, StructLogFormat);
+  return std::make_unique<Envoy::Formatter::StructFormatter>(StructLogFormat, typed, false);
+}
+
 std::unique_ptr<Envoy::TestStreamInfo> makeStreamInfo() {
   auto stream_info = std::make_unique<Envoy::TestStreamInfo>();
   stream_info->setDownstreamRemoteAddress(
@@ -54,7 +72,7 @@ static void BM_AccessLogFormatter(benchmark::State& state) {
   Http::TestResponseHeaderMapImpl response_headers;
   Http::TestResponseTrailerMapImpl response_trailers;
   std::string body;
-  for (auto _ : state) {
+  for (auto _ : state) { // NOLINT: Silences warning about dead store
     output_bytes +=
         formatter->format(request_headers, response_headers, response_trailers, *stream_info, body)
             .length();
@@ -63,6 +81,47 @@ static void BM_AccessLogFormatter(benchmark::State& state) {
 }
 BENCHMARK(BM_AccessLogFormatter);
 
+// NOLINTNEXTLINE(readability-identifier-naming)
+static void BM_StructAccessLogFormatter(benchmark::State& state) {
+  std::unique_ptr<Envoy::TestStreamInfo> stream_info = makeStreamInfo();
+  std::unique_ptr<Envoy::Formatter::StructFormatter> struct_formatter = makeStructFormatter(false);
+
+  size_t output_bytes = 0;
+  Http::TestRequestHeaderMapImpl request_headers;
+  Http::TestResponseHeaderMapImpl response_headers;
+  Http::TestResponseTrailerMapImpl response_trailers;
+  std::string body;
+  for (auto _ : state) { // NOLINT: Silences warning about dead store
+    output_bytes +=
+        struct_formatter
+            ->format(request_headers, response_headers, response_trailers, *stream_info, body)
+            .ByteSize();
+  }
+  benchmark::DoNotOptimize(output_bytes);
+}
+BENCHMARK(BM_StructAccessLogFormatter);
+
+// NOLINTNEXTLINE(readability-identifier-naming)
+static void BM_TypedStructAccessLogFormatter(benchmark::State& state) {
+  std::unique_ptr<Envoy::TestStreamInfo> stream_info = makeStreamInfo();
+  std::unique_ptr<Envoy::Formatter::StructFormatter> typed_struct_formatter =
+      makeStructFormatter(true);
+
+  size_t output_bytes = 0;
+  Http::TestRequestHeaderMapImpl request_headers;
+  Http::TestResponseHeaderMapImpl response_headers;
+  Http::TestResponseTrailerMapImpl response_trailers;
+  std::string body;
+  for (auto _ : state) { // NOLINT: Silences warning about dead store
+    output_bytes +=
+        typed_struct_formatter
+            ->format(request_headers, response_headers, response_trailers, *stream_info, body)
+            .ByteSize();
+  }
+  benchmark::DoNotOptimize(output_bytes);
+}
+BENCHMARK(BM_TypedStructAccessLogFormatter);
+
 // NOLINTNEXTLINE(readability-identifier-naming)
 static void BM_JsonAccessLogFormatter(benchmark::State& state) {
   std::unique_ptr<Envoy::TestStreamInfo> stream_info = makeStreamInfo();
@@ -73,7 +132,7 @@ static void BM_JsonAccessLogFormatter(benchmark::State& state) {
   Http::TestResponseHeaderMapImpl response_headers;
   Http::TestResponseTrailerMapImpl response_trailers;
   std::string body;
-  for (auto _ : state) {
+  for (auto _ : state) { // NOLINT: Silences warning about dead store
     output_bytes +=
         json_formatter
             ->format(request_headers, response_headers, response_trailers, *stream_info, body)
@@ -94,7 +153,7 @@ static void BM_TypedJsonAccessLogFormatter(benchmark::State& state) {
   Http::TestResponseHeaderMapImpl response_headers;
   Http::TestResponseTrailerMapImpl response_trailers;
   std::string body;
-  for (auto _ : state) {
+  for (auto _ : state) { // NOLINT: Silences warning about dead store
     output_bytes +=
         typed_json_formatter
             ->format(request_headers, response_headers, response_trailers, *stream_info, body)