addressed code review comments.

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
This commit is contained in:
Pratik Mankawde
2026-05-27 17:36:06 +01:00
parent 824f63216a
commit 6aa8570d6c
6 changed files with 39 additions and 17 deletions

View File

@@ -20,6 +20,7 @@
* - Per-span attribute keys: bare field name (span name carries the domain).
* - Collision qualifier: <domain>_<field> when bare name collides across
* domains or with OTel reserved `status` (e.g. rpc_status, grpc_status).
* - Shared cross-span attributes: <domain>_<field> (underscore) form.
* - Resource attribute keys: xrpl.<subsystem>.<field> (process-identity).
* - Span prefixes: <subsystem>[.<component>].
*/

View File

@@ -3,13 +3,15 @@
/** Abstract interface for OpenTelemetry distributed tracing.
Provides the Telemetry base class that all components use to create trace
spans. Two concrete implementations exist, selected at construction time
spans. Three concrete implementations exist, selected at construction time
by make_Telemetry():
- TelemetryImpl (Telemetry.cpp): real OTel SDK integration, compiled
only when XRPL_ENABLE_TELEMETRY is defined and enabled at runtime.
- NullTelemetry (NullTelemetry.cpp): no-op stub used when telemetry is
disabled at compile time or runtime.
- NullTelemetryOtel (Telemetry.cpp): no-op stub that still depends on
the OTel API (used during transition or for testing).
Inheritance / dependency diagram:
@@ -37,32 +39,33 @@
1. Root span at a subsystem entry point (typical usage):
@code
#include <xrpld/rpc/detail/RpcSpanNames.h>
using namespace xrpl::telemetry;
// In an RPC handler dispatch:
auto guard = SpanGuard::span(TraceCategory::Rpc, "rpc", commandName);
guard.setAttribute("xrpl.rpc.command", commandName);
auto guard = SpanGuard::span(
TraceCategory::Rpc, rpc_span::prefix::command, commandName);
guard.setAttribute(rpc_span::attr::command, commandName);
// ... process request
// guard destructor automatically ends the span on scope exit
@endcode
2. Child span for a sub-operation (cross-scope):
@code
auto parent = SpanGuard::span(TraceCategory::Transactions, "tx", "process");
{
SpanGuard guard(telemetry.startSpan("rpc.command.submit"));
guard.setAttribute("command", "submit");
// ... guard ends span automatically on scope exit
}
@endcode
3. Child span for a sub-operation (cross-scope):
2. Child span for a sub-operation (scoped child):
@code
auto parent = SpanGuard::span(TraceCategory::Transactions, "tx", "process");
{
auto child = parent.childSpan("tx.apply");
child.setAttribute("xrpl.tx.type", txType);
child.setAttribute("tx_type", txType);
// child ends here
}
// parent continues, then ends here
@endcode
3. Unrelated span (cross-scope, same thread):
@code
// Transactions and RPC can be active simultaneously
auto txSpan = SpanGuard::span(TraceCategory::Transactions, "tx", "process");
auto rpcSpan = SpanGuard::span(TraceCategory::Rpc, "rpc", "info");
// both spans end on scope exit
@endcode
4. Cross-thread context propagation:

View File

@@ -179,6 +179,7 @@ GRPCServerImpl::CallData<Request, Response>::process(std::shared_ptr<JobQueue::C
bool const isUnlimited = clientIsUnlimited();
if (!isUnlimited && usage.disconnect(app_.getJournal("gRPCServer")))
{
span.setAttribute(grpc_span::attr::grpcStatus, grpc_span::val::error);
span.setError(grpc_span::val::resourceExhausted);
grpc::Status const status{
grpc::StatusCode::RESOURCE_EXHAUSTED, "usage balance exceeds threshold"};
@@ -190,6 +191,10 @@ GRPCServerImpl::CallData<Request, Response>::process(std::shared_ptr<JobQueue::C
usage.charge(loadType);
auto role = getRole(isUnlimited);
span.setAttribute(
grpc_span::attr::grpcRole,
role == Role::ADMIN ? grpc_span::val::admin : grpc_span::val::user);
{
std::stringstream toLog;
toLog << "role = " << (int)role;
@@ -225,6 +230,7 @@ GRPCServerImpl::CallData<Request, Response>::process(std::shared_ptr<JobQueue::C
if (conditionMetRes != rpcSUCCESS)
{
RPC::ErrorInfo const errorInfo = RPC::get_error_info(conditionMetRes);
span.setAttribute(grpc_span::attr::grpcStatus, grpc_span::val::error);
span.setError(errorInfo.token.c_str());
grpc::Status const status{
grpc::StatusCode::FAILED_PRECONDITION, errorInfo.message.c_str()};
@@ -234,6 +240,7 @@ GRPCServerImpl::CallData<Request, Response>::process(std::shared_ptr<JobQueue::C
{
std::pair<Response, grpc::Status> result = handler_(context);
setIsUnlimited(result.first, isUnlimited);
span.setAttribute(grpc_span::attr::grpcStatus, grpc_span::val::success);
span.setOk();
responder_.Finish(result.first, result.second, this);
}
@@ -241,6 +248,7 @@ GRPCServerImpl::CallData<Request, Response>::process(std::shared_ptr<JobQueue::C
}
catch (std::exception const& ex)
{
span.setAttribute(grpc_span::attr::grpcStatus, grpc_span::val::error);
span.recordException(ex);
grpc::Status const status{grpc::StatusCode::INTERNAL, ex.what()};
responder_.FinishWithError(status, this);

View File

@@ -51,6 +51,8 @@ inline constexpr auto grpcStatus = makeStr("grpc_status");
namespace val {
using telemetry::attr_val::error;
using telemetry::attr_val::success;
inline constexpr auto admin = makeStr("admin");
inline constexpr auto user = makeStr("user");
inline constexpr auto resourceExhausted = makeStr("resource_exhausted");
inline constexpr auto failedPrecondition = makeStr("failed_precondition");
} // namespace val

View File

@@ -185,7 +185,12 @@ callMethod(JsonContext& context, Method method, std::string const& name, Object&
JLOG(context.j.debug()) << "RPC call " << name << " completed in "
<< ((end - start).count() / 1000000000.0) << "seconds";
perfLog.rpcFinish(name, curId);
span.setAttribute(rpc_span::attr::rpcStatus, rpc_span::val::success);
// Status::operator bool() returns true when there IS an error
// (code_ != OK), so the ternary correctly maps error->error, ok->success.
span.setAttribute(
rpc_span::attr::rpcStatus, ret ? rpc_span::val::error : rpc_span::val::success);
if (!ret)
span.setOk();
return ret;
}
catch (std::exception& e)

View File

@@ -564,6 +564,7 @@ ServerHandler::processSession(
jr[jss::api_version] = jv[jss::api_version];
jr[jss::type] = jss::response;
span.setOk();
return jr;
}
@@ -598,6 +599,7 @@ ServerHandler::processSession(
{
session->close(true);
}
span.setOk();
}
static Json::Value
@@ -1036,6 +1038,7 @@ ServerHandler::processRequest(
}
}
span.setOk();
HTTPReply(httpStatus, response, output, rpcJ);
}