diff --git a/include/envoy/stats/scope.h b/include/envoy/stats/scope.h index 408655bbb8a5c..93e5f00c7c5f1 100644 --- a/include/envoy/stats/scope.h +++ b/include/envoy/stats/scope.h @@ -28,6 +28,8 @@ using TextReadoutOptConstRef = absl::optional; using ScopeSharedPtr = std::shared_ptr; +template using IterateFn = std::function&)>; + /** * A named scope for stats. Scopes are a grouping of stats that can be acted on as a unit if needed * (for example to free/delete all of them). @@ -194,6 +196,47 @@ class Scope { */ virtual const SymbolTable& constSymbolTable() const PURE; virtual SymbolTable& symbolTable() PURE; + + /** + * Calls 'fn' for every counter. Note that in the case of overlapping scopes, + * the implementation may call fn more than one time for each counter. Iteration + * stops if `fn` returns false; + * + * @param fn Function to be run for every counter, or until fn return false. + * @return false if fn(counter) return false during iteration, true if every counter was hit. + */ + virtual bool iterate(const IterateFn& fn) const PURE; + + /** + * Calls 'fn' for every gauge. Note that in the case of overlapping scopes, + * the implementation may call fn more than one time for each gauge. Iteration + * stops if `fn` returns false; + * + * @param fn Function to be run for every gauge, or until fn return false. + * @return false if fn(gauge) return false during iteration, true if every gauge was hit. + */ + virtual bool iterate(const IterateFn& fn) const PURE; + + /** + * Calls 'fn' for every histogram. Note that in the case of overlapping + * scopes, the implementation may call fn more than one time for each + * histogram. Iteration stops if `fn` returns false; + * + * @param fn Function to be run for every histogram, or until fn return false. + * @return false if fn(histogram) return false during iteration, true if every histogram was hit. + */ + virtual bool iterate(const IterateFn& fn) const PURE; + + /** + * Calls 'fn' for every text readout. Note that in the case of overlapping + * scopes, the implementation may call fn more than one time for each + * text readout. Iteration stops if `fn` returns false; + * + * @param fn Function to be run for every text readout, or until fn return false. + * @return false if fn(text_readout) return false during iteration, true if every text readout + * was hit. + */ + virtual bool iterate(const IterateFn& fn) const PURE; }; } // namespace Stats diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h index 2427e71a1b314..57d14ebf45fdc 100644 --- a/source/common/stats/isolated_store_impl.h +++ b/source/common/stats/isolated_store_impl.h @@ -91,6 +91,15 @@ template class IsolatedStatsCache { return vec; } + bool iterate(const IterateFn& fn) const { + for (auto& stat : stats_) { + if (!fn(stat.second)) { + return false; + } + } + return true; + } + private: friend class IsolatedStoreImpl; @@ -154,6 +163,13 @@ class IsolatedStoreImpl : public StoreImpl { return text_readouts_.find(name); } + bool iterate(const IterateFn& fn) const override { return counters_.iterate(fn); } + bool iterate(const IterateFn& fn) const override { return gauges_.iterate(fn); } + bool iterate(const IterateFn& fn) const override { return histograms_.iterate(fn); } + bool iterate(const IterateFn& fn) const override { + return text_readouts_.iterate(fn); + } + // Stats::Store std::vector counters() const override { return counters_.toVector(); } std::vector gauges() const override { diff --git a/source/common/stats/scope_prefixer.h b/source/common/stats/scope_prefixer.h index b7c8743756204..4257c1dd5ddfc 100644 --- a/source/common/stats/scope_prefixer.h +++ b/source/common/stats/scope_prefixer.h @@ -54,7 +54,34 @@ class ScopePrefixer : public Scope { NullGaugeImpl& nullGauge(const std::string& str) override { return scope_.nullGauge(str); } + bool iterate(const IterateFn& fn) const override { return iterHelper(fn); } + bool iterate(const IterateFn& fn) const override { return iterHelper(fn); } + bool iterate(const IterateFn& fn) const override { return iterHelper(fn); } + bool iterate(const IterateFn& fn) const override { return iterHelper(fn); } + private: + template bool iterHelper(const IterateFn& fn) const { + // We determine here what's in the scope by looking at name + // prefixes. Strictly speaking this is not correct, as a stat name can be in + // different scopes. But there is no data in `ScopePrefixer` to resurrect + // actual membership of a stat in a scope, so we go by name matching. Note + // that `ScopePrefixer` is not used in `ThreadLocalStore`, which has + // accurate maps describing which stats are in which scopes. + // + // TODO(jmarantz): In the scope of this limited implementation, it would be + // faster to match on the StatName prefix. This would be possible if + // SymbolTable exposed a split() method. + std::string prefix_str = scope_.symbolTable().toString(prefix_.statName()); + if (!prefix_str.empty() && !absl::EndsWith(prefix_str, ".")) { + prefix_str += "."; + } + IterateFn filter_scope = [&fn, + &prefix_str](const RefcountPtr& stat) -> bool { + return !absl::StartsWith(stat->name(), prefix_str) || fn(stat); + }; + return scope_.iterate(filter_scope); + } + Scope& scope_; StatNameStorage prefix_; }; diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h index 3496c0790e152..bf57eed14d959 100644 --- a/source/common/stats/thread_local_store.h +++ b/source/common/stats/thread_local_store.h @@ -242,6 +242,11 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo return absl::nullopt; } + bool iterate(const IterateFn& fn) const override { return iterHelper(fn); } + bool iterate(const IterateFn& fn) const override { return iterHelper(fn); } + bool iterate(const IterateFn& fn) const override { return iterHelper(fn); } + bool iterate(const IterateFn& fn) const override { return iterHelper(fn); } + // Stats::Store std::vector counters() const override; std::vector gauges() const override; @@ -349,6 +354,28 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo NullGaugeImpl& nullGauge(const std::string&) override { return parent_.null_gauge_; } + template bool iterHelper(StatFn fn, const StatMap& map) const { + for (auto& iter : map) { + if (!fn(iter.second)) { + return false; + } + } + return true; + } + + bool iterate(const IterateFn& fn) const override { + return iterHelper(fn, central_cache_->counters_); + } + bool iterate(const IterateFn& fn) const override { + return iterHelper(fn, central_cache_->gauges_); + } + bool iterate(const IterateFn& fn) const override { + return iterHelper(fn, central_cache_->histograms_); + } + bool iterate(const IterateFn& fn) const override { + return iterHelper(fn, central_cache_->text_readouts_); + } + // NOTE: The find methods assume that `name` is fully-qualified. // Implementations will not add the scope prefix. CounterOptConstRef findCounter(StatName name) const override; @@ -418,6 +445,16 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo absl::flat_hash_map scope_cache_; }; + template bool iterHelper(StatFn fn) const { + Thread::LockGuard lock(lock_); + for (ScopeImpl* scope : scopes_) { + if (!scope->iterate(fn)) { + return false; + } + } + return true; + } + std::string getTagsForName(const std::string& name, TagVector& tags) const; void clearScopeFromCaches(uint64_t scope_id, CentralCacheEntrySharedPtr central_cache); void releaseScopeCrossThread(ScopeImpl* scope); diff --git a/source/common/stats/utility.h b/source/common/stats/utility.h index 46b72234da3ab..4328c2ef5875d 100644 --- a/source/common/stats/utility.h +++ b/source/common/stats/utility.h @@ -5,6 +5,7 @@ #include "envoy/stats/scope.h" #include "envoy/stats/stats.h" +#include "common/common/thread.h" #include "common/stats/symbol_table_impl.h" #include "absl/container/inlined_vector.h" @@ -206,5 +207,55 @@ class Utility { StatNameTagVectorOptConstRef tags = absl::nullopt); }; +/** + * Holds a reference to a stat by name. Note that the stat may not be created + * yet at the time CachedReference is created. Calling get() then does a lazy + * lookup, potentially returning absl::nullopt if the stat doesn't exist yet. + * StatReference works whether the name was constructed symbolically, or with + * StatNameDynamicStorage. + * + * Lookups are very slow, taking time proportional to the size of the scope, + * holding mutexes during the lookup. However once the lookup succeeds, the + * result is cached atomically, and further calls to get() are thus fast and + * mutex-free. The implementation may be faster for stats that are named + * symbolically. + * + * CachedReference is valid for the lifetime of the Scope. When the Scope + * becomes invalid, CachedReferences must also be dropped as they will hold + * pointers into the scope. + */ +template class CachedReference { +public: + CachedReference(Scope& scope, absl::string_view name) : scope_(scope), name_(std::string(name)) {} + + /** + * Finds the named stat, if it exists, returning it as an optional. + */ + absl::optional> get() { + StatType* stat = stat_.get([this]() -> StatType* { + StatType* stat = nullptr; + IterateFn check_stat = [this, + &stat](const RefcountPtr& shared_stat) -> bool { + if (shared_stat->name() == name_) { + stat = shared_stat.get(); + return false; // Stop iteration. + } + return true; + }; + scope_.iterate(check_stat); + return stat; + }); + if (stat == nullptr) { + return absl::nullopt; + } + return *stat; + } + +private: + Scope& scope_; + const std::string name_; + Thread::AtomicPtr stat_; +}; + } // namespace Stats } // namespace Envoy diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD index 5125844c58171..e7c6ebbb01d73 100644 --- a/test/common/stats/BUILD +++ b/test/common/stats/BUILD @@ -268,6 +268,7 @@ envoy_cc_test( srcs = ["utility_test.cc"], deps = [ "//source/common/stats:isolated_store_lib", + "//source/common/stats:thread_local_store_lib", "//source/common/stats:utility_lib", ], ) diff --git a/test/common/stats/utility_test.cc b/test/common/stats/utility_test.cc index 8f4ec260d3bba..bf1643ff4ed5d 100644 --- a/test/common/stats/utility_test.cc +++ b/test/common/stats/utility_test.cc @@ -6,36 +6,169 @@ #include "common/stats/null_counter.h" #include "common/stats/null_gauge.h" #include "common/stats/symbol_table_creator.h" +#include "common/stats/thread_local_store.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" +using testing::UnorderedElementsAre; + namespace Envoy { namespace Stats { namespace { -class StatsUtilityTest : public testing::Test { +// All the tests should be run for both IsolatedStore and ThreadLocalStore. +enum class StoreType { + ThreadLocal, + Isolated, +}; + +class StatsUtilityTest : public testing::TestWithParam { protected: + template + using IterateFn = std::function& stat)>; + using MakeStatFn = std::function; + StatsUtilityTest() - : symbol_table_(SymbolTableCreator::makeSymbolTable()), - store_(std::make_unique(*symbol_table_)), pool_(*symbol_table_), + : symbol_table_(SymbolTableCreator::makeSymbolTable()), pool_(*symbol_table_), tags_( - {{pool_.add("tag1"), pool_.add("value1")}, {pool_.add("tag2"), pool_.add("value2")}}) {} + {{pool_.add("tag1"), pool_.add("value1")}, {pool_.add("tag2"), pool_.add("value2")}}) { + switch (GetParam()) { + case StoreType::ThreadLocal: + alloc_ = std::make_unique(*symbol_table_), + store_ = std::make_unique(*alloc_); + break; + case StoreType::Isolated: + store_ = std::make_unique(*symbol_table_); + break; + } + scope_ = store_->createScope("scope"); + } ~StatsUtilityTest() override { + scope_.reset(); pool_.clear(); store_.reset(); EXPECT_EQ(0, symbol_table_->numSymbols()); } + void init(MakeStatFn make_stat) { + make_stat(*store_, {pool_.add("symbolic1")}); + make_stat(*store_, {Stats::DynamicName("dynamic1")}); + make_stat(*scope_, {pool_.add("symbolic2")}); + make_stat(*scope_, {Stats::DynamicName("dynamic2")}); + } + + template IterateFn iterOnce() { + return [this](const RefcountPtr& stat) -> bool { + results_.insert(stat->name()); + return false; + }; + } + + template IterateFn iterAll() { + return [this](const RefcountPtr& stat) -> bool { + results_.insert(stat->name()); + return true; + }; + } + + static MakeStatFn makeCounter() { + return [](Scope& scope, const ElementVec& elements) { + Utility::counterFromElements(scope, elements).inc(); + }; + } + + static bool checkValue(const Counter& counter) { return counter.value() == 1; } + + static MakeStatFn makeGauge() { + return [](Scope& scope, const ElementVec& elements) { + Utility::gaugeFromElements(scope, elements, Gauge::ImportMode::Accumulate).inc(); + }; + } + + static bool checkValue(const Gauge& gauge) { return gauge.value() == 1; } + + static MakeStatFn makeHistogram() { + return [](Scope& scope, const ElementVec& elements) { + Utility::histogramFromElements(scope, elements, Histogram::Unit::Milliseconds); + }; + } + + static bool checkValue(const Histogram& histogram) { + return histogram.unit() == Histogram::Unit::Milliseconds; + } + + static MakeStatFn makeTextReadout() { + return [](Scope& scope, const ElementVec& elements) { + Utility::textReadoutFromElements(scope, elements).set("my-value"); + }; + } + + static bool checkValue(const TextReadout& text_readout) { + return text_readout.value() == "my-value"; + } + + template void storeOnce(const MakeStatFn make_stat) { + CachedReference symbolic1_ref(*store_, "symbolic1"); + CachedReference dynamic1_ref(*store_, "dynamic1"); + EXPECT_FALSE(symbolic1_ref.get()); + EXPECT_FALSE(dynamic1_ref.get()); + + init(make_stat); + + ASSERT_TRUE(symbolic1_ref.get()); + ASSERT_TRUE(dynamic1_ref.get()); + EXPECT_FALSE(store_->iterate(iterOnce())); + EXPECT_EQ(1, results_.size()); + EXPECT_TRUE(checkValue(*symbolic1_ref.get())); + EXPECT_TRUE(checkValue(*dynamic1_ref.get())); + } + + template void storeAll(const MakeStatFn make_stat) { + init(make_stat); + EXPECT_TRUE(store_->iterate(iterAll())); + EXPECT_THAT(results_, + UnorderedElementsAre("symbolic1", "dynamic1", "scope.symbolic2", "scope.dynamic2")); + } + + template void scopeOnce(const MakeStatFn make_stat) { + CachedReference symbolic2_ref(*store_, "scope.symbolic2"); + CachedReference dynamic2_ref(*store_, "scope.dynamic2"); + EXPECT_FALSE(symbolic2_ref.get()); + EXPECT_FALSE(dynamic2_ref.get()); + + init(make_stat); + + ASSERT_TRUE(symbolic2_ref.get()); + ASSERT_TRUE(dynamic2_ref.get()); + EXPECT_FALSE(scope_->iterate(iterOnce())); + EXPECT_EQ(1, results_.size()); + EXPECT_TRUE(checkValue(*symbolic2_ref.get())); + EXPECT_TRUE(checkValue(*dynamic2_ref.get())); + } + + template void scopeAll(const MakeStatFn make_stat) { + init(make_stat); + EXPECT_TRUE(scope_->iterate(iterAll())); + EXPECT_THAT(results_, UnorderedElementsAre("scope.symbolic2", "scope.dynamic2")); + } + SymbolTablePtr symbol_table_; - std::unique_ptr store_; StatNamePool pool_; + std::unique_ptr alloc_; + std::unique_ptr store_; + ScopePtr scope_; + absl::flat_hash_set results_; StatNameTagVector tags_; }; -TEST_F(StatsUtilityTest, Counters) { +INSTANTIATE_TEST_SUITE_P(StatsUtilityTest, StatsUtilityTest, + testing::ValuesIn({StoreType::ThreadLocal, StoreType::Isolated})); + +TEST_P(StatsUtilityTest, Counters) { ScopePtr scope = store_->createScope("scope."); Counter& c1 = Utility::counterFromElements(*scope, {DynamicName("a"), DynamicName("b")}); EXPECT_EQ("scope.a.b", c1.name()); @@ -54,7 +187,7 @@ TEST_F(StatsUtilityTest, Counters) { EXPECT_EQ("scope.x.token.y.tag1.value1.tag2.value2", ctags.name()); } -TEST_F(StatsUtilityTest, Gauges) { +TEST_P(StatsUtilityTest, Gauges) { ScopePtr scope = store_->createScope("scope."); Gauge& g1 = Utility::gaugeFromElements(*scope, {DynamicName("a"), DynamicName("b")}, Gauge::ImportMode::NeverImport); @@ -73,7 +206,7 @@ TEST_F(StatsUtilityTest, Gauges) { EXPECT_EQ(&g3, &g4); } -TEST_F(StatsUtilityTest, Histograms) { +TEST_P(StatsUtilityTest, Histograms) { ScopePtr scope = store_->createScope("scope."); Histogram& h1 = Utility::histogramFromElements(*scope, {DynamicName("a"), DynamicName("b")}, Histogram::Unit::Milliseconds); @@ -92,7 +225,7 @@ TEST_F(StatsUtilityTest, Histograms) { EXPECT_EQ(&h3, &h4); } -TEST_F(StatsUtilityTest, TextReadouts) { +TEST_P(StatsUtilityTest, TextReadouts) { ScopePtr scope = store_->createScope("scope."); TextReadout& t1 = Utility::textReadoutFromElements(*scope, {DynamicName("a"), DynamicName("b")}); EXPECT_EQ("scope.a.b", t1.name()); @@ -107,6 +240,38 @@ TEST_F(StatsUtilityTest, TextReadouts) { EXPECT_EQ(&t3, &t4); } +TEST_P(StatsUtilityTest, StoreCounterOnce) { storeOnce(makeCounter()); } + +TEST_P(StatsUtilityTest, StoreCounterAll) { storeAll(makeCounter()); } + +TEST_P(StatsUtilityTest, ScopeCounterOnce) { scopeOnce(makeCounter()); } + +TEST_P(StatsUtilityTest, ScopeCounterAll) { scopeAll(makeCounter()); } + +TEST_P(StatsUtilityTest, StoreGaugeOnce) { storeOnce(makeGauge()); } + +TEST_P(StatsUtilityTest, StoreGaugeAll) { storeAll(makeGauge()); } + +TEST_P(StatsUtilityTest, ScopeGaugeOnce) { scopeOnce(makeGauge()); } + +TEST_P(StatsUtilityTest, ScopeGaugeAll) { scopeAll(makeGauge()); } + +TEST_P(StatsUtilityTest, StoreHistogramOnce) { storeOnce(makeHistogram()); } + +TEST_P(StatsUtilityTest, StoreHistogramAll) { storeAll(makeHistogram()); } + +TEST_P(StatsUtilityTest, ScopeHistogramOnce) { scopeOnce(makeHistogram()); } + +TEST_P(StatsUtilityTest, ScopeHistogramAll) { scopeAll(makeHistogram()); } + +TEST_P(StatsUtilityTest, StoreTextReadoutOnce) { storeOnce(makeTextReadout()); } + +TEST_P(StatsUtilityTest, StoreTextReadoutAll) { storeAll(makeTextReadout()); } + +TEST_P(StatsUtilityTest, ScopeTextReadoutOnce) { scopeOnce(makeTextReadout()); } + +TEST_P(StatsUtilityTest, ScopeTextReadoutAll) { scopeAll(makeTextReadout()); } + } // namespace } // namespace Stats } // namespace Envoy diff --git a/test/integration/server.h b/test/integration/server.h index 67e860ea4a4b9..55149d0b6c164 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -150,6 +150,15 @@ class TestScopeWrapper : public Scope { } SymbolTable& symbolTable() override { return wrapped_scope_->symbolTable(); } + bool iterate(const IterateFn& fn) const override { return wrapped_scope_->iterate(fn); } + bool iterate(const IterateFn& fn) const override { return wrapped_scope_->iterate(fn); } + bool iterate(const IterateFn& fn) const override { + return wrapped_scope_->iterate(fn); + } + bool iterate(const IterateFn& fn) const override { + return wrapped_scope_->iterate(fn); + } + private: Thread::MutexBasicLockable& lock_; ScopePtr wrapped_scope_; @@ -333,6 +342,11 @@ class TestIsolatedStoreImpl : public StoreRoot { return store_.textReadouts(); } + bool iterate(const IterateFn& fn) const override { return store_.iterate(fn); } + bool iterate(const IterateFn& fn) const override { return store_.iterate(fn); } + bool iterate(const IterateFn& fn) const override { return store_.iterate(fn); } + bool iterate(const IterateFn& fn) const override { return store_.iterate(fn); } + // Stats::StoreRoot void addSink(Sink&) override {} void setTagProducer(TagProducerPtr&&) override {}