From 60a2ac7bfebce0d3e13bb08115766dbb45377352 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Mon, 6 Jul 2026 19:17:34 +0100 Subject: [PATCH] fix(telemetry): bound RPC error-path span cardinality; balance LCOV markers Resolve the doCommand error-path span suffix and command attribute against the handler registry instead of using raw request input. A recognized command yields its canonical handler name; anything else (unregistered command, or neither command/method present) collapses to the literal "unknown". This bounds telemetry cardinality to the finite set of handler names plus "unknown" while preserving per-command error attribution for known-but-rejected commands (e.g. a submit that hit rpcTOO_BUSY stays rpc.command.submit). The command attribute is promoted to a Prometheus label by the spanmetrics connector, so attacker-controlled input previously enabled unbounded label cardinality. Also add the missing LCOV_EXCL_STOP in the first SpanGuard::linkedSpan overload so its coverage exclusion no longer bleeds into the second overload's guards. Co-Authored-By: Claude Opus 4.8 --- src/libxrpl/telemetry/SpanGuard.cpp | 1 + src/xrpld/rpc/detail/RPCHandler.cpp | 48 ++++++++++++++++++----------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/libxrpl/telemetry/SpanGuard.cpp b/src/libxrpl/telemetry/SpanGuard.cpp index 720750d2e5..e0b552ecc2 100644 --- a/src/libxrpl/telemetry/SpanGuard.cpp +++ b/src/libxrpl/telemetry/SpanGuard.cpp @@ -227,6 +227,7 @@ SpanGuard::linkedSpan(std::string_view name) const return SpanGuard( std::make_unique(tracer->StartSpan( std::string(name), {}, {{spanCtx, {{kLinkTypeKey, kLinkTypeFollowsFrom}}}}, opts))); + // LCOV_EXCL_STOP } SpanGuard diff --git a/src/xrpld/rpc/detail/RPCHandler.cpp b/src/xrpld/rpc/detail/RPCHandler.cpp index 109c991e98..f914fac548 100644 --- a/src/xrpld/rpc/detail/RPCHandler.cpp +++ b/src/xrpld/rpc/detail/RPCHandler.cpp @@ -220,6 +220,31 @@ callMethod(JsonContext& context, Method method, std::string const& name, Object& } } +// 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 +// both fields or names an unregistered command. The raw request value is +// deliberately NOT used: the command attribute is promoted to a Prometheus +// label by the spanmetrics connector, so an attacker-controlled string would +// let arbitrary request input drive unbounded span-name / label cardinality. +// Resolving against the registry keeps per-command error attribution for real +// commands (e.g. a submit rejected with rpcTOO_BUSY stays rpc.command.submit) +// while collapsing garbage input to a single series. +std::string_view +resolveCommandSpanName(JsonContext const& context) +{ + if (!context.params.isMember(jss::command) && !context.params.isMember(jss::method)) + return rpc_span::val::unknownCommand; + + std::string const cmd = context.params.isMember(jss::command) + ? context.params[jss::command].asString() + : context.params[jss::method].asString(); + + auto const* handler = getHandler(context.apiVersion, context.app.config().betaRpcApi, cmd); + return (handler != nullptr) ? std::string_view{handler->name} + : std::string_view{rpc_span::val::unknownCommand}; +} + } // namespace Status @@ -228,25 +253,12 @@ doCommand(RPC::JsonContext& context, json::Value& result) Handler const* handler = nullptr; if (auto error = fillHandler(context, handler)) { - std::string cmdName; - if (context.params.isMember(jss::command)) - { - cmdName = context.params[jss::command].asString(); - } - else if (context.params.isMember(jss::method)) - { - cmdName = context.params[jss::method].asString(); - } - else - { - cmdName = "unknown"; // LCOV_EXCL_LINE - } - // Use the resolved command name as the span suffix so dashboards - // can break out per-command error rates (e.g. rpc.command.submit - // for a submit that hit rpcTOO_BUSY). Falling back to a single - // "unknown" name only when the request truly omits both fields. + // 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. + auto const cmdName = resolveCommandSpanName(context); auto span = SpanGuard::span(TraceCategory::Rpc, rpc_span::prefix::command, cmdName); - span.setAttribute(rpc_span::attr::command, cmdName.c_str()); + span.setAttribute(rpc_span::attr::command, cmdName); span.setAttribute(rpc_span::attr::rpcStatus, rpc_span::val::error); span.setError(getErrorInfo(error).token.cStr());