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 <rfriedma@redhat.com>

diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc
index 68b92c45d37e2e4112a117f80f21f11b712f8509..f45907c96bd450c538e51b33951dcae9b8b8978e 100644 (file)
--- 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 f448359fce7d3c8706ae25c9948fa6e3fd927a79..bd03efe1154357d3e35b6a9967842487118db034 100644 (file)
--- 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 8fdd9d9506f4d888b76704d8285a10530191ea0f..8accc12e44d02814e8acf7b3669ed600e97c5e4b 100644 (file)
--- 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<uint64_t, uint64_t> 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_any_d> 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<void(const CounterMap &)>) 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 f03980eba2d48bc3d14a6869d7a3fa60de429572..a4cab6c763ae442eba04e353812a4da4b250558d 100644 (file)
--- 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 4608a8243dba3666b04e4794337b62657c3338d3..fe02eff9e094ce5f9538990be970c9c32520310c 100644 (file)
--- 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 41da72c9fdeaea8071b4b98c3eee6d1ccbcde5ee..12b006fee9e5380ea28abf6b7e1392e4c6f4c482 100644 (file)
--- 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<tell_result_t>(std::move(f));
   }
 };

diff --git a/src/crimson/common/perf_counters_collection.cc b/src/crimson/common/perf_counters_collection.cc
index 254d85278f648fe05f12682c73027cb30cd8ee57..2efb7a0d8d335bd4041532808e583a8e4d29494b 100644 (file)
--- 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 ae0c8670cb7d5c100c542aabe709c0303e88b0cd..5c6aacf37f4edf9ba96f89757c85fdd06008cbbd 100644 (file)
--- 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 51d224b67c066ba0bed0091001ea93f009cce54b..db10cc63d870c85cfcd186fcbef94837cc14294f 100644 (file)
--- 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 b4fb968413bfb0ef962782ab1268b0f2c5acc998..fcf33acc73539f3c2dcd6e37ceb71399783dc285 100644 (file)
--- 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 b6af9b2ec2d279f41a13bf901cbb65464117c16a..41e10d05f59607fe97012dfd8b1fa84de61d05ff 100644 (file)
--- a/src/librbd/cache/pwl/AbstractWriteLog.cc
+++ b/src/librbd/cache/pwl/AbstractWriteLog.cc
@@ -301,7 +301,8 @@ void AbstractWriteLog<I>::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 2f88acdc93b58a2b1059b1ea2f52ca28ebc183da..ac5408ffd36d3fef2490d5093557bcb87e13bbbd 100644 (file)
--- 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 5549f97ffeaa1e2123f9f322b16fc860463eb1a4..fe3c4a2f6bf8043090c5aff3818787da7f6c7f4b 100644 (file)
--- 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 06115d3cab7c24703f2d07bf45e75035481479f7..05374e5ff8ddcb4f3205b2bae5e706d83b98578c 100644 (file)
--- 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 604a92339b63e06760b707be4b20bcaef8494188..8a76ceef05978943e947db502fd849de9bdfe30b 100644 (file)
--- 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 f5fa9ceca737c4d4ae25724bd96bc18721b335d0..b320c8a72c99765b1c907b338989c9af371cfc4a 100644 (file)
--- 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 e8b83d2dc01f3fc5633f56092e8b1aaff03da700..a7f6e2732583dd263dd0548ea5b9f491f2205a11 100644 (file)
--- 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;