fix(telemetry): make the snapshot portable and repair the mTLS test set

Three CI failures, one cause each.

macOS could not compile the tracker: Apple's libc++ has no std::atomic for a
shared_ptr, so the primary template's trivially-copyable assert fired. Publish
through boost::atomic_shared_ptr instead, which every standard library the
matrix covers can build. boost/smart_ptr is already used in this tree. The
header's note no longer claims libstdc++ as the assumption.

Four tests that predate the metrics_endpoint scheme guard set tls_client_cert
and only put traces_endpoint on https, so the new guard threw before the check
each one asserts. They now set both endpoints. Nine tests in the two files set a
client cert; the other five stay correct because the pairing, use_tls and traces
checks all run ahead of the metrics one.

Eleven clang-tidy findings: redundant member initialisers, two aggregate
initialisations that wanted designated form, two unbraced bodies, three
unparenthesised multiplications, and a reserve before a loop that emplaces.
Dropping std::make_shared also left <memory> unused in both files.
This commit is contained in:
Pratik Mankawde
2026-09-09 15:49:04 +01:00
parent ea4f7fd0d4
commit a59dfe33e3
5 changed files with 36 additions and 14 deletions

View File

@@ -503,6 +503,7 @@ TEST(TelemetryConfig, tls_missing_client_cert_file_throws)
Section section = mtls::makeSection(true);
section.set("use_tls", "1");
section.set(mtls::keyEndpoint, mtls::httpsEndpoint);
section.set(mtls::keyMetricsEndpoint, mtls::metricsHttpsEndpoint);
section.set(mtls::keyClientCert, absentCert);
section.set(mtls::keyClientKey, mtls::writeCertFile(dir.file("k.pem")));
@@ -521,6 +522,7 @@ TEST(TelemetryConfig, tls_missing_client_key_file_throws)
Section section = mtls::makeSection(true);
section.set("use_tls", "1");
section.set(mtls::keyEndpoint, mtls::httpsEndpoint);
section.set(mtls::keyMetricsEndpoint, mtls::metricsHttpsEndpoint);
section.set(mtls::keyClientCert, mtls::writeCertFile(dir.file("c.pem")));
section.set(mtls::keyClientKey, absentKey);
@@ -559,6 +561,7 @@ TEST(TelemetryConfig, tls_client_key_that_is_a_directory_throws)
Section section = mtls::makeSection(true);
section.set("use_tls", "1");
section.set(mtls::keyEndpoint, mtls::httpsEndpoint);
section.set(mtls::keyMetricsEndpoint, mtls::metricsHttpsEndpoint);
section.set(mtls::keyClientCert, mtls::writeCertFile(dir.file("c.pem")));
section.set(mtls::keyClientKey, keyDir);

View File

