sub-formatter: store a bool instead of a string (#37141)

The `FormatterBaseImpl` class stored a string in each instance where it could've stored a bool. Risk Level: low Testing: N/A Docs Changes: N/A Release Notes: N/A Platform Specific Features: N/A Signed-off-by: Adi Suissa-Peleg <[email protected]>
envoyproxy · Nov 16, 2024 · 2acdbdd · 2acdbdd
1 parent 6659950
commit 2acdbdd
Showing 1 changed file with 11 additions and 7 deletions.
diff --git a/source/common/formatter/substitution_formatter.h b/source/common/formatter/substitution_formatter.h
@@ -232,8 +232,14 @@ template <class FormatterContext> class FormatterBaseImpl : public FormatterBase
     log_line.reserve(256);
 
     for (const auto& provider : providers_) {
-      const auto bit = provider->formatWithContext(context, stream_info);
-      log_line += bit.value_or(empty_value_string_);
+      const absl::optional<std::string> 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()) {
+        log_line += bit.value();
+      } else if (!omit_empty_values_) {
+        log_line += DefaultUnspecifiedValueStringView;
+      }
     }
 
     return log_line;
@@ -242,24 +248,22 @@ template <class FormatterContext> class FormatterBaseImpl : public FormatterBase
 protected:
   FormatterBaseImpl(absl::Status& creation_status, absl::string_view format,
                     bool omit_empty_values = false)
-      : empty_value_string_(omit_empty_values ? absl::string_view{}
-                                              : DefaultUnspecifiedValueStringView) {
+      : omit_empty_values_(omit_empty_values) {
     auto providers_or_error = SubstitutionFormatParser::parse<FormatterContext>(format);
     SET_AND_RETURN_IF_NOT_OK(providers_or_error.status(), creation_status);
     providers_ = std::move(*providers_or_error);
   }
   FormatterBaseImpl(absl::Status& creation_status, absl::string_view format, bool omit_empty_values,
                     const CommandParsers& command_parsers = {})
-      : empty_value_string_(omit_empty_values ? absl::string_view{}
-                                              : DefaultUnspecifiedValueStringView) {
+      : omit_empty_values_(omit_empty_values) {
     auto providers_or_error =
         SubstitutionFormatParser::parse<FormatterContext>(format, command_parsers);
     SET_AND_RETURN_IF_NOT_OK(providers_or_error.status(), creation_status);
     providers_ = std::move(*providers_or_error);
   }
 
 private:
-  const std::string empty_value_string_;
+  const bool omit_empty_values_;
   std::vector<FormatterProviderBasePtr<FormatterContext>> providers_;
 };