diff --git a/doc/developer-guide/internal-libraries/Metrics.en.rst b/doc/developer-guide/internal-libraries/Metrics.en.rst index 4822170546e..ba68d1a9df1 100644 --- a/doc/developer-guide/internal-libraries/Metrics.en.rst +++ b/doc/developer-guide/internal-libraries/Metrics.en.rst @@ -98,6 +98,24 @@ never as a side effect of a broad query. renamed or removed between releases without notice. Do not build monitoring on them; use the published aggregate instead. +Enumerating metrics +=================== + +``for_each`` visits every listed metric of a store, in creation order: + +.. code-block:: cpp + + ts::Metrics::instance().for_each([](std::string_view name, ts::Metrics::MetricType type, int64_t value) { + // ... + }); + +This is the only way to enumerate a store. There is no public iterator, and deliberately so: +enumeration is always the whole store, so nothing can hold a cursor across changes to the store or +name a position the walk would skip. Reach a single metric by name with ``lookup`` instead. + +The callback must not create a metric, which would be an attempt to grow the store from inside a +pass over it. + Derived metrics =============== @@ -188,6 +206,42 @@ sampling point*, not the true peak. There are two ways to arrange this, with dif Which is appropriate depends on whether the consumer needs to aggregate over time downstream. +Unlisting a metric +================== + +A metric can be taken out of the store's listing after the fact. An unlisted metric is skipped by +iteration, so it disappears from ``traffic_ctl metric match``, the JSONRPC record lookup and +``stats_over_http``, without either of those consumers needing to know about it: + +.. code-block:: cpp + + auto &m = ts::Metrics::instance(); + + m.unlist(id); // by id + m.unlist("proxy.process.example"); // or by name + + m.relist(id); // put it back + +The slot, the name and the atomic all survive: an unlisted number that still rings. An unlisted +metric still resolves through ``lookup``, so an exact name query, a logging field reference and +``TSStatFindName`` all continue to work, and its value may still be read and written. Creating the +same name again relists it and returns the same id with its accumulated value intact, so a metric +that comes and goes with a configuration setting costs nothing to bring back. + +This exists because the decision to publish a name is otherwise made once, when the metric is first +created, and can never be revisited. Any metric whose name or publication policy depends on a +runtime changeable setting needs a way to retract a name it has already published. + +.. important:: + + Unlisting hides; it does not free. The slot and the name remain allocated against the storage + limit below. Unlisting does not make an unbounded naming scheme safe. + +.. note:: + + The set walked is fixed when ``for_each`` begins, so a metric created while it runs is not + visited. + Storage limits ============== diff --git a/include/tsutil/Metrics.h b/include/tsutil/Metrics.h index 741b1e069a5..344b3cd892b 100644 --- a/include/tsutil/Metrics.h +++ b/include/tsutil/Metrics.h @@ -93,14 +93,25 @@ class Metrics static constexpr int METRIC_TYPE_BITS = 29; static constexpr int METRIC_TYPE_MASK = 0x1FFF; + /// The reserved slot 0 of every store, returned when an id cannot be produced. Present under this + /// name in both stores, so a consumer querying both has to expect it twice. + static constexpr std::string_view BAD_ID_NAME{"proxy.process.api.metrics.bad_id"}; + private: - using NameAndId = std::tuple; - using LookupTable = std::unordered_map; - using NameStorage = std::array; - using AtomicStorage = std::array; - using NamesAndAtomics = std::tuple; + using NameAndId = std::tuple; + using LookupTable = std::unordered_map; + using NameStorage = std::array; + using AtomicStorage = std::array; + /// Per slot flag bits, see @c UNLISTED. A parallel array rather than a member of @c NameAndId + /// because an atomic member would make that tuple neither copyable nor movable, and the slot is + /// written there with a tuple assignment. + using FlagStorage = std::array, MAX_SIZE>; + using NamesAndAtomics = std::tuple; using BlobStorage = std::array, MAX_BLOBS>; + /// The slot exists and is still resolvable by name or id, but is skipped by iteration. + static constexpr uint8_t UNLISTED = 0x01; + public: Metrics(const self_type &) = delete; self_type &operator=(const self_type &) = delete; @@ -145,6 +156,57 @@ class Metrics { return _storage->lookup(id, out_name, type); } + + /** Take @a id out of the store's listing. + * + * An unlisted metric keeps its slot, its name and its atomic. It is skipped by iteration, so it + * vanishes from everything that enumerates the store, but it still resolves through @c lookup and + * its value may still be read and written -- an unlisted number that still rings. Creating the + * same name again relists it and returns the same id. + * + * @return @c false if @a id does not name an allocated slot. + */ + bool + unlist(IdType id) + { + return _storage->set_listed(id, false); + } + + /// Put @a id back in the listing. @see unlist + bool + relist(IdType id) + { + return _storage->set_listed(id, true); + } + + /** Whether @a id is enumerated. + * + * @return @c false for an unlisted metric, and also for an id that names no allocated slot -- + * neither appears in iteration. + */ + bool + listed(IdType id) const + { + return _storage->listed(id); + } + + /// Convenience for callers that publish by name and do not retain the id. @see unlist + bool + unlist(std::string_view name) + { + auto id = lookup(name); + + return id != NOT_FOUND && unlist(id); + } + + /// Convenience for callers that publish by name and do not retain the id. @see relist + bool + relist(std::string_view name) + { + auto id = lookup(name); + + return id != NOT_FOUND && relist(id); + } AtomicType & operator[](IdType id) { @@ -191,87 +253,21 @@ class Metrics return _storage->valid(id); } - // Static methods to encapsulate access to the atomic's - class iterator - { - public: - using iterator_category = std::input_iterator_tag; - using value_type = std::tuple; - using difference_type = ptrdiff_t; - using pointer = value_type *; - using reference = value_type &; - - iterator(const Metrics &m, IdType pos) : _metrics(m), _it(pos) {} - - iterator & - operator++() - { - next(); - - return *this; - } - - iterator - operator++(int) - { - iterator result = *this; - - next(); - - return result; - } - - value_type - operator*() const - { - std::string_view name; - MetricType type; - auto metric = _metrics.lookup(_it, &name, &type); - - return std::make_tuple(name, type, metric->_value.load()); - } - - bool - operator==(const iterator &o) const - { - return _it == o._it && std::addressof(_metrics) == std::addressof(o._metrics); - } - - bool - operator!=(const iterator &o) const - { - return _it != o._it || std::addressof(_metrics) != std::addressof(o._metrics); - } - - private: - void next(); - - const Metrics &_metrics; - Metrics::IdType _it; - }; - - iterator - begin() const - { - return iterator(*this, 0); - } - - iterator - end() const - { - return iterator(*this, _storage->next_free_id()); - } - - iterator - find(const std::string_view name) const + /** Visit every listed metric. + * + * @a func is called as func(std::string_view name, MetricType type, int64_t value) for + * each listed metric, in creation order. Unlisted metrics are skipped, @see unlist. + * + * The set walked is fixed when the call begins: a metric created while it runs is not visited. + * Enumeration is deliberately the whole store and nothing less. There is no cursor to hold, so + * nothing can outlive the walk or name a slot the walk would not visit, and @a func may not + * create a metric, which would be an attempt to grow the store from inside a pass over it. + */ + template + void + for_each(F &&func) const { - auto id = lookup(name); - - if (id == NOT_FOUND) { - return end(); - } else { - return iterator(*this, id); - } + _storage->for_each(std::forward(func)); } private: @@ -337,7 +333,7 @@ class Metrics _blobs[0] = std::make_unique(); release_assert(_blobs[0]); // Reserve slot 0 for errors, this should always be 0 - release_assert(0 == create("proxy.process.api.metrics.bad_id", MetricType::COUNTER)); + release_assert(0 == create(BAD_ID_NAME, MetricType::COUNTER)); } ~Storage() {} @@ -349,6 +345,46 @@ class Metrics AtomicType *lookup(Metrics::IdType id, std::string_view *out_name = nullptr, MetricType *out_type = nullptr) const; std::string_view name(IdType id) const; MetricType type(IdType id) const; + bool set_listed(IdType id, bool listed); + bool listed(IdType id) const; + + /** Visit every listed slot, in creation order. + * + * @see Metrics::for_each, which is how callers reach this. + * + * The bound is read once, up front. Acquiring it acquires every slot below it, which is what + * lets the walk read names and values without the mutex: a slot's name is written before the + * release store that publishes it, and never changes. + */ + template + void + for_each(F &&func) const + { + auto const [last_blob, last_off] = _splitID(next_free_id()); + + for (uint16_t blob = 0; blob <= last_blob; ++blob) { + NamesAndAtomics const *entries = _blobs[blob].get(); + + // The bound covers every blob below it, so this is belt and braces. + if (entries == nullptr) { + break; + } + + uint16_t const limit = blob == last_blob ? last_off : MAX_SIZE; + + for (uint16_t off = 0; off < limit; ++off) { + if ((std::get<2>(*entries)[off].load(MEMORY_ORDER) & UNLISTED) != 0) { + continue; + } + + auto const &slot = std::get<0>(*entries)[off]; + + // The type comes from the slot's own id, not from the position, so it is the type the + // metric was created with. + func(std::string_view{std::get<0>(slot)}, _extractType(std::get<1>(slot)), std::get<1>(*entries)[off].load()); + } + } + } /// The id the next slot will get, which is also iteration's exclusive bound. IdType diff --git a/src/records/RecCore.cc b/src/records/RecCore.cc index 91dd163717c..36ffa3ea489 100644 --- a/src/records/RecCore.cc +++ b/src/records/RecCore.cc @@ -585,7 +585,7 @@ RecLookupMatchingRecords(unsigned rec_type, const char *match, void (*callback)( if ((rec_type & (RECT_PROCESS | RECT_NODE | RECT_PLUGIN))) { // First find the new metrics, this is a bit of a hack, because we still use the old // librecords callback with a "pseudo" record. - for (auto &&[name, type, val] : ts::Metrics::instance()) { + ts::Metrics::instance().for_each([&](std::string_view name, ts::Metrics::MetricType type, int64_t val) { if (regex.exec(name.data())) { RecRecord tmp{}; @@ -596,7 +596,7 @@ RecLookupMatchingRecords(unsigned rec_type, const char *match, void (*callback)( tmp.data.rec_int = val; callback(&tmp, data); } - } + }); // Finally check string metrics ts::Metrics::StaticString::instance().for_each([&](const std::string &name, const std::string &value) { if (regex.exec(name)) { @@ -617,14 +617,14 @@ RecLookupMatchingRecords(unsigned rec_type, const char *match, void (*callback)( if (rec_type & RECT_HIDDEN_METRIC) { // Opt-in only: hidden metrics are never reachable through RECT_ALL, see RecDefs.h. auto &hidden = ts::Metrics::hidden_instance(); - // Slot 0 of every Storage is the reserved bad_id placeholder, so it exists under the same name - // in both stores. Skip it here, otherwise a query matching it returns two identically named - // records that differ only in value. - auto it = hidden.begin(); - ++it; - for (; it != hidden.end(); ++it) { - auto &&[name, type, val] = *it; + hidden.for_each([&](std::string_view name, ts::Metrics::MetricType type, int64_t val) { + // Slot 0 of every Storage is the reserved bad_id placeholder, so it exists under the same + // name in both stores. Skip it here, otherwise a query matching it returns two identically + // named records that differ only in value. + if (name == ts::Metrics::BAD_ID_NAME) { + return; + } if (regex.exec(name.data())) { RecRecord tmp{}; @@ -641,7 +641,7 @@ RecLookupMatchingRecords(unsigned rec_type, const char *match, void (*callback)( tmp.data.rec_int = val; callback(&tmp, data); } - } + }); } int num_records = g_num_records; @@ -968,11 +968,11 @@ RecDumpRecords(RecT rec_type, RecDumpEntryCb callback, void *edata) // Dump all new metrics as well (no "type" for them) RecData datum; - for (auto &&[name, type, val] : ts::Metrics::instance()) { + ts::Metrics::instance().for_each([&](std::string_view name, Metrics::MetricType type, int64_t val) { datum.rec_int = val; callback(RECT_PLUGIN, edata, true, name.data(), type == Metrics::MetricType::COUNTER ? TS_RECORDDATATYPE_COUNTER : TS_RECORDDATATYPE_INT, &datum); - } + }); ts::Metrics::StaticString::instance().for_each([&](const std::string &name, const std::string &value) { datum.rec_string = const_cast(value.c_str()); diff --git a/src/records/unit_tests/test_RecHiddenMetricLookup.cc b/src/records/unit_tests/test_RecHiddenMetricLookup.cc index d1e2d4c4fcb..c4f6e2e3b16 100644 --- a/src/records/unit_tests/test_RecHiddenMetricLookup.cc +++ b/src/records/unit_tests/test_RecHiddenMetricLookup.cc @@ -115,3 +115,59 @@ TEST_CASE("RecLookupMatchingRecords - hidden metrics", "[librecords][RecLookup][ } } } + +TEST_CASE("RecLookupMatchingRecords - unlisted metrics", "[librecords][RecLookup][unlisted]") +{ + const std::string name = "proxy.test.lookup.unlisted_gauge"; + auto *m = ts::Metrics::Gauge::createPtr(name); + + REQUIRE(m != nullptr); + m->store(7); + + auto &metrics = ts::Metrics::instance(); + auto id = metrics.lookup(name); + + REQUIRE(id != ts::Metrics::NOT_FOUND); + REQUIRE(metrics.unlist(id)); + + SECTION("an unlisted metric is not enumerated") + { + std::vector entries; + + REQUIRE(RecLookupMatchingRecords(RECT_ALL, name.c_str(), collect, &entries) == REC_ERR_OKAY); + + for (const auto &e : entries) { + CHECK(e.name != name); + } + } + + SECTION("an unlisted metric is still found by exact name") + { + // RecLookupRecord resolves through Metrics::lookup() rather than iteration, which is what keeps + // logging fields and TSStatFindName working across an unlisting. + std::vector entries; + + REQUIRE(RecLookupRecord(name.c_str(), collect, &entries) == REC_ERR_OKAY); + REQUIRE(entries.size() == 1); + CHECK(entries[0].name == name); + CHECK(entries[0].int_value == 7); + } + + SECTION("relisting puts it back in enumeration") + { + REQUIRE(metrics.relist(id)); + + std::vector entries; + bool found = false; + + REQUIRE(RecLookupMatchingRecords(RECT_ALL, name.c_str(), collect, &entries) == REC_ERR_OKAY); + for (const auto &e : entries) { + if (e.name == name) { + found = true; + CHECK(e.int_value == 7); + } + } + + REQUIRE(found); + } +} diff --git a/src/tsutil/Metrics.cc b/src/tsutil/Metrics.cc index 0b9ef8dc5ac..83c4c8d19a3 100644 --- a/src/tsutil/Metrics.cc +++ b/src/tsutil/Metrics.cc @@ -77,6 +77,10 @@ Metrics::Storage::create(std::string_view name, const MetricType type) auto it = _lookups.find(name); if (it != _lookups.end()) { + // Re-creating a name relists it: same slot, same atomic, and whatever value it accumulated + // while it was out of the listing. A name in _lookups always names an allocated slot. + set_listed(it->second, true); + return it->second; } @@ -137,8 +141,8 @@ Metrics::Storage::lookup(Metrics::IdType id, std::string_view *out_name, Metrics } if (out_type) { - // don't trust the passed in id to get the type as it might have been manufactured (i.e. from iterators) - // so get the type from the storage tuple. + // don't trust the passed in id to get the type as it might have been manufactured, so get the + // type from the storage tuple. *out_type = _extractType(std::get<1>(std::get<0>(*blob)[offset])); } @@ -190,18 +194,37 @@ Metrics::Storage::type(IdType id) const return _extractType(id); } -// Iterator implementation -void -Metrics::iterator::next() +bool +Metrics::Storage::set_listed(Metrics::IdType id, bool listed) { - auto [blob, offset] = _metrics._splitID(_it); + if (!_is_allocated(id)) { + return false; + } - if (++offset == MAX_SIZE) { - ++blob; - offset = 0; + auto [blob_ix, offset] = _splitID(id); + Metrics::NamesAndAtomics *blob = _blobs[blob_ix].get(); + + // Only this bit, so a flag added later is not clobbered by unlisting or relisting. + if (listed) { + std::get<2>(*blob)[offset].fetch_and(static_cast(~UNLISTED), MEMORY_ORDER); + } else { + std::get<2>(*blob)[offset].fetch_or(UNLISTED, MEMORY_ORDER); } - _it = _makeId(blob, offset, MetricType::COUNTER); + return true; +} + +bool +Metrics::Storage::listed(Metrics::IdType id) const +{ + if (!_is_allocated(id)) { + return false; + } + + auto [blob_ix, offset] = _splitID(id); + Metrics::NamesAndAtomics *blob = _blobs[blob_ix].get(); + + return (std::get<2>(*blob)[offset].load(MEMORY_ORDER) & UNLISTED) == 0; } namespace details diff --git a/src/tsutil/unit_tests/test_Metrics.cc b/src/tsutil/unit_tests/test_Metrics.cc index f267097807f..639eeeaf175 100644 --- a/src/tsutil/unit_tests/test_Metrics.cc +++ b/src/tsutil/unit_tests/test_Metrics.cc @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -40,33 +39,37 @@ TEST_CASE("Metrics", "[libtsapi][Metrics]") { auto &m = Metrics::instance(); - SECTION("iterator") + SECTION("for_each") { - auto [name, type, value] = *m.begin(); - REQUIRE(value == 0); - REQUIRE(type == Metrics::MetricType::COUNTER); - REQUIRE(name == "proxy.process.api.metrics.bad_id"); + std::vector names; + int64_t first_value = -1; + Metrics::MetricType first_type{}; + + m.for_each([&](std::string_view name, Metrics::MetricType type, int64_t value) { + if (names.empty()) { + first_value = value; + first_type = type; + } + names.emplace_back(name); + }); - REQUIRE(m.begin() != m.end()); + // The reserved bad_id occupies the first slot of every store, so it is always visited first. + REQUIRE_FALSE(names.empty()); + REQUIRE(names.front() == Metrics::BAD_ID_NAME); + REQUIRE(first_value == 0); + REQUIRE(first_type == Metrics::MetricType::COUNTER); - // Other test cases share this process-wide store, so the number of metrics already present - // is not knowable here. Assert the delta from creating one metric instead of an absolute - // iterator position. - auto pre_count = std::distance(m.begin(), m.end()); + // Other test cases share this process-wide store, so the number of metrics already present is + // not knowable here. Assert the delta from creating one metric instead of an absolute count. + auto const pre_count = names.size(); - Metrics::Counter::create("iterator.marker"); - REQUIRE(std::distance(m.begin(), m.end()) == pre_count + 1); + Metrics::Counter::create("for_each.marker"); - auto it = m.begin(); - std::advance(it, pre_count); - REQUIRE(it != m.end()); - ++it; - REQUIRE(it == m.end()); + names.clear(); + m.for_each([&](std::string_view name, Metrics::MetricType, int64_t) { names.emplace_back(name); }); - auto it2 = m.begin(); - std::advance(it2, pre_count); - it2++; - REQUIRE(it2 == m.end()); + REQUIRE(names.size() == pre_count + 1); + REQUIRE(names.back() == "for_each.marker"); // creation order, so the newest is last } SECTION("New metric") @@ -437,18 +440,12 @@ TEST_CASE("Metrics hidden store", "[libtsapi][Metrics]") // Not visible in the published store, by name or by iteration. REQUIRE(m.lookup("hidden.only") == Metrics::NOT_FOUND); - for (auto &&[name, type, value] : m) { - REQUIRE(name != "hidden.only"); - } + m.for_each([](std::string_view name, Metrics::MetricType, int64_t) { REQUIRE(name != "hidden.only"); }); // Visible in the hidden store. REQUIRE(h.lookup("hidden.only") != Metrics::NOT_FOUND); bool found = false; - for (auto &&[name, type, value] : h) { - if (name == "hidden.only") { - found = true; - } - } + h.for_each([&](std::string_view name, Metrics::MetricType, int64_t) { found |= (name == "hidden.only"); }); REQUIRE(found); } @@ -707,3 +704,214 @@ TEST_CASE("Metrics id lookup is safe against concurrent creation", "[libtsapi][M // would mean the sweep above never left the first one. REQUIRE(hi - lo > Metrics::MAX_SIZE); } + +TEST_CASE("Metrics unlisting", "[libtsapi][Metrics]") +{ + auto &m = Metrics::instance(); + + SECTION("an unlisted metric is skipped by iteration") + { + Metrics::Counter::create("unlisted.iter.before"); + auto target = Metrics::Counter::create("unlisted.iter.target"); + Metrics::Counter::create("unlisted.iter.after"); + + REQUIRE(m.unlist(target)); + + bool saw_before = false, saw_target = false, saw_after = false; + + m.for_each([&](std::string_view name, Metrics::MetricType, int64_t) { + saw_before |= (name == "unlisted.iter.before"); + saw_target |= (name == "unlisted.iter.target"); + saw_after |= (name == "unlisted.iter.after"); + }); + + REQUIRE(saw_before); + REQUIRE_FALSE(saw_target); + REQUIRE(saw_after); + } + + SECTION("creating an unlisted name again relists it") + { + auto p = Metrics::Counter::createPtr("unlisted.resurrect"); + auto id = m.lookup("unlisted.resurrect"); + + Metrics::Counter::increment(p, 5); + REQUIRE(m.unlist(id)); + REQUIRE_FALSE(m.listed(id)); + + // Same name, same id, same atomic, and the mark is gone. + auto p2 = Metrics::Counter::createPtr("unlisted.resurrect"); + REQUIRE(p2 == p); + REQUIRE(m.lookup("unlisted.resurrect") == id); + REQUIRE(m.listed(id)); + + // Visible again, with its value intact. + bool found = false; + m.for_each([&](std::string_view name, Metrics::MetricType, int64_t value) { + if (name == "unlisted.resurrect") { + found = true; + REQUIRE(value == 5); + } + }); + REQUIRE(found); + } + + SECTION("unlist and relist by name") + { + auto id = Metrics::Counter::create("unlisted.byname"); + + REQUIRE(m.unlist("unlisted.byname")); + REQUIRE_FALSE(m.listed(id)); + + REQUIRE(m.relist("unlisted.byname")); + REQUIRE(m.listed(id)); + + bool found = false; + m.for_each([&](std::string_view name, Metrics::MetricType, int64_t) { found |= (name == "unlisted.byname"); }); + REQUIRE(found); + + // A name that was never created cannot be marked. + REQUIRE_FALSE(m.unlist("unlisted.byname.never.created")); + } + + SECTION("an unlisted metric is still resolvable and still counts") + { + auto p = Metrics::Counter::createPtr("unlisted.resolvable"); + auto id = m.lookup("unlisted.resolvable"); + + REQUIRE(m.unlist(id)); + + // Hidden from enumeration is not gone: by name, by id, and through the atomic it is unchanged. + REQUIRE(m.lookup("unlisted.resolvable") == id); + REQUIRE(m.lookup(id) == p); + REQUIRE(m.valid(id)); + REQUIRE(m.name(id) == "unlisted.resolvable"); + REQUIRE(m.type(id) == Metrics::MetricType::COUNTER); + + Metrics::Counter::increment(p, 3); + REQUIRE(Metrics::Counter::load(p) == 3); + } + + SECTION("for_each skips an unlisted first slot") + { + // Slot 0 is the reserved bad_id, so it is the first slot the walk considers. The anchor keeps + // the assertions below from passing on an empty walk. + auto bad_id = m.lookup(Metrics::BAD_ID_NAME); + REQUIRE(bad_id == 0); + + Metrics::Counter::create("unlisted.first.anchor"); + + auto first_name = [&]() { + std::string first; + bool seen = false; + + m.for_each([&](std::string_view name, Metrics::MetricType, int64_t) { + if (!seen) { + first = name; + seen = true; + } + }); + + return first; + }; + + REQUIRE(m.unlist(bad_id)); + auto const while_unlisted = first_name(); + + // Relist before asserting: a failed assertion ends the section, and leaving bad_id unlisted + // would break every later test case that expects to see it. + REQUIRE(m.relist(bad_id)); + auto const while_listed = first_name(); + + REQUIRE_FALSE(while_unlisted.empty()); // the walk did visit something + REQUIRE(while_unlisted != Metrics::BAD_ID_NAME); + REQUIRE(while_listed == Metrics::BAD_ID_NAME); + } + + SECTION("an unlisted run at the end of the store terminates iteration") + { + // Skipping the last slots in the store is the case where the skip loop has nothing unmarked + // left to land on. The anchor is a listed metric of this section's own, so the loop below is + // known to have run without depending on what other sections left in the shared store. + constexpr int COUNT = 8; + std::vector names; + + Metrics::Counter::create("unlisted.tail.anchor"); + + names.reserve(COUNT); + for (int i = 0; i < COUNT; ++i) { + names.push_back("unlisted.tail." + std::to_string(i)); + REQUIRE(m.unlist(Metrics::Counter::create(names[i]))); + } + + bool saw_anchor = false; + + m.for_each([&](std::string_view name, Metrics::MetricType, int64_t) { + saw_anchor |= (name == "unlisted.tail.anchor"); + for (auto const &n : names) { + REQUIRE(name != n); + } + }); + + REQUIRE(saw_anchor); + } + + SECTION("for_each reports the type each metric was created with") + { + // The type comes from the slot's own stored id rather than from the walk's position, which is + // what keeps a gauge from being reported as a counter. + Metrics::Gauge::createPtr("unlisted.typed.gauge"); + Metrics::Counter::createPtr("unlisted.typed.counter"); + + bool saw_gauge = false, saw_counter = false; + + m.for_each([&](std::string_view name, Metrics::MetricType type, int64_t) { + if (name == "unlisted.typed.gauge") { + saw_gauge = true; + REQUIRE(type == Metrics::MetricType::GAUGE); + } else if (name == "unlisted.typed.counter") { + saw_counter = true; + REQUIRE(type == Metrics::MetricType::COUNTER); + } + }); + + REQUIRE(saw_gauge); + REQUIRE(saw_counter); + } + + SECTION("an id that names no allocated slot is neither listed nor unlistable") + { + // Storage::_is_allocated is the gate; this only checks that unlist and listed go through it. + // Blob 100 was never allocated, the largest id names an offset past MAX_SIZE, and create() + // advances after writing so the id one past the last one created is not allocated yet. + auto last = Metrics::Counter::create("unlisted.next.free"); + + for (auto id : {Metrics::IdType{100 << 16}, std::numeric_limits::max(), last + 1}) { + CHECK_FALSE(m.unlist(id)); + CHECK_FALSE(m.listed(id)); + } + } + + SECTION("the hidden store unlists independently") + { + auto &h = Metrics::hidden_instance(); + + Metrics::Counter::createPtr("unlisted.dual"); + Metrics::Counter::createHiddenPtr("unlisted.dual"); + + auto pub_id = m.lookup("unlisted.dual"); + auto hid_id = h.lookup("unlisted.dual"); + + REQUIRE(h.unlist(hid_id)); + REQUIRE_FALSE(h.listed(hid_id)); + REQUIRE(m.listed(pub_id)); + + bool in_published = false, in_hidden = false; + + m.for_each([&](std::string_view name, Metrics::MetricType, int64_t) { in_published |= (name == "unlisted.dual"); }); + h.for_each([&](std::string_view name, Metrics::MetricType, int64_t) { in_hidden |= (name == "unlisted.dual"); }); + + REQUIRE(in_published); + REQUIRE_FALSE(in_hidden); + } +}