stats: Create stats for http codes with the symbol table.

include/envoy/http/codes.h

@@ -3,6 +3,7 @@
 #include <chrono>
 
 #include "envoy/stats/scope.h"
+#include "envoy/stats/symbol_table.h"
 
 namespace Envoy {
 namespace Http {
@@ -86,36 +87,13 @@ class CodeStats {
 public:
   virtual ~CodeStats() = default;
 
-  struct ResponseStatInfo {
-    Stats::Scope& global_scope_;
-    Stats::Scope& cluster_scope_;
-    const std::string& prefix_;
-    uint64_t response_status_code_;
-    bool internal_request_;
-    const std::string& request_vhost_name_;
-    const std::string& request_vcluster_name_;
-    const std::string& from_zone_;
-    const std::string& to_zone_;
-    bool upstream_canary_;
-  };
-
-  struct ResponseTimingInfo {
-    Stats::Scope& global_scope_;
-    Stats::Scope& cluster_scope_;
-    const std::string& prefix_;
-    std::chrono::milliseconds response_time_;
-    bool upstream_canary_;
-    bool internal_request_;
-    const std::string& request_vhost_name_;
-    const std::string& request_vcluster_name_;
-    const std::string& from_zone_;
-    const std::string& to_zone_;
-  };
+  struct ResponseStatInfo;
+  struct ResponseTimingInfo;
 
   /**
    * Charge a simple response stat to an upstream.
    */
-  virtual void chargeBasicResponseStat(Stats::Scope& scope, const std::string& prefix,
+  virtual void chargeBasicResponseStat(Stats::Scope& scope, Stats::StatName prefix,
                                        Code response_code) const PURE;
 
   /**

include/envoy/router/router.h

@@ -332,9 +332,9 @@ class VirtualCluster {
   virtual ~VirtualCluster() {}
 
   /**
-   * @return the name of the virtual cluster.
+   * @return the stat-name of the virtual cluster.
    */
-  virtual const std::string& name() const PURE;
+  virtual Stats::StatName statName() const PURE;
 };
 
 class RateLimitPolicy;
@@ -364,9 +364,9 @@ class VirtualHost {
   virtual const CorsPolicy* corsPolicy() const PURE;
 
   /**
-   * @return const std::string& the name of the virtual host.
+   * @return the stat-name of the virtual host.
    */
-  virtual const std::string& name() const PURE;
+  virtual Stats::StatName statName() const PURE;
 
   /**
    * @return const RateLimitPolicy& the rate limit policy for the virtual host.

include/envoy/upstream/host_description.h

@@ -104,6 +104,11 @@ class HostDescription {
    */
   virtual const envoy::api::v2::core::Locality& locality() const PURE;
 
+  /**
+   * @return the human readable name of the host's locality zone as a StatName.
+   */
+  virtual Stats::StatName localityZoneStatName() const PURE;
+
   /**
    * @return the address used to health check the host.
    */

source/common/http/async_client_impl.h

@@ -183,7 +183,7 @@ class AsyncStreamImpl : public AsyncClient::Stream,
 
   struct NullVirtualHost : public Router::VirtualHost {
     // Router::VirtualHost
-    const std::string& name() const override { return EMPTY_STRING; }
+    Stats::StatName statName() const override { return Stats::StatName(); }
     const Router::RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; }
     const Router::CorsPolicy* corsPolicy() const override { return nullptr; }
     const Router::Config& routeConfig() const override { return route_configuration_; }

source/common/http/codes.cc

@@ -18,111 +18,140 @@
 namespace Envoy {
 namespace Http {
 
-void CodeStatsImpl::chargeBasicResponseStat(Stats::Scope& scope, const std::string& prefix,
+CodeStatsImpl::CodeStatsImpl(Stats::SymbolTable& symbol_table)
+    : stat_name_pool_(symbol_table), symbol_table_(symbol_table),
+      canary_(stat_name_pool_.add("canary")), external_(stat_name_pool_.add("external")),
+      internal_(stat_name_pool_.add("internal")),
+      upstream_rq_1xx_(stat_name_pool_.add("upstream_rq_1xx")),
+      upstream_rq_2xx_(stat_name_pool_.add("upstream_rq_2xx")),
+      upstream_rq_3xx_(stat_name_pool_.add("upstream_rq_3xx")),
+      upstream_rq_4xx_(stat_name_pool_.add("upstream_rq_4xx")),
+      upstream_rq_5xx_(stat_name_pool_.add("upstream_rq_5xx")),
+      upstream_rq_unknown_(stat_name_pool_.add("upstream_rq_unknown")), // Covers invalid http
+                                                                        // response codes e.g. 600.
+      upstream_rq_completed_(stat_name_pool_.add("upstream_rq_completed")),
+      upstream_rq_time_(stat_name_pool_.add("upstream_rq_time")),
+      vcluster_(stat_name_pool_.add("vcluster")), vhost_(stat_name_pool_.add("vhost")),
+      zone_(stat_name_pool_.add("zone")) {
+
+  for (uint32_t i = 0; i < NumHttpCodes; ++i) {
+    rc_stat_names_[i] = nullptr;
+  }
+
+  // Pre-allocate response codes 200, 404, and 503, as those seem quite likely.
+  // We don't pre-allocate all the HTTP codes because the first 127 allocations
+  // are likely to be encoded in one byte, and we would rather spend those on
+  // common components of stat-names that appear frequently.
+  upstreamRqStatName(Code::OK);
+  upstreamRqStatName(Code::NotFound);
+  upstreamRqStatName(Code::ServiceUnavailable);
+}
+
+void CodeStatsImpl::incCounter(Stats::Scope& scope,
+                               const std::vector<Stats::StatName>& names) const {
+  const Stats::SymbolTable::StoragePtr stat_name_storage = symbol_table_.join(names);
+  scope.counterFromStatName(Stats::StatName(stat_name_storage.get())).inc();
+}
+
+void CodeStatsImpl::incCounter(Stats::Scope& scope, Stats::StatName a, Stats::StatName b) const {
+  const Stats::SymbolTable::StoragePtr stat_name_storage = symbol_table_.join({a, b});
+  scope.counterFromStatName(Stats::StatName(stat_name_storage.get())).inc();
+}
+
+void CodeStatsImpl::recordHistogram(Stats::Scope& scope, const std::vector<Stats::StatName>& names,
+                                    uint64_t count) const {
+  const Stats::SymbolTable::StoragePtr stat_name_storage = symbol_table_.join(names);
+  scope.histogramFromStatName(Stats::StatName(stat_name_storage.get())).recordValue(count);
+}
+
+void CodeStatsImpl::chargeBasicResponseStat(Stats::Scope& scope, Stats::StatName prefix,
                                             Code response_code) const {
+  ASSERT(&symbol_table_ == &scope.symbolTable());
+
   // Build a dynamic stat for the response code and increment it.
-  scope.counter(absl::StrCat(prefix, upstream_rq_completed_)).inc();
-  scope
-      .counter(absl::StrCat(prefix, upstream_rq_,
-                            CodeUtility::groupStringForResponseCode(response_code)))
-      .inc();
-  scope.counter(absl::StrCat(prefix, upstream_rq_, enumToInt(response_code))).inc();
+  incCounter(scope, prefix, upstream_rq_completed_);
+  const Stats::StatName rq_group = upstreamRqGroup(response_code);
+  if (!rq_group.empty()) {
+    incCounter(scope, prefix, rq_group);
+  }
+  incCounter(scope, prefix, upstreamRqStatName(response_code));
 }
 
 void CodeStatsImpl::chargeResponseStat(const ResponseStatInfo& info) const {
-  const uint64_t response_code = info.response_status_code_;
-  chargeBasicResponseStat(info.cluster_scope_, info.prefix_, static_cast<Code>(response_code));
+  const Code code = static_cast<Code>(info.response_status_code_);
+
+  ASSERT(&info.cluster_scope_.symbolTable() == &symbol_table_);
+  chargeBasicResponseStat(info.cluster_scope_, info.prefix_, code);
 
-  std::string group_string =
-      CodeUtility::groupStringForResponseCode(static_cast<Code>(response_code));
+  const Stats::StatName rq_group = upstreamRqGroup(code);
+  const Stats::StatName rq_code = upstreamRqStatName(code);
 
   // If the response is from a canary, also create canary stats.
   if (info.upstream_canary_) {
-    info.cluster_scope_.counter(absl::StrCat(info.prefix_, canary_upstream_rq_completed_)).inc();
-    info.cluster_scope_.counter(absl::StrCat(info.prefix_, canary_upstream_rq_, group_string))
-        .inc();
-    info.cluster_scope_.counter(absl::StrCat(info.prefix_, canary_upstream_rq_, response_code))
-        .inc();
+    writeCategory(info, rq_group, rq_code, canary_);
   }
 
   // Split stats into external vs. internal.
   if (info.internal_request_) {
-    info.cluster_scope_.counter(absl::StrCat(info.prefix_, internal_upstream_rq_completed_)).inc();
-    info.cluster_scope_.counter(absl::StrCat(info.prefix_, internal_upstream_rq_, group_string))
-        .inc();
-    info.cluster_scope_.counter(absl::StrCat(info.prefix_, internal_upstream_rq_, response_code))
-        .inc();
+    writeCategory(info, rq_group, rq_code, internal_);
   } else {
-    info.cluster_scope_.counter(absl::StrCat(info.prefix_, external_upstream_rq_completed_)).inc();
-    info.cluster_scope_.counter(absl::StrCat(info.prefix_, external_upstream_rq_, group_string))
-        .inc();
-    info.cluster_scope_.counter(absl::StrCat(info.prefix_, external_upstream_rq_, response_code))
-        .inc();
+    writeCategory(info, rq_group, rq_code, external_);
   }
 
   // Handle request virtual cluster.
   if (!info.request_vcluster_name_.empty()) {
-    info.global_scope_
-        .counter(join({vhost_, info.request_vhost_name_, vcluster_, info.request_vcluster_name_,
-                       upstream_rq_completed_}))
-        .inc();
-    info.global_scope_
-        .counter(join({vhost_, info.request_vhost_name_, vcluster_, info.request_vcluster_name_,
-                       absl::StrCat(upstream_rq_, group_string)}))
-        .inc();
-    info.global_scope_
-        .counter(join({vhost_, info.request_vhost_name_, vcluster_, info.request_vcluster_name_,
-                       absl::StrCat(upstream_rq_, response_code)}))
-        .inc();
+    incCounter(info.global_scope_, {vhost_, info.request_vhost_name_, vcluster_,
+                                    info.request_vcluster_name_, upstream_rq_completed_});
+    incCounter(info.global_scope_, {vhost_, info.request_vhost_name_, vcluster_,
+                                    info.request_vcluster_name_, rq_group});
+    incCounter(info.global_scope_,
+               {vhost_, info.request_vhost_name_, vcluster_, info.request_vcluster_name_, rq_code});
   }
 
   // Handle per zone stats.
   if (!info.from_zone_.empty() && !info.to_zone_.empty()) {
-    absl::string_view prefix_without_trailing_dot = stripTrailingDot(info.prefix_);
-    info.cluster_scope_
-        .counter(join({prefix_without_trailing_dot, zone_, info.from_zone_, info.to_zone_,
-                       upstream_rq_completed_}))
-        .inc();
-    info.cluster_scope_
-        .counter(join({prefix_without_trailing_dot, zone_, info.from_zone_, info.to_zone_,
-                       absl::StrCat(upstream_rq_, group_string)}))
-        .inc();
-    info.cluster_scope_
-        .counter(join({prefix_without_trailing_dot, zone_, info.from_zone_, info.to_zone_,
-                       absl::StrCat(upstream_rq_, response_code)}))
-        .inc();
+    incCounter(info.cluster_scope_,
+               {info.prefix_, zone_, info.from_zone_, info.to_zone_, upstream_rq_completed_});
+    incCounter(info.cluster_scope_,
+               {info.prefix_, zone_, info.from_zone_, info.to_zone_, rq_group});
+    incCounter(info.cluster_scope_, {info.prefix_, zone_, info.from_zone_, info.to_zone_, rq_code});
+  }
+}
+
+void CodeStatsImpl::writeCategory(const ResponseStatInfo& info, Stats::StatName rq_group,
+                                  Stats::StatName rq_code, Stats::StatName category) const {
+  incCounter(info.cluster_scope_, {info.prefix_, category, upstream_rq_completed_});
+  if (!rq_group.empty()) {
+    incCounter(info.cluster_scope_, {info.prefix_, category, rq_group});
   }
+  incCounter(info.cluster_scope_, {info.prefix_, category, rq_code});
 }
 
 void CodeStatsImpl::chargeResponseTiming(const ResponseTimingInfo& info) const {
-  info.cluster_scope_.histogram(absl::StrCat(info.prefix_, upstream_rq_time_))
-      .recordValue(info.response_time_.count());
+  const uint64_t count = info.response_time_.count();
+  recordHistogram(info.cluster_scope_, {info.prefix_, upstream_rq_time_}, count);
   if (info.upstream_canary_) {
-    info.cluster_scope_.histogram(absl::StrCat(info.prefix_, canary_upstream_rq_time_))
-        .recordValue(info.response_time_.count());
+    recordHistogram(info.cluster_scope_, {info.prefix_, canary_, upstream_rq_time_}, count);
   }
 
   if (info.internal_request_) {
-    info.cluster_scope_.histogram(absl::StrCat(info.prefix_, internal_upstream_rq_time_))
-        .recordValue(info.response_time_.count());
+    recordHistogram(info.cluster_scope_, {info.prefix_, internal_, upstream_rq_time_}, count);
   } else {
-    info.cluster_scope_.histogram(absl::StrCat(info.prefix_, external_upstream_rq_time_))
-        .recordValue(info.response_time_.count());
+    recordHistogram(info.cluster_scope_, {info.prefix_, external_, upstream_rq_time_}, count);
   }
 
   if (!info.request_vcluster_name_.empty()) {
-    info.global_scope_
-        .histogram(join({vhost_, info.request_vhost_name_, vcluster_, info.request_vcluster_name_,
-                         upstream_rq_time_}))
-        .recordValue(info.response_time_.count());
+    recordHistogram(info.global_scope_,
+                    {vhost_, info.request_vhost_name_, vcluster_, info.request_vcluster_name_,
+                     upstream_rq_time_},
+                    count);
   }
 
   // Handle per zone stats.
   if (!info.from_zone_.empty() && !info.to_zone_.empty()) {
-    info.cluster_scope_
-        .histogram(join({stripTrailingDot(info.prefix_), zone_, info.from_zone_, info.to_zone_,
-                         upstream_rq_time_}))
-        .recordValue(info.response_time_.count());
+    recordHistogram(info.cluster_scope_,
+                    {info.prefix_, zone_, info.from_zone_, info.to_zone_, upstream_rq_time_},
+                    count);
   }
 }
 
@@ -133,19 +162,48 @@ absl::string_view CodeStatsImpl::stripTrailingDot(absl::string_view str) {
   return str;
 }
 
-std::string CodeStatsImpl::join(const std::vector<absl::string_view>& v) {
-  if (v.empty()) {
-    return "";
+Stats::StatName CodeStatsImpl::upstreamRqGroup(Code response_code) const {
+  switch (enumToInt(response_code) / 100) {
+  case 1:
+    return upstream_rq_1xx_;
+  case 2:
+    return upstream_rq_2xx_;
+  case 3:
+    return upstream_rq_3xx_;
+  case 4:
+    return upstream_rq_4xx_;
+  case 5:
+    return upstream_rq_5xx_;
   }
-  auto iter = v.begin();
-  if (iter->empty()) {
-    ++iter; // Skip any initial empty prefix.
+  return empty_; // Unknown codes do not go into a group.
+}
+
+Stats::StatName CodeStatsImpl::upstreamRqStatName(Code response_code) const {
+  // Take a lock only if we've never seen this response-code before.
+  const uint32_t rc_index = static_cast<uint32_t>(response_code) - HttpCodeOffset;
+  if (rc_index >= NumHttpCodes) {
+    return upstream_rq_unknown_;
+  }
+  std::atomic<uint8_t*>& atomic_ref = rc_stat_names_[rc_index];
+  if (atomic_ref.load() == nullptr) {
+    absl::MutexLock lock(&mutex_);
+
+    // Check again under lock as two threads might have raced to add a StatName
+    // for the same code.
+    if (atomic_ref.load() == nullptr) {
+      atomic_ref = stat_name_pool_.addReturningStorage(
+          absl::StrCat("upstream_rq_", enumToInt(response_code)));
+    }
   }
-  return absl::StrJoin(iter, v.end(), ".");
+  return Stats::StatName(atomic_ref.load());
 }
 
 std::string CodeUtility::groupStringForResponseCode(Code response_code) {
-  if (CodeUtility::is2xx(enumToInt(response_code))) {
+  // Note: this is only used in the unit test and in dynamo_filter.cc, which
+  // needs the same sort of symbolization treatment we are doing here.
+  if (CodeUtility::is1xx(enumToInt(response_code))) {
+    return "1xx";
+  } else if (CodeUtility::is2xx(enumToInt(response_code))) {
     return "2xx";
   } else if (CodeUtility::is3xx(enumToInt(response_code))) {
     return "3xx";