From 7d875c0e763d54f8d5d7b59d5cc53d34863bc113 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 2 Sep 2026 15:30:13 +0100 Subject: [PATCH] refactor(telemetry): name the unit and epoch in the close-time span attrs The three close-time span attributes hold NetClock readings, which are whole seconds since the XRP Ledger epoch of 2000-01-01 rather than the Unix epoch. Neither the unit nor the epoch was recoverable from the key, so a consumer rendering one as a wall-clock time without first adding the epoch offset lands roughly thirty years early. The sibling close_resolution_ms already named its unit, so the header disagreed with itself. close_time -> close_time_ripple_epoch_s parent_close_time -> parent_close_time_ripple_epoch_s close_time_self -> close_time_self_ripple_epoch_s Emitted values do not change; only the keys do. The public RPC response fields of the same name are deliberately untouched, as renaming those would break the ledger API. --- include/xrpl/consensus/ConsensusSpanNames.h | 24 ++++++++++++++----- include/xrpl/telemetry/SpanNames.h | 13 +++++++++- .../libxrpl/telemetry/SpanGuardFactory.cpp | 2 +- src/xrpld/app/consensus/RCLConsensus.cpp | 7 +++--- 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/include/xrpl/consensus/ConsensusSpanNames.h b/include/xrpl/consensus/ConsensusSpanNames.h index fc56adcd44..3db180c463 100644 --- a/include/xrpl/consensus/ConsensusSpanNames.h +++ b/include/xrpl/consensus/ConsensusSpanNames.h @@ -59,10 +59,10 @@ * | | * | +-- consensus.accept.apply [jtACCEPT thread, child of accept] * | Created: Adaptor::doAccept() - * | Attrs: ledger_seq, close_time, close_time_correct, + * | Attrs: ledger_seq, close_time_ripple_epoch_s, close_time_correct, * | close_resolution_ms, consensus_state, proposing, round_time_ms, - * | parent_close_time, close_time_self, close_time_vote_bins, - * | resolution_direction, tx_count + * | parent_close_time_ripple_epoch_s, close_time_self_ripple_epoch_s, + * | close_time_vote_bins, resolution_direction, tx_count * | Events: tx.included (per tx, attrs: tx_id) * | * +~~~ consensus.validation.send [jtACCEPT thread, linked] @@ -140,8 +140,8 @@ namespace attr { * concept, same key, distinguished by span name (not an emitter prefix). */ using ::xrpl::telemetry::attr::closeResolutionMs; -using ::xrpl::telemetry::attr::closeTime; using ::xrpl::telemetry::attr::closeTimeCorrect; +using ::xrpl::telemetry::attr::closeTimeRippleEpochS; using ::xrpl::telemetry::attr::fullValidation; using ::xrpl::telemetry::attr::ledgerHash; using ::xrpl::telemetry::attr::ledgerSeq; @@ -232,8 +232,20 @@ inline constexpr auto positionHashPrefix = makeStr("position_hash_prefix"); * "consensus_state" — domain-qualified (collides with other domains' state). */ inline constexpr auto consensusState = makeStr("consensus_state"); -inline constexpr auto parentCloseTime = makeStr("parent_close_time"); -inline constexpr auto closeTimeSelf = makeStr("close_time_self"); +/** + * Close-time instants, both NetClock readings in whole seconds since the XRP + * Ledger epoch (2000-01-01T00:00:00Z) — see `closeTimeRippleEpochS` in + * SpanNames.h for why the epoch is spelled into the key. + * + * `parentCloseTimeRippleEpochS` is the previous ledger's close time; + * `closeTimeSelfRippleEpochS` is this node's own close-time vote for the round, + * so the pair shows how far the node's position sat from the ledger it built on. + * + * `closeTimeVoteBins` is not a time: it holds the number of distinct close-time + * positions seen from peers this round. + */ +inline constexpr auto parentCloseTimeRippleEpochS = makeStr("parent_close_time_ripple_epoch_s"); +inline constexpr auto closeTimeSelfRippleEpochS = makeStr("close_time_self_ripple_epoch_s"); inline constexpr auto closeTimeVoteBins = makeStr("close_time_vote_bins"); inline constexpr auto resolutionDirection = makeStr("resolution_direction"); inline constexpr auto convergePercent = makeStr("converge_percent"); diff --git a/include/xrpl/telemetry/SpanNames.h b/include/xrpl/telemetry/SpanNames.h index b848f96c03..c24f0b254b 100644 --- a/include/xrpl/telemetry/SpanNames.h +++ b/include/xrpl/telemetry/SpanNames.h @@ -133,8 +133,19 @@ inline constexpr auto ledgerSeq = makeStr("ledger_seq"); /** * Shared close-time attrs — bare names, reused by consensus and ledger. + * + * `closeTimeRippleEpochS` carries a NetClock reading: whole seconds since the + * XRP Ledger epoch (2000-01-01T00:00:00Z), never the Unix epoch. The key names + * both the unit and the epoch because neither is recoverable from the value. + * A consumer rendering it as wall-clock time must first add kEpochOffset + * (946684800 seconds, see basics/chrono.h); read as a Unix timestamp instead, + * it lands roughly 30 years early. + * + * `closeResolutionMs` is a duration, not an instant — the granularity the + * close time is rounded to. NetClock resolution is whole seconds, so this + * value is always a multiple of 1000. */ -inline constexpr auto closeTime = makeStr("close_time"); +inline constexpr auto closeTimeRippleEpochS = makeStr("close_time_ripple_epoch_s"); inline constexpr auto closeTimeCorrect = makeStr("close_time_correct"); inline constexpr auto closeResolutionMs = makeStr("close_resolution_ms"); /** diff --git a/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp b/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp index 36ab40b1b6..6cec7a5c86 100644 --- a/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp +++ b/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp @@ -99,7 +99,7 @@ TEST(SpanGuardFactory, consensus_close_time_attributes) auto span = telemetry::SpanGuard::span( telemetry::TraceCategory::Consensus, telemetry::seg::consensus, "accept.apply"); span.setAttribute("ledger_seq", static_cast(42)); - span.setAttribute("close_time", static_cast(780000000)); + span.setAttribute("close_time_ripple_epoch_s", static_cast(780000000)); span.setAttribute("close_time_correct", true); span.setAttribute("close_resolution_ms", static_cast(30000)); span.setAttribute("consensus_state", std::string("finished")); diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index a0e2f6f6d3..460a447631 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -635,7 +635,8 @@ RCLConsensus::Adaptor::doAccept( : telemetry::SpanGuard::childSpan(cs::acceptApply, roundSpanContext_); doAcceptSpan.setAttribute(cs::attr::ledgerSeq, static_cast(prevLedger.seq()) + 1); doAcceptSpan.setAttribute( - cs::attr::closeTime, static_cast(consensusCloseTime.time_since_epoch().count())); + cs::attr::closeTimeRippleEpochS, + static_cast(consensusCloseTime.time_since_epoch().count())); doAcceptSpan.setAttribute(cs::attr::closeTimeCorrect, closeTimeCorrect); doAcceptSpan.setAttribute( cs::attr::closeResolutionMs, @@ -648,10 +649,10 @@ RCLConsensus::Adaptor::doAccept( doAcceptSpan.setAttribute( cs::attr::roundTimeMs, static_cast(result.roundTime.read().count())); doAcceptSpan.setAttribute( - cs::attr::parentCloseTime, + cs::attr::parentCloseTimeRippleEpochS, static_cast(prevLedger.closeTime().time_since_epoch().count())); doAcceptSpan.setAttribute( - cs::attr::closeTimeSelf, + cs::attr::closeTimeSelfRippleEpochS, static_cast(rawCloseTimes.self.time_since_epoch().count())); doAcceptSpan.setAttribute( cs::attr::closeTimeVoteBins, static_cast(rawCloseTimes.peers.size()));