diff --git a/src/test/nodestore/DatabaseConfig_test.cpp b/src/test/nodestore/DatabaseConfig_test.cpp index 0f132081f0..04669bc6be 100644 --- a/src/test/nodestore/DatabaseConfig_test.cpp +++ b/src/test/nodestore/DatabaseConfig_test.cpp @@ -33,18 +33,23 @@ #include #include -#include #include #include #include #include #include -#include #include #include #include #include +#ifdef XRPL_ENABLE_TELEMETRY +// std::ranges::sort / std::ranges::find_if and std::optional / std::nullopt are +// used only by the gauge-helper suite below, so they are guarded like its uses. +#include +#include +#endif + namespace xrpl::node_store { class DatabaseConfig_test : public beast::unit_test::Suite diff --git a/src/tests/libxrpl/telemetry/MetricsRegistry.cpp b/src/tests/libxrpl/telemetry/MetricsRegistry.cpp index d44a83f147..5d7c8543fb 100644 --- a/src/tests/libxrpl/telemetry/MetricsRegistry.cpp +++ b/src/tests/libxrpl/telemetry/MetricsRegistry.cpp @@ -428,10 +428,8 @@ TEST(MetricsRegistryScaledMean, default_scale_is_one) #include -#include #include #include -#include using namespace xrpl; @@ -699,8 +697,8 @@ public: [[nodiscard]] std::optional const& getTrapTxID() const override { - static std::optional const empty; - return empty; + static std::optional const kEmpty; + return kEmpty; } DatabaseCon& getWalletDB() override diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index baa4e13f87..c617bbd613 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -76,7 +76,6 @@ #include #include #include -#include #include #include #include @@ -117,6 +116,13 @@ #include #include +#ifdef XRPL_ENABLE_TELEMETRY +// The TMGetObjectByHash metric names and label keys. Every use of them sits in +// an XRPL_METRIC_* argument list, and those macros expand to nothing when +// telemetry is off, so the include is guarded like its uses. +#include +#endif + using namespace std::chrono_literals; namespace xrpl { @@ -2857,10 +2863,13 @@ PeerImp::processGetObjectByHash(std::shared_ptr con int const requested = packet.objects_size(); int const iterLimit = std::min(requested, tuning::kHardMaxReplyNodes); +#ifdef XRPL_ENABLE_TELEMETRY // Time the whole loop once, not each iteration: the loop can run up to // kHardMaxReplyNodes times, so per-iteration clock reads would cost more - // than the lookups they measure. + // than the lookups they measure. Both clock reads serve only the metric + // recorded below, so neither happens when telemetry is compiled out. auto const lookupStart = std::chrono::steady_clock::now(); +#endif for (int i = 0; i < iterLimit; ++i) { @@ -2886,8 +2895,12 @@ PeerImp::processGetObjectByHash(std::shared_ptr con newObj.set_ledgerseq(obj.ledgerseq()); } +#ifdef XRPL_ENABLE_TELEMETRY + // Measured here rather than at the call below, which would fold the fee + // computation and charge() into the reported lookup latency. auto const lookupElapsed = std::chrono::duration_cast( std::chrono::steady_clock::now() - lookupStart); +#endif // Apply work-proportional charge. `charge()` posts the disconnect // step (if any) back to strand_, so it is safe to call from this @@ -2902,12 +2915,15 @@ PeerImp::processGetObjectByHash(std::shared_ptr con resource::Charge const fee = computeGetObjectByHashFee(requested, reply.objects_size()); charge(fee, "processed get object by hash request"); +#ifdef XRPL_ENABLE_TELEMETRY recordGetObjectMetrics(requested, reply.objects_size(), lookupElapsed, fee); +#endif JLOG(pJournal_.trace()) << "GetObj: " << reply.objects_size() << " of " << requested; send(std::make_shared(reply, protocol::mtGET_OBJECTS)); } +#ifdef XRPL_ENABLE_TELEMETRY void PeerImp::recordGetObjectMetrics( int const requested, @@ -2936,9 +2952,9 @@ PeerImp::recordGetObjectMetrics( // negative -- the counter takes an unsigned amount, where a wrap would // read as ~1.8e19 rather than as an error. // - // Written as two calls rather than a loop over a {hit, miss} pair: the - // macros expand to empty statements in a telemetry-off build, which would - // leave a loop's induction variable unused and fail the -Werror build. + // Written as two calls rather than a loop over a {hit, miss} pair: the two + // amounts come from different expressions, so there is no single value to + // iterate over. XRPL_METRIC_COUNTER_ADD_LABELED( app_, kGetObjectLookupsTotal, @@ -2953,6 +2969,7 @@ PeerImp::recordGetObjectMetrics( static_cast(std::max(0, requested - found)), {{kLabelResult, std::string(kResultMiss)}}); } +#endif // XRPL_ENABLE_TELEMETRY void PeerImp::onMessage(std::shared_ptr const& m) diff --git a/src/xrpld/overlay/detail/PeerImp.h b/src/xrpld/overlay/detail/PeerImp.h index 25c3ca7618..fc4fe690ac 100644 --- a/src/xrpld/overlay/detail/PeerImp.h +++ b/src/xrpld/overlay/detail/PeerImp.h @@ -701,6 +701,7 @@ private: std::shared_ptr const& m, std::vector nodeIDs); +#ifdef XRPL_ENABLE_TELEMETRY /** * Record the OTel metrics for one completed `TMGetObjectByHash` request. * @@ -711,8 +712,9 @@ private: * * Records `getobject_request_objects`, `getobject_lookup_us`, * `getobject_charge`, and both label values of - * `getobject_lookups_total`. Compiles to nothing when telemetry is - * disabled, because the `XRPL_METRIC_*` macros do. + * `getobject_lookups_total`. Every statement is an `XRPL_METRIC_*` record, + * so the whole method — and its one call site — is compiled out when + * telemetry is disabled rather than left as an empty function. * * @param requested Objects the peer asked for (`objects_size()`). * @param found Objects returned, i.e. the reply's object count. @@ -728,6 +730,7 @@ private: int const found, std::chrono::microseconds const lookupElapsed, resource::Charge const& fee); +#endif // XRPL_ENABLE_TELEMETRY protected: // Kept `protected` so test subclasses (see diff --git a/src/xrpld/telemetry/MetricsRegistry.cpp b/src/xrpld/telemetry/MetricsRegistry.cpp index 04c2670fab..a64f5e219e 100644 --- a/src/xrpld/telemetry/MetricsRegistry.cpp +++ b/src/xrpld/telemetry/MetricsRegistry.cpp @@ -22,6 +22,10 @@ #include +// Unguarded because the constructor's `beast::Journal journal` parameter is +// declared in both configurations; only the member it initialises is guarded. +#include + #ifdef XRPL_ENABLE_TELEMETRY // The app and overlay includes below are why @@ -53,7 +57,6 @@ #include #include #include -#include #include #include #include diff --git a/src/xrpld/telemetry/MetricsRegistry.h b/src/xrpld/telemetry/MetricsRegistry.h index 7acc6f63ee..3ca31af991 100644 --- a/src/xrpld/telemetry/MetricsRegistry.h +++ b/src/xrpld/telemetry/MetricsRegistry.h @@ -143,11 +143,8 @@ #include #include -#include #include -#include #include -#include #include #include #include @@ -158,6 +155,13 @@ #include #include #include + +// These three serve only the telemetry-only members below, so they are guarded +// like their uses: std::atomic by callbacksDetached_, std::function by the +// ObserveFn sink, std::shared_ptr by provider_. +#include +#include +#include #endif namespace xrpl {