runtime: remove runtime envoy.reloadable_features.logging_with_fast_json_formatter

wbpcode · wbpcode · commit 51c4f8ea1663 · 2025-05-06T14:53:53.000Z
Signed-off-by: wangbaiping(wbpcode) &lt;wbphub@gmail.com&gt;
diff --git 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
@@ -37,10 +37,7 @@ absl::StatusOr<FormatterPtr> 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<FormatterPtr> 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<CommandParserPtr>& 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<LegacyJsonFormatterImpl>(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<CommandParserPtr>& commands) {
   return std::make_unique<JsonFormatterImpl>(struct_format, omit_empty_values, commands);
 }
 
diff --git 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<CommandParserPtr>& commands = {});
 };
 
diff --git a/source/common/formatter/substitution_formatter.h b/source/common/formatter/substitution_formatter.h
@@ -127,258 +127,5 @@ class JsonFormatterImpl : public Formatter {
   std::vector<ParsedFormatElement> parsed_elements_;
 };
 
-// Helper classes for StructFormatter::StructFormatMapVisitor.
-template <class... Ts> struct StructFormatMapVisitorHelper : Ts... {
-  using Ts::operator()...;
-};
-template <class... Ts> StructFormatMapVisitorHelper(Ts...) -> StructFormatMapVisitorHelper<Ts...>;
-
-#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<CommandParserPtr>;
-  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<FormatterProviderPtr>& 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 std::vector<FormatterProviderPtr>, 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<std::string, StructFormatValue>;
-  using StructFormatMapPtr = std::unique_ptr<StructFormatMap>;
-  struct StructFormatMapWrapper {
-    StructFormatMapPtr value_;
-  };
-
-  using StructFormatList = std::list<StructFormatValue>;
-  using StructFormatListPtr = std::unique_ptr<StructFormatList>;
-  struct StructFormatListWrapper {
-    StructFormatListPtr value_;
-  };
-
-  using StructFormatMapVisitor = StructFormatMapVisitorHelper<
-      const std::function<ProtobufWkt::Value(const std::vector<FormatterProviderPtr>&)>,
-      const std::function<ProtobufWkt::Value(const StructFormatter::StructFormatMapWrapper&)>,
-      const std::function<ProtobufWkt::Value(const StructFormatter::StructFormatListWrapper&)>>;
-
-  // Methods for building the format map.
-  class FormatBuilder {
-  public:
-    explicit FormatBuilder(const CommandParsers& commands) : commands_(commands) {}
-    absl::StatusOr<std::vector<FormatterProviderPtr>>
-    toFormatStringValue(const std::string& string_format) const {
-      return SubstitutionFormatParser::parse(string_format, commands_);
-    }
-    std::vector<FormatterProviderPtr> toFormatNumberValue(double value) const {
-      std::vector<FormatterProviderPtr> formatters;
-      formatters.emplace_back(FormatterProviderPtr{new PlainNumber(value)});
-      return formatters;
-    }
-    StructFormatMapWrapper toFormatMapValue(const ProtobufWkt::Struct& struct_format) const {
-      auto output = std::make_unique<StructFormatMap>();
-      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<FormatterProviderPtr>));
-          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<StructFormatList>();
-      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<FormatterProviderPtr>));
-          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<FormatterProviderPtr>& 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<ProtobufWkt::Value> 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<StructFormatter>;
-
-class LegacyJsonFormatterImpl : public Formatter {
-public:
-  using CommandParsers = std::vector<CommandParserPtr>;
-
-  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
@@ -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
@@ -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
@@ -66,8 +66,8 @@ AccessLog::InstanceSharedPtr FluentdAccessLogFactory::createAccessLogInstance(
                             std::vector<Formatter::CommandParserPtr>);
 
   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<FluentdFormatterImpl>(std::move(json_formatter));
 
diff --git 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<std::string> 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<CommandParserFactory> command_register(factory);
diff --git a/test/common/formatter/substitution_formatter_speed_test.cc b/test/common/formatter/substitution_formatter_speed_test.cc
diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc
diff --git a/test/extensions/access_loggers/fluentd/substitution_formatter_test.cc b/test/extensions/access_loggers/fluentd/substitution_formatter_test.cc
