fix(telemetry): fix CI failures — clang-tidy, levelization, linker

Clang-tidy fixes:
- Concatenate nested namespaces (modernize-concat-nested-namespaces)
  in OTelCollector.h, OTelCollector.cpp, ValidationTracker.h/.cpp
- Add missing direct includes (misc-include-cleaner) in
  ValidationTracker.cpp, test, CollectorManager.cpp, OTelCollector.cpp
- Make lock_guard variables const (misc-const-correctness)
- Add braces around single-line if/else (readability-braces-around-statements)
- Use designated initializer for WindowEvent (modernize-use-designated-initializers)
- Initialize LedgerEvent::seq field (cppcoreguidelines-pro-type-member-init)

Linker fix:
- Add ValidationTracker.cpp as source to xrpl.test.telemetry target
  (it lives in src/xrpld/ but the test links against libxrpl only)

Levelization fix:
- Remove stale dependency edges from ordering.txt that were introduced
  by the erroneous develop-merge commit

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Pratik Mankawde
2026-05-06 15:07:25 +01:00
parent a61cdf0214
commit 92072ecca4
8 changed files with 62 additions and 41 deletions

View File

@@ -191,18 +191,12 @@ test.toplevel > xrpl.json
test.unit_test > xrpl.basics
test.unit_test > xrpl.protocol
tests.libxrpl > xrpl.basics
tests.libxrpl > xrpl.core
tests.libxrpl > xrpld.telemetry
tests.libxrpl > xrpl.json
tests.libxrpl > xrpl.ledger
tests.libxrpl > xrpl.net
tests.libxrpl > xrpl.nodestore
tests.libxrpl > xrpl.protocol
tests.libxrpl > xrpl.protocol_autogen
tests.libxrpl > xrpl.server
tests.libxrpl > xrpl.shamap
tests.libxrpl > xrpl.telemetry
tests.libxrpl > xrpl.tx
xrpl.conditions > xrpl.basics
xrpl.conditions > xrpl.protocol
xrpl.core > xrpl.basics

View File

