From b1b5d9f5279292a30113cdc40b97a68b7975219d Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 22 May 2019 10:35:49 -0400 Subject: [PATCH 1/6] Improve test coverage by adding tests and small refactors to share more code. Signed-off-by: Joshua Marantz --- include/envoy/stats/scope.h | 2 +- include/envoy/stats/stat_data_allocator.h | 2 +- include/envoy/stats/stats.h | 2 +- source/common/stats/histogram_impl.h | 1 - source/common/stats/metric_impl.cc | 4 ++-- source/common/stats/metric_impl.h | 7 +++++-- source/common/stats/scope_prefixer.h | 2 +- source/common/stats/stat_data_allocator_impl.h | 5 +---- source/common/stats/store_impl.h | 12 +----------- source/common/stats/symbol_table_impl.h | 2 +- source/common/stats/thread_local_store.cc | 4 ++-- source/common/stats/thread_local_store.h | 6 ++---- test/common/stats/isolated_store_impl_test.cc | 9 +++++++++ test/common/stats/thread_local_store_test.cc | 16 ++++++++++++++++ test/integration/server.h | 6 ++++-- test/mocks/stats/mocks.h | 5 +---- 16 files changed, 48 insertions(+), 37 deletions(-) diff --git a/include/envoy/stats/scope.h b/include/envoy/stats/scope.h index 863d5f7d45909..d37ca645e771f 100644 --- a/include/envoy/stats/scope.h +++ b/include/envoy/stats/scope.h @@ -111,7 +111,7 @@ class Scope { /** * @return a reference to the symbol table. */ - virtual const SymbolTable& symbolTable() const PURE; + virtual const SymbolTable& constSymbolTable() const PURE; virtual SymbolTable& symbolTable() PURE; }; diff --git a/include/envoy/stats/stat_data_allocator.h b/include/envoy/stats/stat_data_allocator.h index 8d3ec57409150..8f6b3929ecb0e 100644 --- a/include/envoy/stats/stat_data_allocator.h +++ b/include/envoy/stats/stat_data_allocator.h @@ -49,7 +49,7 @@ class StatDataAllocator { virtual GaugeSharedPtr makeGauge(StatName name, absl::string_view tag_extracted_name, const std::vector& tags) PURE; - virtual const SymbolTable& symbolTable() const PURE; + virtual const SymbolTable& constSymbolTable() const PURE; virtual SymbolTable& symbolTable() PURE; // TODO(jmarantz): create a parallel mechanism to instantiate histograms. At diff --git a/include/envoy/stats/stats.h b/include/envoy/stats/stats.h index 5405a37eb7925..354ef99b8dad3 100644 --- a/include/envoy/stats/stats.h +++ b/include/envoy/stats/stats.h @@ -105,7 +105,7 @@ class Metric { static const uint8_t LogicCached = LogicAccumulate | LogicNeverImport; }; virtual SymbolTable& symbolTable() PURE; - virtual const SymbolTable& symbolTable() const PURE; + virtual const SymbolTable& constSymbolTable() const PURE; }; /** diff --git a/source/common/stats/histogram_impl.h b/source/common/stats/histogram_impl.h index 5fceb00687ff0..9977e357cb8bc 100644 --- a/source/common/stats/histogram_impl.h +++ b/source/common/stats/histogram_impl.h @@ -76,7 +76,6 @@ class HistogramImpl : public Histogram, public MetricImpl { bool used() const override { return true; } StatName statName() const override { return name_.statName(); } - const SymbolTable& symbolTable() const override { return parent_.symbolTable(); } SymbolTable& symbolTable() override { return parent_.symbolTable(); } private: diff --git a/source/common/stats/metric_impl.cc b/source/common/stats/metric_impl.cc index ba45f0311bbad..b88607f4abd1d 100644 --- a/source/common/stats/metric_impl.cc +++ b/source/common/stats/metric_impl.cc @@ -34,7 +34,7 @@ MetricImpl::MetricImpl(absl::string_view tag_extracted_name, const std::vector bool { return fn(Tag{symbol_table.toString(name), symbol_table.toString(value)}); }); diff --git a/source/common/stats/metric_impl.h b/source/common/stats/metric_impl.h index 48c67766c8890..4df03257c920d 100644 --- a/source/common/stats/metric_impl.h +++ b/source/common/stats/metric_impl.h @@ -26,13 +26,17 @@ class MetricImpl : public virtual Metric { SymbolTable& symbol_table); ~MetricImpl(); - std::string name() const override { return symbolTable().toString(statName()); } + std::string name() const override { return constSymbolTable().toString(statName()); } std::string tagExtractedName() const override; std::vector tags() const override; StatName tagExtractedStatName() const override; void iterateTagStatNames(const TagStatNameIterFn& fn) const override; void iterateTags(const TagIterFn& fn) const override; + const SymbolTable& constSymbolTable() const override { + return const_cast(this)->symbolTable(); + } + protected: void clear(); @@ -45,7 +49,6 @@ class NullMetricImpl : public MetricImpl { explicit NullMetricImpl(SymbolTable& symbol_table) : MetricImpl("", std::vector(), symbol_table), stat_name_storage_("", symbol_table) {} - const SymbolTable& symbolTable() const override { return stat_name_storage_.symbolTable(); } SymbolTable& symbolTable() override { return stat_name_storage_.symbolTable(); } bool used() const override { return false; } StatName statName() const override { return stat_name_storage_.statName(); } diff --git a/source/common/stats/scope_prefixer.h b/source/common/stats/scope_prefixer.h index 51c1828393913..c289a59571d72 100644 --- a/source/common/stats/scope_prefixer.h +++ b/source/common/stats/scope_prefixer.h @@ -40,7 +40,7 @@ class ScopePrefixer : public Scope { absl::optional> findHistogram(StatName name) const override; - const SymbolTable& symbolTable() const override { return scope_.symbolTable(); } + const SymbolTable& constSymbolTable() const override { return scope_.constSymbolTable(); } virtual SymbolTable& symbolTable() override { return scope_.symbolTable(); } NullGaugeImpl& nullGauge(const std::string& str) override { return scope_.nullGauge(str); } diff --git a/source/common/stats/stat_data_allocator_impl.h b/source/common/stats/stat_data_allocator_impl.h index 47b0858fe0a8f..546f14c8f3d98 100644 --- a/source/common/stats/stat_data_allocator_impl.h +++ b/source/common/stats/stat_data_allocator_impl.h @@ -37,7 +37,7 @@ template class StatDataAllocatorImpl : public StatDataAllocator virtual void free(StatData& data) PURE; SymbolTable& symbolTable() override { return symbol_table_; } - const SymbolTable& symbolTable() const override { return symbol_table_; } + const SymbolTable& constSymbolTable() const override { return symbol_table_; } private: // SymbolTable encodes encodes stat names as back into strings. This does not @@ -81,7 +81,6 @@ template class CounterImpl : public Counter, public MetricImpl bool used() const override { return data_.flags_ & Flags::Used; } uint64_t value() const override { return data_.value_; } - const SymbolTable& symbolTable() const override { return alloc_.symbolTable(); } SymbolTable& symbolTable() override { return alloc_.symbolTable(); } protected: @@ -164,8 +163,6 @@ template class GaugeImpl : public Gauge, public MetricImpl { } } -private: - const SymbolTable& symbolTable() const override { return alloc_.symbolTable(); } SymbolTable& symbolTable() override { return alloc_.symbolTable(); } protected: diff --git a/source/common/stats/store_impl.h b/source/common/stats/store_impl.h index 94b2db6b06e1c..e4a83f7514372 100644 --- a/source/common/stats/store_impl.h +++ b/source/common/stats/store_impl.h @@ -15,18 +15,8 @@ class StoreImpl : public Store { public: explicit StoreImpl(SymbolTable& symbol_table) : symbol_table_(symbol_table) {} - Counter& counterFromStatName(StatName name) override { - return counter(symbol_table_.toString(name)); - } - - Gauge& gaugeFromStatName(StatName name) override { return gauge(symbol_table_.toString(name)); } - - Histogram& histogramFromStatName(StatName name) override { - return histogram(symbol_table_.toString(name)); - } - SymbolTable& symbolTable() override { return symbol_table_; } - const SymbolTable& symbolTable() const override { return symbol_table_; } + const SymbolTable& constSymbolTable() const override { return symbol_table_; } private: SymbolTable& symbol_table_; diff --git a/source/common/stats/symbol_table_impl.h b/source/common/stats/symbol_table_impl.h index b3405298f1a6d..60c796d1def90 100644 --- a/source/common/stats/symbol_table_impl.h +++ b/source/common/stats/symbol_table_impl.h @@ -401,7 +401,7 @@ class StatNameManagedStorage : public StatNameStorage { ~StatNameManagedStorage() { free(symbol_table_); } SymbolTable& symbolTable() { return symbol_table_; } - const SymbolTable& symbolTable() const { return symbol_table_; } + const SymbolTable& constSymbolTable() const { return symbol_table_; } private: SymbolTable& symbol_table_; diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index d461e342f941b..dcb65e34f2e3b 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -12,7 +12,6 @@ #include "envoy/stats/stats.h" #include "common/common/lock_guard.h" -#include "common/stats/scope_prefixer.h" #include "common/stats/stats_matcher_impl.h" #include "common/stats/tag_producer_impl.h" @@ -86,7 +85,8 @@ bool ThreadLocalStoreImpl::rejects(StatName stat_name) const { // Also note that the elaboration of the stat-name into a string is expensive, // so I think it might be better to move the matcher test until after caching, // unless its acceptsAll/rejectsAll. - return stats_matcher_->rejectsAll() || stats_matcher_->rejects(symbolTable().toString(stat_name)); + return stats_matcher_->rejectsAll() || + stats_matcher_->rejects(constSymbolTable().toString(stat_name)); } std::vector ThreadLocalStoreImpl::counters() const { diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h index 3f53d23ac24a6..c9a70d5ec7183 100644 --- a/source/common/stats/thread_local_store.h +++ b/source/common/stats/thread_local_store.h @@ -51,7 +51,6 @@ class ThreadLocalHistogramImpl : public Histogram, public MetricImpl { // Stats::Metric StatName statName() const override { return name_.statName(); } SymbolTable& symbolTable() override { return symbol_table_; } - const SymbolTable& symbolTable() const override { return symbol_table_; } private: uint64_t otherHistogramIndex() const { return 1 - current_active_; } @@ -98,7 +97,6 @@ class ParentHistogramImpl : public ParentHistogram, public MetricImpl { // Stats::Metric StatName statName() const override { return name_.statName(); } SymbolTable& symbolTable() override { return parent_.symbolTable(); } - const SymbolTable& symbolTable() const override { return parent_.symbolTable(); } private: bool usedLockHeld() const EXCLUSIVE_LOCKS_REQUIRED(merge_lock_); @@ -159,7 +157,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo } Histogram& histogram(const std::string& name) override { return default_scope_->histogram(name); } NullGaugeImpl& nullGauge(const std::string&) override { return null_gauge_; } - const SymbolTable& symbolTable() const override { return alloc_.symbolTable(); } + const SymbolTable& constSymbolTable() const override { return alloc_.constSymbolTable(); } SymbolTable& symbolTable() override { return alloc_.symbolTable(); } const TagProducer& tagProducer() const { return *tag_producer_; } absl::optional> findCounter(StatName name) const override { @@ -250,7 +248,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo ScopePtr createScope(const std::string& name) override { return parent_.createScope(symbolTable().toString(prefix_.statName()) + "." + name); } - const SymbolTable& symbolTable() const override { return parent_.symbolTable(); } + const SymbolTable& constSymbolTable() const override { return parent_.constSymbolTable(); } SymbolTable& symbolTable() override { return parent_.symbolTable(); } Counter& counter(const std::string& name) override { diff --git a/test/common/stats/isolated_store_impl_test.cc b/test/common/stats/isolated_store_impl_test.cc index d8b7d6a985e14..898fb10eb4c91 100644 --- a/test/common/stats/isolated_store_impl_test.cc +++ b/test/common/stats/isolated_store_impl_test.cc @@ -46,6 +46,7 @@ TEST_F(StatsIsolatedStoreImplTest, All) { EXPECT_EQ(0, g2.tags().size()); Histogram& h1 = store_.histogram("h1"); + EXPECT_TRUE(h1.used()); // hardcoded in impl to be true always. Histogram& h2 = scope1->histogram("h2"); scope1->deliverHistogramToSinks(h2, 0); EXPECT_EQ("h1", h1.name()); @@ -111,6 +112,14 @@ TEST_F(StatsIsolatedStoreImplTest, AllWithSymbolTable) { EXPECT_EQ(2UL, store_.gauges().size()); } +TEST_F(StatsIsolatedStoreImplTest, ConstSymtabAccessor) { + ScopePtr scope = store_.createScope("scope."); + const Scope& cscope = *scope; + const SymbolTable& csymtab = cscope.constSymbolTable(); + SymbolTable& symtab = scope->symbolTable(); + EXPECT_EQ(&csymtab, &symtab); +} + TEST_F(StatsIsolatedStoreImplTest, LongStatName) { const std::string long_string(128, 'A'); diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index f573565f441cc..75261063f5d76 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -342,6 +342,14 @@ TEST_F(StatsThreadLocalStoreTest, SanitizePrefix) { tls_.shutdownThread(); } +TEST_F(StatsThreadLocalStoreTest, ConstSymtabAccessor) { + ScopePtr scope = store_->createScope("scope."); + const Scope& cscope = *scope; + const SymbolTable& csymtab = cscope.constSymbolTable(); + SymbolTable& symtab = scope->symbolTable(); + EXPECT_EQ(&csymtab, &symtab); +} + TEST_F(StatsThreadLocalStoreTest, ScopeDelete) { InSequence s; store_->initializeThreading(main_thread_dispatcher_, tls_); @@ -501,6 +509,13 @@ TEST_F(LookupWithStatNameTest, All) { EXPECT_EQ(2UL, store_.gauges().size()); } +TEST_F(LookupWithStatNameTest, NotFound) { + StatName not_found(makeStatName("not_found")); + EXPECT_FALSE(store_.findCounter(not_found)); + EXPECT_FALSE(store_.findGauge(not_found)); + EXPECT_FALSE(store_.findHistogram(not_found)); +} + class StatsMatcherTLSTest : public StatsThreadLocalStoreTest { public: envoy::config::metrics::v2::StatsConfig stats_config_; @@ -527,6 +542,7 @@ TEST_F(StatsMatcherTLSTest, TestNoOpStatImpls) { EXPECT_EQ(noop_counter.value(), 0); Counter& noop_counter_2 = store_->counter("noop_counter_2"); EXPECT_EQ(&noop_counter, &noop_counter_2); + EXPECT_FALSE(noop_counter.used()); // hardcoded to return false in NullMetricImpl. // Gauge Gauge& noop_gauge = store_->gauge("noop_gauge"); diff --git a/test/integration/server.h b/test/integration/server.h index 2ae5492a463b8..a0dca125f8751 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -119,7 +119,9 @@ class TestScopeWrapper : public Scope { return wrapped_scope_->findHistogram(name); } - const SymbolTable& symbolTable() const override { return wrapped_scope_->symbolTable(); } + const SymbolTable& constSymbolTable() const override { + return wrapped_scope_->constSymbolTable(); + } SymbolTable& symbolTable() override { return wrapped_scope_->symbolTable(); } private: @@ -177,7 +179,7 @@ class TestIsolatedStoreImpl : public StoreRoot { Thread::LockGuard lock(lock_); return store_.findHistogram(name); } - const SymbolTable& symbolTable() const override { return store_.symbolTable(); } + const SymbolTable& constSymbolTable() const override { return store_.constSymbolTable(); } SymbolTable& symbolTable() override { return store_.symbolTable(); } // Stats::Store diff --git a/test/mocks/stats/mocks.h b/test/mocks/stats/mocks.h index e50975d8cff90..cc4ed56394983 100644 --- a/test/mocks/stats/mocks.h +++ b/test/mocks/stats/mocks.h @@ -51,7 +51,7 @@ class MockMetric : public virtual Metric { }; SymbolTable& symbolTable() override { return symbol_table_.get(); } - const SymbolTable& symbolTable() const override { return symbol_table_.get(); } + const SymbolTable& constSymbolTable() const override { return symbol_table_.get(); } // Note: cannot be mocked because it is accessed as a Property in a gmock EXPECT_CALL. This // creates a deadlock in gmock and is an unintended use of mock functions. @@ -205,9 +205,6 @@ class MockStore : public SymbolTableProvider, public StoreImpl { return histogram(symbol_table_->toString(name)); } - SymbolTable& symbolTable() override { return symbol_table_.get(); } - const SymbolTable& symbolTable() const override { return symbol_table_.get(); } - Test::Global symbol_table_; testing::NiceMock counter_; std::vector> histograms_; From c551dfcf3f812c18d67b3d39570d4b8f18e64445 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 22 May 2019 11:17:41 -0400 Subject: [PATCH 2/6] fix reference to const symbol table, whose accessor changed names. Signed-off-by: Joshua Marantz --- test/integration/filters/eds_ready_filter.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/integration/filters/eds_ready_filter.cc b/test/integration/filters/eds_ready_filter.cc index 37351fc538685..34719ff5ede06 100644 --- a/test/integration/filters/eds_ready_filter.cc +++ b/test/integration/filters/eds_ready_filter.cc @@ -17,10 +17,14 @@ namespace Envoy { // responds OK to all requests if it is. class EdsReadyFilter : public Http::PassThroughFilter { public: + // TODO(jmarantz): this const-cast of the symbol-table is needed only because + // it lacks a const-method to encode a string, that could fail (e.g. via + // absl::optional) if the tokens in it were not already present in the symbol + // table. No new stats are created by this filter. EdsReadyFilter(const Stats::Scope& root_scope) : root_scope_(root_scope), stat_name_("cluster.cluster_0.membership_healthy", - const_cast(root_scope_.symbolTable())) {} + const_cast(root_scope_.constSymbolTable())) {} Http::FilterHeadersStatus decodeHeaders(Http::HeaderMap&, bool) override { absl::optional> gauge = root_scope_.findGauge(stat_name_.statName()); From 1dc9df9f6302a5ea7dfd5c90efe0cc350851eabf Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 22 May 2019 13:39:54 -0400 Subject: [PATCH 3/6] Add comments and improve variable-naming. Remove TODO that appeared not-well-thought-out. Signed-off-by: Joshua Marantz --- source/common/stats/metric_impl.h | 9 +++++++++ test/common/stats/isolated_store_impl_test.cc | 6 +++--- test/common/stats/thread_local_store_test.cc | 6 +++--- test/integration/filters/eds_ready_filter.cc | 4 ---- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/source/common/stats/metric_impl.h b/source/common/stats/metric_impl.h index 4df03257c920d..9f89c4c038557 100644 --- a/source/common/stats/metric_impl.h +++ b/source/common/stats/metric_impl.h @@ -33,7 +33,16 @@ class MetricImpl : public virtual Metric { void iterateTagStatNames(const TagStatNameIterFn& fn) const override; void iterateTags(const TagIterFn& fn) const override; + // Metric implementations must each implement Metric::symboTable(). However, + // they can inherit the const version of that accessor from MetricImpl. const SymbolTable& constSymbolTable() const override { + // Cast our 'this', which is of type `const MetricImpl*` to a non-const + // pointer, so we can use it to call the subclass implementation of + // symbolTable(). That will be returned as a non-const SymbolTable&, + // which will become const on return. + // + // This pattern is used to share a single non-trival implementation to + // provide const and non-const variants of a method. return const_cast(this)->symbolTable(); } diff --git a/test/common/stats/isolated_store_impl_test.cc b/test/common/stats/isolated_store_impl_test.cc index 898fb10eb4c91..9dd29ed925ad0 100644 --- a/test/common/stats/isolated_store_impl_test.cc +++ b/test/common/stats/isolated_store_impl_test.cc @@ -115,9 +115,9 @@ TEST_F(StatsIsolatedStoreImplTest, AllWithSymbolTable) { TEST_F(StatsIsolatedStoreImplTest, ConstSymtabAccessor) { ScopePtr scope = store_.createScope("scope."); const Scope& cscope = *scope; - const SymbolTable& csymtab = cscope.constSymbolTable(); - SymbolTable& symtab = scope->symbolTable(); - EXPECT_EQ(&csymtab, &symtab); + const SymbolTable& const_symbol_table = cscope.constSymbolTable(); + SymbolTable& symbol_table = scope->symbolTable(); + EXPECT_EQ(&const_symbol_table, &symbol_table); } TEST_F(StatsIsolatedStoreImplTest, LongStatName) { diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index 75261063f5d76..52acdbb65ee8e 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -345,9 +345,9 @@ TEST_F(StatsThreadLocalStoreTest, SanitizePrefix) { TEST_F(StatsThreadLocalStoreTest, ConstSymtabAccessor) { ScopePtr scope = store_->createScope("scope."); const Scope& cscope = *scope; - const SymbolTable& csymtab = cscope.constSymbolTable(); - SymbolTable& symtab = scope->symbolTable(); - EXPECT_EQ(&csymtab, &symtab); + const SymbolTable& const_symbol_table = cscope.constSymbolTable(); + SymbolTable& symbol_table = scope->symbolTable(); + EXPECT_EQ(&const_symbol_table, &symbol_table); } TEST_F(StatsThreadLocalStoreTest, ScopeDelete) { diff --git a/test/integration/filters/eds_ready_filter.cc b/test/integration/filters/eds_ready_filter.cc index 34719ff5ede06..e97a60d2709e0 100644 --- a/test/integration/filters/eds_ready_filter.cc +++ b/test/integration/filters/eds_ready_filter.cc @@ -17,10 +17,6 @@ namespace Envoy { // responds OK to all requests if it is. class EdsReadyFilter : public Http::PassThroughFilter { public: - // TODO(jmarantz): this const-cast of the symbol-table is needed only because - // it lacks a const-method to encode a string, that could fail (e.g. via - // absl::optional) if the tokens in it were not already present in the symbol - // table. No new stats are created by this filter. EdsReadyFilter(const Stats::Scope& root_scope) : root_scope_(root_scope), stat_name_("cluster.cluster_0.membership_healthy", From be758d105cb52f50d147d8500e6da47213e7abb1 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 22 May 2019 13:41:40 -0400 Subject: [PATCH 4/6] spelling Signed-off-by: Joshua Marantz --- source/common/stats/metric_impl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/common/stats/metric_impl.h b/source/common/stats/metric_impl.h index 9f89c4c038557..b1f5dad9de65b 100644 --- a/source/common/stats/metric_impl.h +++ b/source/common/stats/metric_impl.h @@ -33,7 +33,7 @@ class MetricImpl : public virtual Metric { void iterateTagStatNames(const TagStatNameIterFn& fn) const override; void iterateTags(const TagIterFn& fn) const override; - // Metric implementations must each implement Metric::symboTable(). However, + // Metric implementations must each implement Metric::symbolTable(). However, // they can inherit the const version of that accessor from MetricImpl. const SymbolTable& constSymbolTable() const override { // Cast our 'this', which is of type `const MetricImpl*` to a non-const @@ -41,7 +41,7 @@ class MetricImpl : public virtual Metric { // symbolTable(). That will be returned as a non-const SymbolTable&, // which will become const on return. // - // This pattern is used to share a single non-trival implementation to + // This pattern is used to share a single non-trivial implementation to // provide const and non-const variants of a method. return const_cast(this)->symbolTable(); } From 4bf68246edb45fb7ba44f83ce4929fab0d232509 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 22 May 2019 17:20:55 -0400 Subject: [PATCH 5/6] Remove const_cast by passing in non-const symbol table from filter scope. Signed-off-by: Joshua Marantz --- test/integration/filters/eds_ready_filter.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration/filters/eds_ready_filter.cc b/test/integration/filters/eds_ready_filter.cc index e97a60d2709e0..487e05aebdc6f 100644 --- a/test/integration/filters/eds_ready_filter.cc +++ b/test/integration/filters/eds_ready_filter.cc @@ -17,10 +17,9 @@ namespace Envoy { // responds OK to all requests if it is. class EdsReadyFilter : public Http::PassThroughFilter { public: - EdsReadyFilter(const Stats::Scope& root_scope) + EdsReadyFilter(const Stats::Scope& root_scope, Stats::SymbolTable& symbol_table) : root_scope_(root_scope), - stat_name_("cluster.cluster_0.membership_healthy", - const_cast(root_scope_.constSymbolTable())) {} + stat_name_("cluster.cluster_0.membership_healthy", symbol_table) {} Http::FilterHeadersStatus decodeHeaders(Http::HeaderMap&, bool) override { absl::optional> gauge = root_scope_.findGauge(stat_name_.statName()); @@ -52,8 +51,9 @@ class EdsReadyFilterConfig : public Extensions::HttpFilters::Common::EmptyHttpFi createFilter(const std::string&, Server::Configuration::FactoryContext& factory_context) override { return [&factory_context](Http::FilterChainFactoryCallbacks& callbacks) { - callbacks.addStreamFilter( - std::make_shared(factory_context.api().rootScope())); + const Stats::Scope& scope = factory_context.api().rootScope(); + Stats::SymbolTable& symbol_table = factory_context.scope().symbolTable(); + callbacks.addStreamFilter(std::make_shared(scope, symbol_table)); }; } }; From a084921b943f233cc7a087b1a438a3e40a1e5c78 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 22 May 2019 17:23:23 -0400 Subject: [PATCH 6/6] format Signed-off-by: Joshua Marantz --- test/integration/filters/eds_ready_filter.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/integration/filters/eds_ready_filter.cc b/test/integration/filters/eds_ready_filter.cc index 487e05aebdc6f..c1fbf66e9b536 100644 --- a/test/integration/filters/eds_ready_filter.cc +++ b/test/integration/filters/eds_ready_filter.cc @@ -18,8 +18,7 @@ namespace Envoy { class EdsReadyFilter : public Http::PassThroughFilter { public: EdsReadyFilter(const Stats::Scope& root_scope, Stats::SymbolTable& symbol_table) - : root_scope_(root_scope), - stat_name_("cluster.cluster_0.membership_healthy", symbol_table) {} + : root_scope_(root_scope), stat_name_("cluster.cluster_0.membership_healthy", symbol_table) {} Http::FilterHeadersStatus decodeHeaders(Http::HeaderMap&, bool) override { absl::optional> gauge = root_scope_.findGauge(stat_name_.statName());