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,138 @@
 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")),
+ canary_upstream_rq_time_(stat_name_pool_.add("canary.upstream_rq_time")),
+ external_(stat_name_pool_.add("external")),
+ external_upstream_rq_time_(stat_name_pool_.add("external.upstream_rq_time")),
+ internal_(stat_name_pool_.add("internal")),
+ internal_upstream_rq_time_(stat_name_pool_.add("internal.upstream_rq_time")),
+ 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 {
+ 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 {
+ 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 {
+ 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_);
+ incCounter(scope, prefix, upstreamRqGroup(response_code));
+ 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));
+ 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));
+ Stats::StatName rq_group = upstreamRqGroup(code);
+ Stats::StatName rq_code = upstreamRqStatName(code);
+
+ auto write_category = [this, rq_group, rq_code, &info](Stats::StatName category) {
+ incCounter(info.cluster_scope_, {info.prefix_, category, upstream_rq_completed_});
+ incCounter(info.cluster_scope_, {info.prefix_, category, rq_group});
+ incCounter(info.cluster_scope_, {info.prefix_, category, rq_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();
+ write_category(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();
+ write_category(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();
+ write_category(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::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 +160,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 upstream_rq_unknown_;
+}
+
+Stats::StatName CodeStatsImpl::upstreamRqStatName(Code response_code) const {
+ // Take a lock only if we've never seen this response-code before.
+ uint32_t rc_int = static_cast<uint32_t>(response_code);
+ if (rc_int >= NumHttpCodes) {
+ return upstream_rq_unknown_;
+ }
+ std::atomic<uint8_t*>& atomic_ref = rc_stat_names_[rc_int];
+ 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";