@@ -40,8 +40,7 @@
#include <memory>
#include <string>
namespace beast {
namespace insight {
namespace beast::insight {
/**
* @brief A Collector that exports metrics via OpenTelemetry OTLP/HTTP.
@@ -88,5 +87,4 @@ public:
Journal journal);
};
} // namespace insight
} // namespace beast
} // namespace beast::insight

View File

@@ -64,8 +64,7 @@
#include <string>
#include <vector>
namespace beast {
namespace insight {
namespace beast::insight {
namespace detail {
@@ -851,19 +850,19 @@ OTelCollector::New(
return std::make_shared<detail::OTelCollectorImp>(endpoint, prefix, instanceId, journal);
}
} // namespace insight
} // namespace beast
} // namespace beast::insight
#else // !XRPL_ENABLE_TELEMETRY
// When telemetry is disabled at compile time, OTelCollector::New()
// returns a NullCollector so callers do not need conditional logic.
#include <xrpl/beast/insight/Collector.h>
#include <xrpl/beast/insight/NullCollector.h>
#include <xrpl/beast/insight/OTelCollector.h>
#include <xrpl/beast/utility/Journal.h>
namespace beast {
namespace insight {
namespace beast::insight {
std::shared_ptr<Collector>
OTelCollector::New(
@@ -875,7 +874,6 @@ OTelCollector::New(
return NullCollector::New();
}
} // namespace insight
} // namespace beast
} // namespace beast::insight
#endif // XRPL_ENABLE_TELEMETRY

View File

@@ -46,6 +46,12 @@ endif()
xrpl_add_test(telemetry)
target_link_libraries(xrpl.test.telemetry PRIVATE xrpl.imports.test)
target_include_directories(xrpl.test.telemetry PRIVATE ${CMAKE_SOURCE_DIR}/src)
# ValidationTracker lives in src/xrpld/ (not libxrpl), so we compile its
# implementation directly into the test binary.
target_sources(
xrpl.test.telemetry
PRIVATE ${CMAKE_SOURCE_DIR}/src/xrpld/telemetry/detail/ValidationTracker.cpp
)
if(telemetry)
target_link_libraries(
xrpl.test.telemetry

View File

@@ -4,9 +4,14 @@
#include <xrpld/telemetry/ValidationTracker.h>
#include <xrpl/basics/base_uint.h>
#include <xrpl/protocol/Protocol.h>
#include <gtest/gtest.h>
#include <chrono>
#include <cstddef>
#include <cstdint>
#include <thread>
using namespace xrpl;

View File

@@ -5,6 +5,7 @@
#include <xrpl/beast/insight/Group.h>
#include <xrpl/beast/insight/Groups.h>
#include <xrpl/beast/insight/NullCollector.h>
#include <xrpl/beast/insight/OTelCollector.h>
#include <xrpl/beast/insight/StatsDCollector.h>
#include <xrpl/beast/net/IPEndpoint.h>
#include <xrpl/beast/utility/Journal.h>

View File

@@ -14,8 +14,7 @@
#include <deque>
#include <mutex>
namespace xrpl {
namespace telemetry {
namespace xrpl::telemetry {
/**
* Tracks whether this validator's validations agree with network consensus,
@@ -204,7 +203,7 @@ private:
struct LedgerEvent
{
uint256 ledgerHash; ///< Ledger hash being tracked.
LedgerIndex seq; ///< Ledger sequence number.
LedgerIndex seq{0}; ///< Ledger sequence number.
TimePoint recordTime; ///< Time the event was first recorded.
bool weValidated = false; ///< True if we sent a validation.
bool networkValidated = false; ///< True if network reached consensus.
@@ -292,5 +291,4 @@ private:
repairWindowEntry(std::deque<WindowEvent>& window, uint256 const& hash);
};
} // namespace telemetry
} // namespace xrpl
} // namespace xrpl::telemetry

View File

@@ -4,15 +4,21 @@
#include <xrpld/telemetry/ValidationTracker.h>
#include <algorithm>
#include <xrpl/basics/base_uint.h>
#include <xrpl/protocol/Protocol.h>
namespace xrpl {
namespace telemetry {
#include <algorithm>
#include <atomic>
#include <cstdint>
#include <deque>
#include <mutex>
namespace xrpl::telemetry {
void
ValidationTracker::recordOurValidation(uint256 const& ledgerHash, LedgerIndex seq)
{
std::lock_guard lock(mutex_);
std::lock_guard const lock(mutex_);
auto& evt = pending_[ledgerHash];
if (evt.recordTime == TimePoint{})
{
@@ -28,7 +34,7 @@ ValidationTracker::recordOurValidation(uint256 const& ledgerHash, LedgerIndex se
void
ValidationTracker::recordNetworkValidation(uint256 const& ledgerHash, LedgerIndex seq)
{
std::lock_guard lock(mutex_);
std::lock_guard const lock(mutex_);
auto& evt = pending_[ledgerHash];
if (evt.recordTime == TimePoint{})
{
@@ -43,7 +49,7 @@ ValidationTracker::recordNetworkValidation(uint256 const& ledgerHash, LedgerInde
void
ValidationTracker::reconcile()
{
std::lock_guard lock(mutex_);
std::lock_guard const lock(mutex_);
auto const now = Clock::now();
for (auto& [hash, evt] : pending_)
@@ -55,11 +61,15 @@ ValidationTracker::reconcile()
evt.agreed = evt.weValidated && evt.networkValidated;
if (evt.agreed)
{
totalAgreements_.fetch_add(1, std::memory_order_relaxed);
}
else
{
totalMissed_.fetch_add(1, std::memory_order_relaxed);
}
WindowEvent we{now, evt.ledgerHash, evt.agreed};
WindowEvent const we{.time = now, .ledgerHash = evt.ledgerHash, .agreed = evt.agreed};
window1h_.push_back(we);
window24h_.push_back(we);
window7d_.push_back(we);
@@ -107,9 +117,13 @@ ValidationTracker::evictOldPending(TimePoint now)
for (auto it = pending_.begin(); it != pending_.end();)
{
if (it->second.reconciled && it->second.recordTime < cutoff)
{
it = pending_.erase(it);
}
else
{
++it;
}
}
// Hard trim if still over limit -- remove reconciled entries that are
@@ -122,18 +136,26 @@ ValidationTracker::evictOldPending(TimePoint now)
it != pending_.end() && pending_.size() > kMaxPendingEvents;)
{
if (it->second.reconciled && it->second.recordTime < cutoff)
{
it = pending_.erase(it);
}
else
{
++it;
}
}
// Pass 2: any reconciled entry if still over limit.
for (auto it = pending_.begin();
it != pending_.end() && pending_.size() > kMaxPendingEvents;)
{
if (it->second.reconciled)
{
it = pending_.erase(it);
}
else
{
++it;
}
}
}
}
@@ -141,7 +163,7 @@ ValidationTracker::evictOldPending(TimePoint now)
double
ValidationTracker::agreementPct1h() const
{
std::lock_guard lock(mutex_);
std::lock_guard const lock(mutex_);
if (window1h_.empty())
return 0.0;
auto const agreed = static_cast<double>(
@@ -152,7 +174,7 @@ ValidationTracker::agreementPct1h() const
double
ValidationTracker::agreementPct24h() const
{
std::lock_guard lock(mutex_);
std::lock_guard const lock(mutex_);
if (window24h_.empty())
return 0.0;
auto const agreed = static_cast<double>(std::count_if(
@@ -163,7 +185,7 @@ ValidationTracker::agreementPct24h() const
uint64_t
ValidationTracker::agreements1h() const
{
std::lock_guard lock(mutex_);
std::lock_guard const lock(mutex_);
return static_cast<uint64_t>(
std::count_if(window1h_.begin(), window1h_.end(), [](auto const& e) { return e.agreed; }));
}
@@ -171,7 +193,7 @@ ValidationTracker::agreements1h() const
uint64_t
ValidationTracker::missed1h() const
{
std::lock_guard lock(mutex_);
std::lock_guard const lock(mutex_);
return static_cast<uint64_t>(
std::count_if(window1h_.begin(), window1h_.end(), [](auto const& e) { return !e.agreed; }));
}
@@ -179,7 +201,7 @@ ValidationTracker::missed1h() const
uint64_t
ValidationTracker::agreements24h() const
{
std::lock_guard lock(mutex_);
std::lock_guard const lock(mutex_);
return static_cast<uint64_t>(std::count_if(
window24h_.begin(), window24h_.end(), [](auto const& e) { return e.agreed; }));
}
@@ -187,7 +209,7 @@ ValidationTracker::agreements24h() const
uint64_t
ValidationTracker::missed24h() const
{
std::lock_guard lock(mutex_);
std::lock_guard const lock(mutex_);
return static_cast<uint64_t>(std::count_if(
window24h_.begin(), window24h_.end(), [](auto const& e) { return !e.agreed; }));
}
@@ -195,7 +217,7 @@ ValidationTracker::missed24h() const
double
ValidationTracker::agreementPct7d() const
{
std::lock_guard lock(mutex_);
std::lock_guard const lock(mutex_);
if (window7d_.empty())
return 0.0;
auto const agreed = static_cast<double>(
@@ -206,7 +228,7 @@ ValidationTracker::agreementPct7d() const
uint64_t
ValidationTracker::agreements7d() const
{
std::lock_guard lock(mutex_);
std::lock_guard const lock(mutex_);
return static_cast<uint64_t>(
std::count_if(window7d_.begin(), window7d_.end(), [](auto const& e) { return e.agreed; }));
}
@@ -214,7 +236,7 @@ ValidationTracker::agreements7d() const
uint64_t
ValidationTracker::missed7d() const
{
std::lock_guard lock(mutex_);
std::lock_guard const lock(mutex_);
return static_cast<uint64_t>(
std::count_if(window7d_.begin(), window7d_.end(), [](auto const& e) { return !e.agreed; }));
}
@@ -257,5 +279,4 @@ ValidationTracker::repairWindowEntry(std::deque<WindowEvent>& window, uint256 co
}
}
} // namespace telemetry
} // namespace xrpl
} // namespace xrpl::telemetry