formatter: use the new json lib to relace protobuf json (envoyproxy#3…

…6530)

Commit Message: formatter: use the new json lib to relace protobuf json
Additional Description:

This make our formatter and logger won't depend on the full proto and
yaml support. But note the serializer has minor difference with the one
that provided by the proto lib.

The NaN and Infinity values of float will be serialized to ``null`` and
``"inf"`` respectively in the metadata (``DYNAMIC_METADATA``,
``CLUSTER_METADATA``, etc.) formatter. And the proto one will return an
error message.

I personally think in the scenario of formatter and logger, the `null`
and `"inf"` is better. But this is still a minor behavior change.

Risk Level: low.
Testing: n/a.
Docs Changes: n/a.
Release Notes: n/a.
Platform Specific Features: n/a.
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

Signed-off-by: wangbaiping <[email protected]>

changelogs/current.yaml

@@ -5,6 +5,10 @@ behavior_changes:
 
 minor_behavior_changes:
 # *Changes that may cause incompatibilities for some users, but should not for most*
+- 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));
 }