From 769be4ffaceaf003321f4f1af2ea8ebe987effbd Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 31 Jul 2018 18:27:06 -0400
Subject: [PATCH 01/14] Re-order functions to group classes together.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 source/common/stats/stats_impl.cc | 277 +++++++++++++++---------------
 1 file changed, 138 insertions(+), 139 deletions(-)

diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc
index 681bc77d640f..b4556aec0ca4 100644
--- a/source/common/stats/stats_impl.cc
+++ b/source/common/stats/stats_impl.cc
@@ -37,6 +37,105 @@ bool regexStartsWithDot(absl::string_view regex) {
 
 } // namespace
 
+std::string Utility::sanitizeStatsName(const std::string& name) {
+  std::string stats_name = name;
+  std::replace(stats_name.begin(), stats_name.end(), ':', '_');
+  std::replace(stats_name.begin(), stats_name.end(), '\0', '_');
+  return stats_name;
+}
+
+/**
+ * Counter implementation that wraps a StatData. StatData must have data members:
+ *    std::atomic<int64_t> value_;
+ *    std::atomic<int64_t> pending_increment_;
+ *    std::atomic<int16_t> flags_;
+ *    std::atomic<int16_t> ref_count_;
+ */
+template <class StatData> class CounterImpl : public Counter, public MetricImpl {
+public:
+  CounterImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
+              std::string&& tag_extracted_name, std::vector<Tag>&& tags)
+      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
+        alloc_(alloc) {}
+  ~CounterImpl() { alloc_.free(data_); }
+
+  // Stats::Counter
+  void add(uint64_t amount) override {
+    data_.value_ += amount;
+    data_.pending_increment_ += amount;
+    data_.flags_ |= Flags::Used;
+  }
+
+  void inc() override { add(1); }
+  uint64_t latch() override { return data_.pending_increment_.exchange(0); }
+  void reset() override { data_.value_ = 0; }
+  bool used() const override { return data_.flags_ & Flags::Used; }
+  uint64_t value() const override { return data_.value_; }
+
+private:
+  StatData& data_;
+  StatDataAllocatorImpl<StatData>& alloc_;
+};
+
+/**
+ * Gauge implementation that wraps a StatData.
+ */
+template <class StatData> class GaugeImpl : public Gauge, public MetricImpl {
+public:
+  GaugeImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
+            std::string&& tag_extracted_name, std::vector<Tag>&& tags)
+      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
+        alloc_(alloc) {}
+  ~GaugeImpl() { alloc_.free(data_); }
+
+  // Stats::Gauge
+  virtual void add(uint64_t amount) override {
+    data_.value_ += amount;
+    data_.flags_ |= Flags::Used;
+  }
+  virtual void dec() override { sub(1); }
+  virtual void inc() override { add(1); }
+  virtual void set(uint64_t value) override {
+    data_.value_ = value;
+    data_.flags_ |= Flags::Used;
+  }
+  virtual void sub(uint64_t amount) override {
+    ASSERT(data_.value_ >= amount);
+    ASSERT(used());
+    data_.value_ -= amount;
+  }
+  virtual uint64_t value() const override { return data_.value_; }
+  bool used() const override { return data_.flags_ & Flags::Used; }
+
+private:
+  StatData& data_;
+  StatDataAllocatorImpl<StatData>& alloc_;
+};
+
+template <class StatData>
+CounterSharedPtr StatDataAllocatorImpl<StatData>::makeCounter(absl::string_view name,
+                                                              std::string&& tag_extracted_name,
+                                                              std::vector<Tag>&& tags) {
+  StatData* data = alloc(name);
+  if (data == nullptr) {
+    return nullptr;
+  }
+  return std::make_shared<CounterImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
+                                                 std::move(tags));
+}
+
+template <class StatData>
+GaugeSharedPtr StatDataAllocatorImpl<StatData>::makeGauge(absl::string_view name,
+                                                          std::string&& tag_extracted_name,
+                                                          std::vector<Tag>&& tags) {
+  StatData* data = alloc(name);
+  if (data == nullptr) {
+    return nullptr;
+  }
+  return std::make_shared<GaugeImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
+                                               std::move(tags));
+}
+
 // Normally the compiler would do this, but because name_ is a flexible-array-length
 // element, the compiler can't. RawStatData is put into an array in HotRestartImpl, so
 // it's important that each element starts on the required alignment for the type.
@@ -48,11 +147,45 @@ uint64_t RawStatData::structSizeWithOptions(const StatsOptions& stats_options) {
   return structSize(stats_options.maxNameLength());
 }
 
-std::string Utility::sanitizeStatsName(const std::string& name) {
-  std::string stats_name = name;
-  std::replace(stats_name.begin(), stats_name.end(), ':', '_');
-  std::replace(stats_name.begin(), stats_name.end(), '\0', '_');
-  return stats_name;
+void RawStatData::initialize(absl::string_view key, const StatsOptions& stats_options) {
+  ASSERT(!initialized());
+  ASSERT(key.size() <= stats_options.maxNameLength());
+  ref_count_ = 1;
+  memcpy(name_, key.data(), key.size());
+  name_[key.size()] = '\0';
+}
+
+HeapStatData::HeapStatData(absl::string_view key) : name_(key.data(), key.size()) {}
+
+HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {
+  // Any expected truncation of name is done at the callsite. No truncation is
+  // required to use this allocator.
+  auto data = std::make_unique<HeapStatData>(name);
+  Thread::ReleasableLockGuard lock(mutex_);
+  auto ret = stats_.insert(data.get());
+  HeapStatData* existing_data = *ret.first;
+  lock.release();
+
+  if (ret.second) {
+    return data.release();
+  }
+  ++existing_data->ref_count_;
+  return existing_data;
+}
+
+void HeapStatDataAllocator::free(HeapStatData& data) {
+  ASSERT(data.ref_count_ > 0);
+  if (--data.ref_count_ > 0) {
+    return;
+  }
+
+  {
+    Thread::LockGuard lock(mutex_);
+    size_t key_removed = stats_.erase(&data);
+    ASSERT(key_removed == 1);
+  }
+
+  delete &data;
 }
 
 TagExtractorImpl::TagExtractorImpl(const std::string& name, const std::string& regex,
@@ -135,92 +268,6 @@ bool TagExtractorImpl::extractTag(const std::string& stat_name, std::vector<Tag>
   return false;
 }
 
-HeapStatData::HeapStatData(absl::string_view key) : name_(key.data(), key.size()) {}
-
-HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {
-  // Any expected truncation of name is done at the callsite. No truncation is
-  // required to use this allocator.
-  auto data = std::make_unique<HeapStatData>(name);
-  Thread::ReleasableLockGuard lock(mutex_);
-  auto ret = stats_.insert(data.get());
-  HeapStatData* existing_data = *ret.first;
-  lock.release();
-
-  if (ret.second) {
-    return data.release();
-  }
-  ++existing_data->ref_count_;
-  return existing_data;
-}
-
-/**
- * Counter implementation that wraps a StatData. StatData must have data members:
- *    std::atomic<int64_t> value_;
- *    std::atomic<int64_t> pending_increment_;
- *    std::atomic<int16_t> flags_;
- *    std::atomic<int16_t> ref_count_;
- */
-template <class StatData> class CounterImpl : public Counter, public MetricImpl {
-public:
-  CounterImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
-              std::string&& tag_extracted_name, std::vector<Tag>&& tags)
-      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
-        alloc_(alloc) {}
-  ~CounterImpl() { alloc_.free(data_); }
-
-  // Stats::Counter
-  void add(uint64_t amount) override {
-    data_.value_ += amount;
-    data_.pending_increment_ += amount;
-    data_.flags_ |= Flags::Used;
-  }
-
-  void inc() override { add(1); }
-  uint64_t latch() override { return data_.pending_increment_.exchange(0); }
-  void reset() override { data_.value_ = 0; }
-  bool used() const override { return data_.flags_ & Flags::Used; }
-  uint64_t value() const override { return data_.value_; }
-
-private:
-  StatData& data_;
-  StatDataAllocatorImpl<StatData>& alloc_;
-};
-
-/**
- * Gauge implementation that wraps a StatData.
- */
-template <class StatData> class GaugeImpl : public Gauge, public MetricImpl {
-public:
-  GaugeImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
-            std::string&& tag_extracted_name, std::vector<Tag>&& tags)
-      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
-        alloc_(alloc) {}
-  ~GaugeImpl() { alloc_.free(data_); }
-
-  // Stats::Gauge
-  virtual void add(uint64_t amount) override {
-    data_.value_ += amount;
-    data_.flags_ |= Flags::Used;
-  }
-  virtual void dec() override { sub(1); }
-  virtual void inc() override { add(1); }
-  virtual void set(uint64_t value) override {
-    data_.value_ = value;
-    data_.flags_ |= Flags::Used;
-  }
-  virtual void sub(uint64_t amount) override {
-    ASSERT(data_.value_ >= amount);
-    ASSERT(used());
-    data_.value_ -= amount;
-  }
-  virtual uint64_t value() const override { return data_.value_; }
-  bool used() const override { return data_.flags_ & Flags::Used; }
-
-private:
-  StatData& data_;
-  StatDataAllocatorImpl<StatData>& alloc_;
-};
-
 TagProducerImpl::TagProducerImpl(const envoy::config::metrics::v2::StatsConfig& config) {
   // To check name conflict.
   reserveResources(config);
@@ -319,30 +366,6 @@ TagProducerImpl::addDefaultExtractors(const envoy::config::metrics::v2::StatsCon
   return names;
 }
 
-// TODO(jmarantz): move this below HeapStatDataAllocator::alloc.
-void HeapStatDataAllocator::free(HeapStatData& data) {
-  ASSERT(data.ref_count_ > 0);
-  if (--data.ref_count_ > 0) {
-    return;
-  }
-
-  {
-    Thread::LockGuard lock(mutex_);
-    size_t key_removed = stats_.erase(&data);
-    ASSERT(key_removed == 1);
-  }
-
-  delete &data;
-}
-
-void RawStatData::initialize(absl::string_view key, const StatsOptions& stats_options) {
-  ASSERT(!initialized());
-  ASSERT(key.size() <= stats_options.maxNameLength());
-  ref_count_ = 1;
-  memcpy(name_, key.data(), key.size());
-  name_[key.size()] = '\0';
-}
-
 HistogramStatisticsImpl::HistogramStatisticsImpl(const histogram_t* histogram_ptr)
     : computed_quantiles_(supportedQuantiles().size(), 0.0) {
   hist_approx_quantile(histogram_ptr, supportedQuantiles().data(), supportedQuantiles().size(),
@@ -401,30 +424,6 @@ void SourceImpl::clearCache() {
   histograms_.reset();
 }
 
-template <class StatData>
-CounterSharedPtr StatDataAllocatorImpl<StatData>::makeCounter(absl::string_view name,
-                                                              std::string&& tag_extracted_name,
-                                                              std::vector<Tag>&& tags) {
-  StatData* data = alloc(name);
-  if (data == nullptr) {
-    return nullptr;
-  }
-  return std::make_shared<CounterImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
-                                                 std::move(tags));
-}
-
-template <class StatData>
-GaugeSharedPtr StatDataAllocatorImpl<StatData>::makeGauge(absl::string_view name,
-                                                          std::string&& tag_extracted_name,
-                                                          std::vector<Tag>&& tags) {
-  StatData* data = alloc(name);
-  if (data == nullptr) {
-    return nullptr;
-  }
-  return std::make_shared<GaugeImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
-                                               std::move(tags));
-}
-
 template class StatDataAllocatorImpl<HeapStatData>;
 template class StatDataAllocatorImpl<RawStatData>;
 

From e50e73d7d7a16e06b43550fe5e5ca4afda06116a Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 31 Jul 2018 18:29:14 -0400
Subject: [PATCH 02/14] Revert "Re-order functions to group classes together."

This reverts commit 769be4ffaceaf003321f4f1af2ea8ebe987effbd.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 source/common/stats/stats_impl.cc | 277 +++++++++++++++---------------
 1 file changed, 139 insertions(+), 138 deletions(-)

diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc
index b4556aec0ca4..681bc77d640f 100644
--- a/source/common/stats/stats_impl.cc
+++ b/source/common/stats/stats_impl.cc
@@ -37,105 +37,6 @@ bool regexStartsWithDot(absl::string_view regex) {
 
 } // namespace
 
-std::string Utility::sanitizeStatsName(const std::string& name) {
-  std::string stats_name = name;
-  std::replace(stats_name.begin(), stats_name.end(), ':', '_');
-  std::replace(stats_name.begin(), stats_name.end(), '\0', '_');
-  return stats_name;
-}
-
-/**
- * Counter implementation that wraps a StatData. StatData must have data members:
- *    std::atomic<int64_t> value_;
- *    std::atomic<int64_t> pending_increment_;
- *    std::atomic<int16_t> flags_;
- *    std::atomic<int16_t> ref_count_;
- */
-template <class StatData> class CounterImpl : public Counter, public MetricImpl {
-public:
-  CounterImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
-              std::string&& tag_extracted_name, std::vector<Tag>&& tags)
-      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
-        alloc_(alloc) {}
-  ~CounterImpl() { alloc_.free(data_); }
-
-  // Stats::Counter
-  void add(uint64_t amount) override {
-    data_.value_ += amount;
-    data_.pending_increment_ += amount;
-    data_.flags_ |= Flags::Used;
-  }
-
-  void inc() override { add(1); }
-  uint64_t latch() override { return data_.pending_increment_.exchange(0); }
-  void reset() override { data_.value_ = 0; }
-  bool used() const override { return data_.flags_ & Flags::Used; }
-  uint64_t value() const override { return data_.value_; }
-
-private:
-  StatData& data_;
-  StatDataAllocatorImpl<StatData>& alloc_;
-};
-
-/**
- * Gauge implementation that wraps a StatData.
- */
-template <class StatData> class GaugeImpl : public Gauge, public MetricImpl {
-public:
-  GaugeImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
-            std::string&& tag_extracted_name, std::vector<Tag>&& tags)
-      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
-        alloc_(alloc) {}
-  ~GaugeImpl() { alloc_.free(data_); }
-
-  // Stats::Gauge
-  virtual void add(uint64_t amount) override {
-    data_.value_ += amount;
-    data_.flags_ |= Flags::Used;
-  }
-  virtual void dec() override { sub(1); }
-  virtual void inc() override { add(1); }
-  virtual void set(uint64_t value) override {
-    data_.value_ = value;
-    data_.flags_ |= Flags::Used;
-  }
-  virtual void sub(uint64_t amount) override {
-    ASSERT(data_.value_ >= amount);
-    ASSERT(used());
-    data_.value_ -= amount;
-  }
-  virtual uint64_t value() const override { return data_.value_; }
-  bool used() const override { return data_.flags_ & Flags::Used; }
-
-private:
-  StatData& data_;
-  StatDataAllocatorImpl<StatData>& alloc_;
-};
-
-template <class StatData>
-CounterSharedPtr StatDataAllocatorImpl<StatData>::makeCounter(absl::string_view name,
-                                                              std::string&& tag_extracted_name,
-                                                              std::vector<Tag>&& tags) {
-  StatData* data = alloc(name);
-  if (data == nullptr) {
-    return nullptr;
-  }
-  return std::make_shared<CounterImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
-                                                 std::move(tags));
-}
-
-template <class StatData>
-GaugeSharedPtr StatDataAllocatorImpl<StatData>::makeGauge(absl::string_view name,
-                                                          std::string&& tag_extracted_name,
-                                                          std::vector<Tag>&& tags) {
-  StatData* data = alloc(name);
-  if (data == nullptr) {
-    return nullptr;
-  }
-  return std::make_shared<GaugeImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
-                                               std::move(tags));
-}
-
 // Normally the compiler would do this, but because name_ is a flexible-array-length
 // element, the compiler can't. RawStatData is put into an array in HotRestartImpl, so
 // it's important that each element starts on the required alignment for the type.
@@ -147,45 +48,11 @@ uint64_t RawStatData::structSizeWithOptions(const StatsOptions& stats_options) {
   return structSize(stats_options.maxNameLength());
 }
 
-void RawStatData::initialize(absl::string_view key, const StatsOptions& stats_options) {
-  ASSERT(!initialized());
-  ASSERT(key.size() <= stats_options.maxNameLength());
-  ref_count_ = 1;
-  memcpy(name_, key.data(), key.size());
-  name_[key.size()] = '\0';
-}
-
-HeapStatData::HeapStatData(absl::string_view key) : name_(key.data(), key.size()) {}
-
-HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {
-  // Any expected truncation of name is done at the callsite. No truncation is
-  // required to use this allocator.
-  auto data = std::make_unique<HeapStatData>(name);
-  Thread::ReleasableLockGuard lock(mutex_);
-  auto ret = stats_.insert(data.get());
-  HeapStatData* existing_data = *ret.first;
-  lock.release();
-
-  if (ret.second) {
-    return data.release();
-  }
-  ++existing_data->ref_count_;
-  return existing_data;
-}
-
-void HeapStatDataAllocator::free(HeapStatData& data) {
-  ASSERT(data.ref_count_ > 0);
-  if (--data.ref_count_ > 0) {
-    return;
-  }
-
-  {
-    Thread::LockGuard lock(mutex_);
-    size_t key_removed = stats_.erase(&data);
-    ASSERT(key_removed == 1);
-  }
-
-  delete &data;
+std::string Utility::sanitizeStatsName(const std::string& name) {
+  std::string stats_name = name;
+  std::replace(stats_name.begin(), stats_name.end(), ':', '_');
+  std::replace(stats_name.begin(), stats_name.end(), '\0', '_');
+  return stats_name;
 }
 
 TagExtractorImpl::TagExtractorImpl(const std::string& name, const std::string& regex,
@@ -268,6 +135,92 @@ bool TagExtractorImpl::extractTag(const std::string& stat_name, std::vector<Tag>
   return false;
 }
 
+HeapStatData::HeapStatData(absl::string_view key) : name_(key.data(), key.size()) {}
+
+HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {
+  // Any expected truncation of name is done at the callsite. No truncation is
+  // required to use this allocator.
+  auto data = std::make_unique<HeapStatData>(name);
+  Thread::ReleasableLockGuard lock(mutex_);
+  auto ret = stats_.insert(data.get());
+  HeapStatData* existing_data = *ret.first;
+  lock.release();
+
+  if (ret.second) {
+    return data.release();
+  }
+  ++existing_data->ref_count_;
+  return existing_data;
+}
+
+/**
+ * Counter implementation that wraps a StatData. StatData must have data members:
+ *    std::atomic<int64_t> value_;
+ *    std::atomic<int64_t> pending_increment_;
+ *    std::atomic<int16_t> flags_;
+ *    std::atomic<int16_t> ref_count_;
+ */
+template <class StatData> class CounterImpl : public Counter, public MetricImpl {
+public:
+  CounterImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
+              std::string&& tag_extracted_name, std::vector<Tag>&& tags)
+      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
+        alloc_(alloc) {}
+  ~CounterImpl() { alloc_.free(data_); }
+
+  // Stats::Counter
+  void add(uint64_t amount) override {
+    data_.value_ += amount;
+    data_.pending_increment_ += amount;
+    data_.flags_ |= Flags::Used;
+  }
+
+  void inc() override { add(1); }
+  uint64_t latch() override { return data_.pending_increment_.exchange(0); }
+  void reset() override { data_.value_ = 0; }
+  bool used() const override { return data_.flags_ & Flags::Used; }
+  uint64_t value() const override { return data_.value_; }
+
+private:
+  StatData& data_;
+  StatDataAllocatorImpl<StatData>& alloc_;
+};
+
+/**
+ * Gauge implementation that wraps a StatData.
+ */
+template <class StatData> class GaugeImpl : public Gauge, public MetricImpl {
+public:
+  GaugeImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
+            std::string&& tag_extracted_name, std::vector<Tag>&& tags)
+      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
+        alloc_(alloc) {}
+  ~GaugeImpl() { alloc_.free(data_); }
+
+  // Stats::Gauge
+  virtual void add(uint64_t amount) override {
+    data_.value_ += amount;
+    data_.flags_ |= Flags::Used;
+  }
+  virtual void dec() override { sub(1); }
+  virtual void inc() override { add(1); }
+  virtual void set(uint64_t value) override {
+    data_.value_ = value;
+    data_.flags_ |= Flags::Used;
+  }
+  virtual void sub(uint64_t amount) override {
+    ASSERT(data_.value_ >= amount);
+    ASSERT(used());
+    data_.value_ -= amount;
+  }
+  virtual uint64_t value() const override { return data_.value_; }
+  bool used() const override { return data_.flags_ & Flags::Used; }
+
+private:
+  StatData& data_;
+  StatDataAllocatorImpl<StatData>& alloc_;
+};
+
 TagProducerImpl::TagProducerImpl(const envoy::config::metrics::v2::StatsConfig& config) {
   // To check name conflict.
   reserveResources(config);
@@ -366,6 +319,30 @@ TagProducerImpl::addDefaultExtractors(const envoy::config::metrics::v2::StatsCon
   return names;
 }
 
+// TODO(jmarantz): move this below HeapStatDataAllocator::alloc.
+void HeapStatDataAllocator::free(HeapStatData& data) {
+  ASSERT(data.ref_count_ > 0);
+  if (--data.ref_count_ > 0) {
+    return;
+  }
+
+  {
+    Thread::LockGuard lock(mutex_);
+    size_t key_removed = stats_.erase(&data);
+    ASSERT(key_removed == 1);
+  }
+
+  delete &data;
+}
+
+void RawStatData::initialize(absl::string_view key, const StatsOptions& stats_options) {
+  ASSERT(!initialized());
+  ASSERT(key.size() <= stats_options.maxNameLength());
+  ref_count_ = 1;
+  memcpy(name_, key.data(), key.size());
+  name_[key.size()] = '\0';
+}
+
 HistogramStatisticsImpl::HistogramStatisticsImpl(const histogram_t* histogram_ptr)
     : computed_quantiles_(supportedQuantiles().size(), 0.0) {
   hist_approx_quantile(histogram_ptr, supportedQuantiles().data(), supportedQuantiles().size(),
@@ -424,6 +401,30 @@ void SourceImpl::clearCache() {
   histograms_.reset();
 }
 
+template <class StatData>
+CounterSharedPtr StatDataAllocatorImpl<StatData>::makeCounter(absl::string_view name,
+                                                              std::string&& tag_extracted_name,
+                                                              std::vector<Tag>&& tags) {
+  StatData* data = alloc(name);
+  if (data == nullptr) {
+    return nullptr;
+  }
+  return std::make_shared<CounterImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
+                                                 std::move(tags));
+}
+
+template <class StatData>
+GaugeSharedPtr StatDataAllocatorImpl<StatData>::makeGauge(absl::string_view name,
+                                                          std::string&& tag_extracted_name,
+                                                          std::vector<Tag>&& tags) {
+  StatData* data = alloc(name);
+  if (data == nullptr) {
+    return nullptr;
+  }
+  return std::make_shared<GaugeImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
+                                               std::move(tags));
+}
+
 template class StatDataAllocatorImpl<HeapStatData>;
 template class StatDataAllocatorImpl<RawStatData>;
 

From 6f594b5965cd740b6166ace818030ea045a14a07 Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 31 Jul 2018 18:30:26 -0400
Subject: [PATCH 03/14] re-order functions to group implementations of
 different classes together.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 source/common/stats/stats_impl.cc | 277 +++++++++++++++---------------
 1 file changed, 138 insertions(+), 139 deletions(-)

diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc
index 681bc77d640f..b4556aec0ca4 100644
--- a/source/common/stats/stats_impl.cc
+++ b/source/common/stats/stats_impl.cc
@@ -37,6 +37,105 @@ bool regexStartsWithDot(absl::string_view regex) {
 
 } // namespace
 
+std::string Utility::sanitizeStatsName(const std::string& name) {
+  std::string stats_name = name;
+  std::replace(stats_name.begin(), stats_name.end(), ':', '_');
+  std::replace(stats_name.begin(), stats_name.end(), '\0', '_');
+  return stats_name;
+}
+
+/**
+ * Counter implementation that wraps a StatData. StatData must have data members:
+ *    std::atomic<int64_t> value_;
+ *    std::atomic<int64_t> pending_increment_;
+ *    std::atomic<int16_t> flags_;
+ *    std::atomic<int16_t> ref_count_;
+ */
+template <class StatData> class CounterImpl : public Counter, public MetricImpl {
+public:
+  CounterImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
+              std::string&& tag_extracted_name, std::vector<Tag>&& tags)
+      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
+        alloc_(alloc) {}
+  ~CounterImpl() { alloc_.free(data_); }
+
+  // Stats::Counter
+  void add(uint64_t amount) override {
+    data_.value_ += amount;
+    data_.pending_increment_ += amount;
+    data_.flags_ |= Flags::Used;
+  }
+
+  void inc() override { add(1); }
+  uint64_t latch() override { return data_.pending_increment_.exchange(0); }
+  void reset() override { data_.value_ = 0; }
+  bool used() const override { return data_.flags_ & Flags::Used; }
+  uint64_t value() const override { return data_.value_; }
+
+private:
+  StatData& data_;
+  StatDataAllocatorImpl<StatData>& alloc_;
+};
+
+/**
+ * Gauge implementation that wraps a StatData.
+ */
+template <class StatData> class GaugeImpl : public Gauge, public MetricImpl {
+public:
+  GaugeImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
+            std::string&& tag_extracted_name, std::vector<Tag>&& tags)
+      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
+        alloc_(alloc) {}
+  ~GaugeImpl() { alloc_.free(data_); }
+
+  // Stats::Gauge
+  virtual void add(uint64_t amount) override {
+    data_.value_ += amount;
+    data_.flags_ |= Flags::Used;
+  }
+  virtual void dec() override { sub(1); }
+  virtual void inc() override { add(1); }
+  virtual void set(uint64_t value) override {
+    data_.value_ = value;
+    data_.flags_ |= Flags::Used;
+  }
+  virtual void sub(uint64_t amount) override {
+    ASSERT(data_.value_ >= amount);
+    ASSERT(used());
+    data_.value_ -= amount;
+  }
+  virtual uint64_t value() const override { return data_.value_; }
+  bool used() const override { return data_.flags_ & Flags::Used; }
+
+private:
+  StatData& data_;
+  StatDataAllocatorImpl<StatData>& alloc_;
+};
+
+template <class StatData>
+CounterSharedPtr StatDataAllocatorImpl<StatData>::makeCounter(absl::string_view name,
+                                                              std::string&& tag_extracted_name,
+                                                              std::vector<Tag>&& tags) {
+  StatData* data = alloc(name);
+  if (data == nullptr) {
+    return nullptr;
+  }
+  return std::make_shared<CounterImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
+                                                 std::move(tags));
+}
+
+template <class StatData>
+GaugeSharedPtr StatDataAllocatorImpl<StatData>::makeGauge(absl::string_view name,
+                                                          std::string&& tag_extracted_name,
+                                                          std::vector<Tag>&& tags) {
+  StatData* data = alloc(name);
+  if (data == nullptr) {
+    return nullptr;
+  }
+  return std::make_shared<GaugeImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
+                                               std::move(tags));
+}
+
 // Normally the compiler would do this, but because name_ is a flexible-array-length
 // element, the compiler can't. RawStatData is put into an array in HotRestartImpl, so
 // it's important that each element starts on the required alignment for the type.
