From: Ronen Friedman Date: Mon, 20 Jan 2025 18:07:51 +0000 (-0600) Subject: common/perf_counters: select_labeled_t as a parameter to dumpers X-Git-Tag: v20.0.0~280^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d2b910bfef3e76f0acaae73cf629facc2705cd71;p=ceph.git common/perf_counters: select_labeled_t as a parameter to dumpers Some counters' dump functions accept 3 bool parameters to determine some dumping behavior. And multiple 'bool' parameters are considered a code smell. This is a minor refactor to replace one of these bools with a 'select_labeled_t' enum. Signed-off-by: Ronen Friedman --- diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc index 68b92c45d37e..f45907c96bd4 100644 --- a/src/common/ceph_context.cc +++ b/src/common/ceph_context.cc @@ -538,17 +538,18 @@ int CephContext::_do_command( std::string counter; cmd_getval(cmdmap, "logger", logger); cmd_getval(cmdmap, "counter", counter); - _perf_counters_collection->dump_formatted(f, false, false, logger, counter); + _perf_counters_collection->dump_formatted(f, false, select_labeled_t::unlabeled, + logger, counter); } else if (command == "perfcounters_schema" || command == "2" || command == "perf schema") { - _perf_counters_collection->dump_formatted(f, true, false); + _perf_counters_collection->dump_formatted(f, true, select_labeled_t::unlabeled); } else if (command == "counter dump") { - _perf_counters_collection->dump_formatted(f, false, true); + _perf_counters_collection->dump_formatted(f, false, select_labeled_t::labeled); } else if (command == "counter schema") { - _perf_counters_collection->dump_formatted(f, true, true); + _perf_counters_collection->dump_formatted(f, true, select_labeled_t::labeled); } else if (command == "perf histogram dump") { std::string logger; diff --git a/src/common/perf_counters.cc b/src/common/perf_counters.cc index f448359fce7d..bd03efe11543 100644 --- a/src/common/perf_counters.cc +++ b/src/common/perf_counters.cc @@ -108,13 +108,13 @@ void PerfCountersCollectionImpl::dump_formatted_generic( Formatter *f, bool schema, bool histograms, - bool dump_labeled, + select_labeled_t dump_labeled, const std::string &logger, const std::string &counter) const { f->open_object_section("perfcounter_collection"); - - if (dump_labeled) { + + if (dump_labeled == select_labeled_t::labeled) { std::string prev_key_name; for (auto l = m_loggers.begin(); l != m_loggers.end(); ++l) { std::string_view key_name = ceph::perf_counters::key_name((*l)->get_name()); @@ -126,19 +126,28 @@ void PerfCountersCollectionImpl::dump_formatted_generic( prev_key_name = key_name; f->open_array_section(key_name); - (*l)->dump_formatted_generic(f, schema, histograms, true, ""); + (*l)->dump_formatted_generic(f, schema, histograms, select_labeled_t::labeled, ""); } else { - (*l)->dump_formatted_generic(f, schema, histograms, true, ""); + (*l)->dump_formatted_generic(f, schema, histograms, select_labeled_t::labeled, ""); } } if (!m_loggers.empty()) { f->close_section(); // final array section } } else { - for (auto l = m_loggers.begin(); l != m_loggers.end(); ++l) { - // Optionally filter on logger name, pass through counter filter - if (logger.empty() || (*l)->get_name() == logger) { - (*l)->dump_formatted_generic(f, schema, histograms, false, counter); + // unlabeled + if (logger.empty()) { + // dump all loggers + for (auto& l : m_loggers) { + l->dump_formatted_generic(f, schema, histograms, + select_labeled_t::unlabeled, counter); + } + } else { + // dump only specified logger + auto l = m_loggers.find(logger); + if (l != m_loggers.end()) { + (*l)->dump_formatted_generic(f, schema, histograms, + select_labeled_t::unlabeled, counter); } } } @@ -354,10 +363,12 @@ void PerfCounters::reset() } } + void PerfCounters::dump_formatted_generic(Formatter *f, bool schema, - bool histograms, bool dump_labeled, const std::string &counter) const + bool histograms, select_labeled_t dump_labeled, + const std::string &counter) const { - if (dump_labeled) { + if (dump_labeled == select_labeled_t::labeled) { f->open_object_section(""); // should be enclosed by array f->open_object_section("labels"); for (auto label : ceph::perf_counters::key_labels(m_name)) { @@ -377,7 +388,7 @@ void PerfCounters::dump_formatted_generic(Formatter *f, bool schema, f->open_object_section(m_name.c_str()); } - + for (perf_counter_data_vec_t::const_iterator d = m_data.begin(); d != m_data.end(); ++d) { if (!counter.empty() && counter != d->name) { @@ -431,7 +442,7 @@ void PerfCounters::dump_formatted_generic(Formatter *f, bool schema, f->dump_string("nick", ""); } f->dump_int("priority", get_adjusted_priority(d->prio)); - + if (d->unit == UNIT_NONE) { f->dump_string("units", "none"); } else if (d->unit == UNIT_BYTES) { @@ -484,7 +495,7 @@ void PerfCounters::dump_formatted_generic(Formatter *f, bool schema, } } } - if (dump_labeled) { + if (dump_labeled == select_labeled_t::labeled) { f->close_section(); // counters } f->close_section(); diff --git a/src/common/perf_counters.h b/src/common/perf_counters.h index 8fdd9d9506f4..8accc12e44d0 100644 --- a/src/common/perf_counters.h +++ b/src/common/perf_counters.h @@ -54,6 +54,12 @@ enum unit_t : uint8_t UNIT_NONE }; +/// Used to specify whether to dump the labeled counters +enum class select_labeled_t { + labeled, + unlabeled +}; + /* Class for constructing a PerfCounters object. * * This class performs some validation that the parameters we have supplied are @@ -249,13 +255,18 @@ public: void hinc(int idx, int64_t x, int64_t y); void reset(); - void dump_formatted(ceph::Formatter *f, bool schema, bool dump_labeled, - const std::string &counter = "") const { + void dump_formatted( + ceph::Formatter *f, + bool schema, + select_labeled_t dump_labeled, + const std::string &counter = "") const { dump_formatted_generic(f, schema, false, dump_labeled, counter); } - void dump_formatted_histograms(ceph::Formatter *f, bool schema, - const std::string &counter = "") const { - dump_formatted_generic(f, schema, true, false, counter); + void dump_formatted_histograms( + ceph::Formatter *f, + bool schema, + const std::string &counter = "") const { + dump_formatted_generic(f, schema, true, select_labeled_t::unlabeled, counter); } std::pair get_tavg_ns(int idx) const; @@ -281,7 +292,7 @@ private: PerfCounters(const PerfCounters &rhs); PerfCounters& operator=(const PerfCounters &rhs); void dump_formatted_generic(ceph::Formatter *f, bool schema, bool histograms, - bool dump_labeled, + select_labeled_t dump_labeled, const std::string &counter = "") const; typedef std::vector perf_counter_data_vec_t; @@ -334,16 +345,23 @@ public: // a parameter of "all" resets all counters bool reset(std::string_view name); - void dump_formatted(ceph::Formatter *f, bool schema, bool dump_labeled, - const std::string &logger = "", - const std::string &counter = "") const { - dump_formatted_generic(f, schema, false, dump_labeled, logger, counter); + void dump_formatted( + ceph::Formatter *f, + bool schema, + select_labeled_t dump_labeled, + const std::string &logger = "", + const std::string &counter = "") const { + dump_formatted_generic( + f, schema, false, dump_labeled, logger, counter); } - void dump_formatted_histograms(ceph::Formatter *f, bool schema, - const std::string &logger = "", - const std::string &counter = "") const { - dump_formatted_generic(f, schema, true, false, logger, counter); + void dump_formatted_histograms( + ceph::Formatter *f, + bool schema, + const std::string &logger = "", + const std::string &counter = "") const { + dump_formatted_generic( + f, schema, true, select_labeled_t::unlabeled, logger, counter); } // A reference to a perf_counter_data_any_d, with an accompanying @@ -361,10 +379,13 @@ public: void with_counters(std::function) const; private: - void dump_formatted_generic(ceph::Formatter *f, bool schema, bool histograms, - bool dump_labeled, - const std::string &logger = "", - const std::string &counter = "") const; + void dump_formatted_generic( + Formatter *f, + bool schema, + bool histograms, + select_labeled_t dump_labeled, + const std::string &logger, + const std::string &counter) const; perf_counters_set_t m_loggers; diff --git a/src/common/perf_counters_collection.cc b/src/common/perf_counters_collection.cc index f03980eba2d4..a4cab6c763ae 100644 --- a/src/common/perf_counters_collection.cc +++ b/src/common/perf_counters_collection.cc @@ -34,7 +34,7 @@ bool PerfCountersCollection::reset(const std::string &name) return perf_impl.reset(name); } void PerfCountersCollection::dump_formatted(ceph::Formatter *f, bool schema, - bool dump_labeled, + select_labeled_t dump_labeled, const std::string &logger, const std::string &counter) { diff --git a/src/common/perf_counters_collection.h b/src/common/perf_counters_collection.h index 4608a8243dba..fe02eff9e094 100644 --- a/src/common/perf_counters_collection.h +++ b/src/common/perf_counters_collection.h @@ -20,7 +20,8 @@ public: void clear(); bool reset(const std::string &name); - void dump_formatted(ceph::Formatter *f, bool schema, bool dump_labeled, + void dump_formatted(ceph::Formatter *f, bool schema, + select_labeled_t dump_labeled, const std::string &logger = "", const std::string &counter = ""); void dump_formatted_histograms(ceph::Formatter *f, bool schema, diff --git a/src/crimson/admin/osd_admin.cc b/src/crimson/admin/osd_admin.cc index 41da72c9fdea..12b006fee9e5 100644 --- a/src/crimson/admin/osd_admin.cc +++ b/src/crimson/admin/osd_admin.cc @@ -279,7 +279,8 @@ public: cmd_getval(cmdmap, "logger", logger); cmd_getval(cmdmap, "counter", counter); - crimson::common::local_perf_coll().dump_formatted(f.get(), false, false, logger, counter); + crimson::common::local_perf_coll().dump_formatted(f.get(), false, + select_labeled_t::unlabeled, logger, counter); return seastar::make_ready_future(std::move(f)); } }; diff --git a/src/crimson/common/perf_counters_collection.cc b/src/crimson/common/perf_counters_collection.cc index 254d85278f64..2efb7a0d8d33 100644 --- a/src/crimson/common/perf_counters_collection.cc +++ b/src/crimson/common/perf_counters_collection.cc @@ -20,7 +20,7 @@ PerfCountersCollectionImpl* PerfCountersCollection:: get_perf_collection() } void PerfCountersCollection::dump_formatted(ceph::Formatter *f, bool schema, - bool dump_labeled, + select_labeled_t dump_labeled, const std::string &logger, const std::string &counter) { diff --git a/src/crimson/common/perf_counters_collection.h b/src/crimson/common/perf_counters_collection.h index ae0c8670cb7d..5c6aacf37f4e 100644 --- a/src/crimson/common/perf_counters_collection.h +++ b/src/crimson/common/perf_counters_collection.h @@ -23,7 +23,8 @@ public: PerfCountersCollection(); ~PerfCountersCollection(); PerfCountersCollectionImpl* get_perf_collection(); - void dump_formatted(ceph::Formatter *f, bool schema, bool dump_labeled, + void dump_formatted(ceph::Formatter *f, bool schema, + select_labeled_t dump_labeled, const std::string &logger = "", const std::string &counter = ""); }; diff --git a/src/kv/RocksDBStore.cc b/src/kv/RocksDBStore.cc index 51d224b67c06..db10cc63d870 100644 --- a/src/kv/RocksDBStore.cc +++ b/src/kv/RocksDBStore.cc @@ -1469,7 +1469,7 @@ void RocksDBStore::get_statistics(Formatter *f) f->close_section(); } f->open_object_section("rocksdbstore_perf_counters"); - logger->dump_formatted(f, false, false); + logger->dump_formatted(f, false, select_labeled_t::unlabeled); f->close_section(); } if (cct->_conf->rocksdb_collect_memory_stats) { diff --git a/src/libcephsqlite.cc b/src/libcephsqlite.cc index b4fb968413bf..fcf33acc7353 100644 --- a/src/libcephsqlite.cc +++ b/src/libcephsqlite.cc @@ -809,8 +809,8 @@ static void f_perf(sqlite3_context* ctx, int argc, sqlite3_value** argv) auto&& appd = getdata(vfs); JSONFormatter f(false); f.open_object_section("ceph_perf"); - appd.logger->dump_formatted(&f, false, false); - appd.striper_logger->dump_formatted(&f, false, false); + appd.logger->dump_formatted(&f, false, select_labeled_t::unlabeled); + appd.striper_logger->dump_formatted(&f, false, select_labeled_t::unlabeled); f.close_section(); { CachedStackStringStream css; diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc index b6af9b2ec2d2..41e10d05f596 100644 --- a/src/librbd/cache/pwl/AbstractWriteLog.cc +++ b/src/librbd/cache/pwl/AbstractWriteLog.cc @@ -301,7 +301,8 @@ void AbstractWriteLog::log_perf() { ss << "\"image\": \"" << m_image_ctx.name << "\","; bl.append(ss); bl.append("\"stats\": "); - m_image_ctx.cct->get_perfcounters_collection()->dump_formatted(f, false, false); + m_image_ctx.cct->get_perfcounters_collection()->dump_formatted( + f, false, select_labeled_t::unlabeled); f->flush(bl); bl.append(",\n\"histograms\": "); m_image_ctx.cct->get_perfcounters_collection()->dump_formatted_histograms(f, 0); diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index 2f88acdc93b5..ac5408ffd36d 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -625,7 +625,7 @@ uint64_t BlueFS::get_free(unsigned id) void BlueFS::dump_perf_counters(Formatter *f) { f->open_object_section("bluefs_perf_counters"); - logger->dump_formatted(f, false, false); + logger->dump_formatted(f, false, select_labeled_t::unlabeled); f->close_section(); } diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h index 5549f97ffeaa..fe3c4a2f6bf8 100644 --- a/src/os/bluestore/BlueStore.h +++ b/src/os/bluestore/BlueStore.h @@ -3195,7 +3195,7 @@ public: int flush_cache(std::ostream *os = NULL) override; void dump_perf_counters(ceph::Formatter *f) override { f->open_object_section("perf_counters"); - logger->dump_formatted(f, false, false); + logger->dump_formatted(f, false, select_labeled_t::unlabeled); f->close_section(); } diff --git a/src/os/kstore/KStore.h b/src/os/kstore/KStore.h index 06115d3cab7c..05374e5ff8dd 100644 --- a/src/os/kstore/KStore.h +++ b/src/os/kstore/KStore.h @@ -442,7 +442,7 @@ public: } void dump_perf_counters(ceph::Formatter *f) override { f->open_object_section("perf_counters"); - logger->dump_formatted(f, false, false); + logger->dump_formatted(f, false, select_labeled_t::unlabeled); f->close_section(); } void get_db_statistics(ceph::Formatter *f) override { diff --git a/src/test/fio/fio_ceph_messenger.cc b/src/test/fio/fio_ceph_messenger.cc index 604a92339b63..8a76ceef0597 100644 --- a/src/test/fio/fio_ceph_messenger.cc +++ b/src/test/fio/fio_ceph_messenger.cc @@ -132,7 +132,8 @@ static void put_ceph_context(void) Formatter* f; f = Formatter::create("json-pretty"); - g_ceph_context->get_perfcounters_collection()->dump_formatted(f, false, false); + g_ceph_context->get_perfcounters_collection()->dump_formatted( + f, false, select_labeled_t::unlabeled); ostr << ">>>>>>>>>>>>> PERFCOUNTERS BEGIN <<<<<<<<<<<<" << std::endl; f->flush(ostr); ostr << ">>>>>>>>>>>>> PERFCOUNTERS END <<<<<<<<<<<<" << std::endl; diff --git a/src/test/fio/fio_ceph_objectstore.cc b/src/test/fio/fio_ceph_objectstore.cc index f5fa9ceca737..b320c8a72c99 100644 --- a/src/test/fio/fio_ceph_objectstore.cc +++ b/src/test/fio/fio_ceph_objectstore.cc @@ -345,7 +345,8 @@ struct Engine { Formatter* f = Formatter::create( "json-pretty", "json-pretty", "json-pretty"); f->open_object_section("perf_output"); - cct->get_perfcounters_collection()->dump_formatted(f, false, false); + cct->get_perfcounters_collection()->dump_formatted( + f, false, select_labeled_t::unlabeled); if (g_conf()->rocksdb_perf) { f->open_object_section("rocksdb_perf"); os->get_db_statistics(f); diff --git a/src/tools/ceph_objectstore_tool.cc b/src/tools/ceph_objectstore_tool.cc index e8b83d2dc01f..a7f6e2732583 100644 --- a/src/tools/ceph_objectstore_tool.cc +++ b/src/tools/ceph_objectstore_tool.cc @@ -4872,7 +4872,8 @@ out: if (debug) { ostringstream ostr; Formatter* f = Formatter::create("json-pretty", "json-pretty", "json-pretty"); - cct->get_perfcounters_collection()->dump_formatted(f, false, false); + cct->get_perfcounters_collection()->dump_formatted( + f, false, select_labeled_t::unlabeled); ostr << "ceph-objectstore-tool "; f->flush(ostr); delete f;