diff --git a/source/common/formatter/substitution_formatter.cc b/source/common/formatter/substitution_formatter.cc index 9b121db96803f..fc6a92fa99956 100644 --- a/source/common/formatter/substitution_formatter.cc +++ b/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 struct JsonFormatMapVisitor : Ts... { using Ts::operator()...; }; -template JsonFormatMapVisitor(Ts...) -> JsonFormatMapVisitor; +template struct StructFormatMapVisitor : Ts... { using Ts::operator()...; }; +template StructFormatMapVisitor(Ts...) -> StructFormatMapVisitor; } // 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(); - for (const auto& pair : json_format.fields()) { +StructFormatter::StructFormatMapWrapper +StructFormatter::toFormatMap(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())); @@ -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&)> @@ -197,11 +197,11 @@ ProtobufWkt::Struct JsonFormatterImpl::toStruct(const Http::RequestHeaderMap& re } return ValueUtil::stringValue(str); }; - const std::function - json_format_map_callback = [&](const JsonFormatterImpl::JsonFormatMapWrapper& format) { + const std::function + 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, diff --git a/source/common/formatter/substitution_formatter.h b/source/common/formatter/substitution_formatter.h index 6a5fc45fb04e1..66ac8f83e2834 100644 --- a/source/common/formatter/substitution_formatter.h +++ b/source/common/formatter/substitution_formatter.h @@ -107,12 +107,47 @@ class FormatterImpl : public Formatter { std::vector 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 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; + using StructFormatMapPtr = std::unique_ptr; + 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 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; - using JsonFormatMapPtr = std::unique_ptr; - 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_; }; /** diff --git a/test/common/formatter/substitution_format_string_test.cc b/test/common/formatter/substitution_format_string_test.cc index 53c9a2086b29f..105c18f0038b8 100644 --- a/test/common/formatter/substitution_format_string_test.cc +++ b/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."); } } diff --git a/test/common/formatter/substitution_formatter_speed_test.cc b/test/common/formatter/substitution_formatter_speed_test.cc index bf06c39b4e492..5cedca3fe04dc 100644 --- a/test/common/formatter/substitution_formatter_speed_test.cc +++ b/test/common/formatter/substitution_formatter_speed_test.cc @@ -28,6 +28,24 @@ std::unique_ptr makeJsonFormatter(bool type return std::make_unique(JsonLogFormat, typed, false); } +std::unique_ptr 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(StructLogFormat, typed, false); +} + std::unique_ptr makeStreamInfo() { auto stream_info = std::make_unique(); 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 stream_info = makeStreamInfo(); + std::unique_ptr 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 stream_info = makeStreamInfo(); + std::unique_ptr 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 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) diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc index 6e16c79e6992d..81a0ba4f7f365 100644 --- a/test/common/formatter/substitution_formatter_test.cc +++ b/test/common/formatter/substitution_formatter_test.cc @@ -1541,22 +1541,14 @@ TEST(SubstitutionFormatterTest, GrpcStatusFormatterTest) { } } -void verifyJsonOutput(std::string json_string, - absl::node_hash_map expected_map) { - const auto parsed = Json::Factory::loadFromString(json_string); - - // Every json log line should have only one newline character, and it should be the last character - // in the string - const auto newline_pos = json_string.find('\n'); - EXPECT_NE(newline_pos, std::string::npos); - EXPECT_EQ(newline_pos, json_string.length() - 1); - +void verifyStructOutput(ProtobufWkt::Struct output, + absl::node_hash_map expected_map) { for (const auto& pair : expected_map) { - EXPECT_EQ(parsed->getString(pair.first), pair.second); + EXPECT_EQ(output.fields().at(pair.first).string_value(), pair.second); } } -TEST(SubstitutionFormatterTest, JsonFormatterPlainStringTest) { +TEST(SubstitutionFormatterTest, StructFormatterPlainStringTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header; Http::TestResponseHeaderMapImpl response_header; @@ -1576,14 +1568,14 @@ TEST(SubstitutionFormatterTest, JsonFormatterPlainStringTest) { plain_string: plain_string_value )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, false, false); + StructFormatter formatter(key_mapping, false, false); - verifyJsonOutput( + verifyStructOutput( formatter.format(request_header, response_header, response_trailer, stream_info, body), expected_json_map); } -TEST(SubstitutionFormatterTest, JsonFormatterNestedObject) { +TEST(SubstitutionFormatterTest, StructFormatterNestedObject) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header; Http::TestResponseHeaderMapImpl response_header; @@ -1604,9 +1596,9 @@ TEST(SubstitutionFormatterTest, JsonFormatterNestedObject) { protocol: '%PROTOCOL%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, false, false); + StructFormatter formatter(key_mapping, false, false); - const std::string expected = R"EOF({ + const ProtobufWkt::Struct expected = TestUtility::jsonToStruct(R"EOF({ "level_one": { "level_two": { "level_three": { @@ -1615,13 +1607,13 @@ TEST(SubstitutionFormatterTest, JsonFormatterNestedObject) { } } } - })EOF"; - std::string out_json = + })EOF"); + const ProtobufWkt::Struct out_struct = formatter.format(request_header, response_header, response_trailer, stream_info, body); - EXPECT_TRUE(TestUtility::jsonStringEqual(out_json, expected)); + EXPECT_TRUE(TestUtility::protoEqual(out_struct, expected)); } -TEST(SubstitutionFormatterTest, JsonFormatterSingleOperatorTest) { +TEST(SubstitutionFormatterTest, StructFormatterSingleOperatorTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header; Http::TestResponseHeaderMapImpl response_header; @@ -1640,14 +1632,14 @@ TEST(SubstitutionFormatterTest, JsonFormatterSingleOperatorTest) { protocol: '%PROTOCOL%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, false, false); + StructFormatter formatter(key_mapping, false, false); - verifyJsonOutput( + verifyStructOutput( formatter.format(request_header, response_header, response_trailer, stream_info, body), expected_json_map); } -TEST(SubstitutionFormatterTest, JsonFormatterNonExistentHeaderTest) { +TEST(SubstitutionFormatterTest, StructFormatterNonExistentHeaderTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"some_request_header", "SOME_REQUEST_HEADER"}}; Http::TestResponseHeaderMapImpl response_header{{"some_response_header", "SOME_RESPONSE_HEADER"}}; @@ -1668,17 +1660,17 @@ TEST(SubstitutionFormatterTest, JsonFormatterNonExistentHeaderTest) { some_response_header: '%RESP(some_response_header)%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, false, false); + StructFormatter formatter(key_mapping, false, false); absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - verifyJsonOutput( + verifyStructOutput( formatter.format(request_header, response_header, response_trailer, stream_info, body), expected_json_map); } -TEST(SubstitutionFormatterTest, JsonFormatterAlternateHeaderTest) { +TEST(SubstitutionFormatterTest, StructFormatterAlternateHeaderTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{ {"request_present_header", "REQUEST_PRESENT_HEADER"}}; @@ -1701,17 +1693,17 @@ TEST(SubstitutionFormatterTest, JsonFormatterAlternateHeaderTest) { response_present_header_or_response_absent_header: '%RESP(response_present_header?response_absent_header)%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, false, false); + StructFormatter formatter(key_mapping, false, false); absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - verifyJsonOutput( + verifyStructOutput( formatter.format(request_header, response_header, response_trailer, stream_info, body), expected_json_map); } -TEST(SubstitutionFormatterTest, JsonFormatterDynamicMetadataTest) { +TEST(SubstitutionFormatterTest, StructFormatterDynamicMetadataTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestResponseHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; @@ -1735,14 +1727,14 @@ TEST(SubstitutionFormatterTest, JsonFormatterDynamicMetadataTest) { test_obj.inner_key: '%DYNAMIC_METADATA(com.test:test_obj:inner_key)%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, false, false); + StructFormatter formatter(key_mapping, false, false); - verifyJsonOutput( + verifyStructOutput( formatter.format(request_header, response_header, response_trailer, stream_info, body), expected_json_map); } -TEST(SubstitutionFormatterTest, JsonFormatterTypedDynamicMetadataTest) { +TEST(SubstitutionFormatterTest, StructFormatterTypedDynamicMetadataTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestResponseHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; @@ -1761,12 +1753,10 @@ TEST(SubstitutionFormatterTest, JsonFormatterTypedDynamicMetadataTest) { test_obj.inner_key: '%DYNAMIC_METADATA(com.test:test_obj:inner_key)%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, true, false); + StructFormatter formatter(key_mapping, true, false); - const std::string json = + ProtobufWkt::Struct output = formatter.format(request_header, response_header, response_trailer, stream_info, body); - ProtobufWkt::Struct output; - MessageUtil::loadFromJson(json, output); const auto& fields = output.fields(); EXPECT_EQ("test_value", fields.at("test_key").string_value()); @@ -1775,7 +1765,7 @@ TEST(SubstitutionFormatterTest, JsonFormatterTypedDynamicMetadataTest) { fields.at("test_obj").struct_value().fields().at("inner_key").string_value()); } -TEST(SubstitutionFormatterTest, JsonFormatterFilterStateTest) { +TEST(SubstitutionFormatterTest, StructFormatterFilterStateTest) { Http::TestRequestHeaderMapImpl request_headers; Http::TestResponseHeaderMapImpl response_headers; Http::TestResponseTrailerMapImpl response_trailers; @@ -1798,14 +1788,14 @@ TEST(SubstitutionFormatterTest, JsonFormatterFilterStateTest) { test_obj: '%FILTER_STATE(test_obj)%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, false, false); + StructFormatter formatter(key_mapping, false, false); - verifyJsonOutput( + verifyStructOutput( formatter.format(request_headers, response_headers, response_trailers, stream_info, body), expected_json_map); } -TEST(SubstitutionFormatterTest, JsonFormatterOmitEmptyTest) { +TEST(SubstitutionFormatterTest, StructFormatterOmitEmptyTest) { Http::TestRequestHeaderMapImpl request_headers; Http::TestResponseHeaderMapImpl response_headers; Http::TestResponseTrailerMapImpl response_trailers; @@ -1823,14 +1813,14 @@ TEST(SubstitutionFormatterTest, JsonFormatterOmitEmptyTest) { test_key_dynamic_metadata: '%DYNAMIC_METADATA(nonexistent_key)%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, false, true); + StructFormatter formatter(key_mapping, false, true); - verifyJsonOutput( + verifyStructOutput( formatter.format(request_headers, response_headers, response_trailers, stream_info, body), {}); } -TEST(SubstitutionFormatterTest, JsonFormatterTypedFilterStateTest) { +TEST(SubstitutionFormatterTest, StructFormatterTypedFilterStateTest) { Http::TestRequestHeaderMapImpl request_headers; Http::TestResponseHeaderMapImpl response_headers; Http::TestResponseTrailerMapImpl response_trailers; @@ -1850,12 +1840,10 @@ TEST(SubstitutionFormatterTest, JsonFormatterTypedFilterStateTest) { test_obj: '%FILTER_STATE(test_obj)%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, true, false); + StructFormatter formatter(key_mapping, true, false); - std::string json = + ProtobufWkt::Struct output = formatter.format(request_headers, response_headers, response_trailers, stream_info, body); - ProtobufWkt::Struct output; - MessageUtil::loadFromJson(json, output); const auto& fields = output.fields(); EXPECT_EQ("test_value", fields.at("test_key").string_value()); @@ -1887,9 +1875,9 @@ TEST(SubstitutionFormatterTest, FilterStateSpeciferTest) { test_key_typed: '%FILTER_STATE(test_key:TYPED)%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, false, false); + StructFormatter formatter(key_mapping, false, false); - verifyJsonOutput( + verifyStructOutput( formatter.format(request_headers, response_headers, response_trailers, stream_info, body), expected_json_map); } @@ -1913,14 +1901,11 @@ TEST(SubstitutionFormatterTest, TypedFilterStateSpeciferTest) { test_key_typed: '%FILTER_STATE(test_key:TYPED)%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, true, false); + StructFormatter formatter(key_mapping, true, false); - std::string json = + ProtobufWkt::Struct output = formatter.format(request_headers, response_headers, response_trailers, stream_info, body); - ProtobufWkt::Struct output; - MessageUtil::loadFromJson(json, output); - const auto& fields = output.fields(); EXPECT_EQ("test_value By PLAIN", fields.at("test_key_plain").string_value()); EXPECT_EQ("test_value By TYPED", fields.at("test_key_typed").string_value()); @@ -1944,11 +1929,11 @@ TEST(SubstitutionFormatterTest, FilterStateErrorSpeciferTest) { test_key_typed: '%FILTER_STATE(test_key:TYPED)%' )EOF", key_mapping); - EXPECT_THROW_WITH_MESSAGE(JsonFormatterImpl formatter(key_mapping, false, false), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(StructFormatter formatter(key_mapping, false, false), EnvoyException, "Invalid filter state serialize type, only support PLAIN/TYPED."); } -TEST(SubstitutionFormatterTest, JsonFormatterStartTimeTest) { +TEST(SubstitutionFormatterTest, StructFormatterStartTimeTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header; Http::TestResponseHeaderMapImpl response_header; @@ -1975,14 +1960,14 @@ TEST(SubstitutionFormatterTest, JsonFormatterStartTimeTest) { all_zeroes: '%START_TIME(%f.%1f.%2f.%3f)%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, false, false); + StructFormatter formatter(key_mapping, false, false); - verifyJsonOutput( + verifyStructOutput( formatter.format(request_header, response_header, response_trailer, stream_info, body), expected_json_map); } -TEST(SubstitutionFormatterTest, JsonFormatterMultiTokenTest) { +TEST(SubstitutionFormatterTest, StructFormatterMultiTokenTest) { { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"some_request_header", "SOME_REQUEST_HEADER"}}; @@ -2000,21 +1985,19 @@ TEST(SubstitutionFormatterTest, JsonFormatterMultiTokenTest) { )EOF", key_mapping); for (const bool preserve_types : {false, true}) { - JsonFormatterImpl formatter(key_mapping, preserve_types, false); + StructFormatter formatter(key_mapping, preserve_types, false); absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - const auto parsed = Json::Factory::loadFromString( - formatter.format(request_header, response_header, response_trailer, stream_info, body)); - for (const auto& pair : expected_json_map) { - EXPECT_EQ(parsed->getString(pair.first), pair.second); - } + verifyStructOutput( + formatter.format(request_header, response_header, response_trailer, stream_info, body), + expected_json_map); } } } -TEST(SubstitutionFormatterTest, JsonFormatterTypedTest) { +TEST(SubstitutionFormatterTest, StructFormatterTypedTest) { Http::TestRequestHeaderMapImpl request_headers; Http::TestResponseHeaderMapImpl response_headers; Http::TestResponseTrailerMapImpl response_trailers; @@ -2043,12 +2026,10 @@ TEST(SubstitutionFormatterTest, JsonFormatterTypedTest) { filter_state: '%FILTER_STATE(test_obj)%' )EOF", key_mapping); - JsonFormatterImpl formatter(key_mapping, true, false); + StructFormatter formatter(key_mapping, true, false); - const auto json = + ProtobufWkt::Struct output = formatter.format(request_headers, response_headers, response_trailers, stream_info, body); - ProtobufWkt::Struct output; - MessageUtil::loadFromJson(json, output); EXPECT_THAT(output.fields().at("request_duration"), ProtoEq(ValueUtil::numberValue(5.0))); EXPECT_THAT(output.fields().at("request_duration_multi"), ProtoEq(ValueUtil::stringValue("5ms"))); @@ -2058,6 +2039,43 @@ TEST(SubstitutionFormatterTest, JsonFormatterTypedTest) { EXPECT_THAT(output.fields().at("filter_state"), ProtoEq(expected)); } +TEST(SubstitutionFormatterTest, JsonFormatterTest) { + 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)); + EXPECT_CALL(Const(stream_info), lastDownstreamRxByteReceived()) + .WillRepeatedly(Return(std::chrono::nanoseconds(5000000))); + + ProtobufWkt::Struct key_mapping; + TestUtility::loadFromYaml(R"EOF( + request_duration: '%REQUEST_DURATION%' + nested_level: + plain_string: plain_string_value + protocol: '%PROTOCOL%' + )EOF", + key_mapping); + JsonFormatterImpl formatter(key_mapping, false, false); + + const std::string expected = R"EOF({ + "request_duration": "5", + "nested_level": { + "plain_string": "plain_string_value", + "protocol": "HTTP/1.1" + } + })EOF"; + + const std::string out_json = + formatter.format(request_header, response_header, response_trailer, stream_info, body); + EXPECT_TRUE(TestUtility::jsonStringEqual(out_json, expected)); +} + TEST(SubstitutionFormatterTest, CompositeFormatterSuccess) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}};