From 7454ff4e68db5b1a08cd9f93efc56c2547bbcf92 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Wed, 9 Oct 2024 21:38:46 +0800 Subject: [PATCH] formatter: use the new json lib to relace protobuf json Signed-off-by: wangbaiping --- changelogs/current.yaml | 4 ++ source/common/formatter/BUILD | 2 + .../common/formatter/stream_info_formatter.cc | 14 ++---- .../common/formatter/substitution_formatter.h | 46 ++----------------- .../formatter/substitution_formatter_test.cc | 9 +--- 5 files changed, 14 insertions(+), 61 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index fdf30064d8b9..3913fcc9dd5e 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -124,6 +124,10 @@ minor_behavior_changes: change: | Sanitize SNI for potential log injection. The invalid character will be replaced by ``_`` with an ``invalid:`` marker. If runtime flag ``envoy.reloadable_features.sanitize_sni_in_access_log`` is set to ``false``, the sanitize behavior is disabled. +- area: formatter + change: | + The NaN and Infinity values of float will be serialized to ``null`` and ``"inf"`` respectively in the + metadata (``DYNAMIC_METADATA``, ``CLUSTER_METADATA``, etc.) formatter. bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* diff --git a/source/common/formatter/BUILD b/source/common/formatter/BUILD index 76a0fc474cfe..57d08475ef2a 100644 --- a/source/common/formatter/BUILD +++ b/source/common/formatter/BUILD @@ -27,6 +27,7 @@ envoy_cc_library( "//source/common/http:utility_lib", "//source/common/json:json_loader_lib", "//source/common/json:json_streamer_lib", + "//source/common/json:json_utility_lib", "@com_google_absl//absl/strings:str_format", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], @@ -101,6 +102,7 @@ envoy_cc_library( "//source/common/grpc:common_lib", "//source/common/http:utility_lib", "//source/common/json:json_loader_lib", + "//source/common/json:json_utility_lib", "//source/common/protobuf:message_validator_lib", "//source/common/stream_info:utility_lib", ], diff --git a/source/common/formatter/stream_info_formatter.cc b/source/common/formatter/stream_info_formatter.cc index 5ff679a731fd..5463da936071 100644 --- a/source/common/formatter/stream_info_formatter.cc +++ b/source/common/formatter/stream_info_formatter.cc @@ -3,6 +3,7 @@ #include "source/common/common/random_generator.h" #include "source/common/config/metadata.h" #include "source/common/http/utility.h" +#include "source/common/json/json_utility.h" #include "source/common/runtime/runtime_features.h" #include "source/common/stream_info/utility.h" @@ -61,20 +62,11 @@ MetadataFormatter::formatMetadata(const envoy::config::core::v3::Metadata& metad } std::string str; + str.reserve(256); if (value.kind_case() == ProtobufWkt::Value::kStringValue) { str = value.string_value(); } else { -#ifdef ENVOY_ENABLE_YAML - absl::StatusOr json_or_error = - MessageUtil::getJsonStringFromMessage(value, false, true); - if (json_or_error.ok()) { - str = json_or_error.value(); - } else { - str = json_or_error.status().message(); - } -#else - IS_ENVOY_BUG("Json support compiled out"); -#endif + Json::Utility::appendValueToString(value, str); } SubstitutionFormatUtils::truncate(str, max_length_); return str; diff --git a/source/common/formatter/substitution_formatter.h b/source/common/formatter/substitution_formatter.h index 3915f58a08ea..8bd92384eb05 100644 --- a/source/common/formatter/substitution_formatter.h +++ b/source/common/formatter/substitution_formatter.h @@ -16,6 +16,7 @@ #include "source/common/formatter/http_formatter_context.h" #include "source/common/json/json_loader.h" #include "source/common/json/json_streamer.h" +#include "source/common/json/json_utility.h" #include "absl/types/optional.h" #include "re2/re2.h" @@ -439,7 +440,8 @@ class JsonFormatterImplBase : public FormatterBase { } else { // 3. Handle the formatter element with a single provider and value // type needs to be kept. - typedValueToLogLine(formatters, context, info, serializer); + auto value = formatters[0]->formatValueWithContext(context, info); + Json::Utility::appendValueToString(value, log_line); } } @@ -467,48 +469,6 @@ class JsonFormatterImplBase : public FormatterBase { serializer.addRawString(Json::Constants::DoubleQuote); // End the JSON string. } - void typedValueToLogLine(const Formatters& formatters, const FormatterContext& context, - const StreamInfo::StreamInfo& info, - JsonStringSerializer& serializer) const { - - const ProtobufWkt::Value value = formatters[0]->formatValueWithContext(context, info); - - switch (value.kind_case()) { - case ProtobufWkt::Value::KIND_NOT_SET: - case ProtobufWkt::Value::kNullValue: - serializer.addNull(); - break; - case ProtobufWkt::Value::kNumberValue: - serializer.addNumber(value.number_value()); - break; - case ProtobufWkt::Value::kStringValue: - serializer.addString(value.string_value()); - break; - case ProtobufWkt::Value::kBoolValue: - serializer.addBool(value.bool_value()); - break; - case ProtobufWkt::Value::kStructValue: - case ProtobufWkt::Value::kListValue: - // Convert the struct or list to string. This may result in a performance - // degradation. But We rarely hit this case. - // Basically only metadata or filter state formatter may hit this case. -#ifdef ENVOY_ENABLE_YAML - absl::StatusOr json_or = - MessageUtil::getJsonStringFromMessage(value, false, true); - if (json_or.ok()) { - // We assume the output of getJsonStringFromMessage is a valid JSON string piece. - serializer.addRawString(json_or.value()); - } else { - serializer.addString(json_or.status().ToString()); - } -#else - IS_ENVOY_BUG("Json support compiled out"); - serializer.addRawString(R"EOF("Json support compiled out")EOF"); -#endif - break; - } - } - const std::string empty_value_; using ParsedFormatElement = absl::variant; diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc index 9c058bdf46ef..853e0eab1f91 100644 --- a/test/common/formatter/substitution_formatter_test.cc +++ b/test/common/formatter/substitution_formatter_test.cc @@ -2699,9 +2699,7 @@ TEST(SubstitutionFormatterTest, DynamicMetadataFieldExtractor) { DynamicMetadataFormatter formatter("com.test", {"nan_val"}, absl::optional()); absl::optional value = formatter.format(stream_info); - EXPECT_EQ("google.protobuf.Value cannot encode double values for nan, because it would be " - "parsed as a string", - value.value()); + EXPECT_EQ("null", value.value()); } { @@ -2713,9 +2711,7 @@ TEST(SubstitutionFormatterTest, DynamicMetadataFieldExtractor) { DynamicMetadataFormatter formatter("com.test", {"inf_val"}, absl::optional()); absl::optional value = formatter.format(stream_info); - EXPECT_EQ("google.protobuf.Value cannot encode double values for infinity, because it would be " - "parsed as a string", - value.value()); + EXPECT_EQ("inf", value.value()); } } @@ -4475,7 +4471,6 @@ TEST(SubstitutionFormatterTest, JsonFormatterTest) { JsonFormatterImpl formatter(key_mapping, false); const std::string out_json = formatter.formatWithContext(formatter_context, stream_info); - std::cout << out_json << std::endl; EXPECT_TRUE(TestUtility::jsonStringEqual(out_json, expected)); }