From 3f8aa47224dfda58eab724ef10e090d33eaa76c2 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Thu, 14 May 2026 17:27:28 +0100 Subject: [PATCH] fix(telemetry): drop duplicate Beast MetricsRegistry test + remove author-local symlink MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `src/test/telemetry/MetricsRegistry_test.cpp` (Beast `unit_test::suite` format under `src/test/`) duplicates the GTest version already maintained at `src/tests/libxrpl/telemetry/MetricsRegistry.cpp`. Project rule (`tasks/lessons.md` §Test Format): all new tests use GTest under `src/tests/libxrpl/`. The GTest version exercises the same four cases (disabled construction, start/stop lifecycle, recording no-op, destructor-calls-stop). Deleting the Beast duplicate eliminates drift and keeps the test authoritative in one place. - Drop the matching `test.telemetry > xrpl.basics/xrpl.core/xrpld.telemetry` entries from `.github/scripts/levelization/results/ordering.txt` because `xrpl.test.telemetry` (the GTest binary) retains its own entries; the removed ones belonged to the deleted Beast suite. - `.claude/instructions.md` was committed as a symlink to an author-local absolute path (`/home/pratik/sourceCode/personal/Rippled/ instructions.md`) that does not exist for any other contributor or in CI. Remove the symlink from git tracking and add `.claude/` to `.gitignore` so future agent commits do not re-add per-developer settings. --- .claude/instructions.md | 1 - .../scripts/levelization/results/ordering.txt | 3 - .gitignore | 1 + src/test/telemetry/MetricsRegistry_test.cpp | 389 ------------------ 4 files changed, 1 insertion(+), 393 deletions(-) delete mode 120000 .claude/instructions.md delete mode 100644 src/test/telemetry/MetricsRegistry_test.cpp diff --git a/.claude/instructions.md b/.claude/instructions.md deleted file mode 120000 index 7cde73b399..0000000000 --- a/.claude/instructions.md +++ /dev/null @@ -1 +0,0 @@ -/home/pratik/sourceCode/personal/Rippled/instructions.md \ No newline at end of file diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index 0759def1dc..6c94d5a2cd 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -186,9 +186,6 @@ test.shamap > xrpl.basics test.shamap > xrpl.nodestore test.shamap > xrpl.protocol test.shamap > xrpl.shamap -test.telemetry > xrpl.basics -test.telemetry > xrpl.core -test.telemetry > xrpld.telemetry test.toplevel > test.csf test.toplevel > xrpl.json test.unit_test > xrpl.basics diff --git a/.gitignore b/.gitignore index 029f3efa2a..2554435bf1 100644 --- a/.gitignore +++ b/.gitignore @@ -87,3 +87,4 @@ __pycache__ # clangd cache /.cache docker/telemetry/workload/__pycache__/ +.claude/ diff --git a/src/test/telemetry/MetricsRegistry_test.cpp b/src/test/telemetry/MetricsRegistry_test.cpp deleted file mode 100644 index 421c2e27ec..0000000000 --- a/src/test/telemetry/MetricsRegistry_test.cpp +++ /dev/null @@ -1,389 +0,0 @@ -/** Unit tests for MetricsRegistry. - - Tests cover: - - Construction with telemetry disabled (no-op behavior). - - start()/stop() lifecycle when disabled. - - Synchronous instrument recording methods do not crash when disabled. - - Double stop() is safe. - - NOTE: Tests that exercise the OTel SDK path require XRPL_ENABLE_TELEMETRY - to be defined at build time (telemetry=ON). The no-op path tests run - unconditionally. -*/ - -// On Windows, WinSock2.h must be included before WinSock.h. OTel SDK -// headers transitively pull in WinSock.h, so we pre-include Boost.Asio's -// socket header to establish the correct ordering. -#ifdef _WIN32 -#include -#endif - -#include - -#include -#include -#include -#include -#include -#include - -#include - -#include -#include -#include - -namespace xrpl::test { - -/** Minimal mock ServiceRegistry for MetricsRegistry testing. - - Only the getMetricsRegistry() call is used in the tests; other methods - are not invoked because the registry is disabled (enabled=false) so no - gauge callbacks execute. - - All pure virtual methods throw to catch accidental calls during tests. -*/ -class MockServiceRegistry : public ServiceRegistry -{ - [[noreturn]] static void - throwUnimplemented() - { - Throw("MockServiceRegistry: method not implemented"); - } - -public: - // ServiceRegistry interface — stubs that should never be called. - CollectorManager& - getCollectorManager() override - { - throwUnimplemented(); - } - Family& - getNodeFamily() override - { - throwUnimplemented(); - } - TimeKeeper& - getTimeKeeper() override - { - throwUnimplemented(); - } - JobQueue& - getJobQueue() override - { - throwUnimplemented(); - } - NodeCache& - getTempNodeCache() override - { - throwUnimplemented(); - } - CachedSLEs& - getCachedSLEs() override - { - throwUnimplemented(); - } - NetworkIDService& - getNetworkIDService() override - { - throwUnimplemented(); - } - AmendmentTable& - getAmendmentTable() override - { - throwUnimplemented(); - } - HashRouter& - getHashRouter() override - { - throwUnimplemented(); - } - LoadFeeTrack& - getFeeTrack() override - { - throwUnimplemented(); - } - LoadManager& - getLoadManager() override - { - throwUnimplemented(); - } - RCLValidations& - getValidations() override - { - throwUnimplemented(); - } - ValidatorList& - getValidators() override - { - throwUnimplemented(); - } - ValidatorSite& - getValidatorSites() override - { - throwUnimplemented(); - } - ManifestCache& - getValidatorManifests() override - { - throwUnimplemented(); - } - ManifestCache& - getPublisherManifests() override - { - throwUnimplemented(); - } - Overlay& - getOverlay() override - { - throwUnimplemented(); - } - Cluster& - getCluster() override - { - throwUnimplemented(); - } - PeerReservationTable& - getPeerReservations() override - { - throwUnimplemented(); - } - Resource::Manager& - getResourceManager() override - { - throwUnimplemented(); - } - NodeStore::Database& - getNodeStore() override - { - throwUnimplemented(); - } - SHAMapStore& - getSHAMapStore() override - { - throwUnimplemented(); - } - RelationalDatabase& - getRelationalDatabase() override - { - throwUnimplemented(); - } - InboundLedgers& - getInboundLedgers() override - { - throwUnimplemented(); - } - InboundTransactions& - getInboundTransactions() override - { - throwUnimplemented(); - } - TaggedCache& - getAcceptedLedgerCache() override - { - throwUnimplemented(); - } - LedgerMaster& - getLedgerMaster() override - { - throwUnimplemented(); - } - LedgerCleaner& - getLedgerCleaner() override - { - throwUnimplemented(); - } - LedgerReplayer& - getLedgerReplayer() override - { - throwUnimplemented(); - } - PendingSaves& - getPendingSaves() override - { - throwUnimplemented(); - } - [[nodiscard]] OpenLedger& - getOpenLedger() override - { - throwUnimplemented(); - } - [[nodiscard]] OpenLedger const& - getOpenLedger() const override - { - throwUnimplemented(); - } - NetworkOPs& - getOPs() override - { - throwUnimplemented(); - } - OrderBookDB& - getOrderBookDB() override - { - throwUnimplemented(); - } - TransactionMaster& - getMasterTransaction() override - { - throwUnimplemented(); - } - TxQ& - getTxQ() override - { - throwUnimplemented(); - } - PathRequestManager& - getPathRequestManager() override - { - throwUnimplemented(); - } - ServerHandler& - getServerHandler() override - { - throwUnimplemented(); - } - perf::PerfLog& - getPerfLog() override - { - throwUnimplemented(); - } - telemetry::Telemetry& - getTelemetry() override - { - throwUnimplemented(); - } - telemetry::MetricsRegistry* - getMetricsRegistry() override - { - return nullptr; - } - [[nodiscard]] bool - isStopping() const override - { - return false; - } - beast::Journal - getJournal(std::string const&) override - { - return beast::Journal(beast::Journal::getNullSink()); - } - boost::asio::io_context& - getIOContext() override - { - throwUnimplemented(); - } - Logs& - getLogs() override - { - throwUnimplemented(); - } - [[nodiscard]] std::optional const& - getTrapTxID() const override - { - static std::optional const empty; - return empty; - } - DatabaseCon& - getWalletDB() override - { - throwUnimplemented(); - } - Application& - getApp() override - { - throwUnimplemented(); - } -}; - -class MetricsRegistry_test : public beast::unit_test::suite -{ - void - testDisabledConstruction() - { - testcase("Disabled construction"); - - MockServiceRegistry mockApp; - beast::Journal const j(beast::Journal::getNullSink()); - - // Construct with enabled=false; should be a no-op. - telemetry::MetricsRegistry const registry(false, mockApp, j); - BEAST_EXPECT(!registry.isEnabled()); - } - - void - testDisabledStartStop() - { - testcase("Disabled start/stop"); - - MockServiceRegistry mockApp; - beast::Journal const j(beast::Journal::getNullSink()); - - telemetry::MetricsRegistry registry(false, mockApp, j); - - // start() and stop() should be no-ops when disabled. - registry.start("http://localhost:4318/v1/metrics"); - registry.stop(); - - // Double stop should be safe. - registry.stop(); - - pass(); - } - - void - testDisabledRecording() - { - testcase("Disabled recording methods"); - - MockServiceRegistry mockApp; - beast::Journal const j(beast::Journal::getNullSink()); - - telemetry::MetricsRegistry registry(false, mockApp, j); - registry.start("http://localhost:4318/v1/metrics"); - - // All recording methods should be no-ops (not crash). - registry.recordRpcStarted("server_info"); - registry.recordRpcFinished("server_info", 1000); - registry.recordRpcErrored("ledger", 500); - registry.recordJobQueued("ledgerData"); - registry.recordJobStarted("ledgerData", 200); - registry.recordJobFinished("ledgerData", 3000); - - registry.stop(); - - pass(); - } - - void - testDestructorStops() - { - testcase("Destructor calls stop"); - - MockServiceRegistry mockApp; - beast::Journal const j(beast::Journal::getNullSink()); - - { - // Let the destructor handle cleanup. - telemetry::MetricsRegistry registry(false, mockApp, j); - registry.start("http://localhost:4318/v1/metrics"); - } - - // If we get here without crash, the destructor handled stop. - pass(); - } - -public: - void - run() override - { - testDisabledConstruction(); - testDisabledStartStop(); - testDisabledRecording(); - testDestructorStops(); - } -}; - -BEAST_DEFINE_TESTSUITE(MetricsRegistry, telemetry, xrpl); - -} // namespace xrpl::test