diff --git a/source/common/formatter/substitution_formatter.cc b/source/common/formatter/substitution_formatter.cc index d7fca3cf7a493..dac7661ac0708 100644 --- a/source/common/formatter/substitution_formatter.cc +++ b/source/common/formatter/substitution_formatter.cc @@ -49,9 +49,6 @@ const std::regex& getSystemTimeFormatNewlinePattern() { } const std::regex& getNewlinePattern() { CONSTRUCT_ON_FIRST_USE(std::regex, "\n"); } -template struct StructFormatMapVisitor : Ts... { using Ts::operator()...; }; -template StructFormatMapVisitor(Ts...) -> StructFormatMapVisitor; - } // namespace const std::string SubstitutionFormatUtils::DEFAULT_FORMAT = @@ -148,76 +145,145 @@ std::string JsonFormatterImpl::format(const Http::RequestHeaderMap& request_head return absl::StrCat(log_line, "\n"); } +StructFormatter::StructFormatter(const ProtobufWkt::Struct& format_mapping, bool preserve_types, + bool omit_empty_values) + : omit_empty_values_(omit_empty_values), preserve_types_(preserve_types), + empty_value_(omit_empty_values_ ? EMPTY_STRING : DefaultUnspecifiedValueString), + struct_output_format_(toFormatMapValue(format_mapping)) {} + StructFormatter::StructFormatMapWrapper -StructFormatter::toFormatMap(const ProtobufWkt::Struct& struct_format) const { +StructFormatter::toFormatMapValue(const ProtobufWkt::Struct& struct_format) const { auto output = std::make_unique(); 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())); + output->emplace(pair.first, toFormatStringValue(pair.second.string_value())); break; + case ProtobufWkt::Value::kStructValue: - output->emplace(pair.first, toFormatMap(pair.second.struct_value())); + output->emplace(pair.first, toFormatMapValue(pair.second.struct_value())); break; + + case ProtobufWkt::Value::kListValue: + output->emplace(pair.first, toFormatListValue(pair.second.list_value())); + break; + default: - throw EnvoyException( - "Only string values or nested structs are supported in structured access log format."); + throw EnvoyException("Only string values, nested structs and list values are " + "supported in structured access log format."); } } return {std::move(output)}; }; +StructFormatter::StructFormatListWrapper +StructFormatter::toFormatListValue(const ProtobufWkt::ListValue& list_value_format) const { + auto output = std::make_unique(); + for (const auto& value : list_value_format.values()) { + switch (value.kind_case()) { + case ProtobufWkt::Value::kStringValue: + output->emplace_back(toFormatStringValue(value.string_value())); + break; + + case ProtobufWkt::Value::kStructValue: + output->emplace_back(toFormatMapValue(value.struct_value())); + break; + + case ProtobufWkt::Value::kListValue: + output->emplace_back(toFormatListValue(value.list_value())); + break; + default: + throw EnvoyException("Only string values, nested structs and list values are " + "supported in structured access log format."); + } + } + return {std::move(output)}; +} + +std::vector +StructFormatter::toFormatStringValue(const std::string& string_format) const { + return SubstitutionFormatParser::parse(string_format); +}; + +ProtobufWkt::Value StructFormatter::providersCallback( + const std::vector& providers, + 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 { + ASSERT(!providers.empty()); + if (providers.size() == 1) { + const auto& provider = providers.front(); + if (preserve_types_) { + return provider->formatValue(request_headers, response_headers, response_trailers, + stream_info, local_reply_body); + } + + if (omit_empty_values_) { + return ValueUtil::optionalStringValue(provider->format( + request_headers, response_headers, response_trailers, stream_info, local_reply_body)); + } + + const auto str = provider->format(request_headers, response_headers, response_trailers, + stream_info, local_reply_body); + return ValueUtil::stringValue(str.value_or(DefaultUnspecifiedValueString)); + } + // Multiple providers forces string output. + std::string str; + for (const auto& provider : providers) { + const auto bit = provider->format(request_headers, response_headers, response_trailers, + stream_info, local_reply_body); + str += bit.value_or(empty_value_); + } + return ValueUtil::stringValue(str); +} + +ProtobufWkt::Value StructFormatter::structFormatMapCallback( + const StructFormatter::StructFormatMapWrapper& format_map, + const StructFormatter::StructFormatMapVisitor& visitor) const { + ProtobufWkt::Struct output; + auto* fields = output.mutable_fields(); + for (const auto& pair : *format_map.value_) { + ProtobufWkt::Value value = absl::visit(visitor, pair.second); + if (omit_empty_values_ && value.kind_case() == ProtobufWkt::Value::kNullValue) { + continue; + } + (*fields)[pair.first] = value; + } + return ValueUtil::structValue(output); +} + +ProtobufWkt::Value StructFormatter::structFormatListCallback( + const StructFormatter::StructFormatListWrapper& format_list, + const StructFormatter::StructFormatMapVisitor& visitor) const { + std::vector output; + for (const auto& val : *format_list.value_) { + ProtobufWkt::Value value = absl::visit(visitor, val); + if (omit_empty_values_ && value.kind_case() == ProtobufWkt::Value::kNullValue) { + continue; + } + output.push_back(value); + } + return ValueUtil::listValue(output); +} + 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&)> - providers_callback = [&](const std::vector& providers) { - ASSERT(!providers.empty()); - if (providers.size() == 1) { - const auto& provider = providers.front(); - if (preserve_types_) { - return provider->formatValue(request_headers, response_headers, response_trailers, - stream_info, local_reply_body); - } - - if (omit_empty_values_) { - return ValueUtil::optionalStringValue( - provider->format(request_headers, response_headers, response_trailers, stream_info, - local_reply_body)); - } - - const auto str = provider->format(request_headers, response_headers, response_trailers, - stream_info, local_reply_body); - return ValueUtil::stringValue(str.value_or(DefaultUnspecifiedValueString)); - } - // Multiple providers forces string output. - std::string str; - for (const auto& provider : providers) { - const auto bit = provider->format(request_headers, response_headers, response_trailers, - stream_info, local_reply_body); - str += bit.value_or(empty_value); - } - return ValueUtil::stringValue(str); - }; - const std::function - struct_format_map_callback = [&](const StructFormatter::StructFormatMapWrapper& format) { - ProtobufWkt::Struct output; - auto* fields = output.mutable_fields(); - 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) { - continue; - } - (*fields)[pair.first] = value; - } - return ValueUtil::structValue(output); - }; - return struct_format_map_callback(struct_output_format_).struct_value(); + StructFormatMapVisitor visitor{ + [&](const std::vector& providers) { + return providersCallback(providers, request_headers, response_headers, response_trailers, + stream_info, local_reply_body); + }, + [&, this](const StructFormatter::StructFormatMapWrapper& format_map) { + return structFormatMapCallback(format_map, visitor); + }, + [&, this](const StructFormatter::StructFormatListWrapper& format_list) { + return structFormatListCallback(format_list, visitor); + }, + }; + return structFormatMapCallback(struct_output_format_, visitor).struct_value(); } void SubstitutionFormatParser::parseCommandHeader(const std::string& token, const size_t start, @@ -1031,8 +1097,8 @@ MetadataFormatter::formatMetadataValue(const envoy::config::core::v3::Metadata& return val; } -// TODO(glicht): Consider adding support for route/listener/cluster metadata as suggested by @htuch. -// See: https://github.com/envoyproxy/envoy/issues/3006 +// TODO(glicht): Consider adding support for route/listener/cluster metadata as suggested by +// @htuch. See: https://github.com/envoyproxy/envoy/issues/3006 DynamicMetadataFormatter::DynamicMetadataFormatter(const std::string& filter_namespace, const std::vector& path, absl::optional max_length) diff --git a/source/common/formatter/substitution_formatter.h b/source/common/formatter/substitution_formatter.h index 2d1f121c4872e..94a15098d8790 100644 --- a/source/common/formatter/substitution_formatter.h +++ b/source/common/formatter/substitution_formatter.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -122,6 +123,10 @@ class FormatterImpl : public Formatter { std::vector providers_; }; +// Helper classes for StructFormatter::StructFormatMapVisitor. +template struct StructFormatMapVisitorHelper : Ts... { using Ts::operator()...; }; +template StructFormatMapVisitorHelper(Ts...) -> StructFormatMapVisitorHelper; + /** * An formatter for structured log formats, which returns a Struct proto that * can be converted easily into multiple formats. @@ -129,9 +134,7 @@ class FormatterImpl : public Formatter { 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)) {} + bool omit_empty_values); ProtobufWkt::Struct format(const Http::RequestHeaderMap& request_headers, const Http::ResponseHeaderMap& response_headers, @@ -141,22 +144,55 @@ class StructFormatter { private: struct StructFormatMapWrapper; - using StructFormatMapValue = - absl::variant, const StructFormatMapWrapper>; + struct StructFormatListWrapper; + using StructFormatValue = + absl::variant, const StructFormatMapWrapper, + const StructFormatListWrapper>; // 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; + using StructFormatMap = std::map; using StructFormatMapPtr = std::unique_ptr; struct StructFormatMapWrapper { StructFormatMapPtr value_; }; - StructFormatMapWrapper toFormatMap(const ProtobufWkt::Struct& struct_format) const; + using StructFormatList = std::list; + using StructFormatListPtr = std::unique_ptr; + struct StructFormatListWrapper { + StructFormatListPtr value_; + }; + + using StructFormatMapVisitor = StructFormatMapVisitorHelper< + const std::function&)>, + const std::function, + const std::function>; + + // Methods for building the format map. + std::vector toFormatStringValue(const std::string& string_format) const; + StructFormatMapWrapper toFormatMapValue(const ProtobufWkt::Struct& struct_format) const; + StructFormatListWrapper toFormatListValue(const ProtobufWkt::ListValue& list_value_format) const; + + // Methods for doing the actual formatting. + ProtobufWkt::Value providersCallback(const std::vector& providers, + 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::Value + structFormatMapCallback(const StructFormatter::StructFormatMapWrapper& format_map, + const StructFormatMapVisitor& visitor) const; + ProtobufWkt::Value + structFormatListCallback(const StructFormatter::StructFormatListWrapper& format_list, + const StructFormatMapVisitor& visitor) const; const bool omit_empty_values_; const bool preserve_types_; + const std::string empty_value_; const StructFormatMapWrapper struct_output_format_; -}; +}; // namespace Formatter + +using StructFormatterPtr = std::unique_ptr; class JsonFormatterImpl : public Formatter { public: diff --git a/test/common/formatter/substitution_format_string_test.cc b/test/common/formatter/substitution_format_string_test.cc index 3b7bd29abad85..37e83fe3caa9f 100644 --- a/test/common/formatter/substitution_format_string_test.cc +++ b/test/common/formatter/substitution_format_string_test.cc @@ -95,7 +95,8 @@ 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 structured access log format."); + "Only string values, nested structs and list values are supported in structured access log " + "format."); } } diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc index 87d81342169be..76282c11a42fc 100644 --- a/test/common/formatter/substitution_formatter_test.cc +++ b/test/common/formatter/substitution_formatter_test.cc @@ -1624,7 +1624,7 @@ TEST(SubstitutionFormatterTest, StructFormatterPlainStringTest) { expected_json_map); } -TEST(SubstitutionFormatterTest, StructFormatterNestedObject) { +TEST(SubstitutionFormatterTest, StructFormatterTypesTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header; Http::TestResponseHeaderMapImpl response_header; @@ -1638,24 +1638,151 @@ TEST(SubstitutionFormatterTest, StructFormatterNestedObject) { ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( - level_one: - level_two: - level_three: - plain_string: plain_string_value - protocol: '%PROTOCOL%' + string_type: plain_string_value + struct_type: + plain_string: plain_string_value + protocol: '%PROTOCOL%' + list_type: + - plain_string_value + - '%PROTOCOL%' )EOF", key_mapping); StructFormatter formatter(key_mapping, false, false); const ProtobufWkt::Struct expected = TestUtility::jsonToStruct(R"EOF({ - "level_one": { - "level_two": { - "level_three": { - "plain_string": "plain_string_value", - "protocol": "HTTP/1.1" - } - } - } + "string_type": "plain_string_value", + "struct_type": { + "plain_string": "plain_string_value", + "protocol": "HTTP/1.1" + }, + "list_type": [ + "plain_string_value", + "HTTP/1.1" + ] + })EOF"); + const ProtobufWkt::Struct out_struct = + formatter.format(request_header, response_header, response_trailer, stream_info, body); + EXPECT_TRUE(TestUtility::protoEqual(out_struct, expected)); +} + +// Test that nested values are formatted properly, including inter-type nesting. +TEST(SubstitutionFormatterTest, StructFormatterNestedObjectsTest) { + StreamInfo::MockStreamInfo stream_info; + Http::TestRequestHeaderMapImpl request_header; + Http::TestResponseHeaderMapImpl response_header; + Http::TestResponseTrailerMapImpl response_trailer; + std::string body; + + envoy::config::core::v3::Metadata metadata; + populateMetadataTestData(metadata); + absl::optional protocol = Http::Protocol::Http11; + EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); + + ProtobufWkt::Struct key_mapping; + // For both struct and list, we test 3 nesting levels of all types (string, struct and list). + TestUtility::loadFromYaml(R"EOF( + struct: + struct_string: plain_string_value + struct_protocol: '%PROTOCOL%' + struct_struct: + struct_struct_string: plain_string_value + struct_struct_protocol: '%PROTOCOL%' + struct_struct_struct: + struct_struct_struct_string: plain_string_value + struct_struct_struct_protocol: '%PROTOCOL%' + struct_struct_list: + - struct_struct_list_string + - '%PROTOCOL%' + struct_list: + - struct_list_string + - '%PROTOCOL%' + # struct_list_struct + - struct_list_struct_string: plain_string_value + struct_list_struct_protocol: '%PROTOCOL%' + # struct_list_list + - - struct_list_list_string + - '%PROTOCOL%' + list: + - list_string + - '%PROTOCOL%' + # list_struct + - list_struct_string: plain_string_value + list_struct_protocol: '%PROTOCOL%' + list_struct_struct: + list_struct_struct_string: plain_string_value + list_struct_struct_protocol: '%PROTOCOL%' + list_struct_list: + - list_struct_list_string + - '%PROTOCOL%' + # list_list + - - list_list_string + - '%PROTOCOL%' + # list_list_struct + - list_list_struct_string: plain_string_value + list_list_struct_protocol: '%PROTOCOL%' + # list_list_list + - - list_list_list_string + - '%PROTOCOL%' + )EOF", + key_mapping); + StructFormatter formatter(key_mapping, false, false); + const ProtobufWkt::Struct expected = TestUtility::jsonToStruct(R"EOF({ + "struct": { + "struct_string": "plain_string_value", + "struct_protocol": "HTTP/1.1", + "struct_struct": { + "struct_struct_string": "plain_string_value", + "struct_struct_protocol": "HTTP/1.1", + "struct_struct_struct": { + "struct_struct_struct_string": "plain_string_value", + "struct_struct_struct_protocol": "HTTP/1.1", + }, + "struct_struct_list": [ + "struct_struct_list_string", + "HTTP/1.1", + ], + }, + "struct_list": [ + "struct_list_string", + "HTTP/1.1", + { + "struct_list_struct_string": "plain_string_value", + "struct_list_struct_protocol": "HTTP/1.1", + }, + [ + "struct_list_list_string", + "HTTP/1.1", + ], + ], + }, + "list": [ + "list_string", + "HTTP/1.1", + { + "list_struct_string": "plain_string_value", + "list_struct_protocol": "HTTP/1.1", + "list_struct_struct": { + "list_struct_struct_string": "plain_string_value", + "list_struct_struct_protocol": "HTTP/1.1", + }, + "list_struct_list": [ + "list_struct_list_string", + "HTTP/1.1", + ] + }, + [ + "list_list_string", + "HTTP/1.1", + { + "list_list_struct_string": "plain_string_value", + "list_list_struct_protocol": "HTTP/1.1", + }, + [ + "list_list_list_string", + "HTTP/1.1", + ], + ], + ], })EOF"); const ProtobufWkt::Struct out_struct = formatter.format(request_header, response_header, response_trailer, stream_info, body); @@ -1736,10 +1863,14 @@ TEST(SubstitutionFormatterTest, StructFormatterAlternateHeaderTest) { ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( - request_present_header_or_request_absent_header: '%REQ(request_present_header?request_absent_header)%' - request_absent_header_or_request_present_header: '%REQ(request_absent_header?request_present_header)%' - response_absent_header_or_response_absent_header: '%RESP(response_absent_header?response_present_header)%' - response_present_header_or_response_absent_header: '%RESP(response_present_header?response_absent_header)%' + request_present_header_or_request_absent_header: + '%REQ(request_present_header?request_absent_header)%' + request_absent_header_or_request_present_header: + '%REQ(request_absent_header?request_present_header)%' + response_absent_header_or_response_absent_header: + '%RESP(response_absent_header?response_present_header)%' + response_present_header_or_response_absent_header: + '%RESP(response_present_header?response_absent_header)%' )EOF", key_mapping); StructFormatter formatter(key_mapping, false, false); @@ -2030,7 +2161,8 @@ TEST(SubstitutionFormatterTest, StructFormatterMultiTokenTest) { ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( - multi_token_field: '%PROTOCOL% plainstring %REQ(some_request_header)% %RESP(some_response_header)%' + multi_token_field: '%PROTOCOL% plainstring %REQ(some_request_header)% + %RESP(some_response_header)%' )EOF", key_mapping); for (const bool preserve_types : {false, true}) {