@@ -48,11 +147,45 @@ uint64_t RawStatData::structSizeWithOptions(const StatsOptions& stats_options) {
   return structSize(stats_options.maxNameLength());
 }
 
-std::string Utility::sanitizeStatsName(const std::string& name) {
-  std::string stats_name = name;
-  std::replace(stats_name.begin(), stats_name.end(), ':', '_');
-  std::replace(stats_name.begin(), stats_name.end(), '\0', '_');
-  return stats_name;
+void RawStatData::initialize(absl::string_view key, const StatsOptions& stats_options) {
+  ASSERT(!initialized());
+  ASSERT(key.size() <= stats_options.maxNameLength());
+  ref_count_ = 1;
+  memcpy(name_, key.data(), key.size());
+  name_[key.size()] = '\0';
+}
+
+HeapStatData::HeapStatData(absl::string_view key) : name_(key.data(), key.size()) {}
+
+HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {
+  // Any expected truncation of name is done at the callsite. No truncation is
+  // required to use this allocator.
+  auto data = std::make_unique<HeapStatData>(name);
+  Thread::ReleasableLockGuard lock(mutex_);
+  auto ret = stats_.insert(data.get());
+  HeapStatData* existing_data = *ret.first;
+  lock.release();
+
+  if (ret.second) {
+    return data.release();
+  }
+  ++existing_data->ref_count_;
+  return existing_data;
+}
+
+void HeapStatDataAllocator::free(HeapStatData& data) {
+  ASSERT(data.ref_count_ > 0);
+  if (--data.ref_count_ > 0) {
+    return;
+  }
+
+  {
+    Thread::LockGuard lock(mutex_);
+    size_t key_removed = stats_.erase(&data);
+    ASSERT(key_removed == 1);
+  }
+
+  delete &data;
 }
 
 TagExtractorImpl::TagExtractorImpl(const std::string& name, const std::string& regex,
@@ -135,92 +268,6 @@ bool TagExtractorImpl::extractTag(const std::string& stat_name, std::vector<Tag>
   return false;
 }
 
-HeapStatData::HeapStatData(absl::string_view key) : name_(key.data(), key.size()) {}
-
-HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {
-  // Any expected truncation of name is done at the callsite. No truncation is
-  // required to use this allocator.
-  auto data = std::make_unique<HeapStatData>(name);
-  Thread::ReleasableLockGuard lock(mutex_);
-  auto ret = stats_.insert(data.get());
-  HeapStatData* existing_data = *ret.first;
-  lock.release();
-
-  if (ret.second) {
-    return data.release();
-  }
-  ++existing_data->ref_count_;
-  return existing_data;
-}
-
-/**
- * Counter implementation that wraps a StatData. StatData must have data members:
- *    std::atomic<int64_t> value_;
- *    std::atomic<int64_t> pending_increment_;
- *    std::atomic<int16_t> flags_;
- *    std::atomic<int16_t> ref_count_;
- */
-template <class StatData> class CounterImpl : public Counter, public MetricImpl {
-public:
-  CounterImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
-              std::string&& tag_extracted_name, std::vector<Tag>&& tags)
-      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
-        alloc_(alloc) {}
-  ~CounterImpl() { alloc_.free(data_); }
-
-  // Stats::Counter
-  void add(uint64_t amount) override {
-    data_.value_ += amount;
-    data_.pending_increment_ += amount;
-    data_.flags_ |= Flags::Used;
-  }
-
-  void inc() override { add(1); }
-  uint64_t latch() override { return data_.pending_increment_.exchange(0); }
-  void reset() override { data_.value_ = 0; }
-  bool used() const override { return data_.flags_ & Flags::Used; }
-  uint64_t value() const override { return data_.value_; }
-
-private:
-  StatData& data_;
-  StatDataAllocatorImpl<StatData>& alloc_;
-};
-
-/**
- * Gauge implementation that wraps a StatData.
- */
-template <class StatData> class GaugeImpl : public Gauge, public MetricImpl {
-public:
-  GaugeImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
-            std::string&& tag_extracted_name, std::vector<Tag>&& tags)
-      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
-        alloc_(alloc) {}
-  ~GaugeImpl() { alloc_.free(data_); }
-
-  // Stats::Gauge
-  virtual void add(uint64_t amount) override {
-    data_.value_ += amount;
-    data_.flags_ |= Flags::Used;
-  }
-  virtual void dec() override { sub(1); }
-  virtual void inc() override { add(1); }
-  virtual void set(uint64_t value) override {
-    data_.value_ = value;
-    data_.flags_ |= Flags::Used;
-  }
-  virtual void sub(uint64_t amount) override {
-    ASSERT(data_.value_ >= amount);
-    ASSERT(used());
-    data_.value_ -= amount;
-  }
-  virtual uint64_t value() const override { return data_.value_; }
-  bool used() const override { return data_.flags_ & Flags::Used; }
-
-private:
-  StatData& data_;
-  StatDataAllocatorImpl<StatData>& alloc_;
-};
-
 TagProducerImpl::TagProducerImpl(const envoy::config::metrics::v2::StatsConfig& config) {
   // To check name conflict.
   reserveResources(config);
@@ -319,30 +366,6 @@ TagProducerImpl::addDefaultExtractors(const envoy::config::metrics::v2::StatsCon
   return names;
 }
 
-// TODO(jmarantz): move this below HeapStatDataAllocator::alloc.
-void HeapStatDataAllocator::free(HeapStatData& data) {
-  ASSERT(data.ref_count_ > 0);
-  if (--data.ref_count_ > 0) {
-    return;
-  }
-
-  {
-    Thread::LockGuard lock(mutex_);
-    size_t key_removed = stats_.erase(&data);
-    ASSERT(key_removed == 1);
-  }
-
-  delete &data;
-}
-
-void RawStatData::initialize(absl::string_view key, const StatsOptions& stats_options) {
-  ASSERT(!initialized());
-  ASSERT(key.size() <= stats_options.maxNameLength());
-  ref_count_ = 1;
-  memcpy(name_, key.data(), key.size());
-  name_[key.size()] = '\0';
-}
-
 HistogramStatisticsImpl::HistogramStatisticsImpl(const histogram_t* histogram_ptr)
     : computed_quantiles_(supportedQuantiles().size(), 0.0) {
   hist_approx_quantile(histogram_ptr, supportedQuantiles().data(), supportedQuantiles().size(),
@@ -401,30 +424,6 @@ void SourceImpl::clearCache() {
   histograms_.reset();
 }
 
-template <class StatData>
-CounterSharedPtr StatDataAllocatorImpl<StatData>::makeCounter(absl::string_view name,
-                                                              std::string&& tag_extracted_name,
-                                                              std::vector<Tag>&& tags) {
-  StatData* data = alloc(name);
-  if (data == nullptr) {
-    return nullptr;
-  }
-  return std::make_shared<CounterImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
-                                                 std::move(tags));
-}
-
-template <class StatData>
-GaugeSharedPtr StatDataAllocatorImpl<StatData>::makeGauge(absl::string_view name,
-                                                          std::string&& tag_extracted_name,
-                                                          std::vector<Tag>&& tags) {
-  StatData* data = alloc(name);
-  if (data == nullptr) {
-    return nullptr;
-  }
-  return std::make_shared<GaugeImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
-                                               std::move(tags));
-}
-
 template class StatDataAllocatorImpl<HeapStatData>;
 template class StatDataAllocatorImpl<RawStatData>;
 

From 109ab12d580d762af1ea0de45f82b0f51250ed36 Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 31 Jul 2018 18:55:47 -0400
Subject: [PATCH 04/14] A little more aggressively, make the class order match
 between .h an .cc.

And un-inline much of IsolatedStoreImpl, to avoid having to recompile
its large ctor with every other file in Envoy.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 source/common/stats/stats_impl.cc | 341 +++++++++++++++++-------------
 source/common/stats/stats_impl.h  | 136 +++++-------
 2 files changed, 238 insertions(+), 239 deletions(-)

diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc
index b4556aec0ca4..434eaf4dc5e8 100644
--- a/source/common/stats/stats_impl.cc
+++ b/source/common/stats/stats_impl.cc
@@ -37,157 +37,6 @@ bool regexStartsWithDot(absl::string_view regex) {
 
 } // namespace
 
-std::string Utility::sanitizeStatsName(const std::string& name) {
-  std::string stats_name = name;
-  std::replace(stats_name.begin(), stats_name.end(), ':', '_');
-  std::replace(stats_name.begin(), stats_name.end(), '\0', '_');
-  return stats_name;
-}
-
-/**
- * Counter implementation that wraps a StatData. StatData must have data members:
- *    std::atomic<int64_t> value_;
- *    std::atomic<int64_t> pending_increment_;
- *    std::atomic<int16_t> flags_;
- *    std::atomic<int16_t> ref_count_;
- */
-template <class StatData> class CounterImpl : public Counter, public MetricImpl {
-public:
-  CounterImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
-              std::string&& tag_extracted_name, std::vector<Tag>&& tags)
-      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
-        alloc_(alloc) {}
-  ~CounterImpl() { alloc_.free(data_); }
-
-  // Stats::Counter
-  void add(uint64_t amount) override {
-    data_.value_ += amount;
-    data_.pending_increment_ += amount;
-    data_.flags_ |= Flags::Used;
-  }
-
-  void inc() override { add(1); }
-  uint64_t latch() override { return data_.pending_increment_.exchange(0); }
-  void reset() override { data_.value_ = 0; }
-  bool used() const override { return data_.flags_ & Flags::Used; }
-  uint64_t value() const override { return data_.value_; }
-
-private:
-  StatData& data_;
-  StatDataAllocatorImpl<StatData>& alloc_;
-};
-
-/**
- * Gauge implementation that wraps a StatData.
- */
-template <class StatData> class GaugeImpl : public Gauge, public MetricImpl {
-public:
-  GaugeImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
-            std::string&& tag_extracted_name, std::vector<Tag>&& tags)
-      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
-        alloc_(alloc) {}
-  ~GaugeImpl() { alloc_.free(data_); }
-
-  // Stats::Gauge
-  virtual void add(uint64_t amount) override {
-    data_.value_ += amount;
-    data_.flags_ |= Flags::Used;
-  }
-  virtual void dec() override { sub(1); }
-  virtual void inc() override { add(1); }
-  virtual void set(uint64_t value) override {
-    data_.value_ = value;
-    data_.flags_ |= Flags::Used;
-  }
-  virtual void sub(uint64_t amount) override {
-    ASSERT(data_.value_ >= amount);
-    ASSERT(used());
-    data_.value_ -= amount;
-  }
-  virtual uint64_t value() const override { return data_.value_; }
-  bool used() const override { return data_.flags_ & Flags::Used; }
-
-private:
-  StatData& data_;
-  StatDataAllocatorImpl<StatData>& alloc_;
-};
-
-template <class StatData>
-CounterSharedPtr StatDataAllocatorImpl<StatData>::makeCounter(absl::string_view name,
-                                                              std::string&& tag_extracted_name,
-                                                              std::vector<Tag>&& tags) {
-  StatData* data = alloc(name);
-  if (data == nullptr) {
-    return nullptr;
-  }
-  return std::make_shared<CounterImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
-                                                 std::move(tags));
-}
-
-template <class StatData>
-GaugeSharedPtr StatDataAllocatorImpl<StatData>::makeGauge(absl::string_view name,
-                                                          std::string&& tag_extracted_name,
-                                                          std::vector<Tag>&& tags) {
-  StatData* data = alloc(name);
-  if (data == nullptr) {
-    return nullptr;
-  }
-  return std::make_shared<GaugeImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
-                                               std::move(tags));
-}
-
-// Normally the compiler would do this, but because name_ is a flexible-array-length
-// element, the compiler can't. RawStatData is put into an array in HotRestartImpl, so
-// it's important that each element starts on the required alignment for the type.
-uint64_t RawStatData::structSize(uint64_t name_size) {
-  return roundUpMultipleNaturalAlignment(sizeof(RawStatData) + name_size + 1);
-}
-
-uint64_t RawStatData::structSizeWithOptions(const StatsOptions& stats_options) {
-  return structSize(stats_options.maxNameLength());
-}
-
-void RawStatData::initialize(absl::string_view key, const StatsOptions& stats_options) {
-  ASSERT(!initialized());
-  ASSERT(key.size() <= stats_options.maxNameLength());
-  ref_count_ = 1;
-  memcpy(name_, key.data(), key.size());
-  name_[key.size()] = '\0';
-}
-
-HeapStatData::HeapStatData(absl::string_view key) : name_(key.data(), key.size()) {}
-
-HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {
-  // Any expected truncation of name is done at the callsite. No truncation is
-  // required to use this allocator.
-  auto data = std::make_unique<HeapStatData>(name);
-  Thread::ReleasableLockGuard lock(mutex_);
-  auto ret = stats_.insert(data.get());
-  HeapStatData* existing_data = *ret.first;
-  lock.release();
-
-  if (ret.second) {
-    return data.release();
-  }
-  ++existing_data->ref_count_;
-  return existing_data;
-}
-
-void HeapStatDataAllocator::free(HeapStatData& data) {
-  ASSERT(data.ref_count_ > 0);
-  if (--data.ref_count_ > 0) {
-    return;
-  }
-
-  {
-    Thread::LockGuard lock(mutex_);
-    size_t key_removed = stats_.erase(&data);
-    ASSERT(key_removed == 1);
-  }
-
-  delete &data;
-}
-
 TagExtractorImpl::TagExtractorImpl(const std::string& name, const std::string& regex,
                                    const std::string& substr)
     : name_(name), prefix_(std::string(extractRegexPrefix(regex))), substr_(substr),
@@ -366,6 +215,157 @@ TagProducerImpl::addDefaultExtractors(const envoy::config::metrics::v2::StatsCon
   return names;
 }
 
