From 66e6310b567459680f982933cc8fbd7bf5c7c1ab Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Mon, 1 Jun 2026 19:24:20 +0100 Subject: [PATCH] more clang-tidy fixes Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> --- .../scripts/levelization/results/loops.txt | 2 +- .../scripts/levelization/results/ordering.txt | 1 + src/libxrpl/beast/insight/OTelCollector.cpp | 18 +++---- .../telemetry/TraceContextPropagator.cpp | 25 +++++---- src/xrpld/telemetry/MetricsRegistry.cpp | 53 ++++++++++++++----- .../telemetry/detail/ValidationTracker.cpp | 24 ++++----- 6 files changed, 80 insertions(+), 43 deletions(-) diff --git a/.github/scripts/levelization/results/loops.txt b/.github/scripts/levelization/results/loops.txt index 355a4013ed..f50cb168ec 100644 --- a/.github/scripts/levelization/results/loops.txt +++ b/.github/scripts/levelization/results/loops.txt @@ -20,7 +20,7 @@ Loop: xrpld.app xrpld.shamap xrpld.shamap > xrpld.app Loop: xrpld.app xrpld.telemetry - xrpld.telemetry ~= xrpld.app + xrpld.telemetry == xrpld.app Loop: xrpld.overlay xrpld.rpc xrpld.rpc ~= xrpld.overlay diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index 95f3078128..23956bb525 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -310,6 +310,7 @@ xrpld.shamap > xrpl.shamap xrpld.telemetry > xrpl.basics xrpld.telemetry > xrpl.core xrpld.telemetry > xrpld.core +xrpld.telemetry > xrpl.json xrpld.telemetry > xrpl.nodestore xrpld.telemetry > xrpl.protocol xrpld.telemetry > xrpl.rdb diff --git a/src/libxrpl/beast/insight/OTelCollector.cpp b/src/libxrpl/beast/insight/OTelCollector.cpp index c869451511..5db96a87ee 100644 --- a/src/libxrpl/beast/insight/OTelCollector.cpp +++ b/src/libxrpl/beast/insight/OTelCollector.cpp @@ -29,9 +29,9 @@ */ #ifdef XRPL_ENABLE_TELEMETRY - #include +#include #include #include #include @@ -775,15 +775,15 @@ OTelCollectorImp::makeMeter(std::string const& name) void OTelCollectorImp::addHook(OTelHookImpl* hook) { - std::lock_guard lock(mutex_); + std::scoped_lock const lock(mutex_); hooks_.push_back(hook); } void OTelCollectorImp::removeHook(OTelHookImpl* hook) { - std::lock_guard lock(mutex_); - hooks_.erase(std::remove(hooks_.begin(), hooks_.end(), hook), hooks_.end()); + std::scoped_lock const lock(mutex_); + std::erase(hooks_, hook); } void @@ -802,7 +802,7 @@ OTelCollectorImp::callHooks() if (!lastHookCallMs_.compare_exchange_strong(last, now, std::memory_order_acq_rel)) return; // Another thread won the race. - std::lock_guard lock(mutex_); + std::scoped_lock const lock(mutex_); for (auto* hook : hooks_) hook->callHandler(); } @@ -810,15 +810,15 @@ OTelCollectorImp::callHooks() void OTelCollectorImp::addGauge(OTelGaugeImpl* gauge) { - std::lock_guard lock(mutex_); + std::scoped_lock const lock(mutex_); gauges_.push_back(gauge); } void OTelCollectorImp::removeGauge(OTelGaugeImpl* gauge) { - std::lock_guard lock(mutex_); - gauges_.erase(std::remove(gauges_.begin(), gauges_.end(), gauge), gauges_.end()); + std::scoped_lock const lock(mutex_); + std::erase(gauges_, gauge); } opentelemetry::nostd::shared_ptr const& @@ -842,7 +842,7 @@ OTelCollectorImp::formatName(std::string const& name) const result = prefix_; result += '_'; } - for (char c : name) + for (char const c : name) { result += (c == '.') ? '_' : c; } diff --git a/src/tests/libxrpl/telemetry/TraceContextPropagator.cpp b/src/tests/libxrpl/telemetry/TraceContextPropagator.cpp index a8390bf768..d566be3353 100644 --- a/src/tests/libxrpl/telemetry/TraceContextPropagator.cpp +++ b/src/tests/libxrpl/telemetry/TraceContextPropagator.cpp @@ -5,13 +5,20 @@ #include #include +#include #include #include #include +#include #include +#include +#include #include #include +#include + +#include #include namespace trace = opentelemetry::trace; @@ -37,10 +44,10 @@ TEST(TraceContextPropagator, round_trip) 0x10}; std::uint8_t spanIdBuf[8] = {0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff, 0x11, 0x22}; - trace::TraceId traceId(opentelemetry::nostd::span(traceIdBuf, 16)); - trace::SpanId spanId(opentelemetry::nostd::span(spanIdBuf, 8)); - trace::TraceFlags flags(trace::TraceFlags::kIsSampled); - trace::SpanContext spanCtx(traceId, spanId, flags, true); + trace::TraceId const traceId(opentelemetry::nostd::span(traceIdBuf, 16)); + trace::SpanId const spanId(opentelemetry::nostd::span(spanIdBuf, 8)); + trace::TraceFlags const flags(trace::TraceFlags::kIsSampled); + trace::SpanContext const spanCtx(traceId, spanId, flags, true); auto ctx = opentelemetry::context::Context{}.SetValue( trace::kSpanKey, @@ -71,7 +78,7 @@ TEST(TraceContextPropagator, round_trip) TEST(TraceContextPropagator, extract_empty_protobuf) { - protocol::TraceContext proto; + protocol::TraceContext const proto; auto ctx = xrpl::telemetry::extractFromProtobuf(proto); auto span = trace::GetSpan(ctx); if (span) @@ -120,12 +127,12 @@ TEST(TraceContextPropagator, inject_invalid_span) TEST(TraceContextPropagator, flags_preservation) { - std::uint8_t traceIdBuf[16] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; - std::uint8_t spanIdBuf[8] = {1, 2, 3, 4, 5, 6, 7, 8}; + std::uint8_t const traceIdBuf[16] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; + std::uint8_t const spanIdBuf[8] = {1, 2, 3, 4, 5, 6, 7, 8}; // Test with flags NOT sampled (flags = 0) - trace::TraceFlags flags(0); - trace::SpanContext spanCtx( + trace::TraceFlags const flags(0); + trace::SpanContext const spanCtx( trace::TraceId(opentelemetry::nostd::span(traceIdBuf, 16)), trace::SpanId(opentelemetry::nostd::span(spanIdBuf, 8)), flags, diff --git a/src/xrpld/telemetry/MetricsRegistry.cpp b/src/xrpld/telemetry/MetricsRegistry.cpp index 1506723d64..97708e2255 100644 --- a/src/xrpld/telemetry/MetricsRegistry.cpp +++ b/src/xrpld/telemetry/MetricsRegistry.cpp @@ -21,15 +21,8 @@ #include -#include - -#include -#include -#include - #ifdef XRPL_ENABLE_TELEMETRY -#include #include #include #include @@ -39,7 +32,9 @@ #include #include +#include #include +#include #include #include #include @@ -52,16 +47,27 @@ #include #include #include -#include +#include +#include +#include #include #include -#include #include +#include #include #include #include +#include +#include +#include +#include +#include #include +#include +#include +#include +#include namespace metric_sdk = opentelemetry::sdk::metrics; namespace otlp_http = opentelemetry::exporter::otlp; @@ -237,10 +243,12 @@ MetricsRegistry::recordRpcFinished(std::string_view method, std::int64_t duratio return; rpcFinishedCounter_->Add(1, {{"method", std::string(method)}}); if (rpcDurationHistogram_) + { rpcDurationHistogram_->Record( static_cast(durationUs), {{"method", std::string(method)}}, opentelemetry::context::Context{}); + } #else (void)method; (void)durationUs; @@ -256,10 +264,12 @@ MetricsRegistry::recordRpcErrored(std::string_view method, std::int64_t duration return; rpcErroredCounter_->Add(1, {{"method", std::string(method)}}); if (rpcDurationHistogram_) + { rpcDurationHistogram_->Record( static_cast(durationUs), {{"method", std::string(method)}}, opentelemetry::context::Context{}); + } #else (void)method; (void)durationUs; @@ -292,10 +302,12 @@ MetricsRegistry::recordJobStarted(std::string_view jobType, std::int64_t queuedD return; jobStartedCounter_->Add(1, {{"job_type", std::string(jobType)}}); if (jobQueuedDurationHistogram_) + { jobQueuedDurationHistogram_->Record( static_cast(queuedDurUs), {{"job_type", std::string(jobType)}}, opentelemetry::context::Context{}); + } #else (void)jobType; (void)queuedDurUs; @@ -311,10 +323,12 @@ MetricsRegistry::recordJobFinished(std::string_view jobType, std::int64_t runnin return; jobFinishedCounter_->Add(1, {{"job_type", std::string(jobType)}}); if (jobRunningDurationHistogram_) + { jobRunningDurationHistogram_->Record( static_cast(runningDurUs), {{"job_type", std::string(jobType)}}, opentelemetry::context::Context{}); + } #else (void)jobType; (void)runningDurUs; @@ -557,8 +571,9 @@ MetricsRegistry::registerLoadFactorGauge() double combined = static_cast(loadFactorServer) / loadBase; if (refLevel > 0) { - double feeEscalation = static_cast(metrics.openLedgerFeeLevel.fee()) * - loadBaseServer / refLevel; + double const feeEscalation = + static_cast(metrics.openLedgerFeeLevel.fee()) * loadBaseServer / + refLevel; if (feeEscalation > static_cast(loadFactorServer)) { combined = feeEscalation / loadBase; @@ -631,17 +646,23 @@ MetricsRegistry::registerNodeStoreGauge() // Read thread pool stats (native JSON ints, no jss:: constants). if (obj.isMember("read_request_bundle")) + { observe( "read_request_bundle", static_cast(obj["read_request_bundle"].asInt())); + } if (obj.isMember("read_threads_running")) + { observe( "read_threads_running", static_cast(obj["read_threads_running"].asInt())); + } if (obj.isMember("read_threads_total")) + { observe( "read_threads_total", static_cast(obj["read_threads_total"].asInt())); + } } catch (...) // NOLINT(bugprone-empty-catch) { @@ -933,7 +954,7 @@ MetricsRegistry::registerPeerQualityGauge() // P90 latency across connected peers. if (!latencies.empty()) { - std::sort(latencies.begin(), latencies.end()); + std::ranges::sort(latencies); auto p90idx = static_cast(latencies.size() * 0.9); if (p90idx >= latencies.size()) p90idx = latencies.size() - 1; @@ -1015,11 +1036,15 @@ MetricsRegistry::registerLedgerEconomyGauge() auto const txInLedger = openLedger.current()->txCount(); auto const ageVal = age.count(); if (ageVal > 0) + { observe( "transaction_rate", static_cast(txInLedger) / static_cast(ageVal)); + } else + { observe("transaction_rate", 0.0); + } } catch (...) // NOLINT(bugprone-empty-catch) { @@ -1059,9 +1084,13 @@ MetricsRegistry::registerStateTrackingGauge() { auto const info = app.getOPs().getConsensusInfo(); if (info.isMember("proposing") && info["proposing"].asBool()) + { stateValue = 6.0; + } else if (info.isMember("validating") && info["validating"].asBool()) + { stateValue = 5.0; + } } observe("state_value", stateValue); diff --git a/src/xrpld/telemetry/detail/ValidationTracker.cpp b/src/xrpld/telemetry/detail/ValidationTracker.cpp index c0a7fc0b1e..38e065d8b5 100644 --- a/src/xrpld/telemetry/detail/ValidationTracker.cpp +++ b/src/xrpld/telemetry/detail/ValidationTracker.cpp @@ -18,7 +18,7 @@ namespace xrpl::telemetry { void ValidationTracker::recordOurValidation(uint256 const& ledgerHash, LedgerIndex seq) { - std::lock_guard const lock(mutex_); + std::scoped_lock const lock(mutex_); auto& evt = pending_[ledgerHash]; if (evt.recordTime == TimePoint{}) { @@ -34,7 +34,7 @@ ValidationTracker::recordOurValidation(uint256 const& ledgerHash, LedgerIndex se void ValidationTracker::recordNetworkValidation(uint256 const& ledgerHash, LedgerIndex seq) { - std::lock_guard const lock(mutex_); + std::scoped_lock const lock(mutex_); auto& evt = pending_[ledgerHash]; if (evt.recordTime == TimePoint{}) { @@ -49,7 +49,7 @@ ValidationTracker::recordNetworkValidation(uint256 const& ledgerHash, LedgerInde void ValidationTracker::reconcile() { - std::lock_guard const lock(mutex_); + std::scoped_lock const lock(mutex_); auto const now = Clock::now(); for (auto& [hash, evt] : pending_) @@ -163,7 +163,7 @@ ValidationTracker::evictOldPending(TimePoint now) double ValidationTracker::agreementPct1h() const { - std::lock_guard const lock(mutex_); + std::scoped_lock const lock(mutex_); if (window1h_.empty()) return 0.0; auto const agreed = static_cast( @@ -174,7 +174,7 @@ ValidationTracker::agreementPct1h() const double ValidationTracker::agreementPct24h() const { - std::lock_guard const lock(mutex_); + std::scoped_lock const lock(mutex_); if (window24h_.empty()) return 0.0; auto const agreed = static_cast(std::count_if( @@ -185,7 +185,7 @@ ValidationTracker::agreementPct24h() const uint64_t ValidationTracker::agreements1h() const { - std::lock_guard const lock(mutex_); + std::scoped_lock const lock(mutex_); return static_cast( std::count_if(window1h_.begin(), window1h_.end(), [](auto const& e) { return e.agreed; })); } @@ -193,7 +193,7 @@ ValidationTracker::agreements1h() const uint64_t ValidationTracker::missed1h() const { - std::lock_guard const lock(mutex_); + std::scoped_lock const lock(mutex_); return static_cast( std::count_if(window1h_.begin(), window1h_.end(), [](auto const& e) { return !e.agreed; })); } @@ -201,7 +201,7 @@ ValidationTracker::missed1h() const uint64_t ValidationTracker::agreements24h() const { - std::lock_guard const lock(mutex_); + std::scoped_lock const lock(mutex_); return static_cast(std::count_if( window24h_.begin(), window24h_.end(), [](auto const& e) { return e.agreed; })); } @@ -209,7 +209,7 @@ ValidationTracker::agreements24h() const uint64_t ValidationTracker::missed24h() const { - std::lock_guard const lock(mutex_); + std::scoped_lock const lock(mutex_); return static_cast(std::count_if( window24h_.begin(), window24h_.end(), [](auto const& e) { return !e.agreed; })); } @@ -217,7 +217,7 @@ ValidationTracker::missed24h() const double ValidationTracker::agreementPct7d() const { - std::lock_guard const lock(mutex_); + std::scoped_lock const lock(mutex_); if (window7d_.empty()) return 0.0; auto const agreed = static_cast( @@ -228,7 +228,7 @@ ValidationTracker::agreementPct7d() const uint64_t ValidationTracker::agreements7d() const { - std::lock_guard const lock(mutex_); + std::scoped_lock const lock(mutex_); return static_cast( std::count_if(window7d_.begin(), window7d_.end(), [](auto const& e) { return e.agreed; })); } @@ -236,7 +236,7 @@ ValidationTracker::agreements7d() const uint64_t ValidationTracker::missed7d() const { - std::lock_guard const lock(mutex_); + std::scoped_lock const lock(mutex_); return static_cast( std::count_if(window7d_.begin(), window7d_.end(), [](auto const& e) { return !e.agreed; })); }