From a293fb66e50f9f35af0c46d167c0fecdd023365f Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Tue, 22 Sep 2026 21:11:30 +0100 Subject: [PATCH] fix(telemetry): Report pathfind span status from the reply Both pathfind handlers returned on many paths without recording a status, so a failed request produced a span that read as success. Route every exit through one helper that reads the rpc error token off the reply, which also covers the replies built further down the call chain. The token set is fixed by the error registry, so it is safe as a span label; raw request text would not be. Co-Authored-By: Claude Opus 5 (1M context) --- src/xrpld/rpc/detail/PathFindSpanNames.h | 4 +++ src/xrpld/rpc/handlers/orderbook/PathFind.cpp | 30 ++++++++++++------- .../rpc/handlers/orderbook/RipplePathFind.cpp | 24 ++++++++++----- 3 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/xrpld/rpc/detail/PathFindSpanNames.h b/src/xrpld/rpc/detail/PathFindSpanNames.h index 6d8fedc7ab..47376f7e8b 100644 --- a/src/xrpld/rpc/detail/PathFindSpanNames.h +++ b/src/xrpld/rpc/detail/PathFindSpanNames.h @@ -29,6 +29,10 @@ * | +-----------------------------------------------------------+ | * +----------------------------------------------------------------+ * + * pathfind.request ends with status error whenever the handler's reply + * carries an rpc error. The description is that error's registry token, + * never request text. + * * Async recomputation (ledger close): * * +----------------------------------------------------------------+ diff --git a/src/xrpld/rpc/handlers/orderbook/PathFind.cpp b/src/xrpld/rpc/handlers/orderbook/PathFind.cpp index 44830619e4..af15403030 100644 --- a/src/xrpld/rpc/handlers/orderbook/PathFind.cpp +++ b/src/xrpld/rpc/handlers/orderbook/PathFind.cpp @@ -47,18 +47,28 @@ doPathFind(rpc::JsonContext& context) span.setAttribute(pathfind_span::attr::destAccount, redactAccount(dst.asString())); } + // A failed reply carries the rpc error token, so reading the status off the + // reply covers every exit, including the ones whose reply is built further + // down the call chain. The token set is fixed by the error registry, so it + // is safe as a span label; raw request text would not be. + auto const finish = [&span](json::Value&& reply) -> json::Value { + if (span && rpc::containsError(reply)) + span.setError(std::as_const(reply)[jss::error].asString()); + return std::move(reply); + }; + if (context.app.config().pathSearchMax == 0) - return rpcError(RpcNotSupported); + return finish(rpcError(RpcNotSupported)); auto lpLedger = context.ledgerMaster.getClosedLedger(); if (!context.params.isMember(jss::subcommand) || !context.params[jss::subcommand].isString()) { - return rpcError(RpcInvalidParams); + return finish(rpcError(RpcInvalidParams)); } if (!context.infoSub) - return rpcError(RpcNoEvents); + return finish(rpcError(RpcNoEvents)); context.infoSub->setApiVersion(context.apiVersion); @@ -68,8 +78,8 @@ doPathFind(rpc::JsonContext& context) { context.loadType = resource::kFeeHeavyBurdenRpc; context.infoSub->clearRequest(); - return context.app.getPathRequestManager().makePathRequest( - context.infoSub, lpLedger, context.params); + return finish(context.app.getPathRequestManager().makePathRequest( + context.infoSub, lpLedger, context.params)); } if (sSubCommand == "close") @@ -77,10 +87,10 @@ doPathFind(rpc::JsonContext& context) InfoSubRequest::pointer const request = context.infoSub->getRequest(); if (!request) - return rpcError(RpcNoPfRequest); + return finish(rpcError(RpcNoPfRequest)); context.infoSub->clearRequest(); - return request->doClose(); + return finish(request->doClose()); } if (sSubCommand == "status") @@ -88,12 +98,12 @@ doPathFind(rpc::JsonContext& context) InfoSubRequest::pointer const request = context.infoSub->getRequest(); if (!request) - return rpcError(RpcNoPfRequest); + return finish(rpcError(RpcNoPfRequest)); - return request->doStatus(context.params); + return finish(request->doStatus(context.params)); } - return rpcError(RpcInvalidParams); + return finish(rpcError(RpcInvalidParams)); } } // namespace xrpl diff --git a/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp b/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp index 49b90a6e4c..9abe40cdcc 100644 --- a/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp +++ b/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp @@ -56,8 +56,18 @@ doRipplePathFind(rpc::JsonContext& context) span.setAttribute(pathfind_span::attr::destAccount, redactAccount(dst.asString())); } + // A failed reply carries the rpc error token, so reading the status off the + // reply covers every exit, including the ones whose reply is built further + // down the call chain. The token set is fixed by the error registry, so it + // is safe as a span label; raw request text would not be. + auto const finish = [&span](json::Value&& reply) -> json::Value { + if (span && rpc::containsError(reply)) + span.setError(std::as_const(reply)[jss::error].asString()); + return std::move(reply); + }; + if (context.app.config().pathSearchMax == 0) - return rpcError(RpcNotSupported); + return finish(rpcError(RpcNotSupported)); context.loadType = resource::kFeeHeavyBurdenRpc; @@ -73,8 +83,8 @@ doRipplePathFind(rpc::JsonContext& context) rpc::tuning::kMaxValidatedLedgerAge) { if (context.apiVersion == 1) - return rpcError(RpcNoNetwork); - return rpcError(RpcNotSynced); + return finish(rpcError(RpcNoNetwork)); + return finish(rpcError(RpcNotSynced)); } PathRequest::pointer request; @@ -175,17 +185,17 @@ doRipplePathFind(rpc::JsonContext& context) jvResult = request->doStatus(context.params); } - return jvResult; + return finish(std::move(jvResult)); } // The caller specified a ledger jvResult = rpc::lookupLedger(lpLedger, context); if (!lpLedger) - return jvResult; + return finish(std::move(jvResult)); rpc::LegacyPathFind const lpf(isUnlimited(context.role), context.app); if (!lpf.isOk()) - return rpcError(RpcTooBusy); + return finish(rpcError(RpcTooBusy)); auto result = context.app.getPathRequestManager().doLegacyPathRequest( context.consumer, lpLedger, context.params); @@ -193,7 +203,7 @@ doRipplePathFind(rpc::JsonContext& context) for (auto& fieldName : jvResult.getMemberNames()) result[fieldName] = std::move(jvResult[fieldName]); - return result; + return finish(std::move(result)); } } // namespace xrpl