@@ -31,6 +31,14 @@ constexpr char const* clientKey = "/etc/xrpl/tls/node-client-private-key.pem";
constexpr char const* kHttpsEndpoint = "https://collector.example:4318/v1/traces";
/**
* The metric endpoint the section-parsing case needs.
*
* A client certificate requires https on both endpoints, so a case that parses
* a whole section has to set this one as well or the parse throws.
*/
constexpr char const* kHttpsMetricsEndpoint = "https://collector.example:4318/v1/metrics";
/**
* Build a Setup with mutual TLS configured and nothing else set.
*
@@ -133,6 +141,7 @@ TEST(TraceExporterOptions, config_section_reaches_the_exporter_options)
Section section;
section.set("enabled", "1");
section.set("traces_endpoint", kHttpsEndpoint);
section.set("metrics_endpoint", kHttpsMetricsEndpoint);
section.set("use_tls", "1");
section.set("tls_client_cert", cert);
section.set("tls_client_key", key);

View File

@@ -355,7 +355,7 @@ TEST(ValidationTracker, an_event_exactly_one_day_old_has_left_the_day_window)
t.recordNetworkValidation(makeHash(21), 21);
settle(t);
advance(std::chrono::minutes(24 * 60 - 1) - Tracker::gracePeriod());
advance(std::chrono::minutes((24 * 60) - 1) - Tracker::gracePeriod());
t.reconcile();
EXPECT_EQ(t.agreements24h(), 1u);
@@ -396,7 +396,7 @@ TEST(ValidationTracker, steady_traffic_across_the_grid_boundary_keeps_recent_cou
// that was not cleared shows up when that bucket is subtracted.
auto t = makeTracker();
constexpr std::uint64_t kMinutes = 2 * 7 * 24 * 60 + 5;
constexpr std::uint64_t kMinutes = (2 * 7 * 24 * 60) + 5;
for (std::uint64_t i = 0; i < kMinutes; ++i)
{
t.recordOurValidation(makeHash(i), static_cast<LedgerIndex>(i));
@@ -532,7 +532,7 @@ TEST(ValidationTracker, a_burst_larger_than_one_ring_is_counted_in_full_when_dra
{
for (std::size_t i = 0; i < perBatch; ++i)
{
auto const n = b * perBatch + i + 1;
auto const n = (b * perBatch) + i + 1;
t.recordOurValidation(makeHash(n), static_cast<LedgerIndex>(n));
t.recordNetworkValidation(makeHash(n), static_cast<LedgerIndex>(n));
}
@@ -636,11 +636,14 @@ TEST(ValidationTracker, concurrent_reducer_entry_does_not_deadlock_or_double_cou
advance(Tracker::gracePeriod() + std::chrono::seconds(1));
std::vector<std::thread> readers;
readers.reserve(8);
for (int i = 0; i < 8; ++i)
{
readers.emplace_back([&t] {
for (int j = 0; j < 500; ++j)
t.reconcile();
});
}
for (auto& r : readers)
r.join();

View File

@@ -9,13 +9,15 @@
#include <xrpl/basics/base_uint.h>
#include <xrpl/protocol/Protocol.h>
#include <boost/smart_ptr/atomic_shared_ptr.hpp>
#include <boost/smart_ptr/shared_ptr.hpp>
#include <array>
#include <atomic>
#include <chrono>
#include <cstddef>
#include <cstdint>
#include <deque>
#include <memory>
namespace xrpl::telemetry {
@@ -101,8 +103,10 @@ namespace xrpl::telemetry {
* and the ledger master call them. reconcile() and the getters may be called
* from any thread and any number of threads.
* @note reconcile() and the getters share the published snapshot through an
* atomic shared_ptr, which libstdc++ guards with a short internal spin. No
* writer path touches it, so nothing a consensus thread calls can spin.
* atomic shared_ptr, which every implementation guards with a short internal
* spin. No writer path touches it, so nothing a consensus thread calls can
* spin. Boost's is used because Apple's libc++ has no std::atomic for a
* shared_ptr, so the std spelling does not compile there.
* @note A writer whose ring is full discards the event and bumps
* droppedEvents(). Counts are then low but never wrong.
* @note Window edges are rounded to whole minutes, because counts are kept in
@@ -369,7 +373,7 @@ private:
*/
LedgerIndex seq{0};
TimePoint at{}; ///< When the writer recorded it.
TimePoint at; ///< When the writer recorded it.
};
/**
@@ -407,7 +411,7 @@ private:
if (head - tail_.load(std::memory_order_acquire) >= kRingCapacity)
return false;
slots_[head & (kRingCapacity - 1)] = Slot{hash, seq, at};
slots_[head & (kRingCapacity - 1)] = Slot{.hash = hash, .seq = seq, .at = at};
head_.store(head + 1, std::memory_order_release);
return true;
}
@@ -449,7 +453,7 @@ private:
*/
struct LedgerEvent
{
TimePoint recordTime{}; ///< Time the event was first recorded.
TimePoint recordTime; ///< Time the event was first recorded.
std::uint64_t minute{0}; ///< Minute bucket the event belongs to.
bool weValidated{false}; ///< True if we sent a validation.
bool networkValidated{false}; ///< True if network reached consensus.
@@ -620,7 +624,7 @@ private:
* Set while a thread is inside reconcile(). A second caller sees it set
* and returns rather than waiting.
*/
std::atomic_flag reducing_{};
std::atomic_flag reducing_;
/**
* Pending ledger events indexed by ledger hash. Touched only inside
@@ -692,7 +696,7 @@ private:
* a reader that took the old one keeps it alive while it reads. Null until
* the first reconcile().
*/
std::atomic<std::shared_ptr<Snapshot const>> published_;
boost::atomic_shared_ptr<Snapshot const> published_;
/**
* Lifetime count of agreements.

View File

@@ -8,12 +8,13 @@
#include <xrpl/basics/base_uint.h>
#include <xrpl/protocol/Protocol.h>
#include <boost/smart_ptr/make_shared.hpp>
#include <atomic>
#include <chrono>
#include <cstddef>
#include <cstdint>
#include <iterator>
#include <memory>
namespace xrpl::telemetry {
@@ -122,8 +123,10 @@ ValidationTracker::decidePending(TimePoint now)
// Nothing can be repaired past the window, so the entry is dead weight.
auto const cutoff = now - kLateRepairWindow;
for (auto it = pending_.begin(); it != pending_.end();)
{
it = (it->second.decided && it->second.recordTime < cutoff) ? pending_.erase(it)
: std::next(it);
}
}
void
@@ -248,7 +251,7 @@ void
ValidationTracker::publish()
{
published_.store(
std::make_shared<Snapshot const>(Snapshot{c1h_, c24h_, c7d_}), std::memory_order_release);
boost::make_shared<Snapshot const>(Snapshot{.w1h = c1h_, .w24h = c24h_, .w7d = c7d_}));
}
ValidationTracker::Snapshot
@@ -256,7 +259,7 @@ ValidationTracker::read() const
{
// Holding the shared_ptr keeps this snapshot alive for as long as the
// caller needs it, so the reducer can never write the values being read.
auto const s = published_.load(std::memory_order_acquire);
auto const s = published_.load();
return s ? *s : Snapshot{};
}