Replace ':' with '_' in stats name.

docs/configuration/cluster_manager/cluster.rst

@@ -31,7 +31,7 @@ Cluster
 name
   *(required, string)* Supplies the name of the cluster which must be unique across all clusters.
   The cluster name is used when emitting :ref:`statistics <config_cluster_manager_cluster_stats>`.
-  The cluster name can be at most 60 characters long, and must **not** contain ``:``.
+  The cluster name can be at most 60 characters long.
 
 .. _config_cluster_manager_type:
 

docs/configuration/cluster_manager/cluster_manager.rst

@@ -55,7 +55,7 @@ Statistics
 ----------
 
 The cluster manager has a statistics tree rooted at *cluster_manager.* with the following
-statistics:
+statistics. Any ``:`` character in the stats name is replaced with ``_``.
 
 .. csv-table::
   :header: Name, Type, Description

docs/configuration/listeners/listeners.rst

@@ -81,7 +81,7 @@ Statistics
 ----------
 
 The listener manager has a statistics tree rooted at *listener_manager.* with the following
-statistics:
+statistics. Any ``:`` character in the stats name is replaced with ``_``.
 
 .. csv-table::
   :header: Name, Type, Description

source/common/json/config_schemas.cc

@@ -1391,7 +1391,6 @@ const std::string Json::Schema::CLUSTER_SCHEMA(R"EOF(
     "properties" : {
       "name" : {
         "type" : "string",
-        "pattern" : "^[^:]+$",
         "minLength" : 1,
         "maxLength" : 60
       },

source/common/stats/stats_impl.cc

@@ -10,6 +10,12 @@
 namespace Envoy {
 namespace Stats {
 
+std::string Utility::sanitizeStatsName(const std::string& name) {
+  std::string stats_name = name;
+  std::replace(stats_name.begin(), stats_name.end(), ':', '_');
+  return stats_name;
+}
+
 void TimerImpl::TimespanImpl::complete(const std::string& dynamic_name) {
   std::chrono::milliseconds ms = std::chrono::duration_cast<std::chrono::milliseconds>(
       std::chrono::steady_clock::now() - start_);

source/common/stats/stats_impl.h

@@ -17,6 +17,16 @@
 namespace Envoy {
 namespace Stats {
 
+/**
+ * Common stats utility routines.
+ */
+class Utility {
+public:
+  // ':' is a reserved char in statsd. Do a character replacement to avoid costly inline
+  // translations later.
+  static std::string sanitizeStatsName(const std::string& name);
+};
+
 /**
  * This structure is the backing memory for both CounterImpl and GaugeImpl. It is designed so that
  * it can be allocated from shared memory if needed.
@@ -232,7 +242,7 @@ class IsolatedStoreImpl : public Store {
 private:
   struct ScopeImpl : public Scope {
     ScopeImpl(IsolatedStoreImpl& parent, const std::string& prefix)
-        : parent_(parent), prefix_(prefix) {}
+        : parent_(parent), prefix_(Utility::sanitizeStatsName(prefix)) {}
 
     // Stats::Scope
     ScopePtr createScope(const std::string& name) override {

source/common/stats/thread_local_store.h

@@ -81,7 +81,7 @@ class ThreadLocalStoreImpl : public StoreRoot {
 
   struct ScopeImpl : public Scope {
     ScopeImpl(ThreadLocalStoreImpl& parent, const std::string& prefix)
-        : parent_(parent), prefix_(prefix) {}
+        : parent_(parent), prefix_(Utility::sanitizeStatsName(prefix)) {}
     ~ScopeImpl();
 
     // Stats::Scope

source/server/listener_manager_impl.cc

@@ -91,11 +91,8 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, ListenerManag
   ASSERT(config.filter_chains().size() == 1);
   const auto& filter_chain = config.filter_chains()[0];
 
-  // ':' is a reserved char in statsd. Do the translation here to avoid costly inline translations
-  // later.
-  std::string final_stat_name = fmt::format("listener.{}.", address_->asString());
-  std::replace(final_stat_name.begin(), final_stat_name.end(), ':', '_');
-  listener_scope_ = parent_.server_.stats().createScope(final_stat_name);
+  listener_scope_ =
+      parent_.server_.stats().createScope(fmt::format("listener.{}.", address_->asString()));
 
   if (filter_chain.has_tls_context()) {
     Ssl::ServerContextConfigImpl context_config(filter_chain.tls_context());

test/common/upstream/cluster_manager_impl_test.cc

@@ -248,20 +248,29 @@ TEST_F(ClusterManagerImplTest, MaxClusterName) {
                             "document key: #/name");
 }
 
-TEST_F(ClusterManagerImplTest, InvalidClusterNameChars) {
+TEST_F(ClusterManagerImplTest, ValidClusterName) {
   const std::string json = R"EOF(
   {
     "clusters": [
     {
-      "name": "cluster:"
+      "name": "cluster:name",
+      "connect_timeout_ms": 250,
+      "type": "static",
+      "lb_type": "round_robin",
+      "hosts": [{"url": "tcp://127.0.0.1:11001"}]
     }]
   }
   )EOF";
 
-  EXPECT_THROW_WITH_MESSAGE(create(parseBootstrapFromJson(json)), Json::Exception,
-                            "JSON at lines 4-6 does not conform to schema.\n Invalid schema: "
-                            "#/properties/name\n Schema violation: pattern\n Offending document "
-                            "key: #/name");
+  create(parseBootstrapFromJson(json));
+  cluster_manager_->clusters()
+      .find("cluster:name")
+      ->second.get()
+      .info()
+      ->statsScope()
+      .counter("foo")
+      .inc();
+  EXPECT_EQ(1UL, factory_.stats_.counter("cluster.cluster_name.foo").value());
 }
 
 TEST_F(ClusterManagerImplTest, OriginalDstLbRestriction) {

test/server/listener_manager_impl_test.cc

@@ -648,6 +648,21 @@ TEST_F(ListenerManagerImplTest, AddListenerFailure) {
   EXPECT_EQ(1UL, server_.stats_store_.counter("listener_manager.listener_create_failure").value());
 }
 
+TEST_F(ListenerManagerImplTest, StatsNameValidCharacterTest) {
+  const std::string json = R"EOF(
+  {
+    "address": "tcp://[::1]:10000",
+    "filters": [],
+    "bind_to_port": false
+  }
+  )EOF";
+
+  manager_->addOrUpdateListener(parseListenerFromJson(json));
+  manager_->listeners().front().get().listenerScope().counter("foo").inc();
+
+  EXPECT_EQ(1UL, server_.stats_store_.counter("listener.[__1]_10000.foo").value());
+}
+
 TEST_F(ListenerManagerImplTest, DuplicateAddressDontBind) {
   InSequence s;