diff --git a/src/tests/libxrpl/telemetry/TelemetryConfig.cpp b/src/tests/libxrpl/telemetry/TelemetryConfig.cpp index f8ecaa7de7..52e8ea37c6 100644 --- a/src/tests/libxrpl/telemetry/TelemetryConfig.cpp +++ b/src/tests/libxrpl/telemetry/TelemetryConfig.cpp @@ -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); diff --git a/src/tests/libxrpl/telemetry/TraceExporterOptions.cpp b/src/tests/libxrpl/telemetry/TraceExporterOptions.cpp index 8e7d1cb6ba..e1accd12f4 100644 --- a/src/tests/libxrpl/telemetry/TraceExporterOptions.cpp +++ b/src/tests/libxrpl/telemetry/TraceExporterOptions.cpp @@ -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); diff --git a/src/tests/libxrpl/telemetry/ValidationTracker.cpp b/src/tests/libxrpl/telemetry/ValidationTracker.cpp index e9b9558e9b..57b90efe9d 100644 --- a/src/tests/libxrpl/telemetry/ValidationTracker.cpp +++ b/src/tests/libxrpl/telemetry/ValidationTracker.cpp @@ -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(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(n)); t.recordNetworkValidation(makeHash(n), static_cast(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 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(); diff --git a/src/xrpld/telemetry/ValidationTracker.h b/src/xrpld/telemetry/ValidationTracker.h index 30dad9061e..c5f3142bc0 100644 --- a/src/xrpld/telemetry/ValidationTracker.h +++ b/src/xrpld/telemetry/ValidationTracker.h @@ -9,13 +9,15 @@ #include #include +#include +#include + #include #include #include #include #include #include -#include 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> published_; + boost::atomic_shared_ptr published_; /** * Lifetime count of agreements. diff --git a/src/xrpld/telemetry/detail/ValidationTracker.cpp b/src/xrpld/telemetry/detail/ValidationTracker.cpp index 297a6e88b8..d355fb0c76 100644 --- a/src/xrpld/telemetry/detail/ValidationTracker.cpp +++ b/src/xrpld/telemetry/detail/ValidationTracker.cpp @@ -8,12 +8,13 @@ #include #include +#include + #include #include #include #include #include -#include 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{c1h_, c24h_, c7d_}), std::memory_order_release); + boost::make_shared(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{}; }