formatter: use the new json lib to relace protobuf json

Signed-off-by: wangbaiping <wangbaiping@bytedance.com>

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*

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",
     ],

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<std::string> 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;

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<FormatterContext> {
       } 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<FormatterContext> {
     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<std::string> 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<std::string, Formatters>;

test/common/formatter/substitution_formatter_test.cc

@@ -2699,9 +2699,7 @@ TEST(SubstitutionFormatterTest, DynamicMetadataFieldExtractor) {
 
     DynamicMetadataFormatter formatter("com.test", {"nan_val"}, absl::optional<size_t>());
     absl::optional<std::string> 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<size_t>());
     absl::optional<std::string> 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));
 }