From 489cc4841054d1fc1f0d0ef604eb5baef45d350a Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Sun, 2 Feb 2025 09:24:33 -0600 Subject: [PATCH] common/perf_counters: use the RAII helpers when dumping Using Formatter::ObjectSection & Formatter::ArraySection to guarantee consistent dump sections opening & closing. Signed-off-by: Ronen Friedman --- src/common/perf_counters.cc | 62 +++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/src/common/perf_counters.cc b/src/common/perf_counters.cc index aaf0ff3471f14..17828fda8dd63 100644 --- a/src/common/perf_counters.cc +++ b/src/common/perf_counters.cc @@ -112,30 +112,25 @@ void PerfCountersCollectionImpl::dump_formatted_generic( const std::string &logger, const std::string &counter) const { - f->open_object_section("perfcounter_collection"); + Formatter::ObjectSection collection_section(*f, "perfcounter_collection"sv); if (dump_labeled == select_labeled_t::labeled) { + // dump all counters (labeled and unlabeled), using the "labeled format" + std::optional array_section; 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()); if (key_name != prev_key_name) { // close previous set of counters before dumping new one - if (!prev_key_name.empty()) { - f->close_section(); // array section - } + array_section.emplace(*f, key_name); prev_key_name = key_name; - - f->open_array_section(key_name); - (*l)->dump_formatted_generic(f, schema, histograms, select_labeled_t::labeled, ""); + (*l)->dump_formatted_generic(f, schema, histograms, select_labeled_t::labeled, ""s); } else { - (*l)->dump_formatted_generic(f, schema, histograms, select_labeled_t::labeled, ""); + (*l)->dump_formatted_generic(f, schema, histograms, select_labeled_t::labeled, ""s); } } - if (!m_loggers.empty()) { - f->close_section(); // final array section - } } else { - // unlabeled + // unlabeled (only unlabeled, and using the "unlabeled format") if (logger.empty()) { // dump all loggers for (auto& l : m_loggers) { @@ -151,7 +146,6 @@ void PerfCountersCollectionImpl::dump_formatted_generic( } } } - f->close_section(); } void PerfCountersCollectionImpl::with_counters(std::function labeled_2nd_lvl_section; + + // the 'counters_section' is used for both labeled and unlabeled - but + // have different text in the two cases. + std::optional counters_section; + 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)) { + // we are allowed to open an object section here, as - for + // labeled counters format - the caller has opened an array section + labeled_2nd_lvl_section.emplace(*f, ""); + for (Formatter::ObjectSection labels_section{*f, "labels"}; + const auto& label : ceph::perf_counters::key_labels(m_name)) { // don't dump labels with empty label names if (!label.first.empty()) { - f->dump_string(label.first, label.second); + f->dump_string(label.first, label.second); } } - f->close_section(); // labels - f->open_object_section("counters"); + counters_section.emplace(*f, "counters"); } else { auto labels = ceph::perf_counters::key_labels(m_name); // do not dump counters when counter instance is labeled and dump_labeled is not set @@ -403,7 +412,7 @@ void PerfCounters::dump_formatted_generic(Formatter *f, bool schema, return; } - f->open_object_section(m_name.c_str()); + counters_section.emplace(*f, m_name); } for (perf_counter_data_vec_t::const_iterator d = m_data.begin(); @@ -420,7 +429,7 @@ void PerfCounters::dump_formatted_generic(Formatter *f, bool schema, } if (schema) { - f->open_object_section(d->name); + Formatter::ObjectSection schedma_section{*f, d->name}; // we probably should not have exposed this raw field (with bit // values), but existing plugins rely on it so we're stuck with // it. @@ -465,10 +474,9 @@ void PerfCounters::dump_formatted_generic(Formatter *f, bool schema, } else if (d->unit == UNIT_BYTES) { f->dump_string("units", "bytes"); } - f->close_section(); } else { if (d->type & PERFCOUNTER_LONGRUNAVG) { - f->open_object_section(d->name); + Formatter::ObjectSection longrunavg_section{*f, d->name}; pair a = d->read_avg(); if (d->type & PERFCOUNTER_U64) { f->dump_unsigned("avgcount", a.second); @@ -491,13 +499,11 @@ void PerfCounters::dump_formatted_generic(Formatter *f, bool schema, } else { ceph_abort(); } - f->close_section(); } else if (d->type & PERFCOUNTER_HISTOGRAM) { ceph_assert(d->type == (PERFCOUNTER_HISTOGRAM | PERFCOUNTER_COUNTER | PERFCOUNTER_U64)); ceph_assert(d->histogram); - f->open_object_section(d->name); + Formatter::ObjectSection histogram_section{*f, d->name}; d->histogram->dump_formatted(f); - f->close_section(); } else { uint64_t v = d->u64; if (d->type & PERFCOUNTER_U64) { @@ -512,10 +518,6 @@ void PerfCounters::dump_formatted_generic(Formatter *f, bool schema, } } } - if (dump_labeled == select_labeled_t::labeled) { - f->close_section(); // counters - } - f->close_section(); } const std::string &PerfCounters::get_name() const -- 2.39.5