From ddbb39b26f0d709658f831b6d3d3fe97d4c07dd5 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 9 Sep 2026 14:28:07 +0100 Subject: [PATCH] fix(telemetry): pass StartOptions in the two sync-diagnostics start() tests MetricsRegistry::start() takes a StartOptions aggregate. The two telemetry-off tests this branch adds still passed a string literal, which cannot bind to StartOptions const&, so the #ifndef XRPL_ENABLE_TELEMETRY block did not compile. Nothing in CI compiles it: telemetry defaults ON, and the block is skipped whenever the macro is defined. Add kTestStartOptions carrying just the endpoint, and pass it at both sites. Its text is identical to the constant phase-9 introduces, so merging phase-9 forward is an identical addition rather than a second constant with the same meaning. The six call sites phase-9 owns are left for that merge. --- .../libxrpl/telemetry/MetricsRegistry.cpp | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/tests/libxrpl/telemetry/MetricsRegistry.cpp b/src/tests/libxrpl/telemetry/MetricsRegistry.cpp index 14525d59da..d04e8f68a9 100644 --- a/src/tests/libxrpl/telemetry/MetricsRegistry.cpp +++ b/src/tests/libxrpl/telemetry/MetricsRegistry.cpp @@ -639,13 +639,23 @@ using namespace xrpl; namespace { /** - * OTLP/HTTP endpoint passed to every start() call below. Nothing ever dials - * it -- these tests exercise the no-op path -- it just has to be a plausible - * URL. start() takes `std::string const&`, so call sites construct one from - * this view rather than repeating the literal. + * OTLP/HTTP endpoint used by every start() call below. Nothing ever dials it + * -- these tests exercise the no-op path -- it just has to be a plausible URL. + * It reaches start() through @ref kTestStartOptions. */ constexpr std::string_view kTestEndpoint{"http://localhost:4318/v1/metrics"}; +/** + * The only StartOptions field these tests need. + * + * start() takes the StartOptions aggregate, not a string. The other fields -- + * resource identity, network id, TLS paths -- are never read on the no-op + * path, and their defaults already mean "unset". One shared value keeps all + * six call sites on the same endpoint. + */ +telemetry::MetricsRegistry::StartOptions const kTestStartOptions{ + .endpoint = std::string{kTestEndpoint}}; + /** * Minimal mock ServiceRegistry for MetricsRegistry testing. * @@ -1123,7 +1133,7 @@ TEST_F(MetricsRegistryTest, disabled_lifecycle_never_consults_gauge_services) // registerPeerLedgerSupplyGauge() / registerSlotCensusGauge() / // registerAmendmentBlockGauge() / registerNodeStoreGauge() -- // would run. - EXPECT_NO_THROW(registry.start("http://localhost:4318/v1/metrics")); + EXPECT_NO_THROW(registry.start(kTestStartOptions)); // detachCallbacks() is the shutdown hook the real gauges honour. It must be // safe and idempotent even though there is nothing to detach. @@ -1200,7 +1210,7 @@ TEST_F(MetricsRegistryTest, enabled_flag_alone_registers_no_gauges_when_compiled // getLoadManager()/getInboundLedgers()/getNodeFamily()/getJobQueue()/ // getOverlay()/getAmendmentTable()/getNodeStore() and // throw std::logic_error. - EXPECT_NO_THROW(enabledRequest.start("http://localhost:4318/v1/metrics")); + EXPECT_NO_THROW(enabledRequest.start(kTestStartOptions)); EXPECT_NO_THROW(enabledRequest.detachCallbacks()); EXPECT_NO_THROW(enabledRequest.stop());