mobile: moving stats back to string vector (#25324)

Undoing the API change from https://github.com/envoyproxy/envoy/pull/25259/files and adding builder support for the original API.

Risk Level: low
Testing: yes
Docs Changes: no
Release Notes: no
Part of #24976
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

mobile/library/cc/engine_builder.cc

@@ -84,8 +84,8 @@ EngineBuilder& EngineBuilder::setOnEngineRunning(std::function<void()> closure)
   return *this;
 }
 
-EngineBuilder& EngineBuilder::addStatsSink(std::string name, std::string typed_config) {
-  stats_sinks_.push_back(std::make_pair(name, typed_config));
+EngineBuilder& EngineBuilder::addStatsSinks(std::vector<std::string> stats_sinks) {
+  stats_sinks_ = std::move(stats_sinks);
   return *this;
 }
 
@@ -366,22 +366,13 @@ std::string EngineBuilder::generateConfigStr() const {
     config_builder << "- &" << key << " " << value << std::endl;
   }
 
-  bool add_stats_sinks = !stats_sinks_.empty() || !stats_domain_.empty();
-  if (add_stats_sinks) {
-    config_builder << "- &stats_sinks [";
-  }
-  maybe_comma = "";
+  std::vector<std::string> stat_sinks = stats_sinks_;
   if (!stats_domain_.empty()) {
-    config_builder << "*base_metrics_service";
-    maybe_comma = ",";
+    stat_sinks.push_back("*base_metrics_service");
   }
-
-  for (auto& sink_to_add : stats_sinks_) {
-    config_builder << maybe_comma << "{ name: " << sink_to_add.first
-                   << ", typed_config: " << sink_to_add.second << "}";
-    maybe_comma = ",";
-  }
-  if (add_stats_sinks) {
+  if (!stat_sinks.empty()) {
+    config_builder << "- &stats_sinks [";
+    config_builder << absl::StrJoin(stat_sinks, ",");
     config_builder << "] " << std::endl;
   }
 
@@ -903,6 +894,11 @@ std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> EngineBuilder::generate
 
   bootstrap->mutable_typed_dns_resolver_config()->CopyFrom(
       *dns_cache_config->mutable_typed_dns_resolver_config());
+
+  for (const std::string& sink_yaml : stats_sinks_) {
+    auto* sink = bootstrap->add_stats_sinks();
+    MessageUtil::loadFromYaml(sink_yaml, *sink, ProtobufMessage::getStrictValidationVisitor());
+  }
   if (!stats_domain_.empty()) {
     envoy::config::metrics::v3::MetricsServiceConfig metrics_config;
     metrics_config.mutable_grpc_service()->mutable_envoy_grpc()->set_cluster_name("stats");
@@ -939,13 +935,6 @@ std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> EngineBuilder::generate
     ads_config->add_grpc_services()->mutable_google_grpc()->set_target_uri(target_uri);
   }
 
-  for (auto& sink_to_add : stats_sinks_) {
-    auto* sink = bootstrap->add_stats_sinks();
-    sink->set_name(sink_to_add.first);
-    MessageUtil::loadFromYaml(sink_to_add.second, *sink->mutable_typed_config(),
-                              ProtobufMessage::getStrictValidationVisitor());
-  }
-
   // Admin
   if (admin_interface_enabled_) {
     auto* admin_address = bootstrap->mutable_admin()->mutable_address()->mutable_socket_address();

mobile/library/cc/engine_builder.h

@@ -75,7 +75,7 @@ class EngineBuilder {
   EngineBuilder& addDnsPreresolveHostnames(const std::vector<std::string>& hostnames);
   EngineBuilder& addNativeFilter(std::string name, std::string typed_config);
   EngineBuilder& enableAdminInterface(bool admin_interface_on);
-  EngineBuilder& addStatsSink(std::string name, std::string typed_config);
+  EngineBuilder& addStatsSinks(std::vector<std::string> stat_sinks);
   EngineBuilder& addPlatformFilter(std::string name);
   EngineBuilder& addVirtualCluster(std::string virtual_cluster);
 
@@ -153,7 +153,7 @@ class EngineBuilder {
   bool always_use_v6_ = false;
   int dns_min_refresh_seconds_ = 60;
   int max_connections_per_host_ = 7;
-  std::vector<std::pair<std::string, std::string>> stats_sinks_;
+  std::vector<std::string> stats_sinks_;
 
   std::vector<NativeFilterConfig> native_filter_chain_;
   std::vector<std::string> dns_preresolve_hostnames_;

mobile/test/cc/unit/envoy_config_test.cc

@@ -273,11 +273,11 @@ TEST(TestConfig, AddMaxConnectionsPerHost) {
 }
 
 std::string statsdSinkConfig(int port) {
-  std::string config = R"(
-      {
+  std::string config = R"({ name: envoy.stat_sinks.statsd,
+      typed_config: {
         "@type": type.googleapis.com/envoy.config.metrics.v3.StatsdSink,
         address: { socket_address: { address: 127.0.0.1, port_value: )" +
-                       fmt::format("{}", port) + " } } }";
+                       fmt::format("{}", port) + " } } } }";
   return config;
 }
 
@@ -289,8 +289,7 @@ TEST(TestConfig, AddStatsSinks) {
   envoy::config::bootstrap::v3::Bootstrap bootstrap;
   TestUtility::loadFromYaml(absl::StrCat(config_header, config_str), bootstrap);
 
-  engine_builder.addStatsSink("envoy.stat_sinks.statsd", statsdSinkConfig(1));
-  engine_builder.addStatsSink("envoy.stat_sinks.statsd", statsdSinkConfig(2));
+  engine_builder.addStatsSinks({statsdSinkConfig(1), statsdSinkConfig(2)});
   config_str = engine_builder.generateConfigStr();
   ASSERT_THAT(config_str, HasSubstr(statsdSinkConfig(1)));
   ASSERT_THAT(config_str, HasSubstr(statsdSinkConfig(2)));