fix(telemetry): pass StartOptions, not a string, in telemetry-off tests

MetricsRegistry::start() takes a StartOptions aggregate. Six call sites in
the #ifndef XRPL_ENABLE_TELEMETRY block still passed a std::string, so the
block did not compile. Nothing in CI compiles it: telemetry defaults ON, and
the block is skipped whenever the macro is defined.

Add one shared kTestStartOptions carrying just the endpoint -- the other
fields are never read on the no-op path -- and correct two comments that
still described the old signature and a #else stub that does not exist.
This commit is contained in:
Pratik Mankawde
2026-09-09 14:05:33 +01:00
parent 8d2e2d15af
commit db0eae1b35

View File

@@ -603,13 +603,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.
*
@@ -903,7 +913,7 @@ TEST_F(MetricsRegistryTest, disabled_start_stop)
telemetry::MetricsRegistry registry(false, mockApp_, j_);
// start() and stop() should be no-ops when disabled.
registry.start(std::string{kTestEndpoint});
registry.start(kTestStartOptions);
registry.stop();
// Double stop should be safe.
@@ -924,8 +934,9 @@ TEST_F(MetricsRegistryTest, disabled_start_stop)
// (src/tests/libxrpl/CMakeLists.txt:117-126 -- the `else()` branch; when it is
// ON the .cpp needs concrete xrpld types such as LedgerMaster, TxQ, NetworkOPs,
// Overlay and node_store::Database, which a standalone GTest binary cannot
// link). Both start() and startAsyncGauges() therefore compile here to their
// `#else` branch, which only (void)-casts its arguments. So these tests pin the
// link). Both start() and startAsyncGauges() have a single definition whose
// whole body sits inside #ifdef XRPL_ENABLE_TELEMETRY, so here they compile to
// an empty body with a [[maybe_unused]] parameter. So these tests pin the
// API SURFACE -- that both entry points exist, are callable in either order,
// and leave the object usable -- and NOT the gauge behaviour. Real coverage of
// "gauges observe values only after startAsyncGauges()" is unreachable from
@@ -943,7 +954,7 @@ TEST_F(MetricsRegistryTest, async_gauges_start_after_start_is_safe)
telemetry::MetricsRegistry registry(false, mockApp_, j_);
// The documented order: provider/sync instruments first, gauges second.
registry.start(std::string{kTestEndpoint});
registry.start(kTestStartOptions);
registry.startAsyncGauges();
// State: the enable flag is untouched by either phase. Exact value, not
@@ -973,7 +984,7 @@ TEST_F(MetricsRegistryTest, async_gauges_before_start_does_not_break_start)
EXPECT_EQ(registry.isEnabled(), false);
// Phase 1 still works afterwards, so the bad call left no state behind.
registry.start(std::string{kTestEndpoint});
registry.start(kTestStartOptions);
registry.recordJobQueued("ledgerData", "ProcessLData");
EXPECT_EQ(registry.isEnabled(), false);
@@ -994,7 +1005,7 @@ TEST_F(MetricsRegistryTest, async_gauges_respect_the_compile_time_guard)
// return.
EXPECT_EQ(registry.isEnabled(), true);
EXPECT_NO_THROW(registry.start(std::string{kTestEndpoint}));
EXPECT_NO_THROW(registry.start(kTestStartOptions));
EXPECT_NO_THROW(registry.startAsyncGauges());
EXPECT_NO_THROW(registry.stop());
@@ -1004,7 +1015,7 @@ TEST_F(MetricsRegistryTest, async_gauges_respect_the_compile_time_guard)
TEST_F(MetricsRegistryTest, disabled_recording_methods)
{
telemetry::MetricsRegistry registry(false, mockApp_, j_);
registry.start(std::string{kTestEndpoint});
registry.start(kTestStartOptions);
// All recording methods should be no-ops (not crash).
registry.recordRpcStarted("server_info");
@@ -1022,7 +1033,7 @@ TEST_F(MetricsRegistryTest, destructor_calls_stop)
{
// Let the destructor handle cleanup.
telemetry::MetricsRegistry registry(false, mockApp_, j_);
registry.start(std::string{kTestEndpoint});
registry.start(kTestStartOptions);
}
// If we get here without crash, the destructor handled stop.
}