+std::string Utility::sanitizeStatsName(const std::string& name) {
+  std::string stats_name = name;
+  std::replace(stats_name.begin(), stats_name.end(), ':', '_');
+  std::replace(stats_name.begin(), stats_name.end(), '\0', '_');
+  return stats_name;
+}
+
+/**
+ * Counter implementation that wraps a StatData. StatData must have data members:
+ *    std::atomic<int64_t> value_;
+ *    std::atomic<int64_t> pending_increment_;
+ *    std::atomic<int16_t> flags_;
+ *    std::atomic<int16_t> ref_count_;
+ */
+template <class StatData> class CounterImpl : public Counter, public MetricImpl {
+public:
+  CounterImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
+              std::string&& tag_extracted_name, std::vector<Tag>&& tags)
+      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
+        alloc_(alloc) {}
+  ~CounterImpl() { alloc_.free(data_); }
+
+  // Stats::Counter
+  void add(uint64_t amount) override {
+    data_.value_ += amount;
+    data_.pending_increment_ += amount;
+    data_.flags_ |= Flags::Used;
+  }
+
+  void inc() override { add(1); }
+  uint64_t latch() override { return data_.pending_increment_.exchange(0); }
+  void reset() override { data_.value_ = 0; }
+  bool used() const override { return data_.flags_ & Flags::Used; }
+  uint64_t value() const override { return data_.value_; }
+
+private:
+  StatData& data_;
+  StatDataAllocatorImpl<StatData>& alloc_;
+};
+
+/**
+ * Gauge implementation that wraps a StatData.
+ */
+template <class StatData> class GaugeImpl : public Gauge, public MetricImpl {
+public:
+  GaugeImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
+            std::string&& tag_extracted_name, std::vector<Tag>&& tags)
+      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
+        alloc_(alloc) {}
+  ~GaugeImpl() { alloc_.free(data_); }
+
+  // Stats::Gauge
+  virtual void add(uint64_t amount) override {
+    data_.value_ += amount;
+    data_.flags_ |= Flags::Used;
+  }
+  virtual void dec() override { sub(1); }
+  virtual void inc() override { add(1); }
+  virtual void set(uint64_t value) override {
+    data_.value_ = value;
+    data_.flags_ |= Flags::Used;
+  }
+  virtual void sub(uint64_t amount) override {
+    ASSERT(data_.value_ >= amount);
+    ASSERT(used());
+    data_.value_ -= amount;
+  }
+  virtual uint64_t value() const override { return data_.value_; }
+  bool used() const override { return data_.flags_ & Flags::Used; }
+
+private:
+  StatData& data_;
+  StatDataAllocatorImpl<StatData>& alloc_;
+};
+
+// Normally the compiler would do this, but because name_ is a flexible-array-length
+// element, the compiler can't. RawStatData is put into an array in HotRestartImpl, so
+// it's important that each element starts on the required alignment for the type.
+uint64_t RawStatData::structSize(uint64_t name_size) {
+  return roundUpMultipleNaturalAlignment(sizeof(RawStatData) + name_size + 1);
+}
+
+uint64_t RawStatData::structSizeWithOptions(const StatsOptions& stats_options) {
+  return structSize(stats_options.maxNameLength());
+}
+
+void RawStatData::initialize(absl::string_view key, const StatsOptions& stats_options) {
+  ASSERT(!initialized());
+  ASSERT(key.size() <= stats_options.maxNameLength());
+  ref_count_ = 1;
+  memcpy(name_, key.data(), key.size());
+  name_[key.size()] = '\0';
+}
+
+HeapStatData::HeapStatData(absl::string_view key) : name_(key.data(), key.size()) {}
+
+template <class StatData>
+CounterSharedPtr StatDataAllocatorImpl<StatData>::makeCounter(absl::string_view name,
+                                                              std::string&& tag_extracted_name,
+                                                              std::vector<Tag>&& tags) {
+  StatData* data = alloc(name);
+  if (data == nullptr) {
+    return nullptr;
+  }
+  return std::make_shared<CounterImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
+                                                 std::move(tags));
+}
+
+template <class StatData>
+GaugeSharedPtr StatDataAllocatorImpl<StatData>::makeGauge(absl::string_view name,
+                                                          std::string&& tag_extracted_name,
+                                                          std::vector<Tag>&& tags) {
+  StatData* data = alloc(name);
+  if (data == nullptr) {
+    return nullptr;
+  }
+  return std::make_shared<GaugeImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
+                                               std::move(tags));
+}
+
+HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {
+  // Any expected truncation of name is done at the callsite. No truncation is
+  // required to use this allocator.
+  auto data = std::make_unique<HeapStatData>(name);
+  Thread::ReleasableLockGuard lock(mutex_);
+  auto ret = stats_.insert(data.get());
+  HeapStatData* existing_data = *ret.first;
+  lock.release();
+
+  if (ret.second) {
+    return data.release();
+  }
+  ++existing_data->ref_count_;
+  return existing_data;
+}
+
+void HeapStatDataAllocator::free(HeapStatData& data) {
+  ASSERT(data.ref_count_ > 0);
+  if (--data.ref_count_ > 0) {
+    return;
+  }
+
+  {
+    Thread::LockGuard lock(mutex_);
+    size_t key_removed = stats_.erase(&data);
+    ASSERT(key_removed == 1);
+  }
+
+  delete &data;
+}
+
 HistogramStatisticsImpl::HistogramStatisticsImpl(const histogram_t* histogram_ptr)
     : computed_quantiles_(supportedQuantiles().size(), 0.0) {
   hist_approx_quantile(histogram_ptr, supportedQuantiles().data(), supportedQuantiles().size(),
@@ -424,6 +424,45 @@ void SourceImpl::clearCache() {
   histograms_.reset();
 }
 
+IsolatedStoreImpl::IsolatedStoreImpl()
+    : counters_([this](const std::string& name) -> CounterSharedPtr {
+        std::string tag_extracted_name = name;
+        std::vector<Tag> tags;
+        return alloc_.makeCounter(name, std::move(tag_extracted_name), std::move(tags));
+      }),
+      gauges_([this](const std::string& name) -> GaugeSharedPtr {
+        std::string tag_extracted_name = name;
+        std::vector<Tag> tags;
+        return alloc_.makeGauge(name, std::move(tag_extracted_name), std::move(tags));
+      }),
+      histograms_([this](const std::string& name) -> HistogramSharedPtr {
+        return std::make_shared<HistogramImpl>(name, *this, std::string(name), std::vector<Tag>());
+      }) {}
+
+struct IsolatedScopeImpl : public Scope {
+  IsolatedScopeImpl(IsolatedStoreImpl& parent, const std::string& prefix)
+      : parent_(parent), prefix_(Utility::sanitizeStatsName(prefix)) {}
+
+  // Stats::Scope
+  ScopePtr createScope(const std::string& name) override {
+    return ScopePtr{new IsolatedScopeImpl(parent_, prefix_ + name)};
+  }
+  void deliverHistogramToSinks(const Histogram&, uint64_t) override {}
+  Counter& counter(const std::string& name) override { return parent_.counter(prefix_ + name); }
+  Gauge& gauge(const std::string& name) override { return parent_.gauge(prefix_ + name); }
+  Histogram& histogram(const std::string& name) override {
+    return parent_.histogram(prefix_ + name);
+  }
+  const Stats::StatsOptions& statsOptions() const override { return parent_.statsOptions(); }
+
+  IsolatedStoreImpl& parent_;
+  const std::string prefix_;
+};
+
+ScopePtr IsolatedStoreImpl::createScope(const std::string& name) {
+  return ScopePtr{new IsolatedScopeImpl(*this, name)};
+}
+
 template class StatDataAllocatorImpl<HeapStatData>;
 template class StatDataAllocatorImpl<RawStatData>;
 
diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h
index a007d4a2a21d..4fe9c0e1c4d7 100644
--- a/source/common/stats/stats_impl.h
+++ b/source/common/stats/stats_impl.h
@@ -89,7 +89,6 @@ class TagExtractorImpl : public TagExtractor {
    * @return std::string the prefix, or "" if no prefix found.
    */
   static std::string extractRegexPrefix(absl::string_view regex);
-
   const std::string name_;
   const std::string prefix_;
   const std::string substr_;
@@ -245,33 +244,6 @@ struct RawStatData {
   char name_[];
 };
 
-/**
- * Implementation of the Metric interface. Virtual inheritance is used because the interfaces that
- * will inherit from Metric will have other base classes that will also inherit from Metric.
- */
-class MetricImpl : public virtual Metric {
-public:
-  MetricImpl(const std::string& name, std::string&& tag_extracted_name, std::vector<Tag>&& tags)
-      : name_(name), tag_extracted_name_(std::move(tag_extracted_name)), tags_(std::move(tags)) {}
-
-  const std::string& name() const override { return name_; }
-  const std::string& tagExtractedName() const override { return tag_extracted_name_; }
-  const std::vector<Tag>& tags() const override { return tags_; }
-
-protected:
-  /**
-   * Flags used by all stats types to figure out whether they have been used.
-   */
-  struct Flags {
-    static const uint8_t Used = 0x1;
-  };
-
-private:
-  const std::string name_;
-  const std::string tag_extracted_name_;
-  const std::vector<Tag> tags_;
-};
-
 // Partially implements a StatDataAllocator, leaving alloc & free for subclasses.
 // We templatize on StatData rather than defining a virtual base StatData class
 // for performance reasons; stat increment is on the hot path.
@@ -316,6 +288,52 @@ class RawStatDataAllocator : public StatDataAllocatorImpl<RawStatData> {
   bool requiresBoundedStatNameSize() const override { return true; }
 };
 
+/**
+ * This structure is an alternate backing store for both CounterImpl and GaugeImpl. It is designed
+ * so that it can be allocated efficiently from the heap on demand.
+ */
+struct HeapStatData {
+  explicit HeapStatData(absl::string_view key);
+
+  /**
+   * @returns absl::string_view the name as a string_view.
+   */
+  absl::string_view key() const { return name_; }
+
+  std::atomic<uint64_t> value_{0};
+  std::atomic<uint64_t> pending_increment_{0};
+  std::atomic<uint16_t> flags_{0};
+  std::atomic<uint16_t> ref_count_{1};
+  std::string name_;
+};
+
+/**
+ * Implementation of the Metric interface. Virtual inheritance is used because the interfaces that
+ * will inherit from Metric will have other base classes that will also inherit from Metric.
+ */
+class MetricImpl : public virtual Metric {
+public:
+  MetricImpl(const std::string& name, std::string&& tag_extracted_name, std::vector<Tag>&& tags)
+      : name_(name), tag_extracted_name_(std::move(tag_extracted_name)), tags_(std::move(tags)) {}
+
+  const std::string& name() const override { return name_; }
+  const std::string& tagExtractedName() const override { return tag_extracted_name_; }
+  const std::vector<Tag>& tags() const override { return tags_; }
+
+protected:
+  /**
+   * Flags used by all stats types to figure out whether they have been used.
+   */
+  struct Flags {
+    static const uint8_t Used = 0x1;
+  };
+
+private:
+  const std::string name_;
+  const std::string tag_extracted_name_;
+  const std::vector<Tag> tags_;
+};
+
 /**
  * Implementation of HistogramStatistics for circllhist.
  */
@@ -376,25 +394,6 @@ class SourceImpl : public Source {
   absl::optional<std::vector<ParentHistogramSharedPtr>> histograms_;
 };
 
-/**
- * This structure is an alternate backing store for both CounterImpl and GaugeImpl. It is designed
- * so that it can be allocated efficiently from the heap on demand.
- */
-struct HeapStatData {
-  explicit HeapStatData(absl::string_view key);
-
-  /**
-   * @returns absl::string_view the name as a string_view.
-   */
-  absl::string_view key() const { return name_; }
-
-  std::atomic<uint64_t> value_{0};
-  std::atomic<uint64_t> pending_increment_{0};
-  std::atomic<uint16_t> flags_{0};
-  std::atomic<uint16_t> ref_count_{1};
-  std::string name_;
-};
-
 /**
  * Implementation of StatDataAllocator using a pure heap-based strategy, so that
  * Envoy implementations that do not require hot-restart can use less memory.
@@ -470,32 +469,13 @@ template <class Base> class IsolatedStatsCache {
   Allocator alloc_;
 };
 
-/**
- * Store implementation that is isolated from other stores.
- */
 class IsolatedStoreImpl : public Store {
 public:
-  IsolatedStoreImpl()
-      : counters_([this](const std::string& name) -> CounterSharedPtr {
-          std::string tag_extracted_name = name;
-          std::vector<Tag> tags;
-          return alloc_.makeCounter(name, std::move(tag_extracted_name), std::move(tags));
-        }),
-        gauges_([this](const std::string& name) -> GaugeSharedPtr {
-          std::string tag_extracted_name = name;
-          std::vector<Tag> tags;
-          return alloc_.makeGauge(name, std::move(tag_extracted_name), std::move(tags));
-        }),
-        histograms_([this](const std::string& name) -> HistogramSharedPtr {
-          return std::make_shared<HistogramImpl>(name, *this, std::string(name),
-                                                 std::vector<Tag>());
-        }) {}
+  IsolatedStoreImpl();
 
   // Stats::Scope
   Counter& counter(const std::string& name) override { return counters_.get(name); }
-  ScopePtr createScope(const std::string& name) override {
-    return ScopePtr{new ScopeImpl(*this, name)};
-  }
+  ScopePtr createScope(const std::string& name) override;
   void deliverHistogramToSinks(const Histogram&, uint64_t) override {}
   Gauge& gauge(const std::string& name) override { return gauges_.get(name); }
   Histogram& histogram(const std::string& name) override {
@@ -512,26 +492,6 @@ class IsolatedStoreImpl : public Store {
   }
 
 private:
-  struct ScopeImpl : public Scope {
-    ScopeImpl(IsolatedStoreImpl& parent, const std::string& prefix)
-        : parent_(parent), prefix_(Utility::sanitizeStatsName(prefix)) {}
-
-    // Stats::Scope
-    ScopePtr createScope(const std::string& name) override {
-      return ScopePtr{new ScopeImpl(parent_, prefix_ + name)};
-    }
-    void deliverHistogramToSinks(const Histogram&, uint64_t) override {}
-    Counter& counter(const std::string& name) override { return parent_.counter(prefix_ + name); }
-    Gauge& gauge(const std::string& name) override { return parent_.gauge(prefix_ + name); }
-    Histogram& histogram(const std::string& name) override {
-      return parent_.histogram(prefix_ + name);
-    }
-    const Stats::StatsOptions& statsOptions() const override { return parent_.statsOptions(); }
-
-    IsolatedStoreImpl& parent_;
-    const std::string prefix_;
-  };
-
   HeapStatDataAllocator alloc_;
   IsolatedStatsCache<Counter> counters_;
   IsolatedStatsCache<Gauge> gauges_;

From 5a0a7bb6902b6536cc423e05be0f9e7b5d229f42 Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 31 Jul 2018 19:53:38 -0400
Subject: [PATCH 05/14] Split out Stats::Utility into its own files.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 source/common/stats/BUILD                |  7 +++++++
 source/common/stats/stats_impl.cc        |  8 +-------
 source/common/stats/stats_impl.h         | 10 ----------
 source/common/stats/thread_local_store.h |  1 +
 4 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD
index 4c9997138bd1..dfc5e833aad7 100644
--- a/source/common/stats/BUILD
+++ b/source/common/stats/BUILD
@@ -17,6 +17,7 @@ envoy_cc_library(
         "libcircllhist",
     ],
     deps = [
+        ":utility_lib",
         "//include/envoy/common:time_interface",
         "//include/envoy/server:options_interface",
         "//include/envoy/stats:stats_interface",
@@ -42,3 +43,9 @@ envoy_cc_library(
         "//include/envoy/thread_local:thread_local_interface",
     ],
 )
+
+envoy_cc_library(
+    name = "utility_lib",
+    srcs = ["utility.cc"],
+    hdrs = ["utility.h"],
+)
diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc
index 434eaf4dc5e8..4512311e4dd0 100644
--- a/source/common/stats/stats_impl.cc
+++ b/source/common/stats/stats_impl.cc
@@ -13,6 +13,7 @@
 #include "common/common/thread.h"
 #include "common/common/utility.h"
 #include "common/config/well_known_names.h"
+#include "common/stats/utility.h"
 
 #include "absl/strings/ascii.h"
 #include "absl/strings/match.h"
@@ -215,13 +216,6 @@ TagProducerImpl::addDefaultExtractors(const envoy::config::metrics::v2::StatsCon
   return names;
 }
 
-std::string Utility::sanitizeStatsName(const std::string& name) {
-  std::string stats_name = name;
-  std::replace(stats_name.begin(), stats_name.end(), ':', '_');
-  std::replace(stats_name.begin(), stats_name.end(), '\0', '_');
-  return stats_name;
-}
-
 /**
  * Counter implementation that wraps a StatData. StatData must have data members:
  *    std::atomic<int64_t> value_;
diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h
index 4fe9c0e1c4d7..e1b9a0a386ae 100644
--- a/source/common/stats/stats_impl.h
+++ b/source/common/stats/stats_impl.h
@@ -175,16 +175,6 @@ class TagProducerImpl : public TagProducer {
   std::vector<Tag> default_tags_;
 };
 
-/**
- * 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.
diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h
index cc280613ce66..2b3a843282c0 100644
--- a/source/common/stats/thread_local_store.h
+++ b/source/common/stats/thread_local_store.h
@@ -11,6 +11,7 @@
 #include "envoy/thread_local/thread_local.h"
 
 #include "common/stats/stats_impl.h"
+#include "common/stats/utility.h"
 
 namespace Envoy {
 namespace Stats {

From 77375e7237ad68a60cc6321f328d1878b3031dcb Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 31 Jul 2018 19:59:18 -0400
Subject: [PATCH 06/14] Split out StatsOptions into separate files.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 source/common/stats/BUILD        |  9 +++++++++
 source/common/stats/stats_impl.h | 22 +---------------------
 2 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD
index dfc5e833aad7..68e197f7a88c 100644
--- a/source/common/stats/BUILD
+++ b/source/common/stats/BUILD
@@ -17,6 +17,7 @@ envoy_cc_library(
         "libcircllhist",
     ],
     deps = [
+        ":stats_options_lib",
         ":utility_lib",
         "//include/envoy/common:time_interface",
         "//include/envoy/server:options_interface",
@@ -34,6 +35,14 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "stats_options_lib",
+    hdrs = ["stats_options_impl.h"],
+    deps = [
+        "//include/envoy/stats:stats_interface",
+    ],
+)
+
 envoy_cc_library(
     name = "thread_local_store_lib",
     srcs = ["thread_local_store.cc"],
diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h
index e1b9a0a386ae..9d8a683dd582 100644
--- a/source/common/stats/stats_impl.h
+++ b/source/common/stats/stats_impl.h
@@ -23,6 +23,7 @@
 #include "common/common/thread_annotations.h"
 #include "common/common/utility.h"
 #include "common/protobuf/protobuf.h"
+#include "common/stats/stats_options_impl.h"
 
 #include "absl/strings/str_join.h"
 #include "absl/strings/string_view.h"
@@ -32,27 +33,6 @@
 namespace Envoy {
 namespace Stats {
 
-// The max name length is based on current set of stats.
-// As of now, the longest stat is
-// cluster.<cluster_name>.outlier_detection.ejections_consecutive_5xx
-// which is 52 characters long without the cluster name.
-// The max stat name length is 127 (default). So, in order to give room
-// for growth to both the envoy generated stat characters
-// (e.g., outlier_detection...) and user supplied names (e.g., cluster name),
-// we set the max user supplied name length to 60, and the max internally
-// generated stat suffixes to 67 (15 more characters to grow).
-// If you want to increase the max user supplied name length, use the compiler
-// option ENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH or the CLI option
-// max-obj-name-len
-struct StatsOptionsImpl : public StatsOptions {
-  size_t maxNameLength() const override { return max_obj_name_length_ + max_stat_suffix_length_; }
-  size_t maxObjNameLength() const override { return max_obj_name_length_; }
-  size_t maxStatSuffixLength() const override { return max_stat_suffix_length_; }
-
-  size_t max_obj_name_length_ = 60;
-  size_t max_stat_suffix_length_ = 67;
-};
-
 class TagExtractorImpl : public TagExtractor {
 public:
   /**

From 69da286e7be1d8059967ef2d1b360603656106f9 Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 31 Jul 2018 20:08:04 -0400
Subject: [PATCH 07/14] Split out tag-extractor to its own file.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 source/common/stats/BUILD                 |  11 +++
 source/common/stats/stats_impl.cc         |  84 -----------------
 source/common/stats/stats_options_impl.h  |  32 +++++++
 source/common/stats/tag_extractor_impl.cc | 108 ++++++++++++++++++++++
 source/common/stats/tag_extractor_impl.h  |  59 ++++++++++++
 source/common/stats/utility.cc            |  17 ++++
 source/common/stats/utility.h             |  19 ++++
 7 files changed, 246 insertions(+), 84 deletions(-)
 create mode 100644 source/common/stats/stats_options_impl.h
 create mode 100644 source/common/stats/tag_extractor_impl.cc
 create mode 100644 source/common/stats/tag_extractor_impl.h
 create mode 100644 source/common/stats/utility.cc
 create mode 100644 source/common/stats/utility.h

diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD
index 68e197f7a88c..b6f4a7351ea4 100644
--- a/source/common/stats/BUILD
+++ b/source/common/stats/BUILD
@@ -18,6 +18,7 @@ envoy_cc_library(
     ],
     deps = [
         ":stats_options_lib",
+        ":tag_extractor_lib",
         ":utility_lib",
         "//include/envoy/common:time_interface",
         "//include/envoy/server:options_interface",
@@ -43,6 +44,16 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "tag_extractor_lib",
+    srcs = ["tag_extractor_impl.cc"],
+    hdrs = ["tag_extractor_impl.h"],
+    deps = [
+        "//include/envoy/stats:stats_interface",
+        "//source/common/common:perf_annotation_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "thread_local_store_lib",
     srcs = ["thread_local_store.cc"],
diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc
index 4512311e4dd0..ee895ddfc9b1 100644
--- a/source/common/stats/stats_impl.cc
+++ b/source/common/stats/stats_impl.cc
@@ -32,92 +32,8 @@ uint64_t roundUpMultipleNaturalAlignment(uint64_t val) {
   return (val + multiple - 1) & ~(multiple - 1);
 }
 
-bool regexStartsWithDot(absl::string_view regex) {
-  return absl::StartsWith(regex, "\\.") || absl::StartsWith(regex, "(?=\\.)");
-}
-
 } // namespace
 
-TagExtractorImpl::TagExtractorImpl(const std::string& name, const std::string& regex,
-                                   const std::string& substr)
-    : name_(name), prefix_(std::string(extractRegexPrefix(regex))), substr_(substr),
-      regex_(RegexUtil::parseRegex(regex)) {}
-
-std::string TagExtractorImpl::extractRegexPrefix(absl::string_view regex) {
-  std::string prefix;
-  if (absl::StartsWith(regex, "^")) {
-    for (absl::string_view::size_type i = 1; i < regex.size(); ++i) {
-      if (!absl::ascii_isalnum(regex[i]) && (regex[i] != '_')) {
-        if (i > 1) {
-          const bool last_char = i == regex.size() - 1;
-          if ((!last_char && regexStartsWithDot(regex.substr(i))) ||
-              (last_char && (regex[i] == '$'))) {
-            prefix.append(regex.data() + 1, i - 1);
-          }
-        }
-        break;
-      }
-    }
-  }
-  return prefix;
-}
-
-TagExtractorPtr TagExtractorImpl::createTagExtractor(const std::string& name,
-                                                     const std::string& regex,
-                                                     const std::string& substr) {
-
-  if (name.empty()) {
-    throw EnvoyException("tag_name cannot be empty");
-  }
-
-  if (regex.empty()) {
-    throw EnvoyException(fmt::format(
-        "No regex specified for tag specifier and no default regex for name: '{}'", name));
-  }
-  return TagExtractorPtr{new TagExtractorImpl(name, regex, substr)};
-}
-
-bool TagExtractorImpl::substrMismatch(const std::string& stat_name) const {
-  return !substr_.empty() && stat_name.find(substr_) == std::string::npos;
-}
-
-bool TagExtractorImpl::extractTag(const std::string& stat_name, std::vector<Tag>& tags,
-                                  IntervalSet<size_t>& remove_characters) const {
-  PERF_OPERATION(perf);
-
-  if (substrMismatch(stat_name)) {
-    PERF_RECORD(perf, "re-skip-substr", name_);
-    return false;
-  }
-
-  std::smatch match;
-  // The regex must match and contain one or more subexpressions (all after the first are ignored).
-  if (std::regex_search(stat_name, match, regex_) && match.size() > 1) {
-    // remove_subexpr is the first submatch. It represents the portion of the string to be removed.
-    const auto& remove_subexpr = match[1];
-
-    // value_subexpr is the optional second submatch. It is usually inside the first submatch
-    // (remove_subexpr) to allow the expression to strip off extra characters that should be removed
-    // from the string but also not necessary in the tag value ("." for example). If there is no
-    // second submatch, then the value_subexpr is the same as the remove_subexpr.
-    const auto& value_subexpr = match.size() > 2 ? match[2] : remove_subexpr;
-
-    tags.emplace_back();
-    Tag& tag = tags.back();
-    tag.name_ = name_;
-    tag.value_ = value_subexpr.str();
-
-    // Determines which characters to remove from stat_name to elide remove_subexpr.
-    std::string::size_type start = remove_subexpr.first - stat_name.begin();
-    std::string::size_type end = remove_subexpr.second - stat_name.begin();
-    remove_characters.insert(start, end);
-    PERF_RECORD(perf, "re-match", name_);
-    return true;
-  }
-  PERF_RECORD(perf, "re-miss", name_);
-  return false;
-}
-
 TagProducerImpl::TagProducerImpl(const envoy::config::metrics::v2::StatsConfig& config) {
   // To check name conflict.
   reserveResources(config);
diff --git a/source/common/stats/stats_options_impl.h b/source/common/stats/stats_options_impl.h
new file mode 100644
index 000000000000..8f688c6f319b
--- /dev/null
+++ b/source/common/stats/stats_options_impl.h
@@ -0,0 +1,32 @@
+#pragma once
+
+#include <cstdint>
+
+#include "envoy/stats/stats.h"
+
+namespace Envoy {
+namespace Stats {
+
+// The max name length is based on current set of stats.
+// As of now, the longest stat is
+// cluster.<cluster_name>.outlier_detection.ejections_consecutive_5xx
+// which is 52 characters long without the cluster name.
+// The max stat name length is 127 (default). So, in order to give room
+// for growth to both the envoy generated stat characters
+// (e.g., outlier_detection...) and user supplied names (e.g., cluster name),
+// we set the max user supplied name length to 60, and the max internally
+// generated stat suffixes to 67 (15 more characters to grow).
+// If you want to increase the max user supplied name length, use the compiler
+// option ENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH or the CLI option
+// max-obj-name-len
+struct StatsOptionsImpl : public StatsOptions {
+  size_t maxNameLength() const override { return max_obj_name_length_ + max_stat_suffix_length_; }
+  size_t maxObjNameLength() const override { return max_obj_name_length_; }
+  size_t maxStatSuffixLength() const override { return max_stat_suffix_length_; }
+
+  size_t max_obj_name_length_ = 60;
+  size_t max_stat_suffix_length_ = 67;
+};
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/tag_extractor_impl.cc b/source/common/stats/tag_extractor_impl.cc
new file mode 100644
index 000000000000..ce4e3482267d
--- /dev/null
+++ b/source/common/stats/tag_extractor_impl.cc
@@ -0,0 +1,108 @@
+#include "common/stats/tag_extractor_impl.h"
+
+#include <string.h>
+
+#include <string>
+
+#include "envoy/common/exception.h"
+
+#include "common/common/perf_annotation.h"
+#include "common/common/utility.h"
+//#include "common/stats/utility.h"
+
+#include "absl/strings/ascii.h"
+#include "absl/strings/match.h"
+
+namespace Envoy {
+namespace Stats {
+
+namespace {
+
+bool regexStartsWithDot(absl::string_view regex) {
+  return absl::StartsWith(regex, "\\.") || absl::StartsWith(regex, "(?=\\.)");
+}
+
+} // namespace
+
+TagExtractorImpl::TagExtractorImpl(const std::string& name, const std::string& regex,
+                                   const std::string& substr)
+    : name_(name), prefix_(std::string(extractRegexPrefix(regex))), substr_(substr),
+      regex_(RegexUtil::parseRegex(regex)) {}
+
+std::string TagExtractorImpl::extractRegexPrefix(absl::string_view regex) {
+  std::string prefix;
+  if (absl::StartsWith(regex, "^")) {
+    for (absl::string_view::size_type i = 1; i < regex.size(); ++i) {
+      if (!absl::ascii_isalnum(regex[i]) && (regex[i] != '_')) {
+        if (i > 1) {
+          const bool last_char = i == regex.size() - 1;
+          if ((!last_char && regexStartsWithDot(regex.substr(i))) ||
+              (last_char && (regex[i] == '$'))) {
+            prefix.append(regex.data() + 1, i - 1);
+          }
+        }
+        break;
+      }
+    }
+  }
+  return prefix;
+}
+
+TagExtractorPtr TagExtractorImpl::createTagExtractor(const std::string& name,
+                                                     const std::string& regex,
+                                                     const std::string& substr) {
+
+  if (name.empty()) {
+    throw EnvoyException("tag_name cannot be empty");
+  }
+
+  if (regex.empty()) {
+    throw EnvoyException(fmt::format(
+        "No regex specified for tag specifier and no default regex for name: '{}'", name));
+  }
+  return TagExtractorPtr{new TagExtractorImpl(name, regex, substr)};
+}
+
+bool TagExtractorImpl::substrMismatch(const std::string& stat_name) const {
+  return !substr_.empty() && stat_name.find(substr_) == std::string::npos;
+}
+
+bool TagExtractorImpl::extractTag(const std::string& stat_name, std::vector<Tag>& tags,
+                                  IntervalSet<size_t>& remove_characters) const {
+  PERF_OPERATION(perf);
+
+  if (substrMismatch(stat_name)) {
+    PERF_RECORD(perf, "re-skip-substr", name_);
+    return false;
+  }
+
+  std::smatch match;
+  // The regex must match and contain one or more subexpressions (all after the first are ignored).
+  if (std::regex_search(stat_name, match, regex_) && match.size() > 1) {
+    // remove_subexpr is the first submatch. It represents the portion of the string to be removed.
+    const auto& remove_subexpr = match[1];
+
+    // value_subexpr is the optional second submatch. It is usually inside the first submatch
+    // (remove_subexpr) to allow the expression to strip off extra characters that should be removed
+    // from the string but also not necessary in the tag value ("." for example). If there is no
+    // second submatch, then the value_subexpr is the same as the remove_subexpr.
+    const auto& value_subexpr = match.size() > 2 ? match[2] : remove_subexpr;
+
+    tags.emplace_back();
+    Tag& tag = tags.back();
+    tag.name_ = name_;
+    tag.value_ = value_subexpr.str();
+
+    // Determines which characters to remove from stat_name to elide remove_subexpr.
+    std::string::size_type start = remove_subexpr.first - stat_name.begin();
+    std::string::size_type end = remove_subexpr.second - stat_name.begin();
+    remove_characters.insert(start, end);
+    PERF_RECORD(perf, "re-match", name_);
+    return true;
+  }
+  PERF_RECORD(perf, "re-miss", name_);
+  return false;
+}
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/tag_extractor_impl.h b/source/common/stats/tag_extractor_impl.h
new file mode 100644
index 000000000000..0057f60c2ade
--- /dev/null
+++ b/source/common/stats/tag_extractor_impl.h
@@ -0,0 +1,59 @@
+#pragma once
+
+#include <cstdint>
+#include <regex>
+#include <string>
+
+#include "envoy/stats/stats.h"
+
+//#include "common/common/utility.h"
+
+#include "absl/strings/string_view.h"
+
+namespace Envoy {
+namespace Stats {
+
+class TagExtractorImpl : public TagExtractor {
+public:
+  /**
+   * Creates a tag extractor from the regex provided. name and regex must be non-empty.
+   * @param name name for tag extractor.
+   * @param regex regex expression.
+   * @param substr a substring that -- if provided -- must be present in a stat name
+   *               in order to match the regex. This is an optional performance tweak
+   *               to avoid large numbers of failed regex lookups.
+   * @return TagExtractorPtr newly constructed TagExtractor.
+   */
+  static TagExtractorPtr createTagExtractor(const std::string& name, const std::string& regex,
+                                            const std::string& substr = "");
+
+  TagExtractorImpl(const std::string& name, const std::string& regex,
+                   const std::string& substr = "");
+  std::string name() const override { return name_; }
+  bool extractTag(const std::string& tag_extracted_name, std::vector<Tag>& tags,
+                  IntervalSet<size_t>& remove_characters) const override;
+  absl::string_view prefixToken() const override { return prefix_; }
+
+  /**
+   * @param stat_name The stat name
+   * @return bool indicates whether tag extraction should be skipped for this stat_name due
+   * to a substring mismatch.
+   */
+  bool substrMismatch(const std::string& stat_name) const;
+
+private:
+  /**
+   * Examines a regex string, looking for the pattern: ^alphanumerics_with_underscores\.
+   * Returns "alphanumerics_with_underscores" if that pattern is found, empty-string otherwise.
+   * @param regex absl::string_view the regex to scan for prefixes.
+   * @return std::string the prefix, or "" if no prefix found.
+   */
+  static std::string extractRegexPrefix(absl::string_view regex);
+  const std::string name_;
+  const std::string prefix_;
+  const std::string substr_;
+  const std::regex regex_;
+};
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/utility.cc b/source/common/stats/utility.cc
new file mode 100644
index 000000000000..00b86eae51eb
--- /dev/null
+++ b/source/common/stats/utility.cc
@@ -0,0 +1,17 @@
+#include <algorithm>
+#include <string>
+
+#include "common/stats/utility.h"
+
+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(), ':', '_');
+  std::replace(stats_name.begin(), stats_name.end(), '\0', '_');
+  return stats_name;
+}
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/utility.h b/source/common/stats/utility.h
new file mode 100644
index 000000000000..18081360640c
--- /dev/null
+++ b/source/common/stats/utility.h
@@ -0,0 +1,19 @@
+#pragma once
+
+#include <string>
+
+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);
+};
+
+} // namespace Stats
+} // namespace Envoy

