From da68beaef4358f868cb0ea32c3abe9818c0e1704 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 26 Aug 2026 19:44:28 +0100 Subject: [PATCH] Compile out the RPC error-span name resolver with telemetry off resolveCommandSpanName() and the error span in doCommand() exist only to name and label a telemetry span. With telemetry compiled out the resolver still ran on every RPC that fillHandler() rejects: up to five json isMember lookups, up to three string copies, a virtual config() call and a handler-table lookup, all to build a name nobody records. A storm of malformed requests paid that cost once per request. A runtime `if (span)` guard cannot work here. The resolver's result is the span name itself, passed as the third argument of the ScopedSpanGuard constructor, so no span object exists yet to test. That leaves `#ifdef XRPL_ENABLE_TELEMETRY`, matching the house style used elsewhere. The guard covers the whole telemetry block at the call site and the helper definition too, so the file-static helper does not become an unreferenced function, which the build rejects because warnings are errors. injectError() and the error return stay outside the guard, so behaviour on the failure path is unchanged. No include is orphaned: ErrorCodes.h, SpanGuard.h, RpcSpanNames.h and all keep uses outside the guards. With telemetry on nothing changes: only comments and the four directives were added. --- src/xrpld/rpc/detail/RPCHandler.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/xrpld/rpc/detail/RPCHandler.cpp b/src/xrpld/rpc/detail/RPCHandler.cpp index b1c213c463..e3f136c4fe 100644 --- a/src/xrpld/rpc/detail/RPCHandler.cpp +++ b/src/xrpld/rpc/detail/RPCHandler.cpp @@ -222,6 +222,14 @@ callMethod(JsonContext& context, Method method, std::string const& name, Object& } } +// Telemetry-only helper, so it is compiled out with telemetry off. Left +// running it would cost several json lookups, up to two string copies and a +// handler-table lookup on every failed request, for a name nobody records. +// Its result IS the span name, so `if (span)` cannot guard it: at that point +// no span exists to test. The single call site is gated the same way, which +// also keeps this file-static function referenced in both configurations. +#ifdef XRPL_ENABLE_TELEMETRY + // Resolve the span suffix / command attribute for a request that failed in // fillHandler. Returns the canonical handler name for a recognized command // (a finite, bounded set) or the literal "unknown" for a request that omits @@ -255,6 +263,8 @@ resolveCommandSpanName(JsonContext const& context) : std::string_view{rpc_span::val::unknownCommand}; } +#endif // XRPL_ENABLE_TELEMETRY + } // namespace Status @@ -263,6 +273,11 @@ doCommand(rpc::JsonContext& context, json::Value& result) Handler const* handler = nullptr; if (auto error = fillHandler(context, handler)) { + // Every statement below only feeds the error span, and the span name + // itself comes from resolveCommandSpanName(), so there is no span + // object to test with `if (span)`. With telemetry off the whole block + // is compiled out and a storm of malformed requests pays nothing. +#ifdef XRPL_ENABLE_TELEMETRY // Bound the span name and command attribute to the finite set of // registered handler names (plus "unknown") — see the helper for why // raw request input must not reach the telemetry pipeline. @@ -278,6 +293,7 @@ doCommand(rpc::JsonContext& context, json::Value& result) : std::string_view(rpc_span::val::user)); span.setAttribute(rpc_span::attr::rpcStatus, rpc_span::val::error); span.setError(getErrorInfo(error).token.cStr()); +#endif // XRPL_ENABLE_TELEMETRY injectError(error, result); return error;