fix: address PR review round 2 — event name constants, span timing

- Add cons_span::event namespace with disputeResolve and txIncluded
  constants; replace hardcoded strings in Consensus.h and RCLConsensus.cpp
- Move proposal.receive and validation.receive spans in PeerImp into
  shared_ptr captured by job lambdas so they measure checkPropose and
  checkValidation timing, not just message parsing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Pratik Mankawde
2026-04-28 17:58:06 +01:00
parent d990f7f197
commit d50e0ff48e
4 changed files with 34 additions and 21 deletions

View File

@@ -223,6 +223,15 @@ inline constexpr auto disputesCount = join(xrplConsensus, makeStr("disputes_coun
inline constexpr auto trusted = join(xrplConsensus, makeStr("trusted"));
} // namespace attr
// ===== Event names ===========================================================
namespace event {
/// "dispute.resolve"
inline constexpr auto disputeResolve = join(makeStr("dispute"), makeStr("resolve"));
/// "tx.included"
inline constexpr auto txIncluded = join(makeStr("tx"), makeStr("included"));
} // namespace event
// ===== Attribute values ======================================================
namespace val {

View File

@@ -597,7 +597,9 @@ RCLConsensus::Adaptor::doAccept(
JLOG(j_.debug()) << " Tx: " << item.key();
++txCount;
auto const txHash = to_string(item.key());
doAcceptSpan.addEvent("tx.included", {{telemetry::cons_span::attr::txId, txHash}});
doAcceptSpan.addEvent(
telemetry::cons_span::event::txIncluded,
{{telemetry::cons_span::attr::txId, txHash}});
}
catch (std::exception const& ex)
{

View File

@@ -1557,7 +1557,7 @@ Consensus<Adaptor>::updateOurPositions(std::unique_ptr<std::stringstream> const&
auto const yaysStr = std::to_string(dispute.getYays());
auto const naysStr = std::to_string(dispute.getNays());
span.addEvent(
"dispute.resolve",
cons_span::event::disputeResolve,
{{cons_span::attr::txId, to_string(txId)},
{cons_span::attr::disputeOurVote, dispute.getOurVote() ? "yes" : "no"},
{cons_span::attr::disputeYays, yaysStr},

View File

@@ -1944,13 +1944,6 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMProposeSet> const& m)
}
}
{
using namespace telemetry;
auto span = SpanGuard::span(
TraceCategory::Consensus, seg::consensus, cons_span::op::proposalReceive);
span.setAttribute(cons_span::attr::trusted, isTrusted);
}
JLOG(p_journal_.trace()) << "Proposal: " << (isTrusted ? "trusted" : "untrusted");
auto proposal = RCLCxPeerPos(
@@ -1965,9 +1958,17 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMProposeSet> const& m)
app_.getTimeKeeper().closeTime(),
calcNodeID(app_.getValidatorManifests().getMasterKey(publicKey))});
auto span = std::make_shared<telemetry::SpanGuard>(telemetry::SpanGuard::span(
telemetry::TraceCategory::Consensus,
telemetry::seg::consensus,
telemetry::cons_span::op::proposalReceive));
span->setAttribute(telemetry::cons_span::attr::trusted, isTrusted);
std::weak_ptr<PeerImp> const weak = shared_from_this();
app_.getJobQueue().addJob(
isTrusted ? jtPROPOSAL_t : jtPROPOSAL_ut, "checkPropose", [weak, isTrusted, m, proposal]() {
isTrusted ? jtPROPOSAL_t : jtPROPOSAL_ut,
"checkPropose",
[weak, isTrusted, m, proposal, sp = std::move(span)]() {
if (auto peer = weak.lock())
peer->checkPropose(isTrusted, m, proposal);
});
@@ -2542,17 +2543,16 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMValidation> const& m)
return;
}
auto span = std::make_shared<telemetry::SpanGuard>(telemetry::SpanGuard::span(
telemetry::TraceCategory::Consensus,
telemetry::seg::consensus,
telemetry::cons_span::op::validationReceive));
span->setAttribute(telemetry::cons_span::attr::trusted, isTrusted);
if (val->isFieldPresent(sfLedgerSequence))
{
using namespace telemetry;
auto span = SpanGuard::span(
TraceCategory::Consensus, seg::consensus, cons_span::op::validationReceive);
span.setAttribute(cons_span::attr::trusted, isTrusted);
if (val->isFieldPresent(sfLedgerSequence))
{
span.setAttribute(
cons_span::attr::ledgerSeq,
static_cast<int64_t>(val->getFieldU32(sfLedgerSequence)));
}
span->setAttribute(
telemetry::cons_span::attr::ledgerSeq,
static_cast<int64_t>(val->getFieldU32(sfLedgerSequence)));
}
if (!isTrusted && (tracking_.load() == Tracking::diverged))
@@ -2565,7 +2565,9 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMValidation> const& m)
std::weak_ptr<PeerImp> const weak = shared_from_this();
app_.getJobQueue().addJob(
isTrusted ? jtVALIDATION_t : jtVALIDATION_ut, name, [weak, val, m, key]() {
isTrusted ? jtVALIDATION_t : jtVALIDATION_ut,
name,
[weak, val, m, key, sp = std::move(span)]() {
if (auto peer = weak.lock())
peer->checkValidation(val, key, m);
});