diff --git a/src/tests/libxrpl/telemetry/MetricsRegistry.cpp b/src/tests/libxrpl/telemetry/MetricsRegistry.cpp index 534abff3e0..d5781a36a8 100644 --- a/src/tests/libxrpl/telemetry/MetricsRegistry.cpp +++ b/src/tests/libxrpl/telemetry/MetricsRegistry.cpp @@ -468,8 +468,6 @@ TEST(MetricsRegistryScaledMean, default_scale_is_one) #include #include #include -#include -#include using namespace xrpl; diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index 8c5542070f..a7790fbdd6 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -19,7 +19,11 @@ #include #include #include +#ifdef XRPL_ENABLE_TELEMETRY +// The metric-name constants are named only as macro arguments, which the +// macros drop when telemetry is compiled out. #include +#endif #include #include diff --git a/src/xrpld/app/ledger/detail/InboundLedger.cpp b/src/xrpld/app/ledger/detail/InboundLedger.cpp index 78b4fa4165..d38ddaec56 100644 --- a/src/xrpld/app/ledger/detail/InboundLedger.cpp +++ b/src/xrpld/app/ledger/detail/InboundLedger.cpp @@ -13,7 +13,11 @@ #include #include #include +#ifdef XRPL_ENABLE_TELEMETRY +// The metric-name constants are named only as macro arguments, which the +// macros drop when telemetry is compiled out. #include +#endif #include #include @@ -1624,12 +1628,16 @@ InboundLedger::recordBatchOutcome(SHAMapAddNode const& san) stats_ += san; +#ifdef XRPL_ENABLE_TELEMETRY // Emit the tallies the trace log above already printed. receiveNode() walks // every node in the packet, so these MUST stay out here: the loop has // finished and the tallies are aggregated, giving at most three counter Adds // per received packet rather than per node. The split is what separates real // progress (good) from wasted bandwidth (duplicate) and a misbehaving peer // (invalid) -- traffic-level metrics show all three as healthy throughput. + // + // The helper and its calls exist only to report these counters, so they are + // compiled out along with the counters themselves. auto const emit = [this](char const* outcome, int count) { if (count <= 0) return; @@ -1643,6 +1651,7 @@ InboundLedger::recordBatchOutcome(SHAMapAddNode const& san) emit(telemetry::lval::addnode::good, san.getGood()); emit(telemetry::lval::addnode::duplicate, san.getDuplicate()); emit(telemetry::lval::addnode::invalid, san.getBad()); +#endif return san.getGood(); } diff --git a/src/xrpld/app/ledger/detail/LedgerDeltaAcquire.cpp b/src/xrpld/app/ledger/detail/LedgerDeltaAcquire.cpp index 51e5cddb77..3094d20a1f 100644 --- a/src/xrpld/app/ledger/detail/LedgerDeltaAcquire.cpp +++ b/src/xrpld/app/ledger/detail/LedgerDeltaAcquire.cpp @@ -11,7 +11,11 @@ #include #include #include +#ifdef XRPL_ENABLE_TELEMETRY +// The metric-name constants are named only as macro arguments, which the +// macros drop when telemetry is compiled out. #include +#endif #include #include diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp index ae3ea48782..715d605b45 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -19,7 +19,11 @@ #include #include #include +#ifdef XRPL_ENABLE_TELEMETRY +// The metric-name constants are named only as macro arguments, which the +// macros drop when telemetry is compiled out. #include +#endif #include #include diff --git a/src/xrpld/app/ledger/detail/LedgerReplayTask.cpp b/src/xrpld/app/ledger/detail/LedgerReplayTask.cpp index 078e47a3be..242d30fd50 100644 --- a/src/xrpld/app/ledger/detail/LedgerReplayTask.cpp +++ b/src/xrpld/app/ledger/detail/LedgerReplayTask.cpp @@ -227,6 +227,10 @@ LedgerReplayTask::tryAdvance(ScopedLockType& sl) } } +// Not static: the metric macros below read the app_ member. With telemetry +// compiled out they expand to nothing, so the body touches no member and +// clang-tidy sees a method that could be static. +// NOLINTBEGIN(readability-convert-member-functions-to-static) void LedgerReplayTask::recordOutcome(char const* outcome) const { @@ -240,6 +244,7 @@ LedgerReplayTask::recordOutcome(char const* outcome) const "Ledger replay tasks by terminal outcome", {{telemetry::label::outcome, std::string(outcome)}}); } +// NOLINTEND(readability-convert-member-functions-to-static) void LedgerReplayTask::updateSkipList( diff --git a/src/xrpld/app/ledger/detail/SkipListAcquire.cpp b/src/xrpld/app/ledger/detail/SkipListAcquire.cpp index 671a617c82..5238f300bc 100644 --- a/src/xrpld/app/ledger/detail/SkipListAcquire.cpp +++ b/src/xrpld/app/ledger/detail/SkipListAcquire.cpp @@ -9,7 +9,11 @@ #include #include #include +#ifdef XRPL_ENABLE_TELEMETRY +// The metric-name constants are named only as macro arguments, which the +// macros drop when telemetry is compiled out. #include +#endif #include #include diff --git a/src/xrpld/app/main/Application.cpp b/src/xrpld/app/main/Application.cpp index 7c1effbb14..88043e6158 100644 --- a/src/xrpld/app/main/Application.cpp +++ b/src/xrpld/app/main/Application.cpp @@ -40,7 +40,11 @@ #include #include #include +#ifdef XRPL_ENABLE_TELEMETRY +// The metric-name constants are named only as macro arguments, which the +// macros drop when telemetry is compiled out. #include +#endif #include #include @@ -1225,8 +1229,11 @@ public: * faults taken later, as the caches refill and touch the pages the * trim handed back -- see the runbook branch for how to read it. */ + // Not const: with telemetry enabled the metric macros below call + // getMetricsRegistry() on *this, which is non-const (ServiceRegistry.h). + // Only the compiled-out build makes this look like a const method. void - trimHeapAndRecord() + trimHeapAndRecord() // NOLINT(readability-make-member-function-const) { MallocTrimReport const report = mallocTrim("doSweep", journal_); diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index 8ba669b748..1a71eb7fc3 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -32,7 +32,11 @@ #include #include #include +#ifdef XRPL_ENABLE_TELEMETRY +// The metric-name constants are named only as macro arguments, which the +// macros drop when telemetry is compiled out. #include +#endif #include #include #include @@ -2886,15 +2890,6 @@ NetworkOPsImp::setMode(OperatingMode om) if (mode_ == om) return; - // Capture the mode we are leaving before overwriting it: the transition - // edge, not just the destination, is what tells flapping apart from a - // clean climb to FULL. - auto const prevMode = mode_.load(); - - mode_ = om; - - accounting_.mode(om); - // Record the mode transition labelled with source and destination, so the // dashboard can chart which edges of the sync state machine are traversed // (e.g. repeated full->connected flapping vs. a one-way @@ -2902,13 +2897,23 @@ NetworkOPsImp::setMode(OperatingMode om) // never in a hot loop, and there are only five modes so the label // cardinality is bounded. strOperatingMode(mode, admin=false) supplies the // names, which keeps them identical to the ones server_info reports. + // + // This must stay above the assignment below, where mode_ is still the mode + // being left: the transition edge, not just the destination, is what tells + // flapping apart from a clean climb to FULL. Reading it inline also means + // nothing is computed when telemetry is compiled out, since the macro then + // drops its arguments. XRPL_METRIC_COUNTER_INC_LABELED( registry_.get(), telemetry::metric::stateChangesTotal, "Total operating mode changes", - {{telemetry::label::from, strOperatingMode(prevMode, false)}, + {{telemetry::label::from, strOperatingMode(mode_.load(), false)}, {telemetry::label::to, strOperatingMode(om, false)}}); + mode_ = om; + + accounting_.mode(om); + JLOG(journal_.info()) << "STATE->" << strOperatingMode(); pubServer(); } diff --git a/src/xrpld/app/misc/SHAMapStoreImp.cpp b/src/xrpld/app/misc/SHAMapStoreImp.cpp index 373bc5996a..4c795f6ae7 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.cpp +++ b/src/xrpld/app/misc/SHAMapStoreImp.cpp @@ -5,7 +5,11 @@ #include #include #include +#ifdef XRPL_ENABLE_TELEMETRY +// The metric-name constants are named only as macro arguments, which the +// macros drop when telemetry is compiled out. #include +#endif #include #include diff --git a/src/xrpld/overlay/detail/ConnectAttempt.cpp b/src/xrpld/overlay/detail/ConnectAttempt.cpp index f8cb631b66..e7e11253a4 100644 --- a/src/xrpld/overlay/detail/ConnectAttempt.cpp +++ b/src/xrpld/overlay/detail/ConnectAttempt.cpp @@ -9,7 +9,11 @@ #include #include #include +#ifdef XRPL_ENABLE_TELEMETRY +// The metric-name constants are named only as macro arguments, which the +// macros drop when telemetry is compiled out. #include +#endif #include #include diff --git a/src/xrpld/overlay/detail/OverlayImpl.cpp b/src/xrpld/overlay/detail/OverlayImpl.cpp index c2dac4d4f9..c6f6496e80 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.cpp +++ b/src/xrpld/overlay/detail/OverlayImpl.cpp @@ -628,6 +628,10 @@ OverlayImpl::start() timer->asyncWait(); } +// Not static: the metric macros below read the app_ member. With telemetry +// compiled out they expand to nothing, so the body touches no member and +// clang-tidy sees a method that could be static. +// NOLINTBEGIN(readability-convert-member-functions-to-static) void OverlayImpl::reportDnsResolve(std::chrono::steady_clock::time_point start, bool resolved) { @@ -653,7 +657,12 @@ OverlayImpl::reportDnsResolve(std::chrono::steady_clock::time_point start, bool resolved ? telemetry::lval::dns_resolve::resolved : telemetry::lval::dns_resolve::empty)}}); } +// NOLINTEND(readability-convert-member-functions-to-static) +// Not static: the metric macros below read the app_ member. With telemetry +// compiled out they expand to nothing, so the body touches no member and +// clang-tidy sees a method that could be static. +// NOLINTBEGIN(readability-convert-member-functions-to-static) void OverlayImpl::reportAcceptOutcome(char const* outcome) { @@ -663,6 +672,7 @@ OverlayImpl::reportAcceptOutcome(char const* outcome) "Inbound peer connection attempts, by terminal outcome", {{telemetry::label::outcome, std::string(outcome)}}); } +// NOLINTEND(readability-convert-member-functions-to-static) PeerLedgerSupply OverlayImpl::getPeerLedgerSupply(std::uint32_t validatedSeq) const diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 9ff2c3b42d..20eeebb58e 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -663,6 +663,10 @@ PeerImp::close() JLOG((inbound_ ? journal_.debug() : journal_.info())) << "close: Closed"; } +// Not static: the metric macros below read the app_ member. With telemetry +// compiled out they expand to nothing, so the body touches no member and +// clang-tidy sees a method that could be static. +// NOLINTBEGIN(readability-convert-member-functions-to-static) void PeerImp::reportServeRefusal(char const* request, char const* reason) { @@ -673,6 +677,7 @@ PeerImp::reportServeRefusal(char const* request, char const* reason) {{telemetry::label::request, std::string(request)}, {telemetry::label::reason, std::string(reason)}}); } +// NOLINTEND(readability-convert-member-functions-to-static) void PeerImp::fail(std::string const& reason)