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 <[email protected]>

Author: ronen-fr

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;

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();

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 @@ class PerfCounters
   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 @@ class PerfCounters
   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 @@ class PerfCountersCollectionImpl
   // 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 @@ class PerfCountersCollectionImpl
   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;
 

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)
 {

src/common/perf_counters_collection.h

@@ -20,7 +20,8 @@ class PerfCountersCollection
   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,

src/crimson/admin/osd_admin.cc

@@ -279,7 +279,8 @@ class DumpPerfCountersHook final: public AdminSocketHook {
     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));
   }
 };

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)
 {

src/crimson/common/perf_counters_collection.h

@@ -23,7 +23,8 @@ class PerfCountersCollection: public seastar::sharded<PerfCountersCollection>
   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 = "");
 };

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) {

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;