diff --git a/include/envoy/stats/symbol_table.h b/include/envoy/stats/symbol_table.h index 26b84c7be8e19..3463e5c7688cf 100644 --- a/include/envoy/stats/symbol_table.h +++ b/include/envoy/stats/symbol_table.h @@ -2,6 +2,7 @@ #include #include +#include #include #include "envoy/common/pure.h" @@ -26,6 +27,14 @@ class StatNameSet; using StatNameSetPtr = std::unique_ptr; +/** + * Holds a range of indexes indicating which parts of a stat-name are + * dynamic. This is used to transfer stats from hot-restart parent to child, + * retaining the same name structure. + */ +using DynamicSpan = std::pair; +using DynamicSpans = std::vector; + /** * SymbolTable manages a namespace optimized for stat names, exploiting their * typical composition from "."-separated tokens, with a significant overlap @@ -179,6 +188,19 @@ class SymbolTable { */ virtual StatNameSetPtr makeSet(absl::string_view name) PURE; + /** + * Identifies the dynamic components of a stat_name into an array of integer + * pairs, indicating the begin/end of spans of tokens in the stat-name that + * are created from StatNameDynamicStore or StatNameDynamicPool. + * + * This can be used to reconstruct the same exact StatNames in + * StatNames::mergeStats(), to enable stat continuity across hot-restart. + * + * @param stat_name the input stat name. + * @return the array of pairs indicating the bounds. + */ + virtual DynamicSpans getDynamicSpans(StatName stat_name) const PURE; + private: friend struct HeapStatData; friend class StatNameDynamicStorage; diff --git a/source/common/stats/fake_symbol_table_impl.h b/source/common/stats/fake_symbol_table_impl.h index bf23cb466242f..b9639ef44f4a2 100644 --- a/source/common/stats/fake_symbol_table_impl.h +++ b/source/common/stats/fake_symbol_table_impl.h @@ -122,6 +122,7 @@ class FakeSymbolTableImpl : public SymbolTable { void clearRecentLookups() override {} void setRecentLookupCapacity(uint64_t) override {} uint64_t recentLookupCapacity() const override { return 0; } + DynamicSpans getDynamicSpans(StatName) const override { return DynamicSpans(); } private: absl::string_view toStringView(const StatName& stat_name) const { diff --git a/source/common/stats/stat_merger.cc b/source/common/stats/stat_merger.cc index 1f064785bb1c7..b32ff6d7f332b 100644 --- a/source/common/stats/stat_merger.cc +++ b/source/common/stats/stat_merger.cc @@ -7,31 +7,6 @@ namespace Stats { StatMerger::StatMerger(Store& target_store) : temp_scope_(target_store.createScope("")) {} -StatMerger::DynamicSpans StatMerger::DynamicContext::encodeSegments(StatName stat_name) { - DynamicSpans dynamic_spans; - uint32_t index = 0; - auto record_dynamic = [&dynamic_spans, &index](absl::string_view str) { - DynamicSpan span; - span.first = index; - index += std::count(str.begin(), str.end(), '.'); - span.second = index; - ++index; - dynamic_spans.push_back(span); - }; - - // Use decodeTokens to suss out which components of stat_name are - // symbolic vs dynamic. The lambda that takes a Symbol is called - // for symbolic components. The lambda called with a string_view - // is called for dynamic components. - // - // Note that with fake symbol tables, the Symbol lambda is called - // once for each character in the string, and no dynamics will - // be recorded. - SymbolTableImpl::Encoding::decodeTokens( - stat_name.data(), stat_name.dataSize(), [&index](Symbol) { ++index; }, record_dynamic); - return dynamic_spans; -} - StatName StatMerger::DynamicContext::makeDynamicStatName(const std::string& name, const DynamicsMap& map) { auto iter = map.find(name); diff --git a/source/common/stats/stat_merger.h b/source/common/stats/stat_merger.h index 3d751527332a9..2eb1ca2ff7929 100644 --- a/source/common/stats/stat_merger.h +++ b/source/common/stats/stat_merger.h @@ -14,8 +14,6 @@ namespace Stats { // (typically hot restart parent+child) Envoy processes. class StatMerger { public: - using DynamicSpan = std::pair; - using DynamicSpans = std::vector; using DynamicsMap = absl::flat_hash_map; // Holds state needed to construct StatName with mixed dynamic/symbolic @@ -25,19 +23,6 @@ class StatMerger { DynamicContext(SymbolTable& symbol_table) : symbol_table_(symbol_table), symbolic_pool_(symbol_table), dynamic_pool_(symbol_table) {} - /** - * Identifies the dynamic components of a stat_name into an array of integer - * pairs, indicating the begin/end of spans of tokens in the stat-name that - * are created from StatNameDynamicStore or StatNameDynamicPool. - * - * This can be used to reconstruct the same exact StatNames in mergeStats(), - * to enable stat continuity across hot-restart. - * - * @param stat_name the input stat name. - * @return the array pair indicating the bounds. - */ - static DynamicSpans encodeSegments(StatName stat_name); - /** * Generates a StatName with mixed dynamic/symbolic components based on * the string and the dynamic_map obtained from encodeSegments. diff --git a/source/common/stats/symbol_table_impl.cc b/source/common/stats/symbol_table_impl.cc index cdef645b5a267..78d20924c3289 100644 --- a/source/common/stats/symbol_table_impl.cc +++ b/source/common/stats/symbol_table_impl.cc @@ -314,6 +314,32 @@ uint64_t SymbolTableImpl::getRecentLookups(const RecentLookupsFn& iter) const { return total; } +DynamicSpans SymbolTableImpl::getDynamicSpans(StatName stat_name) const { + DynamicSpans dynamic_spans; + + uint32_t index = 0; + auto record_dynamic = [&dynamic_spans, &index](absl::string_view str) { + DynamicSpan span; + span.first = index; + index += std::count(str.begin(), str.end(), '.'); + span.second = index; + ++index; + dynamic_spans.push_back(span); + }; + + // Use decodeTokens to suss out which components of stat_name are + // symbolic vs dynamic. The lambda that takes a Symbol is called + // for symbolic components. The lambda called with a string_view + // is called for dynamic components. + // + // Note that with fake symbol tables, the Symbol lambda is called + // once for each character in the string, and no dynamics will + // be recorded. + Encoding::decodeTokens( + stat_name.data(), stat_name.dataSize(), [&index](Symbol) { ++index; }, record_dynamic); + return dynamic_spans; +} + void SymbolTableImpl::setRecentLookupCapacity(uint64_t capacity) { Thread::LockGuard lock(lock_); recent_lookups_.setCapacity(capacity); diff --git a/source/common/stats/symbol_table_impl.h b/source/common/stats/symbol_table_impl.h index 50e3c565356b2..74319c1f5e1ee 100644 --- a/source/common/stats/symbol_table_impl.h +++ b/source/common/stats/symbol_table_impl.h @@ -206,6 +206,7 @@ class SymbolTableImpl : public SymbolTable { void clearRecentLookups() override; void setRecentLookupCapacity(uint64_t capacity) override; uint64_t recentLookupCapacity() const override; + DynamicSpans getDynamicSpans(StatName stat_name) const override; private: friend class StatName; diff --git a/source/server/hot_restarting_child.cc b/source/server/hot_restarting_child.cc index df7f2c342b379..7240fb1e97087 100644 --- a/source/server/hot_restarting_child.cc +++ b/source/server/hot_restarting_child.cc @@ -91,19 +91,18 @@ void HotRestartingChild::sendParentTerminateRequest() { void HotRestartingChild::mergeParentStats(Stats::Store& stats_store, const HotRestartMessage::Reply::Stats& stats_proto) { - using StatMerger = Stats::StatMerger; if (!stat_merger_) { - stat_merger_ = std::make_unique(stats_store); + stat_merger_ = std::make_unique(stats_store); } // Convert the protobuf for serialized dynamic spans into the structure // required by StatMerger. - StatMerger::DynamicsMap dynamics; + Stats::StatMerger::DynamicsMap dynamics; for (auto iter : stats_proto.dynamics()) { - StatMerger::DynamicSpans& spans = dynamics[iter.first]; + Stats::DynamicSpans& spans = dynamics[iter.first]; for (int i = 0; i < iter.second.spans_size(); ++i) { const HotRestartMessage::Reply::Span& span_proto = iter.second.spans(i); - spans.push_back(StatMerger::DynamicSpan(span_proto.first(), span_proto.last())); + spans.push_back(Stats::DynamicSpan(span_proto.first(), span_proto.last())); } } stat_merger_->mergeStats(stats_proto.counter_deltas(), stats_proto.gauges(), dynamics); diff --git a/source/server/hot_restarting_parent.cc b/source/server/hot_restarting_parent.cc index 510c2d9714e8b..6022b204cf890 100644 --- a/source/server/hot_restarting_parent.cc +++ b/source/server/hot_restarting_parent.cc @@ -163,14 +163,13 @@ void HotRestartingParent::Internal::recordDynamics(HotRestartMessage::Reply::Sta // components using a dynamic representation. // // See https://github.com/envoyproxy/envoy/issues/9874 for more details. - using Stats::StatMerger; - StatMerger::DynamicSpans spans = StatMerger::DynamicContext::encodeSegments(stat_name); + Stats::DynamicSpans spans = server_->stats().symbolTable().getDynamicSpans(stat_name); // Convert that C++ structure (controlled by stat_merger.cc) into a protobuf // for serialization. if (!spans.empty()) { HotRestartMessage::Reply::RepeatedSpan spans_proto; - for (const StatMerger::DynamicSpan& span : spans) { + for (const Stats::DynamicSpan& span : spans) { HotRestartMessage::Reply::Span* span_proto = spans_proto.add_spans(); span_proto->set_first(span.first); span_proto->set_last(span.second); diff --git a/test/common/stats/stat_merger_corpus/example4 b/test/common/stats/stat_merger_corpus/example4 new file mode 100644 index 0000000000000..1e374a32ff54d Binary files /dev/null and b/test/common/stats/stat_merger_corpus/example4 differ diff --git a/test/common/stats/stat_merger_fuzz_test.cc b/test/common/stats/stat_merger_fuzz_test.cc index 18213a0f1b7ab..8e4541778213c 100644 --- a/test/common/stats/stat_merger_fuzz_test.cc +++ b/test/common/stats/stat_merger_fuzz_test.cc @@ -6,6 +6,8 @@ #include "test/fuzz/fuzz_runner.h" #include "test/fuzz/utility.h" +#include "absl/strings/str_replace.h" + namespace Envoy { namespace Stats { namespace Fuzz { @@ -15,6 +17,13 @@ void testDynamicEncoding(absl::string_view data, SymbolTable& symbol_table) { StatNamePool symbolic_pool(symbol_table); std::vector stat_names; + // This local string is write-only; it's used to help when debugging + // a crash. If a crash is found, you can print the unit_test_encoding + // in the debugger and then add that as a test-case in stat_merger_text.cc, + // in StatMergerDynamicTest.DynamicsWithFakeSymbolTable and + // StatMergerDynamicTest.DynamicsWithRealSymbolTable. + std::string unit_test_encoding; + for (uint32_t index = 0; index < data.size();) { // Select component lengths between 0 and 7 bytes inclusive, and ensure it // doesn't overrun our buffer. It's OK to get very small or empty segments. @@ -26,10 +35,15 @@ void testDynamicEncoding(absl::string_view data, SymbolTable& symbol_table) { // determine whether to treat this segment symbolic or not. absl::string_view segment = data.substr(index, num_bytes); bool is_symbolic = (data[index] & 0x8) == 0x0; + if (index != 0) { + unit_test_encoding += "."; + } index += num_bytes + 1; if (is_symbolic) { + absl::StrAppend(&unit_test_encoding, segment); stat_names.push_back(symbolic_pool.add(segment)); } else { + absl::StrAppend(&unit_test_encoding, "D:", absl::StrReplaceAll(segment, {{".", ","}})); stat_names.push_back(dynamic_pool.add(segment)); } } @@ -40,7 +54,7 @@ void testDynamicEncoding(absl::string_view data, SymbolTable& symbol_table) { StatMerger::DynamicContext dynamic_context(symbol_table); std::string name = symbol_table.toString(stat_name); StatMerger::DynamicsMap dynamic_map; - dynamic_map[name] = StatMerger::DynamicContext::encodeSegments(stat_name); + dynamic_map[name] = symbol_table.getDynamicSpans(stat_name); StatName decoded = dynamic_context.makeDynamicStatName(name, dynamic_map); FUZZ_ASSERT(name == symbol_table.toString(decoded)); diff --git a/test/common/stats/stat_merger_test.cc b/test/common/stats/stat_merger_test.cc index e0cc604d38060..3cb0cac86caef 100644 --- a/test/common/stats/stat_merger_test.cc +++ b/test/common/stats/stat_merger_test.cc @@ -51,7 +51,7 @@ class StatMergerTest : public testing::Test { std::string name = symbol_table.toString(stat_name); StatMerger::DynamicsMap dynamic_map; - dynamic_map[name] = StatMerger::DynamicContext::encodeSegments(stat_name); + dynamic_map[name] = symbol_table.getDynamicSpans(stat_name); StatMerger::DynamicContext dynamic_context(symbol_table); StatName decoded = dynamic_context.makeDynamicStatName(name, dynamic_map); EXPECT_EQ(stat_name, decoded) << name; @@ -256,7 +256,7 @@ class StatMergerDynamicTest : public testing::Test { std::string name = symbol_table_->toString(stat_name); StatMerger::DynamicsMap dynamic_map; - StatMerger::DynamicSpans spans = StatMerger::DynamicContext::encodeSegments(stat_name); + DynamicSpans spans = symbol_table_->getDynamicSpans(stat_name); uint32_t size = 0; if (!spans.empty()) { dynamic_map[name] = spans; @@ -276,6 +276,12 @@ class StatMergerDynamicTest : public testing::Test { TEST_F(StatMergerDynamicTest, DynamicsWithRealSymbolTable) { init(std::make_unique()); + for (uint32_t i = 1; i < 256; ++i) { + char ch = static_cast(i); + absl::string_view one_char(&ch, 1); + EXPECT_EQ(1, dynamicEncodeDecodeTest(absl::StrCat("D:", one_char))) << "dynamic=" << one_char; + EXPECT_EQ(0, dynamicEncodeDecodeTest(one_char)) << "symbolic=" << one_char; + } EXPECT_EQ(0, dynamicEncodeDecodeTest("normal")); EXPECT_EQ(1, dynamicEncodeDecodeTest("D:dynamic")); EXPECT_EQ(0, dynamicEncodeDecodeTest("hello.world")); @@ -299,6 +305,12 @@ TEST_F(StatMergerDynamicTest, DynamicsWithRealSymbolTable) { TEST_F(StatMergerDynamicTest, DynamicsWithFakeSymbolTable) { init(std::make_unique()); + for (uint32_t i = 1; i < 256; ++i) { + char ch = static_cast(i); + absl::string_view one_char(&ch, 1); + EXPECT_EQ(0, dynamicEncodeDecodeTest(absl::StrCat("D:", one_char))) << "dynamic=" << one_char; + EXPECT_EQ(0, dynamicEncodeDecodeTest(one_char)) << "symbolic=" << one_char; + } EXPECT_EQ(0, dynamicEncodeDecodeTest("normal")); EXPECT_EQ(0, dynamicEncodeDecodeTest("D:dynamic")); EXPECT_EQ(0, dynamicEncodeDecodeTest("hello.world"));