From 5106baeecd792f3a42152b7717fd84d8aae7fcf4 Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 31 Jul 2018 20:28:19 -0400
Subject: [PATCH 08/14] Split out TagProducer.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 source/common/config/BUILD                |   1 +
 source/common/config/utility.cc           |   1 +
 source/common/stats/BUILD                 |  16 ++-
 source/common/stats/stats_impl.cc         |  99 ------------------
 source/common/stats/stats_impl.h          | 122 ----------------------
 source/common/stats/tag_producer_impl.cc  | 116 ++++++++++++++++++++
 source/common/stats/tag_producer_impl.h   | 105 +++++++++++++++++++
 source/common/stats/thread_local_store.cc |   1 +
 source/common/upstream/BUILD              |   1 +
 test/common/stats/stats_impl_test.cc      |   2 +
 10 files changed, 242 insertions(+), 222 deletions(-)
 create mode 100644 source/common/stats/tag_producer_impl.cc
 create mode 100644 source/common/stats/tag_producer_impl.h

diff --git a/source/common/config/BUILD b/source/common/config/BUILD
index a463141313e3..966fe94e5e76 100644
--- a/source/common/config/BUILD
+++ b/source/common/config/BUILD
@@ -332,6 +332,7 @@ envoy_cc_library(
         "//source/common/protobuf:utility_lib",
         "//source/common/singleton:const_singleton",
         "//source/common/stats:stats_lib",
+        "//source/common/stats:tag_producer_lib",
         "@envoy_api//envoy/api/v2/core:base_cc",
         "@envoy_api//envoy/config/filter/network/http_connection_manager/v2:http_connection_manager_cc",
     ],
diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc
index 37fac934f2ad..dddd63037a9b 100644
--- a/source/common/config/utility.cc
+++ b/source/common/config/utility.cc
@@ -15,6 +15,7 @@
 #include "common/json/config_schemas.h"
 #include "common/protobuf/protobuf.h"
 #include "common/protobuf/utility.h"
+#include "common/stats/tag_producer_impl.h"
 #include "common/stats/stats_impl.h"
 
 namespace Envoy {
diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD
index b6f4a7351ea4..09a5b1d12a29 100644
--- a/source/common/stats/BUILD
+++ b/source/common/stats/BUILD
@@ -29,7 +29,6 @@ envoy_cc_library(
         "//source/common/common:perf_annotation_lib",
         "//source/common/common:thread_annotations",
         "//source/common/common:utility_lib",
-        "//source/common/config:well_known_names",
         "//source/common/protobuf",
         "//source/common/singleton:const_singleton",
         "@envoy_api//envoy/config/metrics/v2:stats_cc",
@@ -54,12 +53,27 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "tag_producer_lib",
+    srcs = ["tag_producer_impl.cc"],
+    hdrs = ["tag_producer_impl.h"],
+    deps = [
+        ":tag_extractor_lib",
+        "//include/envoy/stats:stats_interface",
+        "//source/common/common:perf_annotation_lib",
+        "//source/common/config:well_known_names",
+        "//source/common/protobuf",
+        "@envoy_api//envoy/config/metrics/v2:stats_cc",
+    ],
+)
+
 envoy_cc_library(
     name = "thread_local_store_lib",
     srcs = ["thread_local_store.cc"],
     hdrs = ["thread_local_store.h"],
     deps = [
         ":stats_lib",
+        ":tag_producer_lib",
         "//include/envoy/thread_local:thread_local_interface",
     ],
 )
diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc
index ee895ddfc9b1..a556a1af2e1a 100644
--- a/source/common/stats/stats_impl.cc
+++ b/source/common/stats/stats_impl.cc
@@ -12,7 +12,6 @@
 #include "common/common/perf_annotation.h"
 #include "common/common/thread.h"
 #include "common/common/utility.h"
-#include "common/config/well_known_names.h"
 #include "common/stats/utility.h"
 
 #include "absl/strings/ascii.h"
@@ -34,104 +33,6 @@ uint64_t roundUpMultipleNaturalAlignment(uint64_t val) {
 
 } // namespace
 
-TagProducerImpl::TagProducerImpl(const envoy::config::metrics::v2::StatsConfig& config) {
-  // To check name conflict.
-  reserveResources(config);
-  std::unordered_set<std::string> names = addDefaultExtractors(config);
-
-  for (const auto& tag_specifier : config.stats_tags()) {
-    const std::string& name = tag_specifier.tag_name();
-    if (!names.emplace(name).second) {
-      throw EnvoyException(fmt::format("Tag name '{}' specified twice.", name));
-    }
-
-    // If no tag value is found, fallback to default regex to keep backward compatibility.
-    if (tag_specifier.tag_value_case() ==
-            envoy::config::metrics::v2::TagSpecifier::TAG_VALUE_NOT_SET ||
-        tag_specifier.tag_value_case() == envoy::config::metrics::v2::TagSpecifier::kRegex) {
-
-      if (tag_specifier.regex().empty()) {
-        if (addExtractorsMatching(name) == 0) {
-          throw EnvoyException(fmt::format(
-              "No regex specified for tag specifier and no default regex for name: '{}'", name));
-        }
-      } else {
-        addExtractor(Stats::TagExtractorImpl::createTagExtractor(name, tag_specifier.regex()));
-      }
-    } else if (tag_specifier.tag_value_case() ==
-               envoy::config::metrics::v2::TagSpecifier::kFixedValue) {
-      default_tags_.emplace_back(Stats::Tag{.name_ = name, .value_ = tag_specifier.fixed_value()});
-    }
-  }
-}
-
-int TagProducerImpl::addExtractorsMatching(absl::string_view name) {
-  int num_found = 0;
-  for (const auto& desc : Config::TagNames::get().descriptorVec()) {
-    if (desc.name_ == name) {
-      addExtractor(
-          Stats::TagExtractorImpl::createTagExtractor(desc.name_, desc.regex_, desc.substr_));
-      ++num_found;
-    }
-  }
-  return num_found;
-}
-
-void TagProducerImpl::addExtractor(TagExtractorPtr extractor) {
-  const absl::string_view prefix = extractor->prefixToken();
-  if (prefix.empty()) {
-    tag_extractors_without_prefix_.emplace_back(std::move(extractor));
-  } else {
-    tag_extractor_prefix_map_[prefix].emplace_back(std::move(extractor));
-  }
-}
-
-void TagProducerImpl::forEachExtractorMatching(
-    const std::string& stat_name, std::function<void(const TagExtractorPtr&)> f) const {
-  IntervalSetImpl<size_t> remove_characters;
-  for (const TagExtractorPtr& tag_extractor : tag_extractors_without_prefix_) {
-    f(tag_extractor);
-  }
-  const std::string::size_type dot = stat_name.find('.');
-  if (dot != std::string::npos) {
-    const absl::string_view token = absl::string_view(stat_name.data(), dot);
-    const auto iter = tag_extractor_prefix_map_.find(token);
-    if (iter != tag_extractor_prefix_map_.end()) {
-      for (const TagExtractorPtr& tag_extractor : iter->second) {
-        f(tag_extractor);
-      }
-    }
-  }
-}
-
-std::string TagProducerImpl::produceTags(const std::string& metric_name,
-                                         std::vector<Tag>& tags) const {
-  tags.insert(tags.end(), default_tags_.begin(), default_tags_.end());
-  IntervalSetImpl<size_t> remove_characters;
-  forEachExtractorMatching(
-      metric_name, [&remove_characters, &tags, &metric_name](const TagExtractorPtr& tag_extractor) {
-        tag_extractor->extractTag(metric_name, tags, remove_characters);
-      });
-  return StringUtil::removeCharacters(metric_name, remove_characters);
-}
-
-void TagProducerImpl::reserveResources(const envoy::config::metrics::v2::StatsConfig& config) {
-  default_tags_.reserve(config.stats_tags().size());
-}
-
-std::unordered_set<std::string>
-TagProducerImpl::addDefaultExtractors(const envoy::config::metrics::v2::StatsConfig& config) {
-  std::unordered_set<std::string> names;
-  if (!config.has_use_all_default_tags() || config.use_all_default_tags().value()) {
-    for (const auto& desc : Config::TagNames::get().descriptorVec()) {
-      names.emplace(desc.name_);
-      addExtractor(
-          Stats::TagExtractorImpl::createTagExtractor(desc.name_, desc.regex_, desc.substr_));
-    }
-  }
-  return names;
-}
-
 /**
  * Counter implementation that wraps a StatData. StatData must have data members:
  *    std::atomic<int64_t> value_;
diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h
index 9d8a683dd582..6fed6601394b 100644
--- a/source/common/stats/stats_impl.h
+++ b/source/common/stats/stats_impl.h
@@ -33,128 +33,6 @@
 namespace Envoy {
 namespace Stats {
 
-class TagExtractorImpl : public TagExtractor {
-public:
-  /**
-   * Creates a tag extractor from the regex provided. name and regex must be non-empty.
-   * @param name name for tag extractor.
-   * @param regex regex expression.
-   * @param substr a substring that -- if provided -- must be present in a stat name
-   *               in order to match the regex. This is an optional performance tweak
-   *               to avoid large numbers of failed regex lookups.
-   * @return TagExtractorPtr newly constructed TagExtractor.
-   */
-  static TagExtractorPtr createTagExtractor(const std::string& name, const std::string& regex,
-                                            const std::string& substr = "");
-
-  TagExtractorImpl(const std::string& name, const std::string& regex,
-                   const std::string& substr = "");
-  std::string name() const override { return name_; }
-  bool extractTag(const std::string& tag_extracted_name, std::vector<Tag>& tags,
-                  IntervalSet<size_t>& remove_characters) const override;
-  absl::string_view prefixToken() const override { return prefix_; }
-
-  /**
-   * @param stat_name The stat name
-   * @return bool indicates whether tag extraction should be skipped for this stat_name due
-   * to a substring mismatch.
-   */
-  bool substrMismatch(const std::string& stat_name) const;
-
-private:
-  /**
-   * Examines a regex string, looking for the pattern: ^alphanumerics_with_underscores\.
-   * Returns "alphanumerics_with_underscores" if that pattern is found, empty-string otherwise.
-   * @param regex absl::string_view the regex to scan for prefixes.
-   * @return std::string the prefix, or "" if no prefix found.
-   */
-  static std::string extractRegexPrefix(absl::string_view regex);
-  const std::string name_;
-  const std::string prefix_;
-  const std::string substr_;
-  const std::regex regex_;
-};
-
-/**
- * Organizes a collection of TagExtractors so that stat-names can be processed without
- * iterating through all extractors.
- */
-class TagProducerImpl : public TagProducer {
-public:
-  TagProducerImpl(const envoy::config::metrics::v2::StatsConfig& config);
-  TagProducerImpl() {}
-
-  /**
-   * Take a metric name and a vector then add proper tags into the vector and
-   * return an extracted metric name.
-   * @param metric_name std::string a name of Stats::Metric (Counter, Gauge, Histogram).
-   * @param tags std::vector a set of Stats::Tag.
-   */
-  std::string produceTags(const std::string& metric_name, std::vector<Tag>& tags) const override;
-
-private:
-  friend class DefaultTagRegexTester;
-
-  /**
-   * Adds a TagExtractor to the collection of tags, tracking prefixes to help make
-   * produceTags run efficiently by trying only extractors that have a chance to match.
-   * @param extractor TagExtractorPtr the extractor to add.
-   */
-  void addExtractor(TagExtractorPtr extractor);
-
-  /**
-   * Adds all default extractors matching the specified tag name. In this model,
-   * more than one TagExtractor can be used to generate a given tag. The default
-   * extractors are specified in common/config/well_known_names.cc.
-   * @param name absl::string_view the extractor to add.
-   * @return int the number of matching extractors.
-   */
-  int addExtractorsMatching(absl::string_view name);
-
-  /**
-   * Roughly estimate the size of the vectors.
-   * @param config const envoy::config::metrics::v2::StatsConfig& the config.
-   */
-  void reserveResources(const envoy::config::metrics::v2::StatsConfig& config);
-
-  /**
-   * Adds all default extractors from well_known_names.cc into the
-   * collection. Returns a set of names of all default extractors
-   * into a string-set for dup-detection against new stat names
-   * specified in the configuration.
-   * @param config const envoy::config::metrics::v2::StatsConfig& the config.
-   * @return names std::unordered_set<std::string> the set of names to populate
-   */
-  std::unordered_set<std::string>
-  addDefaultExtractors(const envoy::config::metrics::v2::StatsConfig& config);
-
-  /**
-   * Iterates over every tag extractor that might possibly match stat_name, calling
-   * callback f for each one. This is broken out this way to reduce code redundancy
-   * during testing, where we want to verify that extraction is order-independent.
-   * The possibly-matching-extractors list is computed by:
-   *   1. Finding the first '.' separated token in stat_name.
-   *   2. Collecting the TagExtractors whose regexes have that same prefix "^prefix\\."
-   *   3. Collecting also the TagExtractors whose regexes don't start with any prefix.
-   * In the future, we may also do substring searches in some cases.
-   * See DefaultTagRegexTester::produceTagsReverse in test/common/stats/stats_impl_test.cc.
-   *
-   * @param stat_name const std::string& the stat name.
-   * @param f std::function<void(const TagExtractorPtr&)> function to call for each extractor.
-   */
-  void forEachExtractorMatching(const std::string& stat_name,
-                                std::function<void(const TagExtractorPtr&)> f) const;
-
-  std::vector<TagExtractorPtr> tag_extractors_without_prefix_;
-
-  // Maps a prefix word extracted out of a regex to a vector of TagExtractors. Note that
-  // the storage for the prefix string is owned by the TagExtractor, which, depending on
-  // implementation, may need make a copy of the prefix.
-  std::unordered_map<absl::string_view, std::vector<TagExtractorPtr>, StringViewHash>
-      tag_extractor_prefix_map_;
-  std::vector<Tag> default_tags_;
-};
-
 /**
  * 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.
diff --git a/source/common/stats/tag_producer_impl.cc b/source/common/stats/tag_producer_impl.cc
new file mode 100644
index 000000000000..41739704b27e
--- /dev/null
+++ b/source/common/stats/tag_producer_impl.cc
@@ -0,0 +1,116 @@
+#include "common/stats/tag_producer_impl.h"
+
+//#include <string.h>
+
+//#include <algorithm>
+//#include <chrono>
+#include <string>
+
+#include "envoy/common/exception.h"
+
+#include "common/common/utility.h"
+#include "common/stats/tag_extractor_impl.h"
+
+namespace Envoy {
+namespace Stats {
+
+TagProducerImpl::TagProducerImpl(const envoy::config::metrics::v2::StatsConfig& config) {
+  // To check name conflict.
+  reserveResources(config);
+  std::unordered_set<std::string> names = addDefaultExtractors(config);
+
+  for (const auto& tag_specifier : config.stats_tags()) {
+    const std::string& name = tag_specifier.tag_name();
+    if (!names.emplace(name).second) {
+      throw EnvoyException(fmt::format("Tag name '{}' specified twice.", name));
+    }
+
+    // If no tag value is found, fallback to default regex to keep backward compatibility.
+    if (tag_specifier.tag_value_case() ==
+            envoy::config::metrics::v2::TagSpecifier::TAG_VALUE_NOT_SET ||
+        tag_specifier.tag_value_case() == envoy::config::metrics::v2::TagSpecifier::kRegex) {
+
+      if (tag_specifier.regex().empty()) {
+        if (addExtractorsMatching(name) == 0) {
+          throw EnvoyException(fmt::format(
+              "No regex specified for tag specifier and no default regex for name: '{}'", name));
+        }
+      } else {
+        addExtractor(Stats::TagExtractorImpl::createTagExtractor(name, tag_specifier.regex()));
+      }
+    } else if (tag_specifier.tag_value_case() ==
+               envoy::config::metrics::v2::TagSpecifier::kFixedValue) {
+      default_tags_.emplace_back(Stats::Tag{.name_ = name, .value_ = tag_specifier.fixed_value()});
+    }
+  }
+}
+
+int TagProducerImpl::addExtractorsMatching(absl::string_view name) {
+  int num_found = 0;
+  for (const auto& desc : Config::TagNames::get().descriptorVec()) {
+    if (desc.name_ == name) {
+      addExtractor(
+          Stats::TagExtractorImpl::createTagExtractor(desc.name_, desc.regex_, desc.substr_));
+      ++num_found;
+    }
+  }
+  return num_found;
+}
+
+void TagProducerImpl::addExtractor(TagExtractorPtr extractor) {
+  const absl::string_view prefix = extractor->prefixToken();
+  if (prefix.empty()) {
+    tag_extractors_without_prefix_.emplace_back(std::move(extractor));
+  } else {
+    tag_extractor_prefix_map_[prefix].emplace_back(std::move(extractor));
+  }
+}
+
+void TagProducerImpl::forEachExtractorMatching(
+    const std::string& stat_name, std::function<void(const TagExtractorPtr&)> f) const {
+  IntervalSetImpl<size_t> remove_characters;
+  for (const TagExtractorPtr& tag_extractor : tag_extractors_without_prefix_) {
+    f(tag_extractor);
+  }
+  const std::string::size_type dot = stat_name.find('.');
+  if (dot != std::string::npos) {
+    const absl::string_view token = absl::string_view(stat_name.data(), dot);
+    const auto iter = tag_extractor_prefix_map_.find(token);
+    if (iter != tag_extractor_prefix_map_.end()) {
+      for (const TagExtractorPtr& tag_extractor : iter->second) {
+        f(tag_extractor);
+      }
+    }
+  }
+}
+
+std::string TagProducerImpl::produceTags(const std::string& metric_name,
+                                         std::vector<Tag>& tags) const {
+  tags.insert(tags.end(), default_tags_.begin(), default_tags_.end());
+  IntervalSetImpl<size_t> remove_characters;
+  forEachExtractorMatching(
+      metric_name, [&remove_characters, &tags, &metric_name](const TagExtractorPtr& tag_extractor) {
+        tag_extractor->extractTag(metric_name, tags, remove_characters);
+      });
+  return StringUtil::removeCharacters(metric_name, remove_characters);
+}
+
+void TagProducerImpl::reserveResources(const envoy::config::metrics::v2::StatsConfig& config) {
+  default_tags_.reserve(config.stats_tags().size());
+}
+
+std::unordered_set<std::string>
+TagProducerImpl::addDefaultExtractors(const envoy::config::metrics::v2::StatsConfig& config) {
+  std::unordered_set<std::string> names;
+  if (!config.has_use_all_default_tags() || config.use_all_default_tags().value()) {
+    for (const auto& desc : Config::TagNames::get().descriptorVec()) {
+      names.emplace(desc.name_);
+      addExtractor(
+          Stats::TagExtractorImpl::createTagExtractor(desc.name_, desc.regex_, desc.substr_));
+    }
+  }
+  return names;
+}
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/tag_producer_impl.h b/source/common/stats/tag_producer_impl.h
new file mode 100644
index 000000000000..2e60f86564f4
--- /dev/null
+++ b/source/common/stats/tag_producer_impl.h
@@ -0,0 +1,105 @@
+#pragma once
+
+#include <cstdint>
+#include <functional>
+#include <memory>
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <vector>
+
+#include "envoy/config/metrics/v2/stats.pb.h"
+#include "envoy/stats/stats.h"
+
+#include "common/common/hash.h"
+#include "common/common/utility.h"
+#include "common/config/well_known_names.h"
+#include "common/protobuf/protobuf.h"
+
+#include "absl/strings/string_view.h"
+
+namespace Envoy {
+namespace Stats {
+
+/**
+ * Organizes a collection of TagExtractors so that stat-names can be processed without
+ * iterating through all extractors.
+ */
+class TagProducerImpl : public TagProducer {
+public:
+  TagProducerImpl(const envoy::config::metrics::v2::StatsConfig& config);
+  TagProducerImpl() {}
+
+  /**
+   * Take a metric name and a vector then add proper tags into the vector and
+   * return an extracted metric name.
+   * @param metric_name std::string a name of Stats::Metric (Counter, Gauge, Histogram).
+   * @param tags std::vector a set of Stats::Tag.
+   */
+  std::string produceTags(const std::string& metric_name, std::vector<Tag>& tags) const override;
+
+private:
+  friend class DefaultTagRegexTester;
+
+  /**
+   * Adds a TagExtractor to the collection of tags, tracking prefixes to help make
+   * produceTags run efficiently by trying only extractors that have a chance to match.
+   * @param extractor TagExtractorPtr the extractor to add.
+   */
+  void addExtractor(TagExtractorPtr extractor);
+
+  /**
+   * Adds all default extractors matching the specified tag name. In this model,
+   * more than one TagExtractor can be used to generate a given tag. The default
+   * extractors are specified in common/config/well_known_names.cc.
+   * @param name absl::string_view the extractor to add.
+   * @return int the number of matching extractors.
+   */
+  int addExtractorsMatching(absl::string_view name);
+
+  /**
+   * Roughly estimate the size of the vectors.
+   * @param config const envoy::config::metrics::v2::StatsConfig& the config.
+   */
+  void reserveResources(const envoy::config::metrics::v2::StatsConfig& config);
+
+  /**
+   * Adds all default extractors from well_known_names.cc into the
+   * collection. Returns a set of names of all default extractors
+   * into a string-set for dup-detection against new stat names
+   * specified in the configuration.
+   * @param config const envoy::config::metrics::v2::StatsConfig& the config.
+   * @return names std::unordered_set<std::string> the set of names to populate
+   */
+  std::unordered_set<std::string>
+  addDefaultExtractors(const envoy::config::metrics::v2::StatsConfig& config);
+
+  /**
+   * Iterates over every tag extractor that might possibly match stat_name, calling
+   * callback f for each one. This is broken out this way to reduce code redundancy
+   * during testing, where we want to verify that extraction is order-independent.
+   * The possibly-matching-extractors list is computed by:
+   *   1. Finding the first '.' separated token in stat_name.
+   *   2. Collecting the TagExtractors whose regexes have that same prefix "^prefix\\."
+   *   3. Collecting also the TagExtractors whose regexes don't start with any prefix.
+   * In the future, we may also do substring searches in some cases.
+   * See DefaultTagRegexTester::produceTagsReverse in test/common/stats/stats_impl_test.cc.
+   *
+   * @param stat_name const std::string& the stat name.
+   * @param f std::function<void(const TagExtractorPtr&)> function to call for each extractor.
+   */
+  void forEachExtractorMatching(const std::string& stat_name,
+                                std::function<void(const TagExtractorPtr&)> f) const;
+
+  std::vector<TagExtractorPtr> tag_extractors_without_prefix_;
+
+  // Maps a prefix word extracted out of a regex to a vector of TagExtractors. Note that
+  // the storage for the prefix string is owned by the TagExtractor, which, depending on
+  // implementation, may need make a copy of the prefix.
+  std::unordered_map<absl::string_view, std::vector<TagExtractorPtr>, StringViewHash>
+      tag_extractor_prefix_map_;
+  std::vector<Tag> default_tags_;
+};
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc
index 0467e47f587a..c4336d288e8d 100644
--- a/source/common/stats/thread_local_store.cc
+++ b/source/common/stats/thread_local_store.cc
@@ -8,6 +8,7 @@
 #include <unordered_set>
 
 #include "common/common/lock_guard.h"
