From f5fff9b5f9d2bf8dd550ea8d4bcbeed49a7ac7b7 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:44:04 +0100 Subject: [PATCH 1/2] fix(telemetry): wire the metric export cadence constants to the reader kMetricExportInterval and kMetricExportTimeout were declared but the reader options set the same values as literals, so both constants were unused. constexpr at namespace scope has internal linkage, so clang reports them under -Wunused-const-variable, which -Dwerr=ON makes fatal: it failed the compile on ubuntu-clang-release-amd64 and macos-arm64-release, and clang-tidy as well. Using them removes two magic numbers and keeps the comment that explains why the interval is 1 s. --- src/libxrpl/telemetry/Telemetry.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libxrpl/telemetry/Telemetry.cpp b/src/libxrpl/telemetry/Telemetry.cpp index 7b54b76c8d..aa7cec6a6f 100644 --- a/src/libxrpl/telemetry/Telemetry.cpp +++ b/src/libxrpl/telemetry/Telemetry.cpp @@ -543,11 +543,9 @@ public: auto metricExporter = otlp_http::OtlpHttpMetricExporterFactory::Create(metricExporterOpts); - // Configure periodic metric reader (1-second export interval, - // matching the beast OTelCollector path). metrics_sdk::PeriodicExportingMetricReaderOptions readerOpts; - readerOpts.export_interval_millis = std::chrono::milliseconds(1000); - readerOpts.export_timeout_millis = std::chrono::milliseconds(500); + readerOpts.export_interval_millis = kMetricExportInterval; + readerOpts.export_timeout_millis = kMetricExportTimeout; auto reader = metrics_sdk::PeriodicExportingMetricReaderFactory::Create( std::move(metricExporter), readerOpts); From 39ef1806ba3bd57992974377ad3f343825d3f746 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:44:08 +0100 Subject: [PATCH 2/2] fix(telemetry): satisfy the optional-access and include checks in the range tests clang-tidy reported six bugprone-unchecked-optional-access errors: the dataflow analysis does not treat ASSERT_TRUE(x.has_value()) as establishing the precondition, because the assertion's early return is hidden inside a macro. Three sites now guard with a plain if + FAIL(), the form already used elsewhere in this file and one the analysis does model. The fourth site compares the whole optional instead. That covers both "was it parsed" and "are the bounds right" in one exact assertion, and because it is EXPECT rather than ASSERT every row of the table is now checked -- previously one bad row returned from the test and hid the other five. parseLedgerRange uses std::errc, which comes from ; the header included only , which misc-include-cleaner flagged. --- .../libxrpl/telemetry/MetricsRegistry.cpp | 21 ++++++++++++------- src/xrpld/telemetry/MetricsRegistry.h | 1 + 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/tests/libxrpl/telemetry/MetricsRegistry.cpp b/src/tests/libxrpl/telemetry/MetricsRegistry.cpp index fc06d1cc39..11fbbfaed8 100644 --- a/src/tests/libxrpl/telemetry/MetricsRegistry.cpp +++ b/src/tests/libxrpl/telemetry/MetricsRegistry.cpp @@ -488,7 +488,8 @@ TEST(MetricsRegistryParseLedgerRange, single_ledger_segment_is_a_range_not_a_rej // treating a dashless segment as malformed reports nothing at all for that // node -- the reading an operator most needs while a node is catching up. auto const one = Registry::parseLedgerRange("5000"); - ASSERT_TRUE(one.has_value()); + if (!one.has_value()) + FAIL() << "a single-ledger segment must parse"; EXPECT_EQ(one->first, 5000u); EXPECT_EQ(one->second, 5000u); @@ -503,9 +504,11 @@ TEST(MetricsRegistryParseLedgerRange, every_producible_segment_parses_exactly) { for (auto const& [segment, expected] : kProducibleSegments) { - auto const parsed = Registry::parseLedgerRange(segment); - ASSERT_TRUE(parsed.has_value()) << "rejected a producible segment: " << segment; - EXPECT_EQ(*parsed, expected) << "wrong bounds for segment: " << segment; + // Comparing the whole optional covers both "was it parsed" and "are the + // bounds right" in one exact assertion, and keeps the loop going so one + // bad row cannot hide the other five. + EXPECT_EQ(Registry::parseLedgerRange(segment), std::optional{expected}) + << "segment: " << segment; } } @@ -527,7 +530,8 @@ TEST(MetricsRegistryParseLedgerRange, bounds_are_exact_at_the_sequence_limits) auto const maxText = std::to_string(kMaxSeq); auto const atLimit = Registry::parseLedgerRange(maxText); - ASSERT_TRUE(atLimit.has_value()) << "rejected the largest representable sequence"; + if (!atLimit.has_value()) + FAIL() << "rejected the largest representable sequence: " << maxText; EXPECT_EQ(atLimit->first, kMaxSeq); EXPECT_EQ(atLimit->second, kMaxSeq); @@ -558,8 +562,11 @@ TEST(MetricsRegistryParseLedgerRange, reads_back_what_the_real_producer_wrote) auto const segment = rest.substr(0, comma); auto const parsed = Registry::parseLedgerRange(segment); - ASSERT_TRUE(parsed.has_value()) << "producer emitted a segment the parser refuses: [" - << segment << "] from " << rendered; + if (!parsed.has_value()) + { + FAIL() << "producer emitted a segment the parser refuses: [" << segment << "] from " + << rendered; + } recovered.push_back(*parsed); rest = (comma == std::string_view::npos) ? std::string_view{} : rest.substr(comma + 1); diff --git a/src/xrpld/telemetry/MetricsRegistry.h b/src/xrpld/telemetry/MetricsRegistry.h index a450c25cce..852e638fdc 100644 --- a/src/xrpld/telemetry/MetricsRegistry.h +++ b/src/xrpld/telemetry/MetricsRegistry.h @@ -155,6 +155,7 @@ #include #include #include +#include #include #ifdef XRPL_ENABLE_TELEMETRY