diff --git a/.github/workflows/_precheck_deps.yml b/.github/workflows/_precheck_deps.yml index e0926f16223f..82cc5bbe3d27 100644 --- a/.github/workflows/_precheck_deps.yml +++ b/.github/workflows/_precheck_deps.yml @@ -65,4 +65,4 @@ jobs: ref: ${{ fromJSON(inputs.request).request.sha }} persist-credentials: false - name: Dependency Review - uses: actions/dependency-review-action@3b139cfc5fae8b618d3eae3675e383bb1769c019 # v4.5.0 + uses: actions/dependency-review-action@38ecb5b593bf0eb19e335c03f97670f792489a8b # v4.7.0 diff --git a/changelogs/current.yaml b/changelogs/current.yaml index d77a54b11784..c79d75bec2eb 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -66,6 +66,9 @@ removed_config_or_runtime: - area: runtime change: | Removed runtime guard ``envoy_reloadable_features_boolean_to_string_fix`` and legacy code paths. +- area: logging + change: | + Removed runtime guard ``envoy.reloadable_features.logging_with_fast_json_formatter`` and legacy code paths. - area: sni change: | Removed runtime guard ``envoy.reloadable_features.use_route_host_mutation_for_auto_sni_san`` and legacy code paths. diff --git a/source/common/formatter/substitution_format_string.cc b/source/common/formatter/substitution_format_string.cc index 5149f93b5af3..c6c305de1bcc 100644 --- a/source/common/formatter/substitution_format_string.cc +++ b/source/common/formatter/substitution_format_string.cc @@ -37,10 +37,7 @@ absl::StatusOr SubstitutionFormatStringUtils::fromProtoConfig( case envoy::config::core::v3::SubstitutionFormatString::FormatCase::kTextFormat: return FormatterImpl::create(config.text_format(), config.omit_empty_values(), *commands); case envoy::config::core::v3::SubstitutionFormatString::FormatCase::kJsonFormat: - return createJsonFormatter( - config.json_format(), true, config.omit_empty_values(), - config.has_json_format_options() ? config.json_format_options().sort_properties() : false, - *commands); + return createJsonFormatter(config.json_format(), config.omit_empty_values(), *commands); case envoy::config::core::v3::SubstitutionFormatString::FormatCase::kTextFormatSource: { auto data_source_or_error = Config::DataSource::read(config.text_format_source(), true, context.serverFactoryContext().api()); @@ -54,23 +51,10 @@ absl::StatusOr SubstitutionFormatStringUtils::fromProtoConfig( return nullptr; } -FormatterPtr SubstitutionFormatStringUtils::createJsonFormatter( - const ProtobufWkt::Struct& struct_format, bool preserve_types, bool omit_empty_values, - bool sort_properties, const std::vector& commands) { - -// TODO(alyssawilk, wbpcode) when deprecating logging_with_fast_json_formatter -// remove LegacyJsonFormatterImpl and StructFormatterBase -#ifndef ENVOY_DISABLE_EXCEPTIONS - if (!Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.logging_with_fast_json_formatter")) { - return std::make_unique(struct_format, preserve_types, - omit_empty_values, sort_properties, commands); - } -#else - UNREFERENCED_PARAMETER(preserve_types); - UNREFERENCED_PARAMETER(sort_properties); -#endif - +FormatterPtr +SubstitutionFormatStringUtils::createJsonFormatter(const ProtobufWkt::Struct& struct_format, + bool omit_empty_values, + const std::vector& commands) { return std::make_unique(struct_format, omit_empty_values, commands); } diff --git a/source/common/formatter/substitution_format_string.h b/source/common/formatter/substitution_format_string.h index 558c3e56bd63..cbf31a130ad3 100644 --- a/source/common/formatter/substitution_format_string.h +++ b/source/common/formatter/substitution_format_string.h @@ -45,8 +45,7 @@ class SubstitutionFormatStringUtils { * Generate a Json formatter object from proto::Struct config */ static FormatterPtr createJsonFormatter(const ProtobufWkt::Struct& struct_format, - bool preserve_types, bool omit_empty_values, - bool sort_properties, + bool omit_empty_values, const std::vector& commands = {}); }; diff --git a/source/common/formatter/substitution_formatter.h b/source/common/formatter/substitution_formatter.h index 1192097b6f5d..cc9fc64346c6 100644 --- a/source/common/formatter/substitution_formatter.h +++ b/source/common/formatter/substitution_formatter.h @@ -127,258 +127,5 @@ class JsonFormatterImpl : public Formatter { std::vector parsed_elements_; }; -// Helper classes for StructFormatter::StructFormatMapVisitor. -template struct StructFormatMapVisitorHelper : Ts... { - using Ts::operator()...; -}; -template StructFormatMapVisitorHelper(Ts...) -> StructFormatMapVisitorHelper; - -#ifndef ENVOY_DISABLE_EXCEPTIONS -/** - * An formatter for structured log formats, which returns a Struct proto that - * can be converted easily into multiple formats. - */ -class StructFormatter { -public: - using CommandParsers = std::vector; - using PlainNumber = PlainNumberFormatter; - using PlainString = PlainStringFormatter; - - StructFormatter(const ProtobufWkt::Struct& format_mapping, bool preserve_types, - bool omit_empty_values, const CommandParsers& commands = {}) - : omit_empty_values_(omit_empty_values), preserve_types_(preserve_types), - struct_output_format_(FormatBuilder(commands).toFormatMapValue(format_mapping)) {} - - ProtobufWkt::Struct formatWithContext(const Context& context, - const StreamInfo::StreamInfo& info) const { - StructFormatMapVisitor visitor{ - [&](const std::vector& providers) { - return providersCallback(providers, context, info); - }, - [&, 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(); - } - -private: - struct 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 StructFormatMapPtr = std::unique_ptr; - struct StructFormatMapWrapper { - StructFormatMapPtr value_; - }; - - 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. - class FormatBuilder { - public: - explicit FormatBuilder(const CommandParsers& commands) : commands_(commands) {} - absl::StatusOr> - toFormatStringValue(const std::string& string_format) const { - return SubstitutionFormatParser::parse(string_format, commands_); - } - std::vector toFormatNumberValue(double value) const { - std::vector formatters; - formatters.emplace_back(FormatterProviderPtr{new PlainNumber(value)}); - return formatters; - } - StructFormatMapWrapper 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, - THROW_OR_RETURN_VALUE(toFormatStringValue(pair.second.string_value()), - std::vector)); - break; - - case ProtobufWkt::Value::kStructValue: - output->emplace(pair.first, toFormatMapValue(pair.second.struct_value())); - break; - - case ProtobufWkt::Value::kListValue: - output->emplace(pair.first, toFormatListValue(pair.second.list_value())); - break; - - case ProtobufWkt::Value::kNumberValue: - output->emplace(pair.first, toFormatNumberValue(pair.second.number_value())); - break; - default: - throw EnvoyException( - "Only string values, nested structs, list values and number values are " - "supported in structured access log format."); - } - } - return {std::move(output)}; - } - StructFormatListWrapper - 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(THROW_OR_RETURN_VALUE(toFormatStringValue(value.string_value()), - std::vector)); - 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; - - case ProtobufWkt::Value::kNumberValue: - output->emplace_back(toFormatNumberValue(value.number_value())); - break; - - default: - throw EnvoyException( - "Only string values, nested structs, list values and number values are " - "supported in structured access log format."); - } - } - return {std::move(output)}; - } - - private: - const CommandParsers& commands_; - }; - - // Methods for doing the actual formatting. - ProtobufWkt::Value providersCallback(const std::vector& providers, - const Context& context, - const StreamInfo::StreamInfo& stream_info) const { - ASSERT(!providers.empty()); - if (providers.size() == 1) { - const auto& provider = providers.front(); - if (preserve_types_) { - return provider->formatValueWithContext(context, stream_info); - } - - if (omit_empty_values_) { - return ValueUtil::optionalStringValue(provider->formatWithContext(context, stream_info)); - } - - const auto str = provider->formatWithContext(context, stream_info); - if (str.has_value()) { - return ValueUtil::stringValue(*str); - } - // Returning an "empty string" (depending on omit_empty_values_) in case - // of a formatting error. - return ValueUtil::stringValue(omit_empty_values_ ? EMPTY_STRING - : DefaultUnspecifiedValueStringView); - } - // Multiple providers forces string output. - std::string str; - for (const auto& provider : providers) { - const auto bit = provider->formatWithContext(context, stream_info); - // Add the formatted value if there is one. Otherwise add a default value - // of "-" if omit_empty_values_ is not set. - if (bit.has_value()) { - str += bit.value(); - } else if (!omit_empty_values_) { - str += DefaultUnspecifiedValueStringView; - } - } - return ValueUtil::stringValue(str); - } - ProtobufWkt::Value - structFormatMapCallback(const StructFormatter::StructFormatMapWrapper& format_map, - const 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; - } - if (omit_empty_values_ && output.fields().empty()) { - return ValueUtil::nullValue(); - } - return ValueUtil::structValue(output); - } - ProtobufWkt::Value - structFormatListCallback(const StructFormatter::StructFormatListWrapper& format_list, - const 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); - } - - const bool omit_empty_values_; - const bool preserve_types_; - - const StructFormatMapWrapper struct_output_format_; -}; - -using StructFormatterPtr = std::unique_ptr; - -class LegacyJsonFormatterImpl : public Formatter { -public: - using CommandParsers = std::vector; - - LegacyJsonFormatterImpl(const ProtobufWkt::Struct& format_mapping, bool preserve_types, - bool omit_empty_values, bool sort_properties, - const CommandParsers& commands = {}) - : struct_formatter_(format_mapping, preserve_types, omit_empty_values, commands), - sort_properties_(sort_properties) {} - - // Formatter - std::string formatWithContext(const Context& context, - const StreamInfo::StreamInfo& info) const override { - const ProtobufWkt::Struct output_struct = struct_formatter_.formatWithContext(context, info); - - std::string log_line = ""; -#ifdef ENVOY_ENABLE_YAML - if (sort_properties_) { - log_line = Json::Factory::loadFromProtobufStruct(output_struct)->asJsonString(); - } else { - log_line = MessageUtil::getJsonStringFromMessageOrError(output_struct, false, true); - } -#else - UNREFERENCED_PARAMETER(sort_properties_); - IS_ENVOY_BUG("Json support compiled out"); -#endif - return absl::StrCat(log_line, "\n"); - } - -private: - const StructFormatter struct_formatter_; - const bool sort_properties_; -}; - -#endif // ENVOY_DISABLE_EXCEPTIONS - } // namespace Formatter } // namespace Envoy diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 2085affefd63..910742ac4ff1 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -62,7 +62,6 @@ RUNTIME_GUARD(envoy_reloadable_features_jwt_authn_remove_jwt_from_query_params); RUNTIME_GUARD(envoy_reloadable_features_jwt_authn_validate_uri); RUNTIME_GUARD(envoy_reloadable_features_jwt_fetcher_use_scheme_from_uri); RUNTIME_GUARD(envoy_reloadable_features_local_reply_traverses_filter_chain_after_1xx); -RUNTIME_GUARD(envoy_reloadable_features_logging_with_fast_json_formatter); RUNTIME_GUARD(envoy_reloadable_features_mmdb_files_reload_enabled); RUNTIME_GUARD(envoy_reloadable_features_no_extension_lookup_by_name); RUNTIME_GUARD(envoy_reloadable_features_normalize_rds_provider_config); diff --git a/source/extensions/access_loggers/file/config.cc b/source/extensions/access_loggers/file/config.cc index 69846d1e0b93..7aa65f9c47c7 100644 --- a/source/extensions/access_loggers/file/config.cc +++ b/source/extensions/access_loggers/file/config.cc @@ -44,7 +44,7 @@ AccessLog::InstanceSharedPtr FileAccessLogFactory::createAccessLogInstance( break; case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase::kJsonFormat: formatter = Formatter::SubstitutionFormatStringUtils::createJsonFormatter( - fal_config.json_format(), false, false, false, command_parsers); + fal_config.json_format(), false, command_parsers); break; case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: kTypedJsonFormat: { diff --git a/source/extensions/access_loggers/fluentd/config.cc b/source/extensions/access_loggers/fluentd/config.cc index e7e40fce64cd..15a96c6252f8 100644 --- a/source/extensions/access_loggers/fluentd/config.cc +++ b/source/extensions/access_loggers/fluentd/config.cc @@ -66,8 +66,8 @@ AccessLog::InstanceSharedPtr FluentdAccessLogFactory::createAccessLogInstance( std::vector); Formatter::FormatterPtr json_formatter = - Formatter::SubstitutionFormatStringUtils::createJsonFormatter(proto_config.record(), true, - false, false, commands); + Formatter::SubstitutionFormatStringUtils::createJsonFormatter(proto_config.record(), false, + commands); FluentdFormatterPtr fluentd_formatter = std::make_unique(std::move(json_formatter)); diff --git a/test/common/formatter/substitution_format_string_test.cc b/test/common/formatter/substitution_format_string_test.cc index f651b12ffb7b..fbcb648636ed 100644 --- a/test/common/formatter/substitution_format_string_test.cc +++ b/test/common/formatter/substitution_format_string_test.cc @@ -78,26 +78,6 @@ TEST_F(SubstitutionFormatStringUtilsTest, TestFromProtoConfigJson) { EXPECT_TRUE(TestUtility::jsonStringEqual(out_json, expected)); } -TEST_F(SubstitutionFormatStringUtilsTest, TestInvalidConfigs) { - TestScopedRuntime runtime; - runtime.mergeValues({{"envoy.reloadable_features.logging_with_fast_json_formatter", "false"}}); - - const std::vector invalid_configs = { - R"( - json_format: - field: true -)", - }; - for (const auto& yaml : invalid_configs) { - TestUtility::loadFromYaml(yaml, config_); - EXPECT_THROW_WITH_MESSAGE( - SubstitutionFormatStringUtils::fromProtoConfig(config_, context_).IgnoreError(), - EnvoyException, - "Only string values, nested structs, list values and number values " - "are supported in structured access log format."); - } -} - TEST_F(SubstitutionFormatStringUtilsTest, TestFromProtoConfigFormatterExtension) { TestCommandFactory factory; Registry::InjectFactory command_register(factory); diff --git a/test/common/formatter/substitution_formatter_speed_test.cc b/test/common/formatter/substitution_formatter_speed_test.cc index 0c006993484e..560f94cd5e30 100644 --- a/test/common/formatter/substitution_formatter_speed_test.cc +++ b/test/common/formatter/substitution_formatter_speed_test.cc @@ -29,24 +29,6 @@ std::unique_ptr makeJsonFormatter() { return std::make_unique(JsonLogFormat, 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%' - response_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(TimeSource& time_source) { auto stream_info = std::make_unique(time_source); stream_info->downstream_connection_info_provider_->setRemoteAddress( @@ -95,37 +77,6 @@ static void BM_AccessLogFormatter(benchmark::State& state) { } BENCHMARK(BM_AccessLogFormatter); -// NOLINTNEXTLINE(readability-identifier-naming) -static void BM_StructAccessLogFormatter(benchmark::State& state) { - testing::NiceMock time_system; - - std::unique_ptr stream_info = makeStreamInfo(time_system); - std::unique_ptr struct_formatter = makeStructFormatter(false); - - size_t output_bytes = 0; - for (auto _ : state) { // NOLINT: Silences warning about dead store - output_bytes += struct_formatter->formatWithContext({}, *stream_info).ByteSize(); - } - benchmark::DoNotOptimize(output_bytes); -} -BENCHMARK(BM_StructAccessLogFormatter); - -// NOLINTNEXTLINE(readability-identifier-naming) -static void BM_TypedStructAccessLogFormatter(benchmark::State& state) { - testing::NiceMock time_system; - - std::unique_ptr stream_info = makeStreamInfo(time_system); - std::unique_ptr typed_struct_formatter = - makeStructFormatter(true); - - size_t output_bytes = 0; - for (auto _ : state) { // NOLINT: Silences warning about dead store - output_bytes += typed_struct_formatter->formatWithContext({}, *stream_info).ByteSize(); - } - benchmark::DoNotOptimize(output_bytes); -} -BENCHMARK(BM_TypedStructAccessLogFormatter); - // NOLINTNEXTLINE(readability-identifier-naming) static void BM_JsonAccessLogFormatter(benchmark::State& state) { testing::NiceMock time_system; diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc index e95d0ebb2f1e..822047b87a66 100644 --- a/test/common/formatter/substitution_formatter_test.cc +++ b/test/common/formatter/substitution_formatter_test.cc @@ -3597,7 +3597,7 @@ void verifyStructOutput(ProtobufWkt::Struct output, } } -TEST(SubstitutionFormatterTest, StructFormatterPlainStringTest) { +TEST(SubstitutionFormatterTest, JsonFormatterPlainStringTest) { StreamInfo::MockStreamInfo stream_info; envoy::config::core::v3::Metadata metadata; @@ -3605,20 +3605,22 @@ TEST(SubstitutionFormatterTest, StructFormatterPlainStringTest) { absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - absl::node_hash_map expected_json_map = { - {"plain_string", "plain_string_value"}}; + const std::string expected_json_map = R"EOF( + {"plain_string": "plain_string_value"} + )EOF"; ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( plain_string: plain_string_value )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); - verifyStructOutput(formatter.formatWithContext({}, stream_info), expected_json_map); + EXPECT_TRUE(TestUtility::jsonStringEqual(formatter.formatWithContext({}, stream_info), + expected_json_map)); } -TEST(SubstitutionFormatterTest, StructFormatterPlainNumberTest) { +TEST(SubstitutionFormatterTest, JsonFormatterPlainNumberTest) { StreamInfo::MockStreamInfo stream_info; envoy::config::core::v3::Metadata metadata; @@ -3626,19 +3628,22 @@ TEST(SubstitutionFormatterTest, StructFormatterPlainNumberTest) { absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - absl::node_hash_map expected_json_map = {{"plain_number", "400"}}; + const std::string expected_json_map = R"EOF( + {"plain_number": 400} + )EOF"; ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( plain_number: 400 )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); - verifyStructOutput(formatter.formatWithContext({}, stream_info), expected_json_map); + EXPECT_TRUE(TestUtility::jsonStringEqual(formatter.formatWithContext({}, stream_info), + expected_json_map)); } -TEST(SubstitutionFormatterTest, StructFormatterTypesTest) { +TEST(SubstitutionFormatterTest, JsonFormatterTypesTest) { StreamInfo::MockStreamInfo stream_info; envoy::config::core::v3::Metadata metadata; @@ -3657,7 +3662,7 @@ TEST(SubstitutionFormatterTest, StructFormatterTypesTest) { - '%PROTOCOL%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); const ProtobufWkt::Struct expected = TestUtility::jsonToStruct(R"EOF({ "string_type": "plain_string_value", @@ -3670,12 +3675,13 @@ TEST(SubstitutionFormatterTest, StructFormatterTypesTest) { "HTTP/1.1" ] })EOF"); - const ProtobufWkt::Struct out_struct = formatter.formatWithContext({}, stream_info); + const ProtobufWkt::Struct out_struct = + TestUtility::jsonToStruct(formatter.formatWithContext({}, stream_info)); EXPECT_TRUE(TestUtility::protoEqual(out_struct, expected)); } // Test that nested values are formatted properly, including inter-type nesting. -TEST(SubstitutionFormatterTest, StructFormatterNestedObjectsTest) { +TEST(SubstitutionFormatterTest, JsonFormatterNestedObjectsTest) { StreamInfo::MockStreamInfo stream_info; envoy::config::core::v3::Metadata metadata; @@ -3730,7 +3736,7 @@ TEST(SubstitutionFormatterTest, StructFormatterNestedObjectsTest) { - '%PROTOCOL%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); const ProtobufWkt::Struct expected = TestUtility::jsonToStruct(R"EOF({ "struct": { "struct_string": "plain_string_value", @@ -3789,11 +3795,12 @@ TEST(SubstitutionFormatterTest, StructFormatterNestedObjectsTest) { ], ], })EOF"); - const ProtobufWkt::Struct out_struct = formatter.formatWithContext({}, stream_info); + const ProtobufWkt::Struct out_struct = + TestUtility::jsonToStruct(formatter.formatWithContext({}, stream_info)); EXPECT_TRUE(TestUtility::protoEqual(out_struct, expected)); } -TEST(SubstitutionFormatterTest, StructFormatterSingleOperatorTest) { +TEST(SubstitutionFormatterTest, JsonFormatterSingleOperatorTest) { StreamInfo::MockStreamInfo stream_info; envoy::config::core::v3::Metadata metadata; @@ -3808,12 +3815,13 @@ TEST(SubstitutionFormatterTest, StructFormatterSingleOperatorTest) { protocol: '%PROTOCOL%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); - verifyStructOutput(formatter.formatWithContext({}, stream_info), expected_json_map); + verifyStructOutput(TestUtility::jsonToStruct(formatter.formatWithContext({}, stream_info)), + expected_json_map); } -TEST(SubstitutionFormatterTest, EmptyStructFormatterTest) { +TEST(SubstitutionFormatterTest, EmptyJsonFormatterTest) { StreamInfo::MockStreamInfo stream_info; envoy::config::core::v3::Metadata metadata; @@ -3828,12 +3836,13 @@ TEST(SubstitutionFormatterTest, EmptyStructFormatterTest) { protocol: '' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); - verifyStructOutput(formatter.formatWithContext({}, stream_info), expected_json_map); + verifyStructOutput(TestUtility::jsonToStruct(formatter.formatWithContext({}, stream_info)), + expected_json_map); } -TEST(SubstitutionFormatterTest, StructFormatterNonExistentHeaderTest) { +TEST(SubstitutionFormatterTest, JsonFormatterNonExistentHeaderTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"some_request_header", "SOME_REQUEST_HEADER"}}; Http::TestResponseHeaderMapImpl response_header{{"some_response_header", "SOME_RESPONSE_HEADER"}}; @@ -3843,11 +3852,12 @@ TEST(SubstitutionFormatterTest, StructFormatterNonExistentHeaderTest) { HttpFormatterContext formatter_context(&request_header, &response_header, &response_trailer, body); - absl::node_hash_map expected_json_map = { - {"protocol", "HTTP/1.1"}, - {"some_request_header", "SOME_REQUEST_HEADER"}, - {"nonexistent_response_header", "-"}, - {"some_response_header", "SOME_RESPONSE_HEADER"}}; + const std::string expected_json_map = R"EOF({ + "protocol": "HTTP/1.1", + "some_request_header": "SOME_REQUEST_HEADER", + "nonexistent_response_header": null, + "some_response_header": "SOME_RESPONSE_HEADER" + })EOF"; ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( @@ -3857,16 +3867,16 @@ TEST(SubstitutionFormatterTest, StructFormatterNonExistentHeaderTest) { some_response_header: '%RESP(some_response_header)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); + EXPECT_TRUE(TestUtility::jsonStringEqual( + formatter.formatWithContext(formatter_context, stream_info), expected_json_map)); } -TEST(SubstitutionFormatterTest, StructFormatterAlternateHeaderTest) { +TEST(SubstitutionFormatterTest, JsonFormatterAlternateHeaderTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{ {"request_present_header", "REQUEST_PRESENT_HEADER"}}; @@ -3896,16 +3906,17 @@ TEST(SubstitutionFormatterTest, StructFormatterAlternateHeaderTest) { '%RESP(response_present_header?response_absent_header)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); + verifyStructOutput( + TestUtility::jsonToStruct(formatter.formatWithContext(formatter_context, stream_info)), + expected_json_map); } -TEST(SubstitutionFormatterTest, StructFormatterDynamicMetadataTest) { +TEST(SubstitutionFormatterTest, JsonFormatterDynamicMetadataTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestResponseHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; @@ -3920,10 +3931,13 @@ TEST(SubstitutionFormatterTest, StructFormatterDynamicMetadataTest) { EXPECT_CALL(stream_info, dynamicMetadata()).WillRepeatedly(ReturnRef(metadata)); EXPECT_CALL(Const(stream_info), dynamicMetadata()).WillRepeatedly(ReturnRef(metadata)); - absl::node_hash_map expected_json_map = { - {"test_key", "test_value"}, - {"test_obj", "{\"inner_key\":\"inner_value\"}"}, - {"test_obj.inner_key", "inner_value"}}; + const std::string expected_json_map = R"EOF({ + "test_key": "test_value", + "test_obj": { + "inner_key": "inner_value" + }, + "test_obj.inner_key": "inner_value" + })EOF"; ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( @@ -3932,13 +3946,13 @@ TEST(SubstitutionFormatterTest, StructFormatterDynamicMetadataTest) { test_obj.inner_key: '%DYNAMIC_METADATA(com.test:test_obj:inner_key)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); + EXPECT_TRUE(TestUtility::jsonStringEqual( + formatter.formatWithContext(formatter_context, stream_info), expected_json_map)); } -TEST(SubstitutionFormatterTest, StructFormatterTypedDynamicMetadataTest) { +TEST(SubstitutionFormatterTest, JsonFormatterTypedDynamicMetadataTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestResponseHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; @@ -3960,9 +3974,10 @@ TEST(SubstitutionFormatterTest, StructFormatterTypedDynamicMetadataTest) { test_obj.inner_key: '%DYNAMIC_METADATA(com.test:test_obj:inner_key)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, true, false); + JsonFormatterImpl formatter(key_mapping, false); - ProtobufWkt::Struct output = formatter.formatWithContext(formatter_context, stream_info); + ProtobufWkt::Struct output = + TestUtility::jsonToStruct(formatter.formatWithContext(formatter_context, stream_info)); const auto& fields = output.fields(); EXPECT_EQ("test_value", fields.at("test_key").string_value()); @@ -3971,7 +3986,7 @@ TEST(SubstitutionFormatterTest, StructFormatterTypedDynamicMetadataTest) { fields.at("test_obj").struct_value().fields().at("inner_key").string_value()); } -TEST(SubstitutionFormatterTest, StructFormatterClusterMetadataTest) { +TEST(SubstitutionFormatterTest, JsonFormatterClusterMetadataTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestResponseHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; @@ -3989,12 +4004,14 @@ TEST(SubstitutionFormatterTest, StructFormatterClusterMetadataTest) { EXPECT_CALL(stream_info, upstreamClusterInfo()).WillRepeatedly(ReturnPointee(cluster)); EXPECT_CALL(Const(stream_info), upstreamClusterInfo()).WillRepeatedly(ReturnPointee(cluster)); - absl::node_hash_map expected_json_map = { - {"test_key", "test_value"}, - {"test_obj", "{\"inner_key\":\"inner_value\"}"}, - {"test_obj.inner_key", "inner_value"}, - {"test_obj.non_existing_key", "-"}, - }; + const std::string expected_json_map = R"EOF({ + "test_key": "test_value", + "test_obj": { + "inner_key": "inner_value" + }, + "test_obj.inner_key": "inner_value", + "test_obj.non_existing_key": null + })EOF"; ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( @@ -4004,13 +4021,13 @@ TEST(SubstitutionFormatterTest, StructFormatterClusterMetadataTest) { test_obj.non_existing_key: '%CLUSTER_METADATA(com.test:test_obj:non_existing_key)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); + EXPECT_TRUE(TestUtility::jsonStringEqual( + formatter.formatWithContext(formatter_context, stream_info), expected_json_map)); } -TEST(SubstitutionFormatterTest, StructFormatterTypedClusterMetadataTest) { +TEST(SubstitutionFormatterTest, JsonFormatterTypedClusterMetadataTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestResponseHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; @@ -4035,9 +4052,10 @@ TEST(SubstitutionFormatterTest, StructFormatterTypedClusterMetadataTest) { test_obj.inner_key: '%CLUSTER_METADATA(com.test:test_obj:inner_key)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, true, false); + JsonFormatterImpl formatter(key_mapping, false); - ProtobufWkt::Struct output = formatter.formatWithContext(formatter_context, stream_info); + ProtobufWkt::Struct output = + TestUtility::jsonToStruct(formatter.formatWithContext(formatter_context, stream_info)); const auto& fields = output.fields(); EXPECT_EQ("test_value", fields.at("test_key").string_value()); @@ -4046,7 +4064,7 @@ TEST(SubstitutionFormatterTest, StructFormatterTypedClusterMetadataTest) { fields.at("test_obj").struct_value().fields().at("inner_key").string_value()); } -TEST(SubstitutionFormatterTest, StructFormatterClusterMetadataNoClusterInfoTest) { +TEST(SubstitutionFormatterTest, JsonFormatterClusterMetadataNoClusterInfoTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestResponseHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; @@ -4056,30 +4074,32 @@ TEST(SubstitutionFormatterTest, StructFormatterClusterMetadataNoClusterInfoTest) HttpFormatterContext formatter_context(&request_header, &response_header, &response_trailer, body); - absl::node_hash_map expected_json_map = {{"test_key", "-"}}; + const std::string expected_json_map = R"EOF( + {"test_key": null} + )EOF"; ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( test_key: '%CLUSTER_METADATA(com.test:test_key)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); // Empty optional (absl::nullopt) { EXPECT_CALL(Const(stream_info), upstreamClusterInfo()).WillOnce(Return(absl::nullopt)); - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); + EXPECT_TRUE(TestUtility::jsonStringEqual( + formatter.formatWithContext(formatter_context, stream_info), expected_json_map)); } // Empty cluster info (nullptr) { EXPECT_CALL(Const(stream_info), upstreamClusterInfo()).WillOnce(Return(nullptr)); - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); + EXPECT_TRUE(TestUtility::jsonStringEqual( + formatter.formatWithContext(formatter_context, stream_info), expected_json_map)); } } -TEST(SubstitutionFormatterTest, StructFormatterUpstreamHostMetadataTest) { +TEST(SubstitutionFormatterTest, JsonFormatterUpstreamHostMetadataTest) { NiceMock stream_info; Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestResponseHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; @@ -4099,12 +4119,14 @@ TEST(SubstitutionFormatterTest, StructFormatterUpstreamHostMetadataTest) { mock_upstream_info->upstreamHost()); EXPECT_CALL(*mock_host_description, metadata()).WillRepeatedly(Return(metadata)); - absl::node_hash_map expected_json_map = { - {"test_key", "test_value"}, - {"test_obj", "{\"inner_key\":\"inner_value\"}"}, - {"test_obj.inner_key", "inner_value"}, - {"test_obj.non_existing_key", "-"}, - }; + const std::string expected_json_map = R"EOF( + {"test_key": "test_value", + "test_obj": { + "inner_key": "inner_value" + }, + "test_obj.inner_key": "inner_value", + "test_obj.non_existing_key": null + })EOF"; ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( @@ -4114,13 +4136,13 @@ TEST(SubstitutionFormatterTest, StructFormatterUpstreamHostMetadataTest) { test_obj.non_existing_key: '%UPSTREAM_METADATA(com.test:test_obj:non_existing_key)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); + EXPECT_TRUE(TestUtility::jsonStringEqual( + formatter.formatWithContext(formatter_context, stream_info), expected_json_map)); } -TEST(SubstitutionFormatterTest, StructFormatterUpstreamHostMetadataNullPtrs) { +TEST(SubstitutionFormatterTest, JsonFormatterUpstreamHostMetadataNullPtrs) { testing::NiceMock stream_info; Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestResponseHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; @@ -4130,20 +4152,22 @@ TEST(SubstitutionFormatterTest, StructFormatterUpstreamHostMetadataNullPtrs) { HttpFormatterContext formatter_context(&request_header, &response_header, &response_trailer, body); - absl::node_hash_map expected_json_map = {{"test_key", "-"}}; + const std::string expected_json_map = R"EOF( + {"test_key": null} + )EOF"; ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( test_key: '%UPSTREAM_METADATA(com.test:test_key)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); // Empty optional (absl::nullopt) { EXPECT_CALL(Const(stream_info), upstreamInfo()).WillOnce(Return(absl::nullopt)); - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); + EXPECT_TRUE(TestUtility::jsonStringEqual( + formatter.formatWithContext(formatter_context, stream_info), expected_json_map)); testing::Mock::VerifyAndClearExpectations(&stream_info); } // Empty host description info (nullptr) @@ -4151,12 +4175,12 @@ TEST(SubstitutionFormatterTest, StructFormatterUpstreamHostMetadataNullPtrs) { std::shared_ptr mock_upstream_info = std::dynamic_pointer_cast(stream_info.upstreamInfo()); EXPECT_CALL(*mock_upstream_info, upstreamHost()).WillOnce(Return(nullptr)); - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); + EXPECT_TRUE(TestUtility::jsonStringEqual( + formatter.formatWithContext(formatter_context, stream_info), expected_json_map)); } } -TEST(SubstitutionFormatterTest, StructFormatterFilterStateTest) { +TEST(SubstitutionFormatterTest, JsonFormatterFilterStateTest) { Http::TestRequestHeaderMapImpl request_headers; Http::TestResponseHeaderMapImpl response_headers; Http::TestResponseTrailerMapImpl response_trailers; @@ -4174,91 +4198,14 @@ TEST(SubstitutionFormatterTest, StructFormatterFilterStateTest) { StreamInfo::FilterState::StateType::ReadOnly); EXPECT_CALL(Const(stream_info), filterState()).Times(testing::AtLeast(1)); - absl::node_hash_map expected_json_map = { - {"test_key", "\"test_value\""}, {"test_obj", "{\"inner_key\":\"inner_value\"}"}}; - - ProtobufWkt::Struct key_mapping; - TestUtility::loadFromYaml(R"EOF( - test_key: '%FILTER_STATE(test_key)%' - test_obj: '%FILTER_STATE(test_obj)%' - )EOF", - key_mapping); - StructFormatter formatter(key_mapping, false, false); - - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); -} - -TEST(SubstitutionFormatterTest, StructFormatterOmitEmptyTest) { - Http::TestRequestHeaderMapImpl request_headers; - Http::TestResponseHeaderMapImpl response_headers; - Http::TestResponseTrailerMapImpl response_trailers; - StreamInfo::MockStreamInfo stream_info; - std::string body; - - HttpFormatterContext formatter_context(&request_headers, &response_headers, &response_trailers, - body); - - EXPECT_CALL(Const(stream_info), filterState()).Times(testing::AtLeast(1)); - EXPECT_CALL(Const(stream_info), dynamicMetadata()).Times(testing::AtLeast(1)); - - ProtobufWkt::Struct key_mapping; - TestUtility::loadFromYaml(R"EOF( - test_key_filter_state: '%FILTER_STATE(nonexistent_key)%' - test_key_req: '%REQ(nonexistent_key)%' - test_key_res: '%RESP(nonexistent_key)%' - test_key_dynamic_metadata: '%DYNAMIC_METADATA(nonexistent_key)%' - )EOF", - key_mapping); - StructFormatter formatter(key_mapping, false, true); - - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), {}); -} - -TEST(SubstitutionFormatterTest, StructFormatterOmitEmptyNestedTest) { - Http::TestRequestHeaderMapImpl request_headers; - Http::TestResponseHeaderMapImpl response_headers; - Http::TestResponseTrailerMapImpl response_trailers; - StreamInfo::MockStreamInfo stream_info; - std::string body; - - HttpFormatterContext formatter_context(&request_headers, &response_headers, &response_trailers, - body); - - EXPECT_CALL(Const(stream_info), filterState()).Times(testing::AtLeast(1)); - EXPECT_CALL(Const(stream_info), dynamicMetadata()).Times(testing::AtLeast(1)); - - ProtobufWkt::Struct key_mapping; - TestUtility::loadFromYaml(R"EOF( - test_key_root: - test_key_filter_state: '%FILTER_STATE(nonexistent_key)%' - test_key_req: '%REQ(nonexistent_key)%' - test_key_res: '%RESP(nonexistent_key)%' - test_key_dynamic_metadata: '%DYNAMIC_METADATA(nonexistent_key)%' - )EOF", - key_mapping); - StructFormatter formatter(key_mapping, false, true); - - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), {}); -} - -TEST(SubstitutionFormatterTest, StructFormatterTypedFilterStateTest) { - Http::TestRequestHeaderMapImpl request_headers; - Http::TestResponseHeaderMapImpl response_headers; - Http::TestResponseTrailerMapImpl response_trailers; - StreamInfo::MockStreamInfo stream_info; - std::string body; - - HttpFormatterContext formatter_context(&request_headers, &response_headers, &response_trailers, - body); - - stream_info.filter_state_->setData("test_key", - std::make_unique("test_value"), - StreamInfo::FilterState::StateType::ReadOnly); - stream_info.filter_state_->setData("test_obj", - std::make_unique(), - StreamInfo::FilterState::StateType::ReadOnly); - EXPECT_CALL(Const(stream_info), filterState()).Times(testing::AtLeast(1)); + const std::string expected_json_map = R"EOF( + { + "test_key": "test_value", + "test_obj": { + "inner_key": "inner_value" + } + } + )EOF"; ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( @@ -4266,14 +4213,10 @@ TEST(SubstitutionFormatterTest, StructFormatterTypedFilterStateTest) { test_obj: '%FILTER_STATE(test_obj)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, true, false); - - ProtobufWkt::Struct output = formatter.formatWithContext(formatter_context, stream_info); + JsonFormatterImpl formatter(key_mapping, false); - const auto& fields = output.fields(); - EXPECT_EQ("test_value", fields.at("test_key").string_value()); - EXPECT_EQ("inner_value", - fields.at("test_obj").struct_value().fields().at("inner_key").string_value()); + EXPECT_TRUE(TestUtility::jsonStringEqual( + formatter.formatWithContext(formatter_context, stream_info), expected_json_map)); } // Test new specifier (PLAIN/TYPED) of FilterState. Ensure that after adding additional specifier, @@ -4293,11 +4236,13 @@ TEST(SubstitutionFormatterTest, FilterStateSpeciferTest) { StreamInfo::FilterState::StateType::ReadOnly); EXPECT_CALL(Const(stream_info), filterState()).Times(testing::AtLeast(1)); - absl::node_hash_map expected_json_map = { - {"test_key_plain", "test_value By PLAIN"}, - {"test_key_typed", "\"test_value By TYPED\""}, - {"test_key_field", "test_value"}, - }; + const std::string expected_json_map = R"EOF( + { + "test_key_plain": "test_value By PLAIN", + "test_key_typed": "test_value By TYPED", + "test_key_field": "test_value" + } + )EOF"; ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( @@ -4306,10 +4251,10 @@ TEST(SubstitutionFormatterTest, FilterStateSpeciferTest) { test_key_field: '%FILTER_STATE(test_key:FIELD:test_field)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); + EXPECT_TRUE(TestUtility::jsonStringEqual( + formatter.formatWithContext(formatter_context, stream_info), expected_json_map)); } // Test new specifier (PLAIN/TYPED) of FilterState and convert the output log string to proto @@ -4336,9 +4281,10 @@ TEST(SubstitutionFormatterTest, TypedFilterStateSpeciferTest) { test_key_field: '%FILTER_STATE(test_key:FIELD:test_field)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, true, false); + JsonFormatterImpl formatter(key_mapping, false); - ProtobufWkt::Struct output = formatter.formatWithContext(formatter_context, stream_info); + ProtobufWkt::Struct output = + TestUtility::jsonToStruct(formatter.formatWithContext(formatter_context, stream_info)); const auto& fields = output.fields(); EXPECT_EQ("test_value By PLAIN", fields.at("test_key_plain").string_value()); @@ -4368,7 +4314,7 @@ TEST(SubstitutionFormatterTest, FilterStateErrorSpeciferTest) { test_key_typed: '%FILTER_STATE(test_key:TYPED)%' )EOF", key_mapping); - EXPECT_THROW_WITH_MESSAGE(StructFormatter formatter(key_mapping, false, false), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(JsonFormatterImpl formatter(key_mapping, false), EnvoyException, "Invalid filter state serialize type, only support PLAIN/TYPED/FIELD."); } @@ -4379,7 +4325,7 @@ TEST(SubstitutionFormatterTest, FilterStateErrorSpeciferFieldTest) { test_key_plain: '%FILTER_STATE(test_key:PLAIN:test_field)%' )EOF", key_mapping); - EXPECT_THROW_WITH_MESSAGE(StructFormatter formatter(key_mapping, false, false), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(JsonFormatterImpl formatter(key_mapping, false), EnvoyException, "Invalid filter state serialize type, FIELD " "should be used with the field name."); } @@ -4391,12 +4337,12 @@ TEST(SubstitutionFormatterTest, FilterStateErrorSpeciferFieldNoNameTest) { test_key_plain: '%FILTER_STATE(test_key:FIELD)%' )EOF", key_mapping); - EXPECT_THROW_WITH_MESSAGE(StructFormatter formatter(key_mapping, false, false), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(JsonFormatterImpl formatter(key_mapping, false), EnvoyException, "Invalid filter state serialize type, FIELD " "should be used with the field name."); } -TEST(SubstitutionFormatterTest, StructFormatterStartTimeTest) { +TEST(SubstitutionFormatterTest, JsonFormatterStartTimeTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header; Http::TestResponseHeaderMapImpl response_header; @@ -4431,13 +4377,14 @@ TEST(SubstitutionFormatterTest, StructFormatterStartTimeTest) { all_zeroes: '%START_TIME(%f.%1f.%2f.%3f)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, false, false); + JsonFormatterImpl formatter(key_mapping, false); - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); + verifyStructOutput( + TestUtility::jsonToStruct(formatter.formatWithContext(formatter_context, stream_info)), + expected_json_map); } -TEST(SubstitutionFormatterTest, StructFormatterMultiTokenTest) { +TEST(SubstitutionFormatterTest, JsonFormatterMultiTokenTest) { { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"some_request_header", "SOME_REQUEST_HEADER"}}; @@ -4458,19 +4405,18 @@ TEST(SubstitutionFormatterTest, StructFormatterMultiTokenTest) { %RESP(some_response_header)%' )EOF", key_mapping); - for (const bool preserve_types : {false, true}) { - StructFormatter formatter(key_mapping, preserve_types, false); + JsonFormatterImpl formatter(key_mapping, false); - absl::optional protocol = Http::Protocol::Http11; - EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); + absl::optional protocol = Http::Protocol::Http11; + EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - verifyStructOutput(formatter.formatWithContext(formatter_context, stream_info), - expected_json_map); - } + verifyStructOutput( + TestUtility::jsonToStruct(formatter.formatWithContext(formatter_context, stream_info)), + expected_json_map); } } -TEST(SubstitutionFormatterTest, StructFormatterTypedTest) { +TEST(SubstitutionFormatterTest, JsonFormatterTypedTest) { Http::TestRequestHeaderMapImpl request_headers; Http::TestResponseHeaderMapImpl response_headers; Http::TestResponseTrailerMapImpl response_trailers; @@ -4505,9 +4451,10 @@ TEST(SubstitutionFormatterTest, StructFormatterTypedTest) { filter_state: '%FILTER_STATE(test_obj)%' )EOF", key_mapping); - StructFormatter formatter(key_mapping, true, false); + JsonFormatterImpl formatter(key_mapping, false); - ProtobufWkt::Struct output = formatter.formatWithContext(formatter_context, stream_info); + ProtobufWkt::Struct output = + TestUtility::jsonToStruct(formatter.formatWithContext(formatter_context, stream_info)); EXPECT_THAT(output.fields().at("request_duration"), ProtoEq(ValueUtil::numberValue(5.0))); EXPECT_THAT(output.fields().at("request_duration_multi"), ProtoEq(ValueUtil::stringValue("5ms"))); @@ -4582,51 +4529,6 @@ TEST(SubstitutionFormatterTest, JsonFormatterTest) { EXPECT_TRUE(TestUtility::jsonStringEqual(out_json, expected)); } -TEST(SubstitutionFormatterTest, LegacyJsonFormatterTest) { - NiceMock stream_info; - Http::TestRequestHeaderMapImpl request_header{{"key_1", "value_1"}, - {"key_2", R"(value_with_quotes_"_)"}}; - Http::TestResponseHeaderMapImpl response_header; - Http::TestResponseTrailerMapImpl response_trailer; - std::string body; - - HttpFormatterContext formatter_context(&request_header, &response_header, &response_trailer, - body); - - envoy::config::core::v3::Metadata metadata; - populateMetadataTestData(metadata); - absl::optional protocol = Http::Protocol::Http11; - EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - MockTimeSystem time_system; - EXPECT_CALL(time_system, monotonicTime) - .WillOnce(Return(MonotonicTime(std::chrono::nanoseconds(5000000)))); - stream_info.downstream_timing_.onLastDownstreamRxByteReceived(time_system); - - ProtobufWkt::Struct key_mapping; - TestUtility::loadFromYaml(R"EOF( - request_duration: '%REQUEST_DURATION%' - nested_level: - plain_string: plain_string_value - protocol: '%PROTOCOL%' - request_key: '%REQ(key_1)%_@!!!_"_%REQ(key_2)%' - )EOF", - key_mapping); - - const std::string expected = R"EOF({ - "request_duration": "5", - "nested_level": { - "plain_string": "plain_string_value", - "protocol": "HTTP/1.1" - }, - "request_key": "value_1_@!!!_\"_value_with_quotes_\"_" - })EOF"; - - LegacyJsonFormatterImpl legacy_formatter(key_mapping, false, false, false); - const std::string legacy_out_json = - legacy_formatter.formatWithContext(formatter_context, stream_info); - EXPECT_TRUE(TestUtility::jsonStringEqual(legacy_out_json, expected)); -} - TEST(SubstitutionFormatterTest, JsonFormatterWithOrderedPropertiesTest) { NiceMock stream_info; Http::TestRequestHeaderMapImpl request_header; @@ -4668,17 +4570,6 @@ TEST(SubstitutionFormatterTest, JsonFormatterWithOrderedPropertiesTest) { const std::string out_json = formatter.formatWithContext(formatter_context, stream_info); // Check string equality to verify the order. EXPECT_EQ(out_json, expected); - - const std::string legacy_expected = - "{\"afield\":\"vala\",\"bfield\":\"valb\",\"nested_level\":" - "{\"cfield\":\"valc\",\"plain_string\":\"plain_string_value\",\"protocol\":\"HTTP/1.1\"}," - "\"request_duration\":5.0}\n"; - - // The legacy formatter needs a flag to tell it to order the properties. - LegacyJsonFormatterImpl legacy_formatter(key_mapping, true, false, true); - const std::string legacy_out_json = - legacy_formatter.formatWithContext(formatter_context, stream_info); - EXPECT_EQ(legacy_out_json, legacy_expected); } TEST(SubstitutionFormatterTest, CompositeFormatterSuccess) { diff --git a/test/extensions/access_loggers/fluentd/substitution_formatter_test.cc b/test/extensions/access_loggers/fluentd/substitution_formatter_test.cc index a98a0e2b58f6..962d60a1ce4a 100644 --- a/test/extensions/access_loggers/fluentd/substitution_formatter_test.cc +++ b/test/extensions/access_loggers/fluentd/substitution_formatter_test.cc @@ -12,8 +12,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -using testing::Return; - namespace Envoy { namespace Extensions { namespace AccessLoggers { @@ -26,7 +24,7 @@ TEST(FluentdFormatterImplTest, FormatMsgpack) { (*log_struct.mutable_fields())["LogType"].set_string_value("%ACCESS_LOG_TYPE%"); auto json_formatter = - Formatter::SubstitutionFormatStringUtils::createJsonFormatter(log_struct, true, false, true); + Formatter::SubstitutionFormatStringUtils::createJsonFormatter(log_struct, false); auto fluentd_formatter = FluentdFormatterImpl(std::move(json_formatter)); auto expected_json = "{\"Message\":\"SomeValue\",\"LogType\":\"NotSet\"}";