+#include "common/stats/tag_producer_impl.h"
 
 namespace Envoy {
 namespace Stats {
diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD
index f938b86ae079..a1b75781dcdd 100644
--- a/source/common/upstream/BUILD
+++ b/source/common/upstream/BUILD
@@ -388,6 +388,7 @@ envoy_cc_library(
         "//source/common/common:enum_to_int",
         "//source/common/common:minimal_logger_lib",
         "//source/common/config:metadata_lib",
+        "//source/common/config:well_known_names",
         "//source/common/stats:stats_lib",
         "//source/common/upstream:locality_lib",
         "@envoy_api//envoy/api/v2/core:base_cc",
diff --git a/test/common/stats/stats_impl_test.cc b/test/common/stats/stats_impl_test.cc
index d976a5a5ffdf..11493d2b0c99 100644
--- a/test/common/stats/stats_impl_test.cc
+++ b/test/common/stats/stats_impl_test.cc
@@ -8,6 +8,8 @@
 #include "common/common/hex.h"
 #include "common/config/well_known_names.h"
 #include "common/stats/stats_impl.h"
+#include "common/stats/tag_extractor_impl.h"
+#include "common/stats/tag_producer_impl.h"
 
 #include "test/mocks/stats/mocks.h"
 #include "test/test_common/logging.h"

From dfd05ac36bdcdf6ef35c5683995e0d698d4f1a8a Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 31 Jul 2018 20:38:16 -0400
Subject: [PATCH 09/14] Spelling error, formatting fixes, and verified
 //test/... passes.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 source/common/config/utility.cc           | 2 +-
 source/common/stats/stats_impl.h          | 2 +-
 source/common/stats/tag_extractor_impl.cc | 1 +
 source/common/stats/utility.cc            | 4 ++--
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc
index dddd63037a9b..7bf047b3d4fa 100644
--- a/source/common/config/utility.cc
+++ b/source/common/config/utility.cc
@@ -15,8 +15,8 @@
 #include "common/json/config_schemas.h"
 #include "common/protobuf/protobuf.h"
 #include "common/protobuf/utility.h"
-#include "common/stats/tag_producer_impl.h"
 #include "common/stats/stats_impl.h"
+#include "common/stats/tag_producer_impl.h"
 
 namespace Envoy {
 namespace Config {
diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h
index 6fed6601394b..6485476f207f 100644
--- a/source/common/stats/stats_impl.h
+++ b/source/common/stats/stats_impl.h
@@ -44,7 +44,7 @@ struct RawStatData {
 
   /**
    * Due to the flexible-array-length of name_, c-style allocation
-   * and initialization are neccessary.
+   * and initialization are necessary.
    */
   RawStatData() = delete;
   ~RawStatData() = delete;
diff --git a/source/common/stats/tag_extractor_impl.cc b/source/common/stats/tag_extractor_impl.cc
index ce4e3482267d..9c6acef9574f 100644
--- a/source/common/stats/tag_extractor_impl.cc
+++ b/source/common/stats/tag_extractor_impl.cc
@@ -8,6 +8,7 @@
 
 #include "common/common/perf_annotation.h"
 #include "common/common/utility.h"
+
 //#include "common/stats/utility.h"
 
 #include "absl/strings/ascii.h"
diff --git a/source/common/stats/utility.cc b/source/common/stats/utility.cc
index 00b86eae51eb..afc4e8c4f4ca 100644
--- a/source/common/stats/utility.cc
+++ b/source/common/stats/utility.cc
@@ -1,8 +1,8 @@
+#include "common/stats/utility.h"
+
 #include <algorithm>
 #include <string>
 
-#include "common/stats/utility.h"
-
 namespace Envoy {
 namespace Stats {
 

From 871c1a34b11c6f8d44bee136ae29c5e21199d68b Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 31 Jul 2018 21:39:54 -0400
Subject: [PATCH 10/14] Break out IsoaltedStore into a separate file.  Man this
 was painful.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 source/common/stats/BUILD                     |  21 +++
 source/common/stats/isolated_store_impl.cc    |  54 ++++++++
 source/common/stats/isolated_store_impl.h     |  81 ++++++++++++
 source/common/stats/raw_stat_data.cc          |  45 +++++++
 source/common/stats/raw_stat_data.h           |  84 ++++++++++++
 source/common/stats/stats_impl.cc             |  72 +---------
 source/common/stats/stats_impl.h              | 124 ------------------
 source/common/upstream/BUILD                  |   1 +
 source/common/upstream/upstream_impl.h        |   1 +
 source/server/hot_restart_impl.cc             |   1 +
 source/server/hot_restart_impl.h              |   1 +
 source/server/http/BUILD                      |   1 +
 source/server/http/admin.h                    |   1 +
 test/common/access_log/BUILD                  |   1 +
 .../access_log_manager_impl_test.cc           |   1 +
 test/common/filesystem/BUILD                  |   1 +
 .../common/filesystem/filesystem_impl_test.cc |   1 +
 test/common/grpc/BUILD                        |   1 +
 .../grpc/google_async_client_impl_test.cc     |   1 +
 test/common/runtime/BUILD                     |   1 +
 test/common/runtime/runtime_impl_test.cc      |   1 +
 test/common/ssl/BUILD                         |   2 +
 test/common/ssl/context_impl_test.cc          |   1 +
 .../filters/network/redis_proxy/BUILD         |   1 +
 .../redis_proxy/command_splitter_impl_test.cc |   1 +
 test/integration/BUILD                        |   1 +
 test/integration/fake_upstream.h              |   1 +
 test/mocks/network/BUILD                      |   1 +
 test/mocks/network/mocks.h                    |   1 +
 test/mocks/stats/BUILD                        |   1 +
 test/mocks/stats/mocks.h                      |   1 +
 test/test_common/utility.h                    |   1 +
 32 files changed, 312 insertions(+), 195 deletions(-)
 create mode 100644 source/common/stats/isolated_store_impl.cc
 create mode 100644 source/common/stats/isolated_store_impl.h
 create mode 100644 source/common/stats/raw_stat_data.cc
 create mode 100644 source/common/stats/raw_stat_data.h

diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD
index 09a5b1d12a29..7ceefbb501d7 100644
--- a/source/common/stats/BUILD
+++ b/source/common/stats/BUILD
@@ -8,6 +8,26 @@ load(
 
 envoy_package()
 
+envoy_cc_library(
+    name = "isolated_store_lib",
+    srcs = ["isolated_store_impl.cc"],
+    hdrs = ["isolated_store_impl.h"],
+    deps = [
+        "//source/common/stats:stats_lib",
+    ],
+)
+
+envoy_cc_library(
+    name = "raw_stat_data_lib",
+    srcs = ["raw_stat_data.cc"],
+    hdrs = ["raw_stat_data.h"],
+    deps = [
+        "//include/envoy/stats:stats_interface",
+        "//source/common/common:assert_lib",
+        "//source/common/common:hash_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "stats_lib",
     srcs = ["stats_impl.cc"],
@@ -17,6 +37,7 @@ envoy_cc_library(
         "libcircllhist",
     ],
     deps = [
+        ":raw_stat_data_lib",
         ":stats_options_lib",
         ":tag_extractor_lib",
         ":utility_lib",
diff --git a/source/common/stats/isolated_store_impl.cc b/source/common/stats/isolated_store_impl.cc
new file mode 100644
index 000000000000..b7459fc4380d
--- /dev/null
+++ b/source/common/stats/isolated_store_impl.cc
@@ -0,0 +1,54 @@
+#include "common/stats/isolated_store_impl.h"
+
+#include <string.h>
+
+#include <algorithm>
+#include <string>
+
+#include "common/common/utility.h"
+#include "common/stats/utility.h"
+
+namespace Envoy {
+namespace Stats {
+
+IsolatedStoreImpl::IsolatedStoreImpl()
+    : counters_([this](const std::string& name) -> CounterSharedPtr {
+        std::string tag_extracted_name = name;
+        std::vector<Tag> tags;
+        return alloc_.makeCounter(name, std::move(tag_extracted_name), std::move(tags));
+      }),
+      gauges_([this](const std::string& name) -> GaugeSharedPtr {
+        std::string tag_extracted_name = name;
+        std::vector<Tag> tags;
+        return alloc_.makeGauge(name, std::move(tag_extracted_name), std::move(tags));
+      }),
+      histograms_([this](const std::string& name) -> HistogramSharedPtr {
+        return std::make_shared<HistogramImpl>(name, *this, std::string(name), std::vector<Tag>());
+      }) {}
+
+struct IsolatedScopeImpl : public Scope {
+  IsolatedScopeImpl(IsolatedStoreImpl& parent, const std::string& prefix)
+      : parent_(parent), prefix_(Utility::sanitizeStatsName(prefix)) {}
+
+  // Stats::Scope
+  ScopePtr createScope(const std::string& name) override {
+    return ScopePtr{new IsolatedScopeImpl(parent_, prefix_ + name)};
+  }
+  void deliverHistogramToSinks(const Histogram&, uint64_t) override {}
+  Counter& counter(const std::string& name) override { return parent_.counter(prefix_ + name); }
+  Gauge& gauge(const std::string& name) override { return parent_.gauge(prefix_ + name); }
+  Histogram& histogram(const std::string& name) override {
+    return parent_.histogram(prefix_ + name);
+  }
+  const Stats::StatsOptions& statsOptions() const override { return parent_.statsOptions(); }
+
+  IsolatedStoreImpl& parent_;
+  const std::string prefix_;
+};
+
+ScopePtr IsolatedStoreImpl::createScope(const std::string& name) {
+  return ScopePtr{new IsolatedScopeImpl(*this, name)};
+}
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h
new file mode 100644
index 000000000000..344c62153d0c
--- /dev/null
+++ b/source/common/stats/isolated_store_impl.h
@@ -0,0 +1,81 @@
+#pragma once
+
+#include <string.h>
+
+#include <algorithm>
+#include <string>
+
+#include "common/common/utility.h"
+#include "common/stats/stats_impl.h"
+#include "common/stats/utility.h"
+
+namespace Envoy {
+namespace Stats {
+
+/**
+ * A stats cache template that is used by the isolated store.
+ */
+template <class Base> class IsolatedStatsCache {
+public:
+  typedef std::function<std::shared_ptr<Base>(const std::string& name)> Allocator;
+
+  IsolatedStatsCache(Allocator alloc) : alloc_(alloc) {}
+
+  Base& get(const std::string& name) {
+    auto stat = stats_.find(name);
+    if (stat != stats_.end()) {
+      return *stat->second;
+    }
+
+    std::shared_ptr<Base> new_stat = alloc_(name);
+    stats_.emplace(name, new_stat);
+    return *new_stat;
+  }
+
+  std::vector<std::shared_ptr<Base>> toVector() const {
+    std::vector<std::shared_ptr<Base>> vec;
+    vec.reserve(stats_.size());
+    for (auto& stat : stats_) {
+      vec.push_back(stat.second);
+    }
+
+    return vec;
+  }
+
+private:
+  std::unordered_map<std::string, std::shared_ptr<Base>> stats_;
+  Allocator alloc_;
+};
+
+class IsolatedStoreImpl : public Store {
+public:
+  IsolatedStoreImpl();
+
+  // Stats::Scope
+  Counter& counter(const std::string& name) override { return counters_.get(name); }
+  ScopePtr createScope(const std::string& name) override;
+  void deliverHistogramToSinks(const Histogram&, uint64_t) override {}
+  Gauge& gauge(const std::string& name) override { return gauges_.get(name); }
+  Histogram& histogram(const std::string& name) override {
+    Histogram& histogram = histograms_.get(name);
+    return histogram;
+  }
+  const Stats::StatsOptions& statsOptions() const override { return stats_options_; }
+
+  // Stats::Store
+  std::vector<CounterSharedPtr> counters() const override { return counters_.toVector(); }
+  std::vector<GaugeSharedPtr> gauges() const override { return gauges_.toVector(); }
+  std::vector<ParentHistogramSharedPtr> histograms() const override {
+    return std::vector<ParentHistogramSharedPtr>{};
+  }
+
+private:
+  HeapStatDataAllocator alloc_;
+  IsolatedStatsCache<Counter> counters_;
+  IsolatedStatsCache<Gauge> gauges_;
+  IsolatedStatsCache<Histogram> histograms_;
+  const StatsOptionsImpl stats_options_;
+};
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/raw_stat_data.cc b/source/common/stats/raw_stat_data.cc
new file mode 100644
index 000000000000..e62549dbedbe
--- /dev/null
+++ b/source/common/stats/raw_stat_data.cc
@@ -0,0 +1,45 @@
+#include "common/stats/raw_stat_data.h"
+
+#include <string.h>
+
+#include <algorithm>
+#include <chrono>
+#include <string>
+
+namespace Envoy {
+namespace Stats {
+
+namespace {
+
+// Round val up to the next multiple of the natural alignment.
+// Note: this implementation only works because 8 is a power of 2.
+uint64_t roundUpMultipleNaturalAlignment(uint64_t val) {
+  const uint64_t multiple = alignof(RawStatData);
+  static_assert(multiple == 1 || multiple == 2 || multiple == 4 || multiple == 8 || multiple == 16,
+                "multiple must be a power of 2 for this algorithm to work");
+  return (val + multiple - 1) & ~(multiple - 1);
+}
+
+} // namespace
+
+// Normally the compiler would do this, but because name_ is a flexible-array-length
+// element, the compiler can't. RawStatData is put into an array in HotRestartImpl, so
+// it's important that each element starts on the required alignment for the type.
+uint64_t RawStatData::structSize(uint64_t name_size) {
+  return roundUpMultipleNaturalAlignment(sizeof(RawStatData) + name_size + 1);
+}
+
+uint64_t RawStatData::structSizeWithOptions(const StatsOptions& stats_options) {
+  return structSize(stats_options.maxNameLength());
+}
+
+void RawStatData::initialize(absl::string_view key, const StatsOptions& stats_options) {
+  ASSERT(!initialized());
+  ASSERT(key.size() <= stats_options.maxNameLength());
+  ref_count_ = 1;
+  memcpy(name_, key.data(), key.size());
+  name_[key.size()] = '\0';
+}
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/raw_stat_data.h b/source/common/stats/raw_stat_data.h
new file mode 100644
index 000000000000..34013180c87f
--- /dev/null
+++ b/source/common/stats/raw_stat_data.h
@@ -0,0 +1,84 @@
+#pragma once
+
+#include <algorithm>
+#include <atomic>
+#include <chrono>
+#include <cstdint>
+#include <functional>
+#include <list>
+#include <memory>
+#include <regex>
+#include <string>
+#include <unordered_map>
+
+#include "envoy/stats/stats.h"
+
+#include "common/common/assert.h"
+#include "common/common/hash.h"
+
+#include "absl/strings/string_view.h"
+
+namespace Envoy {
+namespace Stats {
+
+/**
+ * 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.
+ *
+ * @note Due to name_ being variable size, sizeof(RawStatData) probably isn't useful. Use
+ * RawStatData::structSize() or RawStatData::structSizeWithOptions() instead.
+ */
+struct RawStatData {
+
+  /**
+   * Due to the flexible-array-length of name_, c-style allocation
+   * and initialization are necessary.
+   */
+  RawStatData() = delete;
+  ~RawStatData() = delete;
+
+  /**
+   * Returns the size of this struct, accounting for the length of name_
+   * and padding for alignment.
+   */
+  static uint64_t structSize(uint64_t name_size);
+
+  /**
+   * Wrapper for structSize, taking a StatsOptions struct.
+   * Required by BlockMemoryHashSet, which has the context to supply StatsOptions.
+   */
+  static uint64_t structSizeWithOptions(const StatsOptions& stats_options);
+
+  /**
+   * Initializes this object to have the specified key,
+   * a refcount of 1, and all other values zero. Required for the HeapRawStatDataAllocator, which
+   * does not expect stat name truncation. We pass in the number of bytes allocated in order to
+   * assert the copy is safe inline.
+   */
+  void initialize(absl::string_view key, const StatsOptions& stats_options);
+
+  /**
+   * Returns a hash of the key. This is required by BlockMemoryHashSet.
+   */
+  static uint64_t hash(absl::string_view key) { return HashUtil::xxHash64(key); }
+
+  /**
+   * Returns true if object is in use.
+   */
+  bool initialized() { return name_[0] != '\0'; }
+
+  /**
+   * Returns the name as a string_view with no truncation.
+   */
+  absl::string_view key() const { return absl::string_view(name_); }
+
+  std::atomic<uint64_t> value_;
+  std::atomic<uint64_t> pending_increment_;
+  std::atomic<uint16_t> flags_;
+  std::atomic<uint16_t> ref_count_;
+  std::atomic<uint32_t> unused_;
+  char name_[];
+};
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc
index a556a1af2e1a..6a2a6c3a2e84 100644
--- a/source/common/stats/stats_impl.cc
+++ b/source/common/stats/stats_impl.cc
@@ -12,6 +12,7 @@
 #include "common/common/perf_annotation.h"
 #include "common/common/thread.h"
 #include "common/common/utility.h"
+#include "common/stats/raw_stat_data.h"
 #include "common/stats/utility.h"
 
 #include "absl/strings/ascii.h"
@@ -20,19 +21,6 @@
 namespace Envoy {
 namespace Stats {
 
-namespace {
-
-// Round val up to the next multiple of the natural alignment.
-// Note: this implementation only works because 8 is a power of 2.
-uint64_t roundUpMultipleNaturalAlignment(uint64_t val) {
-  const uint64_t multiple = alignof(RawStatData);
-  static_assert(multiple == 1 || multiple == 2 || multiple == 4 || multiple == 8 || multiple == 16,
-                "multiple must be a power of 2 for this algorithm to work");
-  return (val + multiple - 1) & ~(multiple - 1);
-}
-
-} // namespace
-
 /**
  * Counter implementation that wraps a StatData. StatData must have data members:
  *    std::atomic<int64_t> value_;
@@ -101,25 +89,6 @@ template <class StatData> class GaugeImpl : public Gauge, public MetricImpl {
   StatDataAllocatorImpl<StatData>& alloc_;
 };
 
-// Normally the compiler would do this, but because name_ is a flexible-array-length
-// element, the compiler can't. RawStatData is put into an array in HotRestartImpl, so
-// it's important that each element starts on the required alignment for the type.
-uint64_t RawStatData::structSize(uint64_t name_size) {
-  return roundUpMultipleNaturalAlignment(sizeof(RawStatData) + name_size + 1);
-}
-
-uint64_t RawStatData::structSizeWithOptions(const StatsOptions& stats_options) {
-  return structSize(stats_options.maxNameLength());
-}
-
-void RawStatData::initialize(absl::string_view key, const StatsOptions& stats_options) {
-  ASSERT(!initialized());
-  ASSERT(key.size() <= stats_options.maxNameLength());
-  ref_count_ = 1;
-  memcpy(name_, key.data(), key.size());
-  name_[key.size()] = '\0';
-}
-
 HeapStatData::HeapStatData(absl::string_view key) : name_(key.data(), key.size()) {}
 
 template <class StatData>
@@ -235,45 +204,6 @@ void SourceImpl::clearCache() {
   histograms_.reset();
 }
 
-IsolatedStoreImpl::IsolatedStoreImpl()
-    : counters_([this](const std::string& name) -> CounterSharedPtr {
-        std::string tag_extracted_name = name;
-        std::vector<Tag> tags;
-        return alloc_.makeCounter(name, std::move(tag_extracted_name), std::move(tags));
-      }),
-      gauges_([this](const std::string& name) -> GaugeSharedPtr {
-        std::string tag_extracted_name = name;
-        std::vector<Tag> tags;
-        return alloc_.makeGauge(name, std::move(tag_extracted_name), std::move(tags));
-      }),
-      histograms_([this](const std::string& name) -> HistogramSharedPtr {
-        return std::make_shared<HistogramImpl>(name, *this, std::string(name), std::vector<Tag>());
-      }) {}
-
-struct IsolatedScopeImpl : public Scope {
-  IsolatedScopeImpl(IsolatedStoreImpl& parent, const std::string& prefix)
-      : parent_(parent), prefix_(Utility::sanitizeStatsName(prefix)) {}
-
-  // Stats::Scope
-  ScopePtr createScope(const std::string& name) override {
-    return ScopePtr{new IsolatedScopeImpl(parent_, prefix_ + name)};
-  }
-  void deliverHistogramToSinks(const Histogram&, uint64_t) override {}
-  Counter& counter(const std::string& name) override { return parent_.counter(prefix_ + name); }
-  Gauge& gauge(const std::string& name) override { return parent_.gauge(prefix_ + name); }
-  Histogram& histogram(const std::string& name) override {
-    return parent_.histogram(prefix_ + name);
-  }
-  const Stats::StatsOptions& statsOptions() const override { return parent_.statsOptions(); }
-
-  IsolatedStoreImpl& parent_;
-  const std::string prefix_;
-};
-
-ScopePtr IsolatedStoreImpl::createScope(const std::string& name) {
-  return ScopePtr{new IsolatedScopeImpl(*this, name)};
-}
-
 template class StatDataAllocatorImpl<HeapStatData>;
 template class StatDataAllocatorImpl<RawStatData>;
 
diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h
index 6485476f207f..f37c81c8ab0f 100644
--- a/source/common/stats/stats_impl.h
+++ b/source/common/stats/stats_impl.h
@@ -33,65 +33,6 @@
 namespace Envoy {
 namespace Stats {
 
-/**
- * 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.
- *
- * @note Due to name_ being variable size, sizeof(RawStatData) probably isn't useful. Use
- * RawStatData::structSize() or RawStatData::structSizeWithOptions() instead.
- */
-struct RawStatData {
-
-  /**
-   * Due to the flexible-array-length of name_, c-style allocation
-   * and initialization are necessary.
-   */
-  RawStatData() = delete;
-  ~RawStatData() = delete;
-
-  /**
-   * Returns the size of this struct, accounting for the length of name_
-   * and padding for alignment.
-   */
-  static uint64_t structSize(uint64_t name_size);
-
-  /**
-   * Wrapper for structSize, taking a StatsOptions struct.
-   * Required by BlockMemoryHashSet, which has the context to supply StatsOptions.
-   */
-  static uint64_t structSizeWithOptions(const StatsOptions& stats_options);
-
-  /**
-   * Initializes this object to have the specified key,
-   * a refcount of 1, and all other values zero. Required for the HeapRawStatDataAllocator, which
-   * does not expect stat name truncation. We pass in the number of bytes allocated in order to
-   * assert the copy is safe inline.
-   */
-  void initialize(absl::string_view key, const StatsOptions& stats_options);
-
-  /**
-   * Returns a hash of the key. This is required by BlockMemoryHashSet.
-   */
-  static uint64_t hash(absl::string_view key) { return HashUtil::xxHash64(key); }
-
-  /**
-   * Returns true if object is in use.
-   */
-  bool initialized() { return name_[0] != '\0'; }
-
-  /**
-   * Returns the name as a string_view with no truncation.
-   */
-  absl::string_view key() const { return absl::string_view(name_); }
-
-  std::atomic<uint64_t> value_;
-  std::atomic<uint64_t> pending_increment_;
-  std::atomic<uint16_t> flags_;
-  std::atomic<uint16_t> ref_count_;
-  std::atomic<uint32_t> unused_;
-  char name_[];
-};
-
 // Partially implements a StatDataAllocator, leaving alloc & free for subclasses.
 // We templatize on StatData rather than defining a virtual base StatData class
 // for performance reasons; stat increment is on the hot path.
@@ -282,70 +223,5 @@ class HeapStatDataAllocator : public StatDataAllocatorImpl<HeapStatData> {
   Thread::MutexBasicLockable mutex_;
 };
 
-/**
- * A stats cache template that is used by the isolated store.
- */
-template <class Base> class IsolatedStatsCache {
-public:
-  typedef std::function<std::shared_ptr<Base>(const std::string& name)> Allocator;
-
-  IsolatedStatsCache(Allocator alloc) : alloc_(alloc) {}
-
-  Base& get(const std::string& name) {
-    auto stat = stats_.find(name);
-    if (stat != stats_.end()) {
-      return *stat->second;
-    }
-
-    std::shared_ptr<Base> new_stat = alloc_(name);
-    stats_.emplace(name, new_stat);
-    return *new_stat;
-  }
-
-  std::vector<std::shared_ptr<Base>> toVector() const {
-    std::vector<std::shared_ptr<Base>> vec;
-    vec.reserve(stats_.size());
-    for (auto& stat : stats_) {
-      vec.push_back(stat.second);
-    }
-
-    return vec;
-  }
-
-private:
-  std::unordered_map<std::string, std::shared_ptr<Base>> stats_;
-  Allocator alloc_;
-};
-
-class IsolatedStoreImpl : public Store {
-public:
-  IsolatedStoreImpl();
-
-  // Stats::Scope
-  Counter& counter(const std::string& name) override { return counters_.get(name); }
-  ScopePtr createScope(const std::string& name) override;
-  void deliverHistogramToSinks(const Histogram&, uint64_t) override {}
-  Gauge& gauge(const std::string& name) override { return gauges_.get(name); }
-  Histogram& histogram(const std::string& name) override {
-    Histogram& histogram = histograms_.get(name);
-    return histogram;
-  }
-  const Stats::StatsOptions& statsOptions() const override { return stats_options_; }
-
-  // Stats::Store
-  std::vector<CounterSharedPtr> counters() const override { return counters_.toVector(); }
-  std::vector<GaugeSharedPtr> gauges() const override { return gauges_.toVector(); }
-  std::vector<ParentHistogramSharedPtr> histograms() const override {
-    return std::vector<ParentHistogramSharedPtr>{};
-  }
-
-private:
-  HeapStatDataAllocator alloc_;
-  IsolatedStatsCache<Counter> counters_;
-  IsolatedStatsCache<Gauge> gauges_;
-  IsolatedStatsCache<Histogram> histograms_;
-  const StatsOptionsImpl stats_options_;
-};
-
 } // namespace Stats
 } // namespace Envoy
diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD
index a1b75781dcdd..939f0f58b220 100644
--- a/source/common/upstream/BUILD
+++ b/source/common/upstream/BUILD
@@ -389,6 +389,7 @@ envoy_cc_library(
         "//source/common/common:minimal_logger_lib",
         "//source/common/config:metadata_lib",
         "//source/common/config:well_known_names",
+        "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//source/common/upstream:locality_lib",
         "@envoy_api//envoy/api/v2/core:base_cc",
diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h
index 90b7935d55d4..e06f065e9ee2 100644
--- a/source/common/upstream/upstream_impl.h
+++ b/source/common/upstream/upstream_impl.h
@@ -33,6 +33,7 @@
 #include "common/config/metadata.h"
 #include "common/config/well_known_names.h"
 #include "common/network/utility.h"
+#include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 #include "common/upstream/load_balancer_impl.h"
 #include "common/upstream/locality.h"
diff --git a/source/server/hot_restart_impl.cc b/source/server/hot_restart_impl.cc
index 80b4f2531307..84b01896bc90 100644
--- a/source/server/hot_restart_impl.cc
+++ b/source/server/hot_restart_impl.cc
@@ -18,6 +18,7 @@
 #include "common/common/lock_guard.h"
 #include "common/common/utility.h"
 #include "common/network/utility.h"
+#include "common/stats/raw_stat_data.h"
 
 #include "absl/strings/string_view.h"
 
diff --git a/source/server/hot_restart_impl.h b/source/server/hot_restart_impl.h
index c24c07b10904..710e3434c702 100644
--- a/source/server/hot_restart_impl.h
+++ b/source/server/hot_restart_impl.h
@@ -13,6 +13,7 @@
 
 #include "common/common/assert.h"
 #include "common/common/block_memory_hash_set.h"
+#include "common/stats/raw_stat_data.h"
 #include "common/stats/stats_impl.h"
 
 namespace Envoy {
diff --git a/source/server/http/BUILD b/source/server/http/BUILD
index 375f9a96c58d..19364ffca3b2 100644
--- a/source/server/http/BUILD
+++ b/source/server/http/BUILD
@@ -52,6 +52,7 @@ envoy_cc_library(
         "//source/common/network:utility_lib",
         "//source/common/profiler:profiler_lib",
         "//source/common/router:config_lib",
+        "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//source/common/upstream:host_utility_lib",
         "//source/extensions/access_loggers/file:file_access_log_lib",
diff --git a/source/server/http/admin.h b/source/server/http/admin.h
index b16a58e2b285..bcf3a4d22153 100644
--- a/source/server/http/admin.h
+++ b/source/server/http/admin.h
@@ -26,6 +26,7 @@
 #include "common/http/default_server_string.h"
 #include "common/http/utility.h"
 #include "common/network/raw_buffer_socket.h"
+#include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 
 #include "server/http/config_tracker_impl.h"
diff --git a/test/common/access_log/BUILD b/test/common/access_log/BUILD
index 6f080bd91a44..99aad705ac83 100644
--- a/test/common/access_log/BUILD
+++ b/test/common/access_log/BUILD
@@ -54,6 +54,7 @@ envoy_cc_test(
     srcs = ["access_log_manager_impl_test.cc"],
     deps = [
         "//source/common/access_log:access_log_manager_lib",
+        "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//test/mocks/access_log:access_log_mocks",
         "//test/mocks/api:api_mocks",
diff --git a/test/common/access_log/access_log_manager_impl_test.cc b/test/common/access_log/access_log_manager_impl_test.cc
index 063703707431..9f54272a20e8 100644
--- a/test/common/access_log/access_log_manager_impl_test.cc
+++ b/test/common/access_log/access_log_manager_impl_test.cc
@@ -1,6 +1,7 @@
 #include <memory>
 
 #include "common/access_log/access_log_manager_impl.h"
+#include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 
 #include "test/mocks/access_log/mocks.h"
diff --git a/test/common/filesystem/BUILD b/test/common/filesystem/BUILD
index 5db2ed03a0cb..8ae711154adb 100644
--- a/test/common/filesystem/BUILD
+++ b/test/common/filesystem/BUILD
@@ -17,6 +17,7 @@ envoy_cc_test(
         "//source/common/event:dispatcher_includes",
         "//source/common/event:dispatcher_lib",
         "//source/common/filesystem:filesystem_lib",
+        "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//test/mocks/api:api_mocks",
         "//test/mocks/event:event_mocks",
diff --git a/test/common/filesystem/filesystem_impl_test.cc b/test/common/filesystem/filesystem_impl_test.cc
index d078bc1807ff..5d159bb38ee2 100644
--- a/test/common/filesystem/filesystem_impl_test.cc
+++ b/test/common/filesystem/filesystem_impl_test.cc
@@ -6,6 +6,7 @@
 #include "common/common/thread.h"
 #include "common/event/dispatcher_impl.h"
 #include "common/filesystem/filesystem_impl.h"
+#include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 
 #include "test/mocks/api/mocks.h"
diff --git a/test/common/grpc/BUILD b/test/common/grpc/BUILD
index 87a75d94944f..3294b133c88a 100644
--- a/test/common/grpc/BUILD
+++ b/test/common/grpc/BUILD
@@ -61,6 +61,7 @@ envoy_cc_test(
     srcs = envoy_select_google_grpc(["google_async_client_impl_test.cc"]),
     deps = [
         "//source/common/event:dispatcher_lib",
+        "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//source/common/tracing:http_tracer_lib",
         "//test/mocks/grpc:grpc_mocks",
diff --git a/test/common/grpc/google_async_client_impl_test.cc b/test/common/grpc/google_async_client_impl_test.cc
index 3f69613832ae..27077dd86318 100644
--- a/test/common/grpc/google_async_client_impl_test.cc
+++ b/test/common/grpc/google_async_client_impl_test.cc
@@ -1,5 +1,6 @@
 #include "common/event/dispatcher_impl.h"
 #include "common/grpc/google_async_client_impl.h"
+#include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 
 #include "test/mocks/grpc/mocks.h"
diff --git a/test/common/runtime/BUILD b/test/common/runtime/BUILD
index e9452ff24b93..a170485d54bd 100644
--- a/test/common/runtime/BUILD
+++ b/test/common/runtime/BUILD
@@ -21,6 +21,7 @@ envoy_cc_test(
     data = glob(["test_data/**"]) + ["filesystem_setup.sh"],
     deps = [
         "//source/common/runtime:runtime_lib",
+        "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//test/mocks/api:api_mocks",
         "//test/mocks/event:event_mocks",
diff --git a/test/common/runtime/runtime_impl_test.cc b/test/common/runtime/runtime_impl_test.cc
index f908c516c5bc..bd7bedd31d0c 100644
--- a/test/common/runtime/runtime_impl_test.cc
+++ b/test/common/runtime/runtime_impl_test.cc
@@ -2,6 +2,7 @@
 #include <string>
 
 #include "common/runtime/runtime_impl.h"
+#include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 
 #include "test/mocks/api/mocks.h"
diff --git a/test/common/ssl/BUILD b/test/common/ssl/BUILD
index 8a9265f691da..70a45354264f 100644
--- a/test/common/ssl/BUILD
+++ b/test/common/ssl/BUILD
@@ -30,6 +30,7 @@ envoy_cc_test(
         "//source/common/ssl:context_config_lib",
         "//source/common/ssl:context_lib",
         "//source/common/ssl:ssl_socket_lib",
+        "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//source/extensions/filters/listener/tls_inspector:tls_inspector_lib",
         "//test/mocks/buffer:buffer_mocks",
@@ -59,6 +60,7 @@ envoy_cc_test(
         "//source/common/secret:secret_manager_impl_lib",
         "//source/common/ssl:context_config_lib",
         "//source/common/ssl:context_lib",
+        "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//test/mocks/runtime:runtime_mocks",
         "//test/mocks/secret:secret_mocks",
diff --git a/test/common/ssl/context_impl_test.cc b/test/common/ssl/context_impl_test.cc
index 8efaad4f499f..e6b4ad265347 100644
--- a/test/common/ssl/context_impl_test.cc
+++ b/test/common/ssl/context_impl_test.cc
@@ -5,6 +5,7 @@
 #include "common/secret/secret_manager_impl.h"
 #include "common/ssl/context_config_impl.h"
 #include "common/ssl/context_impl.h"
+#include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 
 #include "test/common/ssl/ssl_certs_test.h"
diff --git a/test/extensions/filters/network/redis_proxy/BUILD b/test/extensions/filters/network/redis_proxy/BUILD
index 0ac33ba5eee5..5044e36884c7 100644
--- a/test/extensions/filters/network/redis_proxy/BUILD
+++ b/test/extensions/filters/network/redis_proxy/BUILD
@@ -31,6 +31,7 @@ envoy_extension_cc_test(
     extension_name = "envoy.filters.network.redis_proxy",
     deps = [
         ":redis_mocks",
+        "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//source/extensions/filters/network/redis_proxy:command_splitter_lib",
         "//test/mocks:common_lib",
diff --git a/test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc b/test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc
index 3287e5b2e8e6..0e3d6533187b 100644
--- a/test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc
+++ b/test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc
@@ -4,6 +4,7 @@
 #include <vector>
 
 #include "common/common/fmt.h"
+#include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 
 #include "extensions/filters/network/redis_proxy/command_splitter_impl.h"
diff --git a/test/integration/BUILD b/test/integration/BUILD
index 034d9682e084..fbcdf003a5c1 100644
--- a/test/integration/BUILD
+++ b/test/integration/BUILD
@@ -278,6 +278,7 @@ envoy_cc_test_library(
         "//source/common/network:filter_lib",
         "//source/common/network:listen_socket_lib",
         "//source/common/network:utility_lib",
+        "//source/common/stats:isolated_store_lib",
         "//source/common/stats:thread_local_store_lib",
         "//source/common/thread_local:thread_local_lib",
         "//source/common/upstream:upstream_includes",
diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h
index 87bc4cfe022f..73bb43b5986e 100644
--- a/test/integration/fake_upstream.h
+++ b/test/integration/fake_upstream.h
@@ -24,6 +24,7 @@
 #include "common/grpc/common.h"
 #include "common/network/filter_impl.h"
 #include "common/network/listen_socket_impl.h"
+#include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 
 #include "test/test_common/printers.h"
diff --git a/test/mocks/network/BUILD b/test/mocks/network/BUILD
index 6680d5a6bbf8..665047ccbb16 100644
--- a/test/mocks/network/BUILD
+++ b/test/mocks/network/BUILD
@@ -22,6 +22,7 @@ envoy_cc_mock(
         "//include/envoy/server:listener_manager_interface",
         "//source/common/network:address_lib",
         "//source/common/network:utility_lib",
+        "//source/common/stats:isolated_store_lib",
         "//test/mocks/event:event_mocks",
         "//test/test_common:printers_lib",
         "@envoy_api//envoy/api/v2/core:base_cc",
diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h
index e5e820d4e036..b96cca9fdc00 100644
--- a/test/mocks/network/mocks.h
+++ b/test/mocks/network/mocks.h
@@ -12,6 +12,7 @@
 #include "envoy/network/resolver.h"
 #include "envoy/network/transport_socket.h"
 
+#include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 
 #include "test/mocks/event/mocks.h"
diff --git a/test/mocks/stats/BUILD b/test/mocks/stats/BUILD
index b119f793bf24..0c38afb4a3ac 100644
--- a/test/mocks/stats/BUILD
+++ b/test/mocks/stats/BUILD
@@ -17,6 +17,7 @@ envoy_cc_mock(
         "//include/envoy/stats:timespan",
         "//include/envoy/thread_local:thread_local_interface",
         "//include/envoy/upstream:cluster_manager_interface",
+        "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//test/mocks:common_lib",
     ],
diff --git a/test/mocks/stats/mocks.h b/test/mocks/stats/mocks.h
index c98fa2758f1b..ef844a3ec815 100644
--- a/test/mocks/stats/mocks.h
+++ b/test/mocks/stats/mocks.h
@@ -10,6 +10,7 @@
 #include "envoy/thread_local/thread_local.h"
 #include "envoy/upstream/cluster_manager.h"
 
+#include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 
 #include "gmock/gmock.h"
diff --git a/test/test_common/utility.h b/test/test_common/utility.h
index 04bca5ca951c..ad1c983cd722 100644
--- a/test/test_common/utility.h
+++ b/test/test_common/utility.h
@@ -16,6 +16,7 @@
 #include "common/common/c_smart_ptr.h"
 #include "common/http/header_map_impl.h"
 #include "common/protobuf/utility.h"
+#include "common/stats/raw_stat_data.h"
 #include "common/stats/stats_impl.h"
 
 #include "test/test_common/printers.h"

From 9c1093121a6483599341f3c85240b09ad1dc9c0f Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 31 Jul 2018 22:33:23 -0400
Subject: [PATCH 11/14] Split out HeapStatData and MetricsImpl, moving the
 allocator template to its own file.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 source/common/stats/BUILD                 |  34 +++++
 source/common/stats/heap_stat_data.cc     |  50 ++++++++
 source/common/stats/heap_stat_data.h      |  73 +++++++++++
 source/common/stats/isolated_store_impl.h |   1 +
 source/common/stats/metric_impl.h         |  39 ++++++
 source/common/stats/raw_stat_data.cc      |   2 +
 source/common/stats/raw_stat_data.h       |   7 ++
 source/common/stats/stat_data_allocator.h | 147 ++++++++++++++++++++++
 source/common/stats/stats_impl.cc         | 128 -------------------
 source/common/stats/stats_impl.h          | 132 +------------------
 source/common/stats/thread_local_store.h  |   1 +
 test/common/stats/BUILD                   |   1 +
 test/common/stats/stats_impl_test.cc      |   1 +
 13 files changed, 357 insertions(+), 259 deletions(-)
 create mode 100644 source/common/stats/heap_stat_data.cc
 create mode 100644 source/common/stats/heap_stat_data.h
 create mode 100644 source/common/stats/metric_impl.h
 create mode 100644 source/common/stats/stat_data_allocator.h

diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD
index 7ceefbb501d7..ab7408ff5b79 100644
--- a/source/common/stats/BUILD
+++ b/source/common/stats/BUILD
@@ -8,26 +8,58 @@ load(
 
 envoy_package()
 
+envoy_cc_library(
+    name = "heap_stat_data_lib",
+    srcs = ["heap_stat_data.cc"],
+    hdrs = ["heap_stat_data.h"],
+    deps = [
+        ":stat_data_allocator_lib",
+        "//source/common/common:assert_lib",
+        "//source/common/common:hash_lib",
+        "//source/common/stats:stats_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "isolated_store_lib",
     srcs = ["isolated_store_impl.cc"],
     hdrs = ["isolated_store_impl.h"],
     deps = [
+        "//source/common/stats:heap_stat_data_lib",
         "//source/common/stats:stats_lib",
     ],
 )
 
+envoy_cc_library(
+    name = "metric_impl_lib",
+    hdrs = ["metric_impl.h"],
+    deps = [
+        "//include/envoy/stats:stats_interface",
+    ],
+)
+
 envoy_cc_library(
     name = "raw_stat_data_lib",
     srcs = ["raw_stat_data.cc"],
     hdrs = ["raw_stat_data.h"],
     deps = [
+        ":stat_data_allocator_lib",
         "//include/envoy/stats:stats_interface",
         "//source/common/common:assert_lib",
         "//source/common/common:hash_lib",
     ],
 )
 
+envoy_cc_library(
+    name = "stat_data_allocator_lib",
+    hdrs = ["stat_data_allocator.h"],
+    deps = [
+        ":metric_impl_lib",
+        "//include/envoy/stats:stats_interface",
+        "//source/common/common:assert_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "stats_lib",
     srcs = ["stats_impl.cc"],
@@ -37,6 +69,7 @@ envoy_cc_library(
         "libcircllhist",
     ],
     deps = [
+        ":metric_impl_lib",
         ":raw_stat_data_lib",
         ":stats_options_lib",
         ":tag_extractor_lib",
@@ -93,6 +126,7 @@ envoy_cc_library(
     srcs = ["thread_local_store.cc"],
     hdrs = ["thread_local_store.h"],
     deps = [
+        ":heap_stat_data_lib",
         ":stats_lib",
         ":tag_producer_lib",
         "//include/envoy/thread_local:thread_local_interface",
diff --git a/source/common/stats/heap_stat_data.cc b/source/common/stats/heap_stat_data.cc
new file mode 100644
index 000000000000..6921baaa6d61
--- /dev/null
+++ b/source/common/stats/heap_stat_data.cc
@@ -0,0 +1,50 @@
+#include "common/stats/heap_stat_data.h"
+
+#include "common/common/lock_guard.h"
+#include "common/common/thread.h"
+#include "common/stats/stats_impl.h"
+
+namespace Envoy {
+namespace Stats {
+
+HeapStatData::HeapStatData(absl::string_view key) : name_(key.data(), key.size()) {}
+
+HeapStatDataAllocator::HeapStatDataAllocator() {}
+
+HeapStatDataAllocator::~HeapStatDataAllocator() { ASSERT(stats_.empty()); }
+
+HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {
+  // Any expected truncation of name is done at the callsite. No truncation is
+  // required to use this allocator.
+  auto data = std::make_unique<HeapStatData>(name);
+  Thread::ReleasableLockGuard lock(mutex_);
+  auto ret = stats_.insert(data.get());
+  HeapStatData* existing_data = *ret.first;
+  lock.release();
+
+  if (ret.second) {
+    return data.release();
+  }
+  ++existing_data->ref_count_;
+  return existing_data;
+}
+
+void HeapStatDataAllocator::free(HeapStatData& data) {
+  ASSERT(data.ref_count_ > 0);
+  if (--data.ref_count_ > 0) {
+    return;
+  }
+
+  {
+    Thread::LockGuard lock(mutex_);
+    size_t key_removed = stats_.erase(&data);
+    ASSERT(key_removed == 1);
+  }
+
+  delete &data;
+}
+
+template class StatDataAllocatorImpl<HeapStatData>;
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/heap_stat_data.h b/source/common/stats/heap_stat_data.h
new file mode 100644
index 000000000000..04bbde4e289f
--- /dev/null
+++ b/source/common/stats/heap_stat_data.h
@@ -0,0 +1,73 @@
+#pragma once
+
+#include <cstdint>
+#include <string>
+#include <unordered_set>
+
+#include "common/stats/stat_data_allocator.h"
+#include "common/stats/stats_impl.h"
+
+namespace Envoy {
+namespace Stats {
+
+/**
+ * This structure is an alternate backing store for both CounterImpl and GaugeImpl. It is designed
+ * so that it can be allocated efficiently from the heap on demand.
+ */
+struct HeapStatData {
+  explicit HeapStatData(absl::string_view key);
+
+  /**
+   * @returns absl::string_view the name as a string_view.
+   */
+  absl::string_view key() const { return name_; }
+
+  std::atomic<uint64_t> value_{0};
+  std::atomic<uint64_t> pending_increment_{0};
+  std::atomic<uint16_t> flags_{0};
+  std::atomic<uint16_t> ref_count_{1};
+  std::string name_;
+};
+
+/**
+ * Implementation of StatDataAllocator using a pure heap-based strategy, so that
+ * Envoy implementations that do not require hot-restart can use less memory.
+ */
+class HeapStatDataAllocator : public StatDataAllocatorImpl<HeapStatData> {
+public:
+  HeapStatDataAllocator();
+  ~HeapStatDataAllocator();
+
+  // StatDataAllocatorImpl
+  HeapStatData* alloc(absl::string_view name) override;
+  void free(HeapStatData& data) override;
+
+  // StatDataAllocator
+  bool requiresBoundedStatNameSize() const override { return false; }
+
+private:
+  struct HeapStatHash_ {
+    size_t operator()(const HeapStatData* a) const { return HashUtil::xxHash64(a->key()); }
+  };
+  struct HeapStatCompare_ {
+    bool operator()(const HeapStatData* a, const HeapStatData* b) const {
+      return (a->key() == b->key());
+    }
+  };
+
+  // TODO(jmarantz): See https://github.com/envoyproxy/envoy/pull/3927 and
+  //  https://github.com/envoyproxy/envoy/issues/3585, which can help reorganize
+  // the heap stats using a ref-counted symbol table to compress the stat strings.
+  typedef std::unordered_set<HeapStatData*, HeapStatHash_, HeapStatCompare_> StatSet;
+
+  // An unordered set of HeapStatData pointers which keys off the key()
+  // field in each object. This necessitates a custom comparator and hasher.
+  StatSet stats_ GUARDED_BY(mutex_);
+  // A mutex is needed here to protect the stats_ object from both alloc() and free() operations.
+  // Although alloc() operations are called under existing locking, free() operations are made from
+  // the destructors of the individual stat objects, which are not protected by locks.
+  Thread::MutexBasicLockable mutex_;
+};
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h
index 344c62153d0c..13c6beaed6b2 100644
--- a/source/common/stats/isolated_store_impl.h
+++ b/source/common/stats/isolated_store_impl.h
@@ -6,6 +6,7 @@
 #include <string>
 
 #include "common/common/utility.h"
+#include "common/stats/heap_stat_data.h"
 #include "common/stats/stats_impl.h"
 #include "common/stats/utility.h"
 
diff --git a/source/common/stats/metric_impl.h b/source/common/stats/metric_impl.h
new file mode 100644
index 000000000000..34ea7ed83037
--- /dev/null
+++ b/source/common/stats/metric_impl.h
@@ -0,0 +1,39 @@
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "envoy/stats/stats.h"
+
+namespace Envoy {
+namespace Stats {
+
+/**
+ * Implementation of the Metric interface. Virtual inheritance is used because the interfaces that
+ * will inherit from Metric will have other base classes that will also inherit from Metric.
+ */
+class MetricImpl : public virtual Metric {
+public:
+  MetricImpl(const std::string& name, std::string&& tag_extracted_name, std::vector<Tag>&& tags)
+      : name_(name), tag_extracted_name_(std::move(tag_extracted_name)), tags_(std::move(tags)) {}
+
+  const std::string& name() const override { return name_; }
+  const std::string& tagExtractedName() const override { return tag_extracted_name_; }
+  const std::vector<Tag>& tags() const override { return tags_; }
+
+protected:
+  /**
+   * Flags used by all stats types to figure out whether they have been used.
+   */
+  struct Flags {
+    static const uint8_t Used = 0x1;
+  };
+
+private:
+  const std::string name_;
+  const std::string tag_extracted_name_;
+  const std::vector<Tag> tags_;
+};
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/raw_stat_data.cc b/source/common/stats/raw_stat_data.cc
index e62549dbedbe..dc03bba090c6 100644
--- a/source/common/stats/raw_stat_data.cc
+++ b/source/common/stats/raw_stat_data.cc
@@ -41,5 +41,7 @@ void RawStatData::initialize(absl::string_view key, const StatsOptions& stats_op
   name_[key.size()] = '\0';
 }
 
+template class StatDataAllocatorImpl<RawStatData>;
+
 } // namespace Stats
 } // namespace Envoy
diff --git a/source/common/stats/raw_stat_data.h b/source/common/stats/raw_stat_data.h
index 34013180c87f..0cb6dac4f81c 100644
--- a/source/common/stats/raw_stat_data.h
+++ b/source/common/stats/raw_stat_data.h
@@ -15,6 +15,7 @@
 
 #include "common/common/assert.h"
 #include "common/common/hash.h"
+#include "common/stats/stat_data_allocator.h"
 
 #include "absl/strings/string_view.h"
 
@@ -80,5 +81,11 @@ struct RawStatData {
   char name_[];
 };
 
+class RawStatDataAllocator : public StatDataAllocatorImpl<RawStatData> {
+public:
+  // StatDataAllocator
+  bool requiresBoundedStatNameSize() const override { return true; }
+};
+
 } // namespace Stats
 } // namespace Envoy
diff --git a/source/common/stats/stat_data_allocator.h b/source/common/stats/stat_data_allocator.h
new file mode 100644
index 000000000000..74ffea4af2e8
--- /dev/null
+++ b/source/common/stats/stat_data_allocator.h
@@ -0,0 +1,147 @@
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "envoy/stats/stats.h"
+
+#include "common/common/assert.h"
+#include "common/stats/metric_impl.h"
+
+#include "absl/strings/string_view.h"
+
+namespace Envoy {
+namespace Stats {
+
+// Partially implements a StatDataAllocator, leaving alloc & free for subclasses.
+// We templatize on StatData rather than defining a virtual base StatData class
+// for performance reasons; stat increment is on the hot path.
+//
+// The two production derivations cover using a fixed block of shared-memory for
+// hot restart stat continuity, and heap allocation for more efficient RAM usage
+// for when hot-restart is not required.
+//
+// Also note that RawStatData needs to live in a shared memory block, and it's
+// possible, but not obvious, that a vptr would be usable across processes. In
+// any case, RawStatData is allocated from a shared-memory block rather than via
+// new, so the usual C++ compiler assistance for setting up vptrs will not be
+// available. This could be resolved with placed new, or another nesting level.
+template <class StatData> class StatDataAllocatorImpl : public StatDataAllocator {
+public:
+  // StatDataAllocator
+  CounterSharedPtr makeCounter(absl::string_view name, std::string&& tag_extracted_name,
+                               std::vector<Tag>&& tags) override;
+  GaugeSharedPtr makeGauge(absl::string_view name, std::string&& tag_extracted_name,
+                           std::vector<Tag>&& tags) override;
+
+  /**
+   * @param name the full name of the stat.
+   * @return StatData* a data block for a given stat name or nullptr if there is no more memory
+   *         available for stats. The allocator should return a reference counted data location
+   *         by name if one already exists with the same name. This is used for intra-process
+   *         scope swapping as well as inter-process hot restart.
+   */
+  virtual StatData* alloc(absl::string_view name) PURE;
+
+  /**
+   * Free a raw stat data block. The allocator should handle reference counting and only truly
+   * free the block if it is no longer needed.
+   * @param data the data returned by alloc().
+   */
+  virtual void free(StatData& data) PURE;
+};
+
+/**
+ * Counter implementation that wraps a StatData. StatData must have data members:
+ *    std::atomic<int64_t> value_;
+ *    std::atomic<int64_t> pending_increment_;
+ *    std::atomic<int16_t> flags_;
+ *    std::atomic<int16_t> ref_count_;
+ */
+template <class StatData> class CounterImpl : public Counter, public MetricImpl {
+public:
+  CounterImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
+              std::string&& tag_extracted_name, std::vector<Tag>&& tags)
+      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
+        alloc_(alloc) {}
+  ~CounterImpl() { alloc_.free(data_); }
+
+  // Stats::Counter
+  void add(uint64_t amount) override {
+    data_.value_ += amount;
+    data_.pending_increment_ += amount;
+    data_.flags_ |= Flags::Used;
+  }
+
+  void inc() override { add(1); }
+  uint64_t latch() override { return data_.pending_increment_.exchange(0); }
+  void reset() override { data_.value_ = 0; }
+  bool used() const override { return data_.flags_ & Flags::Used; }
+  uint64_t value() const override { return data_.value_; }
+
+private:
+  StatData& data_;
+  StatDataAllocatorImpl<StatData>& alloc_;
+};
+
+/**
+ * Gauge implementation that wraps a StatData.
+ */
+template <class StatData> class GaugeImpl : public Gauge, public MetricImpl {
+public:
+  GaugeImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
+            std::string&& tag_extracted_name, std::vector<Tag>&& tags)
+      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
+        alloc_(alloc) {}
+  ~GaugeImpl() { alloc_.free(data_); }
+
+  // Stats::Gauge
+  virtual void add(uint64_t amount) override {
+    data_.value_ += amount;
+    data_.flags_ |= Flags::Used;
+  }
+  virtual void dec() override { sub(1); }
+  virtual void inc() override { add(1); }
+  virtual void set(uint64_t value) override {
+    data_.value_ = value;
+    data_.flags_ |= Flags::Used;
+  }
+  virtual void sub(uint64_t amount) override {
+    ASSERT(data_.value_ >= amount);
+    ASSERT(used());
+    data_.value_ -= amount;
+  }
+  virtual uint64_t value() const override { return data_.value_; }
+  bool used() const override { return data_.flags_ & Flags::Used; }
+
+private:
+  StatData& data_;
+  StatDataAllocatorImpl<StatData>& alloc_;
+};
+
+template <class StatData>
+CounterSharedPtr StatDataAllocatorImpl<StatData>::makeCounter(absl::string_view name,
+                                                              std::string&& tag_extracted_name,
+                                                              std::vector<Tag>&& tags) {
+  StatData* data = alloc(name);
+  if (data == nullptr) {
+    return nullptr;
+  }
+  return std::make_shared<CounterImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
+                                                 std::move(tags));
+}
+
+template <class StatData>
+GaugeSharedPtr StatDataAllocatorImpl<StatData>::makeGauge(absl::string_view name,
+                                                          std::string&& tag_extracted_name,
+                                                          std::vector<Tag>&& tags) {
+  StatData* data = alloc(name);
+  if (data == nullptr) {
+    return nullptr;
+  }
+  return std::make_shared<GaugeImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
+                                               std::move(tags));
+}
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc
index 6a2a6c3a2e84..c20a23527139 100644
--- a/source/common/stats/stats_impl.cc
+++ b/source/common/stats/stats_impl.cc
@@ -21,131 +21,6 @@
 namespace Envoy {
 namespace Stats {
 
-/**
- * Counter implementation that wraps a StatData. StatData must have data members:
- *    std::atomic<int64_t> value_;
- *    std::atomic<int64_t> pending_increment_;
- *    std::atomic<int16_t> flags_;
- *    std::atomic<int16_t> ref_count_;
- */
-template <class StatData> class CounterImpl : public Counter, public MetricImpl {
-public:
-  CounterImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
-              std::string&& tag_extracted_name, std::vector<Tag>&& tags)
-      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
-        alloc_(alloc) {}
-  ~CounterImpl() { alloc_.free(data_); }
-
-  // Stats::Counter
-  void add(uint64_t amount) override {
-    data_.value_ += amount;
-    data_.pending_increment_ += amount;
-    data_.flags_ |= Flags::Used;
-  }
-
-  void inc() override { add(1); }
-  uint64_t latch() override { return data_.pending_increment_.exchange(0); }
-  void reset() override { data_.value_ = 0; }
-  bool used() const override { return data_.flags_ & Flags::Used; }
-  uint64_t value() const override { return data_.value_; }
-
-private:
-  StatData& data_;
-  StatDataAllocatorImpl<StatData>& alloc_;
-};
-
-/**
- * Gauge implementation that wraps a StatData.
- */
-template <class StatData> class GaugeImpl : public Gauge, public MetricImpl {
-public:
-  GaugeImpl(StatData& data, StatDataAllocatorImpl<StatData>& alloc,
-            std::string&& tag_extracted_name, std::vector<Tag>&& tags)
-      : MetricImpl(data.name_, std::move(tag_extracted_name), std::move(tags)), data_(data),
-        alloc_(alloc) {}
-  ~GaugeImpl() { alloc_.free(data_); }
-
-  // Stats::Gauge
-  virtual void add(uint64_t amount) override {
-    data_.value_ += amount;
-    data_.flags_ |= Flags::Used;
-  }
-  virtual void dec() override { sub(1); }
-  virtual void inc() override { add(1); }
-  virtual void set(uint64_t value) override {
-    data_.value_ = value;
-    data_.flags_ |= Flags::Used;
-  }
-  virtual void sub(uint64_t amount) override {
-    ASSERT(data_.value_ >= amount);
-    ASSERT(used());
-    data_.value_ -= amount;
-  }
-  virtual uint64_t value() const override { return data_.value_; }
-  bool used() const override { return data_.flags_ & Flags::Used; }
-
-private:
-  StatData& data_;
-  StatDataAllocatorImpl<StatData>& alloc_;
-};
-
-HeapStatData::HeapStatData(absl::string_view key) : name_(key.data(), key.size()) {}
-
-template <class StatData>
-CounterSharedPtr StatDataAllocatorImpl<StatData>::makeCounter(absl::string_view name,
-                                                              std::string&& tag_extracted_name,
-                                                              std::vector<Tag>&& tags) {
-  StatData* data = alloc(name);
-  if (data == nullptr) {
-    return nullptr;
-  }
-  return std::make_shared<CounterImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
-                                                 std::move(tags));
-}
-
-template <class StatData>
-GaugeSharedPtr StatDataAllocatorImpl<StatData>::makeGauge(absl::string_view name,
-                                                          std::string&& tag_extracted_name,
-                                                          std::vector<Tag>&& tags) {
-  StatData* data = alloc(name);
-  if (data == nullptr) {
-    return nullptr;
-  }
-  return std::make_shared<GaugeImpl<StatData>>(*data, *this, std::move(tag_extracted_name),
-                                               std::move(tags));
-}
-
-HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {
-  // Any expected truncation of name is done at the callsite. No truncation is
-  // required to use this allocator.
-  auto data = std::make_unique<HeapStatData>(name);
-  Thread::ReleasableLockGuard lock(mutex_);
-  auto ret = stats_.insert(data.get());
-  HeapStatData* existing_data = *ret.first;
-  lock.release();
-
-  if (ret.second) {
-    return data.release();
-  }
-  ++existing_data->ref_count_;
-  return existing_data;
-}
-
-void HeapStatDataAllocator::free(HeapStatData& data) {
-  ASSERT(data.ref_count_ > 0);
-  if (--data.ref_count_ > 0) {
-    return;
-  }
-
-  {
-    Thread::LockGuard lock(mutex_);
-    size_t key_removed = stats_.erase(&data);
-    ASSERT(key_removed == 1);
-  }
-
-  delete &data;
-}
-
 HistogramStatisticsImpl::HistogramStatisticsImpl(const histogram_t* histogram_ptr)
     : computed_quantiles_(supportedQuantiles().size(), 0.0) {
   hist_approx_quantile(histogram_ptr, supportedQuantiles().data(), supportedQuantiles().size(),
@@ -204,8 +79,5 @@ void SourceImpl::clearCache() {
   histograms_.reset();
 }
 
-template class StatDataAllocatorImpl<HeapStatData>;
-template class StatDataAllocatorImpl<RawStatData>;
-
 } // namespace Stats
 } // namespace Envoy
diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h
index f37c81c8ab0f..f3a2f5ce7535 100644
--- a/source/common/stats/stats_impl.h
+++ b/source/common/stats/stats_impl.h
@@ -1,7 +1,6 @@
 #pragma once
 
 #include <algorithm>
-#include <atomic>
 #include <chrono>
 #include <cstdint>
 #include <functional>
@@ -23,6 +22,7 @@
 #include "common/common/thread_annotations.h"
 #include "common/common/utility.h"
 #include "common/protobuf/protobuf.h"
+#include "common/stats/metric_impl.h"
 #include "common/stats/stats_options_impl.h"
 
 #include "absl/strings/str_join.h"
@@ -33,96 +33,6 @@
 namespace Envoy {
 namespace Stats {
 
-// Partially implements a StatDataAllocator, leaving alloc & free for subclasses.
-// We templatize on StatData rather than defining a virtual base StatData class
-// for performance reasons; stat increment is on the hot path.
-//
-// The two production derivations cover using a fixed block of shared-memory for
-// hot restart stat continuity, and heap allocation for more efficient RAM usage
-// for when hot-restart is not required.
-//
-// Also note that RawStatData needs to live in a shared memory block, and it's
-// possible, but not obvious, that a vptr would be usable across processes. In
-// any case, RawStatData is allocated from a shared-memory block rather than via
-// new, so the usual C++ compiler assistance for setting up vptrs will not be
-// available. This could be resolved with placed new, or another nesting level.
-template <class StatData> class StatDataAllocatorImpl : public StatDataAllocator {
-public:
-  // StatDataAllocator
-  CounterSharedPtr makeCounter(absl::string_view name, std::string&& tag_extracted_name,
-                               std::vector<Tag>&& tags) override;
-  GaugeSharedPtr makeGauge(absl::string_view name, std::string&& tag_extracted_name,
-                           std::vector<Tag>&& tags) override;
-
-  /**
-   * @param name the full name of the stat.
-   * @return StatData* a data block for a given stat name or nullptr if there is no more memory
-   *         available for stats. The allocator should return a reference counted data location
-   *         by name if one already exists with the same name. This is used for intra-process
-   *         scope swapping as well as inter-process hot restart.
-   */
-  virtual StatData* alloc(absl::string_view name) PURE;
-
-  /**
-   * Free a raw stat data block. The allocator should handle reference counting and only truly
-   * free the block if it is no longer needed.
-   * @param data the data returned by alloc().
-   */
-  virtual void free(StatData& data) PURE;
-};
-
-class RawStatDataAllocator : public StatDataAllocatorImpl<RawStatData> {
-public:
-  // StatDataAllocator
-  bool requiresBoundedStatNameSize() const override { return true; }
-};
-
-/**
- * This structure is an alternate backing store for both CounterImpl and GaugeImpl. It is designed
- * so that it can be allocated efficiently from the heap on demand.
- */
-struct HeapStatData {
-  explicit HeapStatData(absl::string_view key);
-
-  /**
-   * @returns absl::string_view the name as a string_view.
-   */
-  absl::string_view key() const { return name_; }
-
-  std::atomic<uint64_t> value_{0};
-  std::atomic<uint64_t> pending_increment_{0};
-  std::atomic<uint16_t> flags_{0};
-  std::atomic<uint16_t> ref_count_{1};
-  std::string name_;
-};
-
-/**
- * Implementation of the Metric interface. Virtual inheritance is used because the interfaces that
- * will inherit from Metric will have other base classes that will also inherit from Metric.
- */
-class MetricImpl : public virtual Metric {
-public:
-  MetricImpl(const std::string& name, std::string&& tag_extracted_name, std::vector<Tag>&& tags)
-      : name_(name), tag_extracted_name_(std::move(tag_extracted_name)), tags_(std::move(tags)) {}
-
-  const std::string& name() const override { return name_; }
-  const std::string& tagExtractedName() const override { return tag_extracted_name_; }
-  const std::vector<Tag>& tags() const override { return tags_; }
-
-protected:
-  /**
-   * Flags used by all stats types to figure out whether they have been used.
-   */
-  struct Flags {
-    static const uint8_t Used = 0x1;
-  };
-
-private:
-  const std::string name_;
-  const std::string tag_extracted_name_;
-  const std::vector<Tag> tags_;
-};
-
 /**
  * Implementation of HistogramStatistics for circllhist.
  */
@@ -183,45 +93,5 @@ class SourceImpl : public Source {
   absl::optional<std::vector<ParentHistogramSharedPtr>> histograms_;
 };
 
-/**
- * Implementation of StatDataAllocator using a pure heap-based strategy, so that
- * Envoy implementations that do not require hot-restart can use less memory.
- */
-class HeapStatDataAllocator : public StatDataAllocatorImpl<HeapStatData> {
-public:
-  HeapStatDataAllocator() {}
-  ~HeapStatDataAllocator() { ASSERT(stats_.empty()); }
-
-  // StatDataAllocatorImpl
-  HeapStatData* alloc(absl::string_view name) override;
-  void free(HeapStatData& data) override;
-
-  // StatDataAllocator
-  bool requiresBoundedStatNameSize() const override { return false; }
-
-private:
-  struct HeapStatHash_ {
-    size_t operator()(const HeapStatData* a) const { return HashUtil::xxHash64(a->key()); }
-  };
-  struct HeapStatCompare_ {
-    bool operator()(const HeapStatData* a, const HeapStatData* b) const {
-      return (a->key() == b->key());
-    }
-  };
-
-  // TODO(jmarantz): See https://github.com/envoyproxy/envoy/pull/3927 and
-  //  https://github.com/envoyproxy/envoy/issues/3585, which can help reorganize
-  // the heap stats using a ref-counted symbol table to compress the stat strings.
-  typedef std::unordered_set<HeapStatData*, HeapStatHash_, HeapStatCompare_> StatSet;
-
-  // An unordered set of HeapStatData pointers which keys off the key()
-  // field in each object. This necessitates a custom comparator and hasher.
-  StatSet stats_ GUARDED_BY(mutex_);
-  // A mutex is needed here to protect the stats_ object from both alloc() and free() operations.
-  // Although alloc() operations are called under existing locking, free() operations are made from
-  // the destructors of the individual stat objects, which are not protected by locks.
-  Thread::MutexBasicLockable mutex_;
-};
-
 } // namespace Stats
 } // namespace Envoy
diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h
index 2b3a843282c0..d5d616fe99b0 100644
--- a/source/common/stats/thread_local_store.h
+++ b/source/common/stats/thread_local_store.h
@@ -10,6 +10,7 @@
 
 #include "envoy/thread_local/thread_local.h"
 
+#include "common/stats/heap_stat_data.h"
 #include "common/stats/stats_impl.h"
 #include "common/stats/utility.h"
 
diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD
index a157fb6cdad8..88914db802f4 100644
--- a/test/common/stats/BUILD
+++ b/test/common/stats/BUILD
@@ -13,6 +13,7 @@ envoy_cc_test(
     srcs = ["stats_impl_test.cc"],
     deps = [
         "//source/common/common:hex_lib",
+        "//source/common/stats:heap_stat_data_lib",
         "//source/common/stats:stats_lib",
         "//test/mocks/stats:stats_mocks",
         "//test/test_common:logging_lib",
diff --git a/test/common/stats/stats_impl_test.cc b/test/common/stats/stats_impl_test.cc
index 11493d2b0c99..85486e20b989 100644
--- a/test/common/stats/stats_impl_test.cc
+++ b/test/common/stats/stats_impl_test.cc
@@ -7,6 +7,7 @@
 
 #include "common/common/hex.h"
 #include "common/config/well_known_names.h"
+#include "common/stats/heap_stat_data.h"
 #include "common/stats/stats_impl.h"
 #include "common/stats/tag_extractor_impl.h"
 #include "common/stats/tag_producer_impl.h"

From 64e8c9c2cd1b8b900a0cf5f9f17819482ce2da68 Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 31 Jul 2018 23:34:49 -0400
Subject: [PATCH 12/14] Pull out HistogramImpl and HistogramStatisticsImpl into
 their own file.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 include/envoy/stats/stats.h                   |  1 +
 source/common/stats/BUILD                     | 28 ++++++--
 source/common/stats/heap_stat_data.cc         |  1 -
 source/common/stats/heap_stat_data.h          |  4 +-
 source/common/stats/histogram.cc              | 49 ++++++++++++++
 source/common/stats/histogram.h               | 60 +++++++++++++++++
 source/common/stats/isolated_store_impl.cc    |  1 +
 source/common/stats/isolated_store_impl.h     |  1 +
 source/common/stats/stats_impl.cc             | 34 ----------
 source/common/stats/stats_impl.h              | 65 +------------------
 source/common/stats/thread_local_store.cc     |  2 +
 source/common/stats/thread_local_store.h      |  3 +
 source/server/BUILD                           |  2 +-
 source/server/hot_restart_impl.cc             |  1 +
 source/server/http/BUILD                      |  1 +
 source/server/http/admin.cc                   |  2 +
 source/server/options_impl.h                  |  2 +-
 .../common/block_memory_hash_set_test.cc      |  2 +-
 .../stats_sinks/metrics_service/BUILD         |  1 +
 .../metrics_service_integration_test.cc       |  1 +
 .../integration/websocket_integration_test.cc |  1 +
 test/mocks/stats/BUILD                        |  1 +
 test/mocks/stats/mocks.h                      |  1 +
 test/test_common/BUILD                        |  2 +
 test/test_common/utility.cc                   |  1 +
 test/test_common/utility.h                    |  1 +
 tools/BUILD                                   |  2 +-
 tools/v1_to_bootstrap.cc                      |  2 +-
 28 files changed, 162 insertions(+), 110 deletions(-)
 create mode 100644 source/common/stats/histogram.cc
 create mode 100644 source/common/stats/histogram.h

diff --git a/include/envoy/stats/stats.h b/include/envoy/stats/stats.h
index c39e41749699..cdaafd610d18 100644
--- a/include/envoy/stats/stats.h
+++ b/include/envoy/stats/stats.h
@@ -15,6 +15,7 @@
 
 namespace Envoy {
 namespace Event {
+
 class Dispatcher;
 }
 
diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD
index ab7408ff5b79..84210f394fbf 100644
--- a/source/common/stats/BUILD
+++ b/source/common/stats/BUILD
@@ -16,7 +16,24 @@ envoy_cc_library(
         ":stat_data_allocator_lib",
         "//source/common/common:assert_lib",
         "//source/common/common:hash_lib",
-        "//source/common/stats:stats_lib",
+        "//source/common/common:thread_annotations",
+        "//source/common/common:thread_lib",
+    ],
+)
+
+envoy_cc_library(
+    name = "histogram_lib",
+    srcs = ["histogram.cc"],
+    hdrs = ["histogram.h"],
+    external_deps = [
+        "abseil_optional",
+        "libcircllhist",
+    ],
+    deps = [
+        ":metric_impl_lib",
+        "//source/common/common:assert_lib",
+        "//source/common/common:hash_lib",
+        "//source/common/common:utility_lib",
     ],
 )
 
@@ -25,8 +42,10 @@ envoy_cc_library(
     srcs = ["isolated_store_impl.cc"],
     hdrs = ["isolated_store_impl.h"],
     deps = [
+        ":histogram_lib",
+        ":stats_lib",
+        ":stats_options_lib",
         "//source/common/stats:heap_stat_data_lib",
-        "//source/common/stats:stats_lib",
     ],
 )
 
@@ -64,11 +83,8 @@ envoy_cc_library(
     name = "stats_lib",
     srcs = ["stats_impl.cc"],
     hdrs = ["stats_impl.h"],
-    external_deps = [
-        "abseil_optional",
-        "libcircllhist",
-    ],
     deps = [
+        ":histogram_lib",
         ":metric_impl_lib",
         ":raw_stat_data_lib",
         ":stats_options_lib",
diff --git a/source/common/stats/heap_stat_data.cc b/source/common/stats/heap_stat_data.cc
index 6921baaa6d61..7a31fa71396d 100644
--- a/source/common/stats/heap_stat_data.cc
+++ b/source/common/stats/heap_stat_data.cc
@@ -2,7 +2,6 @@
 
 #include "common/common/lock_guard.h"
 #include "common/common/thread.h"
-#include "common/stats/stats_impl.h"
 
 namespace Envoy {
 namespace Stats {
diff --git a/source/common/stats/heap_stat_data.h b/source/common/stats/heap_stat_data.h
index 04bbde4e289f..61a949c39a23 100644
--- a/source/common/stats/heap_stat_data.h
+++ b/source/common/stats/heap_stat_data.h
@@ -4,8 +4,10 @@
 #include <string>
 #include <unordered_set>
 
+#include "common/common/hash.h"
+#include "common/common/thread.h"
+#include "common/common/thread_annotations.h"
 #include "common/stats/stat_data_allocator.h"
-#include "common/stats/stats_impl.h"
 
 namespace Envoy {
 namespace Stats {
diff --git a/source/common/stats/histogram.cc b/source/common/stats/histogram.cc
new file mode 100644
index 000000000000..5dca4bf94e16
--- /dev/null
+++ b/source/common/stats/histogram.cc
@@ -0,0 +1,49 @@
+#include "common/stats/histogram.h"
+
+#include <algorithm>
+#include <string>
+
+#include "common/common/utility.h"
+
+//#include "common/stats/utility.h"
+
+#include "absl/strings/str_join.h"
+
+namespace Envoy {
+namespace Stats {
+
+HistogramStatisticsImpl::HistogramStatisticsImpl(const histogram_t* histogram_ptr)
+    : computed_quantiles_(supportedQuantiles().size(), 0.0) {
+  hist_approx_quantile(histogram_ptr, supportedQuantiles().data(), supportedQuantiles().size(),
+                       computed_quantiles_.data());
+}
+
+const std::vector<double>& HistogramStatisticsImpl::supportedQuantiles() const {
+  static const std::vector<double> supported_quantiles = {0,    0.25, 0.5,   0.75, 0.90,
+                                                          0.95, 0.99, 0.999, 1};
+  return supported_quantiles;
+}
+
+std::string HistogramStatisticsImpl::summary() const {
+  std::vector<std::string> summary;
+  const std::vector<double>& supported_quantiles_ref = supportedQuantiles();
+  summary.reserve(supported_quantiles_ref.size());
+  for (size_t i = 0; i < supported_quantiles_ref.size(); ++i) {
+    summary.push_back(
+        fmt::format("P{}: {}", 100 * supported_quantiles_ref[i], computed_quantiles_[i]));
+  }
+  return absl::StrJoin(summary, ", ");
+}
+
+/**
+ * Clears the old computed values and refreshes it with values computed from passed histogram.
+ */
+void HistogramStatisticsImpl::refresh(const histogram_t* new_histogram_ptr) {
+  std::fill(computed_quantiles_.begin(), computed_quantiles_.end(), 0.0);
+  ASSERT(supportedQuantiles().size() == computed_quantiles_.size());
+  hist_approx_quantile(new_histogram_ptr, supportedQuantiles().data(), supportedQuantiles().size(),
+                       computed_quantiles_.data());
+}
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/histogram.h b/source/common/stats/histogram.h
new file mode 100644
index 000000000000..2df56bfa2d87
--- /dev/null
+++ b/source/common/stats/histogram.h
@@ -0,0 +1,60 @@
+#pragma once
+
+#include <cstdint>
+#include <string>
+
+#include "envoy/stats/stats.h"
+
+#include "common/common/non_copyable.h"
+#include "common/stats/metric_impl.h"
+
+#include "circllhist.h"
+
+namespace Envoy {
+namespace Stats {
+
+/**
+ * Implementation of HistogramStatistics for circllhist.
+ */
+class HistogramStatisticsImpl : public HistogramStatistics, NonCopyable {
+public:
+  HistogramStatisticsImpl() : computed_quantiles_(supportedQuantiles().size(), 0.0) {}
+  /**
+   * HistogramStatisticsImpl object is constructed using the passed in histogram.
+   * @param histogram_ptr pointer to the histogram for which stats will be calculated. This pointer
+   * will not be retained.
+   */
+  HistogramStatisticsImpl(const histogram_t* histogram_ptr);
+
+  void refresh(const histogram_t* new_histogram_ptr);
+
+  // HistogramStatistics
+  std::string summary() const override;
+  const std::vector<double>& supportedQuantiles() const override;
+  const std::vector<double>& computedQuantiles() const override { return computed_quantiles_; }
+
+private:
+  std::vector<double> computed_quantiles_;
+};
+
+/**
+ * Histogram implementation for the heap.
+ */
+class HistogramImpl : public Histogram, public MetricImpl {
+public:
+  HistogramImpl(const std::string& name, Store& parent, std::string&& tag_extracted_name,
+                std::vector<Tag>&& tags)
+      : MetricImpl(name, std::move(tag_extracted_name), std::move(tags)), parent_(parent) {}
+
+  // Stats::Histogram
+  void recordValue(uint64_t value) override { parent_.deliverHistogramToSinks(*this, value); }
+
+  bool used() const override { return true; }
+
+private:
+  // This is used for delivering the histogram data to sinks.
+  Store& parent_;
+};
+
+} // namespace Stats
+} // namespace Envoy
diff --git a/source/common/stats/isolated_store_impl.cc b/source/common/stats/isolated_store_impl.cc
index b7459fc4380d..4ebce873ea7b 100644
--- a/source/common/stats/isolated_store_impl.cc
+++ b/source/common/stats/isolated_store_impl.cc
@@ -6,6 +6,7 @@
 #include <string>
 
 #include "common/common/utility.h"
+#include "common/stats/histogram.h"
 #include "common/stats/utility.h"
 
 namespace Envoy {
diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h
index 13c6beaed6b2..48b35da8dcf8 100644
--- a/source/common/stats/isolated_store_impl.h
+++ b/source/common/stats/isolated_store_impl.h
@@ -8,6 +8,7 @@
 #include "common/common/utility.h"
 #include "common/stats/heap_stat_data.h"
 #include "common/stats/stats_impl.h"
+#include "common/stats/stats_options_impl.h"
 #include "common/stats/utility.h"
 
 namespace Envoy {
diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc
index c20a23527139..bda71a3d5d62 100644
--- a/source/common/stats/stats_impl.cc
+++ b/source/common/stats/stats_impl.cc
@@ -12,7 +12,6 @@
 #include "common/common/perf_annotation.h"
 #include "common/common/thread.h"
 #include "common/common/utility.h"
-#include "common/stats/raw_stat_data.h"
 #include "common/stats/utility.h"
 
 #include "absl/strings/ascii.h"
@@ -21,39 +20,6 @@
 namespace Envoy {
 namespace Stats {
 
-HistogramStatisticsImpl::HistogramStatisticsImpl(const histogram_t* histogram_ptr)
-    : computed_quantiles_(supportedQuantiles().size(), 0.0) {
-  hist_approx_quantile(histogram_ptr, supportedQuantiles().data(), supportedQuantiles().size(),
-                       computed_quantiles_.data());
-}
-
-const std::vector<double>& HistogramStatisticsImpl::supportedQuantiles() const {
-  static const std::vector<double> supported_quantiles = {0,    0.25, 0.5,   0.75, 0.90,
-                                                          0.95, 0.99, 0.999, 1};
-  return supported_quantiles;
-}
-
-std::string HistogramStatisticsImpl::summary() const {
-  std::vector<std::string> summary;
-  const std::vector<double>& supported_quantiles_ref = supportedQuantiles();
-  summary.reserve(supported_quantiles_ref.size());
-  for (size_t i = 0; i < supported_quantiles_ref.size(); ++i) {
-    summary.push_back(
-        fmt::format("P{}: {}", 100 * supported_quantiles_ref[i], computed_quantiles_[i]));
-  }
-  return absl::StrJoin(summary, ", ");
-}
-
-/**
- * Clears the old computed values and refreshes it with values computed from passed histogram.
- */
-void HistogramStatisticsImpl::refresh(const histogram_t* new_histogram_ptr) {
-  std::fill(computed_quantiles_.begin(), computed_quantiles_.end(), 0.0);
-  ASSERT(supportedQuantiles().size() == computed_quantiles_.size());
-  hist_approx_quantile(new_histogram_ptr, supportedQuantiles().data(), supportedQuantiles().size(),
-                       computed_quantiles_.data());
-}
-
 std::vector<CounterSharedPtr>& SourceImpl::cachedCounters() {
   if (!counters_) {
     counters_ = store_.counters();
diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h
index f3a2f5ce7535..d79626150f62 100644
--- a/source/common/stats/stats_impl.h
+++ b/source/common/stats/stats_impl.h
@@ -1,80 +1,19 @@
 #pragma once
 
 #include <algorithm>
-#include <chrono>
-#include <cstdint>
 #include <functional>
-#include <list>
-#include <memory>
-#include <regex>
-#include <string>
-#include <unordered_map>
 
-#include "envoy/common/time.h"
-#include "envoy/config/metrics/v2/stats.pb.h"
-#include "envoy/server/options.h"
 #include "envoy/stats/stats.h"
 
-#include "common/common/assert.h"
-#include "common/common/hash.h"
-#include "common/common/non_copyable.h"
-#include "common/common/thread.h"
-#include "common/common/thread_annotations.h"
 #include "common/common/utility.h"
-#include "common/protobuf/protobuf.h"
-#include "common/stats/metric_impl.h"
-#include "common/stats/stats_options_impl.h"
 
-#include "absl/strings/str_join.h"
-#include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
-#include "circllhist.h"
 
 namespace Envoy {
 namespace Stats {
 
-/**
- * Implementation of HistogramStatistics for circllhist.
- */
-class HistogramStatisticsImpl : public HistogramStatistics, NonCopyable {
-public:
-  HistogramStatisticsImpl() : computed_quantiles_(supportedQuantiles().size(), 0.0) {}
-  /**
-   * HistogramStatisticsImpl object is constructed using the passed in histogram.
-   * @param histogram_ptr pointer to the histogram for which stats will be calculated. This pointer
-   * will not be retained.
-   */
-  HistogramStatisticsImpl(const histogram_t* histogram_ptr);
-
-  void refresh(const histogram_t* new_histogram_ptr);
-
-  // HistogramStatistics
-  std::string summary() const override;
-  const std::vector<double>& supportedQuantiles() const override;
-  const std::vector<double>& computedQuantiles() const override { return computed_quantiles_; }
-
-private:
-  std::vector<double> computed_quantiles_;
-};
-
-/**
- * Histogram implementation for the heap.
- */
-class HistogramImpl : public Histogram, public MetricImpl {
-public:
-  HistogramImpl(const std::string& name, Store& parent, std::string&& tag_extracted_name,
-                std::vector<Tag>&& tags)
-      : MetricImpl(name, std::move(tag_extracted_name), std::move(tags)), parent_(parent) {}
-
-  // Stats::Histogram
-  void recordValue(uint64_t value) override { parent_.deliverHistogramToSinks(*this, value); }
-
-  bool used() const override { return true; }
-
-private:
-  // This is used for delivering the histogram data to sinks.
-  Store& parent_;
-};
+// TODO(jmarantz): rename this to source_impl.h -- this will have a pretty large
+// blast radius, as there are around ~60 references to this file.
 
 class SourceImpl : public Source {
 public:
diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc
index c4336d288e8d..af910c0bf732 100644
--- a/source/common/stats/thread_local_store.cc
+++ b/source/common/stats/thread_local_store.cc
@@ -10,6 +10,8 @@
 #include "common/common/lock_guard.h"
 #include "common/stats/tag_producer_impl.h"
 
+#include "absl/strings/str_join.h"
+
 namespace Envoy {
 namespace Stats {
 
diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h
index d5d616fe99b0..a65037185fbb 100644
--- a/source/common/stats/thread_local_store.h
+++ b/source/common/stats/thread_local_store.h
@@ -11,9 +11,12 @@
 #include "envoy/thread_local/thread_local.h"
 
 #include "common/stats/heap_stat_data.h"
+#include "common/stats/histogram.h"
 #include "common/stats/stats_impl.h"
 #include "common/stats/utility.h"
 
+#include "circllhist.h"
+
 namespace Envoy {
 namespace Stats {
 
diff --git a/source/server/BUILD b/source/server/BUILD
index dd46a38892f2..49f2efc73908 100644
--- a/source/server/BUILD
+++ b/source/server/BUILD
@@ -122,7 +122,7 @@ envoy_cc_library(
         "//source/common/common:block_memory_hash_set_lib",
         "//source/common/common:utility_lib",
         "//source/common/network:utility_lib",
-        "//source/common/stats:stats_lib",
+        "//source/common/stats:stats_options_lib",
     ],
 )
 
diff --git a/source/server/hot_restart_impl.cc b/source/server/hot_restart_impl.cc
index 84b01896bc90..a082eba18a6f 100644
--- a/source/server/hot_restart_impl.cc
+++ b/source/server/hot_restart_impl.cc
@@ -19,6 +19,7 @@
 #include "common/common/utility.h"
 #include "common/network/utility.h"
 #include "common/stats/raw_stat_data.h"
+#include "common/stats/stats_options_impl.h"
 
 #include "absl/strings/string_view.h"
 
diff --git a/source/server/http/BUILD b/source/server/http/BUILD
index 19364ffca3b2..bcdb6d25208e 100644
--- a/source/server/http/BUILD
+++ b/source/server/http/BUILD
@@ -52,6 +52,7 @@ envoy_cc_library(
         "//source/common/network:utility_lib",
         "//source/common/profiler:profiler_lib",
         "//source/common/router:config_lib",
+        "//source/common/stats:histogram_lib",
         "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//source/common/upstream:host_utility_lib",
diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc
index 267e557ca063..39cd61a95bcf 100644
--- a/source/server/http/admin.cc
+++ b/source/server/http/admin.cc
@@ -38,11 +38,13 @@
 #include "common/network/utility.h"
 #include "common/profiler/profiler.h"
 #include "common/router/config_impl.h"
+#include "common/stats/histogram.h"
 #include "common/stats/stats_impl.h"
 #include "common/upstream/host_utility.h"
 
 #include "extensions/access_loggers/file/file_access_log_impl.h"
 
+#include "absl/strings/str_join.h"
 #include "absl/strings/str_replace.h"
 #include "absl/strings/string_view.h"
 
diff --git a/source/server/options_impl.h b/source/server/options_impl.h
index bbb97472ec8f..b9b293f8c189 100644
--- a/source/server/options_impl.h
+++ b/source/server/options_impl.h
@@ -7,7 +7,7 @@
 #include "envoy/common/exception.h"
 #include "envoy/server/options.h"
 
-#include "common/stats/stats_impl.h"
+#include "common/stats/stats_options_impl.h"
 
 #include "spdlog/spdlog.h"
 
diff --git a/test/common/common/block_memory_hash_set_test.cc b/test/common/common/block_memory_hash_set_test.cc
index f0082da91ad5..8ae7b7466b95 100644
--- a/test/common/common/block_memory_hash_set_test.cc
+++ b/test/common/common/block_memory_hash_set_test.cc
@@ -9,7 +9,7 @@
 #include "common/common/block_memory_hash_set.h"
 #include "common/common/fmt.h"
 #include "common/common/hash.h"
-#include "common/stats/stats_impl.h"
+#include "common/stats/stats_options_impl.h"
 
 #include "absl/strings/string_view.h"
 #include "gtest/gtest.h"
diff --git a/test/extensions/stats_sinks/metrics_service/BUILD b/test/extensions/stats_sinks/metrics_service/BUILD
index 3dba545540d4..b3d91b1e19ab 100644
--- a/test/extensions/stats_sinks/metrics_service/BUILD
+++ b/test/extensions/stats_sinks/metrics_service/BUILD
@@ -36,6 +36,7 @@ envoy_extension_cc_test(
         "//source/common/buffer:zero_copy_input_stream_lib",
         "//source/common/grpc:codec_lib",
         "//source/common/grpc:common_lib",
+        "//source/common/stats:histogram_lib",
         "//source/extensions/stat_sinks/metrics_service:config",
         "//test/common/grpc:grpc_client_integration_lib",
         "//test/integration:http_integration_lib",
diff --git a/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc b/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc
index 35e325223889..47bf6892557e 100644
--- a/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc
+++ b/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc
@@ -4,6 +4,7 @@
 #include "common/common/version.h"
 #include "common/grpc/codec.h"
 #include "common/grpc/common.h"
+#include "common/stats/histogram.h"
 
 #include "test/common/grpc/grpc_client_integration.h"
 #include "test/integration/http_integration.h"
diff --git a/test/integration/websocket_integration_test.cc b/test/integration/websocket_integration_test.cc
index aa9f838e4572..eb95ca365321 100644
--- a/test/integration/websocket_integration_test.cc
+++ b/test/integration/websocket_integration_test.cc
@@ -13,6 +13,7 @@
 #include "test/test_common/printers.h"
 #include "test/test_common/utility.h"
 
+#include "absl/strings/str_cat.h"
 #include "gtest/gtest.h"
 
 using testing::MatchesRegex;
diff --git a/test/mocks/stats/BUILD b/test/mocks/stats/BUILD
index 0c38afb4a3ac..73cda48a2b41 100644
--- a/test/mocks/stats/BUILD
+++ b/test/mocks/stats/BUILD
@@ -17,6 +17,7 @@ envoy_cc_mock(
         "//include/envoy/stats:timespan",
         "//include/envoy/thread_local:thread_local_interface",
         "//include/envoy/upstream:cluster_manager_interface",
+        "//source/common/stats:histogram_lib",
         "//source/common/stats:isolated_store_lib",
         "//source/common/stats:stats_lib",
         "//test/mocks:common_lib",
diff --git a/test/mocks/stats/mocks.h b/test/mocks/stats/mocks.h
index ef844a3ec815..69ae64d153e8 100644
--- a/test/mocks/stats/mocks.h
+++ b/test/mocks/stats/mocks.h
@@ -10,6 +10,7 @@
 #include "envoy/thread_local/thread_local.h"
 #include "envoy/upstream/cluster_manager.h"
 
+#include "common/stats/histogram.h"
 #include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 
diff --git a/test/test_common/BUILD b/test/test_common/BUILD
index cda60b85c985..d861e0b99ec7 100644
--- a/test/test_common/BUILD
+++ b/test/test_common/BUILD
@@ -78,6 +78,7 @@ envoy_cc_test_library(
         "//include/envoy/http:codec_interface",
         "//include/envoy/network:address_interface",
         "//source/common/common:empty_string",
+        "//source/common/common:thread_lib",
         "//source/common/common:utility_lib",
         "//source/common/config:bootstrap_json_lib",
         "//source/common/http:header_map_lib",
@@ -86,6 +87,7 @@ envoy_cc_test_library(
         "//source/common/network:utility_lib",
         "//source/common/protobuf:utility_lib",
         "//source/common/stats:stats_lib",
+        "//source/common/stats:stats_options_lib",
         "@envoy_api//envoy/config/bootstrap/v2:bootstrap_cc",
     ],
 )
diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc
index 5427a548ea28..d28d32abb9c6 100644
--- a/test/test_common/utility.cc
+++ b/test/test_common/utility.cc
@@ -21,6 +21,7 @@
 #include "common/json/json_loader.h"
 #include "common/network/address_impl.h"
 #include "common/network/utility.h"
+#include "common/stats/stats_options_impl.h"
 
 #include "test/test_common/printers.h"
 
diff --git a/test/test_common/utility.h b/test/test_common/utility.h
index ad1c983cd722..18ce4afd2649 100644
--- a/test/test_common/utility.h
+++ b/test/test_common/utility.h
@@ -14,6 +14,7 @@
 
 #include "common/buffer/buffer_impl.h"
 #include "common/common/c_smart_ptr.h"
+#include "common/common/thread.h"
 #include "common/http/header_map_impl.h"
 #include "common/protobuf/utility.h"
 #include "common/stats/raw_stat_data.h"
diff --git a/tools/BUILD b/tools/BUILD
index 87880b11b6d5..94957cd7a738 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -41,7 +41,7 @@ envoy_cc_binary(
         "//source/common/config:bootstrap_json_lib",
         "//source/common/json:json_loader_lib",
         "//source/common/protobuf:utility_lib",
-        "//source/common/stats:stats_lib",
+        "//source/common/stats:stats_options_lib",
         "@envoy_api//envoy/config/bootstrap/v2:bootstrap_cc",
     ],
 )
diff --git a/tools/v1_to_bootstrap.cc b/tools/v1_to_bootstrap.cc
index e712e1bcb2c2..1407e8f3896e 100644
--- a/tools/v1_to_bootstrap.cc
+++ b/tools/v1_to_bootstrap.cc
@@ -13,7 +13,7 @@
 #include "common/config/bootstrap_json.h"
 #include "common/json/json_loader.h"
 #include "common/protobuf/utility.h"
-#include "common/stats/stats_impl.h"
+#include "common/stats/stats_options_impl.h"
 
 // NOLINT(namespace-envoy)
 int main(int argc, char** argv) {

From 9248ee1508fc1b3f834825a86f8745b3e5b9eddf Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Wed, 1 Aug 2018 08:10:52 -0400
Subject: [PATCH 13/14] rename histogram.* to histogram_impl.* for consistency.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 source/common/stats/BUILD                                     | 4 ++--
 source/common/stats/{histogram.cc => histogram_impl.cc}       | 2 +-
 source/common/stats/{histogram.h => histogram_impl.h}         | 0
 source/common/stats/isolated_store_impl.cc                    | 2 +-
 source/common/stats/thread_local_store.h                      | 2 +-
 source/server/http/admin.cc                                   | 2 +-
 .../metrics_service/metrics_service_integration_test.cc       | 2 +-
 test/mocks/stats/mocks.h                                      | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)
 rename source/common/stats/{histogram.cc => histogram_impl.cc} (97%)
 rename source/common/stats/{histogram.h => histogram_impl.h} (100%)

diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD
index 84210f394fbf..2b17f23ee9f6 100644
--- a/source/common/stats/BUILD
+++ b/source/common/stats/BUILD
@@ -23,8 +23,8 @@ envoy_cc_library(
 
 envoy_cc_library(
     name = "histogram_lib",
-    srcs = ["histogram.cc"],
-    hdrs = ["histogram.h"],
+    srcs = ["histogram_impl.cc"],
+    hdrs = ["histogram_impl.h"],
     external_deps = [
         "abseil_optional",
         "libcircllhist",
diff --git a/source/common/stats/histogram.cc b/source/common/stats/histogram_impl.cc
similarity index 97%
rename from source/common/stats/histogram.cc
rename to source/common/stats/histogram_impl.cc
index 5dca4bf94e16..9145f6dccb17 100644
--- a/source/common/stats/histogram.cc
+++ b/source/common/stats/histogram_impl.cc
@@ -1,4 +1,4 @@
-#include "common/stats/histogram.h"
+#include "common/stats/histogram_impl.h"
 
 #include <algorithm>
 #include <string>
diff --git a/source/common/stats/histogram.h b/source/common/stats/histogram_impl.h
similarity index 100%
rename from source/common/stats/histogram.h
rename to source/common/stats/histogram_impl.h
diff --git a/source/common/stats/isolated_store_impl.cc b/source/common/stats/isolated_store_impl.cc
index 4ebce873ea7b..d85d38ef0321 100644
--- a/source/common/stats/isolated_store_impl.cc
+++ b/source/common/stats/isolated_store_impl.cc
@@ -6,7 +6,7 @@
 #include <string>
 
 #include "common/common/utility.h"
-#include "common/stats/histogram.h"
+#include "common/stats/histogram_impl.h"
 #include "common/stats/utility.h"
 
 namespace Envoy {
diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h
index a65037185fbb..414e8cfb7f6c 100644
--- a/source/common/stats/thread_local_store.h
+++ b/source/common/stats/thread_local_store.h
@@ -11,7 +11,7 @@
 #include "envoy/thread_local/thread_local.h"
 
 #include "common/stats/heap_stat_data.h"
-#include "common/stats/histogram.h"
+#include "common/stats/histogram_impl.h"
 #include "common/stats/stats_impl.h"
 #include "common/stats/utility.h"
 
diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc
index 39cd61a95bcf..58dd1a885f3f 100644
--- a/source/server/http/admin.cc
+++ b/source/server/http/admin.cc
@@ -38,7 +38,7 @@
 #include "common/network/utility.h"
 #include "common/profiler/profiler.h"
 #include "common/router/config_impl.h"
-#include "common/stats/histogram.h"
+#include "common/stats/histogram_impl.h"
 #include "common/stats/stats_impl.h"
 #include "common/upstream/host_utility.h"
 
diff --git a/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc b/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc
index 47bf6892557e..9208cda05d20 100644
--- a/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc
+++ b/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc
@@ -4,7 +4,7 @@
 #include "common/common/version.h"
 #include "common/grpc/codec.h"
 #include "common/grpc/common.h"
-#include "common/stats/histogram.h"
+#include "common/stats/histogram_impl.h"
 
 #include "test/common/grpc/grpc_client_integration.h"
 #include "test/integration/http_integration.h"
diff --git a/test/mocks/stats/mocks.h b/test/mocks/stats/mocks.h
index 69ae64d153e8..8c5ccf7bd7fe 100644
--- a/test/mocks/stats/mocks.h
+++ b/test/mocks/stats/mocks.h
@@ -10,7 +10,7 @@
 #include "envoy/thread_local/thread_local.h"
 #include "envoy/upstream/cluster_manager.h"
 
-#include "common/stats/histogram.h"
+#include "common/stats/histogram_impl.h"
 #include "common/stats/isolated_store_impl.h"
 #include "common/stats/stats_impl.h"
 

From 571b8d37e8b8b321978a5a71b62ed0c6602b75fc Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Wed, 1 Aug 2018 11:21:25 -0400
Subject: [PATCH 14/14] Add a comment to re-kick CI.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 test/common/stats/stats_impl_test.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/test/common/stats/stats_impl_test.cc b/test/common/stats/stats_impl_test.cc
index 85486e20b989..14e08379df43 100644
--- a/test/common/stats/stats_impl_test.cc
+++ b/test/common/stats/stats_impl_test.cc
@@ -24,6 +24,9 @@ using testing::ReturnPointee;
 namespace Envoy {
 namespace Stats {
 
+// TODO(jmarantz): break this up into distinct test files for each class, to match
+// the breakup of stats_impl.h.
+
 TEST(StatsIsolatedStoreImplTest, All) {
   IsolatedStoreImpl store;