merge: bring the review fixes forward from phase4-consensus-tracing

Two conflicts, both additive.

TelemetryConfig.cpp: this branch added requireHttpsEndpoint next to
requireReadableFile; upstream added readConsensusTraceStrategy at the same spot.
Both kept.

05-configuration-reference.md: this branch added the two client-certificate rows
while upstream corrected the consensus strategy value from attribute to random.
Both kept. Also drops the stale "not yet implemented" row for
consensus_trace_strategy, which the merged table now contradicts twice over: the
option is parsed, and its value is no longer spelled attribute.
This commit is contained in:
Pratik Mankawde
2026-09-08 15:31:40 +01:00
14 changed files with 243 additions and 61 deletions

View File

@@ -269,7 +269,7 @@ Establish-phase gap fill and cross-node correlation attributes (Phase 4a):
| --------------------- | ------ | --------------------------------------------------------- |
| `consensus_round_id` | int64 | Consensus round number |
| `consensus_ledger_id` | string | `previousLedger.id()` — shared across nodes |
| `trace_strategy` | string | `"deterministic"` or `"attribute"` |
| `trace_strategy` | string | `"deterministic"` or `"random"` |
| `converge_percent` | int64 | Convergence % (0-100+) |
| `establish_count` | int64 | Number of establish iterations |
| `disputes_count` | int64 | Active disputed transactions |
@@ -504,7 +504,8 @@ The first 16 bytes are used as trace_id. See [Phase 4a implementation status](./
and `createDeterministicContext()` in `RCLConsensus.cpp` for the implementation.
Switchable via `consensus_trace_strategy` config:
`"deterministic"` (default) or `"attribute"` (random trace_id, correlation via attribute queries).
`"deterministic"` (default) or `"random"` (random trace_id, correlation via attribute queries).
`"random"` is experimental and not used: it would break cross-node trace correlation.
#### Why Not Random IDs with Propagation Only?

View File

@@ -15,39 +15,38 @@ The authoritative `[telemetry]` example lives in `cfg/xrpld-example.cfg`. Teleme
### 5.1.2 Configuration Options Summary
| Option | Type | Default | Description |
| -------------------------- | ------ | --------------------------------- | ---------------------------------------------------------------------------------------------------------- |
| `enabled` | 0 or 1 | `0` | Enable/disable telemetry |
| `traces_endpoint` | string | `http://localhost:4318/v1/traces` | Full OTLP/HTTP URL for spans, used verbatim |
| `use_tls` | 0 or 1 | `0` | Enable TLS for exporter connection |
| `tls_ca_cert` | string | `""` | Path to CA certificate file |
| `tls_client_cert` | string | `""` | Client cert (PEM) for mTLS; empty = one-way; if `enabled=1`, needs key + `use_tls=1` or startup fails |
| `tls_client_key` | string | `""` | Private key (PEM) for `tls_client_cert`; if set with `enabled=1`, needs the cert + `use_tls=1` or fails |
| `batch_size` | uint | `512` | Spans per export batch |
| `batch_delay_ms` | uint | `5000` | Max delay before sending batch (ms) |
| `max_queue_size` | uint | `2048` | Maximum queued spans |
| `trace_transactions` | 0 or 1 | `1` | Enable transaction tracing |
| `trace_consensus` | 0 or 1 | `1` | Enable consensus tracing |
| `trace_rpc` | 0 or 1 | `1` | Enable RPC tracing |
| `trace_peer` | 0 or 1 | `1` | Enable peer message tracing (high volume) |
| `trace_ledger` | 0 or 1 | `1` | Enable ledger tracing |
| `tx_trace_strategy` | string | `"deterministic"` | TX trace ID strategy: `"deterministic"` (trace_id = txHash[0:16]) or `"attribute"` (random) |
| `consensus_trace_strategy` | string | `"deterministic"` | Consensus trace ID strategy: `"deterministic"` (trace_id = prevLedgerHash[0:16]) or `"attribute"` (random) |
| `service_name` | string | `"xrpld"` | Service name (`service.name`) for traces and metrics |
| `service_instance_id` | string | `<node_pubkey>` | Instance identifier |
| Option | Type | Default | Description |
| -------------------------- | ------ | --------------------------------- | ----------------------------------------------------------------------------------------------------------------------- |
| `enabled` | 0 or 1 | `0` | Enable/disable telemetry |
| `traces_endpoint` | string | `http://localhost:4318/v1/traces` | Full OTLP/HTTP URL for spans, used verbatim |
| `use_tls` | 0 or 1 | `0` | Enable TLS for exporter connection |
| `tls_ca_cert` | string | `""` | Path to CA certificate file |
| `tls_client_cert` | string | `""` | Client cert (PEM) for mTLS; empty = one-way; if `enabled=1`, needs key + `use_tls=1` or startup fails |
| `tls_client_key` | string | `""` | Private key (PEM) for `tls_client_cert`; if set with `enabled=1`, needs the cert + `use_tls=1` or fails |
| `batch_size` | uint | `512` | Spans per export batch |
| `batch_delay_ms` | uint | `5000` | Max delay before sending batch (ms) |
| `max_queue_size` | uint | `2048` | Maximum queued spans |
| `trace_transactions` | 0 or 1 | `1` | Enable transaction tracing |
| `trace_consensus` | 0 or 1 | `1` | Enable consensus tracing |
| `trace_rpc` | 0 or 1 | `1` | Enable RPC tracing |
| `trace_peer` | 0 or 1 | `1` | Enable peer message tracing (high volume) |
| `trace_ledger` | 0 or 1 | `1` | Enable ledger tracing |
| `tx_trace_strategy` | string | `"deterministic"` | TX trace ID strategy: `"deterministic"` (trace_id = txHash[0:16]) or `"attribute"` (random) |
| `consensus_trace_strategy` | string | `"deterministic"` | Consensus trace ID strategy: `"deterministic"` (trace_id = prevLedgerHash[0:16]) or `"random"` (experimental, not used) |
| `service_name` | string | `"xrpld"` | Service name (`service.name`) for traces and metrics |
| `service_instance_id` | string | `<node_pubkey>` | Instance identifier |
**Planned (not yet implemented)**: the following options appear in the design
documents but are not parsed by `TelemetryConfig.cpp` in Phase 1b and later
phases. They will be added as the corresponding subsystems are instrumented:
| Option | Planned Phase | Purpose |
| -------------------------- | ------------- | ----------------------------------------------------------------------- |
| `exporter` | Future | Select between OTLP/HTTP and OTLP/gRPC |
| `trace_pathfind` | Phase 2 | Path computation tracing toggle |
| `trace_txq` | Phase 3 | Transaction queue tracing toggle |
| `trace_validator` | Future | Validator list / manifest update tracing |
| `trace_amendment` | Future | Amendment voting tracing |
| `consensus_trace_strategy` | Phase 4 | Trace ID strategy for consensus rounds (`deterministic` \| `attribute`) |
| Option | Planned Phase | Purpose |
| ----------------- | ------------- | ---------------------------------------- |
| `exporter` | Future | Select between OTLP/HTTP and OTLP/gRPC |
| `trace_pathfind` | Phase 2 | Path computation tracing toggle |
| `trace_txq` | Phase 3 | Transaction queue tracing toggle |
| `trace_validator` | Future | Validator list / manifest update tracing |
| `trace_amendment` | Future | Amendment voting tracing |
---

View File

@@ -205,7 +205,8 @@ Phase 4a (establish-phase gap fill & cross-node correlation) adds:
- **Deterministic trace ID** derived from `previousLedger.id()` so all validators
in the same round share the same `trace_id` (switchable via
`consensus_trace_strategy` config: `"deterministic"` or `"attribute"`).
`consensus_trace_strategy` config: `"deterministic"`, or `"random"` which is
experimental and not used).
See [Configuration Reference](./05-configuration-reference.md) for full
configuration options.
- **Round lifecycle spans**: `consensus.round` with round-to-round span links.

View File

@@ -1812,6 +1812,20 @@ validators.txt
# Enable tracing for ledger close and accept operations — ledger
# building, state hashing, and write-back to the node store. Default: 1.
#
# consensus_trace_strategy=deterministic
#
# How the consensus round span picks its trace id. Two values are
# accepted, and anything else makes xrpld fail to start.
#
# deterministic (the default, and the value to use): the trace id comes
# from the previous ledger hash, so every validator of a round reports
# into one trace and the round can be read end to end across nodes.
#
# random: experimental only, and not used. Each node invents its own
# trace id, so a single round arrives as one separate trace per node.
# Those traces can only be lined up by hand through the
# consensus_ledger_id span attribute.
#
# --- Batch processor tuning ---
#
# batch_size=512

View File

@@ -317,7 +317,9 @@ namespace event {
*/
inline constexpr auto disputeResolve = join(makeStr("dispute"), makeStr("resolve"));
/**
* "tx.included"
* "tx.included" — one per transaction of the agreed consensus set, recorded
* before the ledger is built. A transaction that then fails to apply still
* has an event, so this is a superset of the accepted ledger's contents.
*/
inline constexpr auto txIncluded = join(makeStr("tx"), makeStr("included"));

View File

@@ -123,6 +123,69 @@ namespace xrpl::telemetry {
inline constexpr std::string_view kTracerName{"xrpld"};
#endif
/**
* How a consensus round span picks its trace id.
*
* consensus_trace_strategy (xrpld.cfg)
* |
* v
* makeTelemetrySetup() ──> Setup::consensusTraceStrategy
* |
* v
* RCLConsensus::Adaptor::startRoundTracing()
* |
* +-- Deterministic ──> SpanGuard::hashSpan(prev ledger hash)
* +-- Random ──> SpanGuard::span() / linkedSpan()
*
* Deterministic is the strategy in use. Every validator of a round hashes the
* same previous ledger id, so all of them land in one trace.
*
* Random is experimental and not used. Each node would invent its own trace
* id, so one round would arrive as one trace per node, joinable only by the
* `consensus_ledger_id` attribute.
*
* @code
* // Branch on the strategy rather than on a string.
* if (telemetry.getConsensusTraceStrategy() == ConsensusTraceStrategy::Random)
* span = SpanGuard::span(TraceCategory::Consensus, seg::consensus, op::round);
* else
* span = SpanGuard::hashSpan(TraceCategory::Consensus, name, id.data(), id.kBytes);
*
* // Edge case: the value also goes on a span attribute, so it needs its
* // config spelling back.
* span.setAttribute(attr::traceStrategy, strategyName(ConsensusTraceStrategy::Random));
* @endcode
*
* @note Adding an enumerator means adding a spelling to strategyName() below
* and to the parser in TelemetryConfig.cpp. Both switch without a default, so
* the compiler catches a missed one.
*/
enum class ConsensusTraceStrategy : std::uint8_t { Deterministic, Random };
/**
* Config spelling of a consensus trace strategy.
*
* This is the same text `consensus_trace_strategy` accepts, and it is what
* goes on the `trace_strategy` span attribute, so the two cannot drift.
*
* @param strategy Strategy to name.
* @return "deterministic" or "random", pointing at a string literal.
*/
[[nodiscard]] constexpr char const*
strategyName(ConsensusTraceStrategy strategy)
{
switch (strategy)
{
case ConsensusTraceStrategy::Deterministic:
return "deterministic";
case ConsensusTraceStrategy::Random:
return "random";
}
// The switch covers every enumerator. This return only satisfies the
// compiler, which cannot rule out a value outside the enumeration.
return "deterministic";
}
class Telemetry
{
/**
@@ -281,12 +344,12 @@ public:
bool traceLedger = true;
/**
* Strategy for cross-node consensus trace correlation.
* "deterministic" — derive trace_id from ledger hash so all
* validators in the same round share the same trace_id.
* "attribute" — random trace_id, correlate via ledger_id attribute.
* How a consensus round span picks its trace id.
*
* Read from `consensus_trace_strategy`. Deterministic is the strategy
* in use; Random is experimental. See ConsensusTraceStrategy.
*/
std::string consensusTraceStrategy = "deterministic";
ConsensusTraceStrategy consensusTraceStrategy = ConsensusTraceStrategy::Deterministic;
};
virtual ~Telemetry() = default;
@@ -359,9 +422,9 @@ public:
shouldTraceLedger() const = 0;
/**
* @return The configured consensus trace correlation strategy.
* @return How a consensus round span picks its trace id.
*/
[[nodiscard]] virtual std::string const&
[[nodiscard]] virtual ConsensusTraceStrategy
getConsensusTraceStrategy() const = 0;
#ifdef XRPL_ENABLE_TELEMETRY

View File

@@ -104,7 +104,7 @@ public:
return false;
}
[[nodiscard]] std::string const&
[[nodiscard]] ConsensusTraceStrategy
getConsensusTraceStrategy() const override
{
return setup_.consensusTraceStrategy;

View File

@@ -216,7 +216,7 @@ public:
return false;
}
[[nodiscard]] std::string const&
[[nodiscard]] ConsensusTraceStrategy
getConsensusTraceStrategy() const override
{
return setup_.consensusTraceStrategy;
@@ -429,7 +429,7 @@ public:
return setup_.traceLedger;
}
[[nodiscard]] std::string const&
[[nodiscard]] ConsensusTraceStrategy
getConsensusTraceStrategy() const override
{
return setup_.consensusTraceStrategy;

View File

@@ -51,6 +51,7 @@ constexpr char const* traceConsensus = "trace_consensus";
constexpr char const* traceRpc = "trace_rpc";
constexpr char const* tracePeer = "trace_peer";
constexpr char const* traceLedger = "trace_ledger";
constexpr char const* consensusTraceStrategy = "consensus_trace_strategy";
} // namespace key
/**
@@ -212,6 +213,33 @@ requireHttpsEndpoint(std::string const& endpoint, char const* configKey)
" is set, but is '" + endpoint + "'.");
}
/**
* Map a `consensus_trace_strategy` value onto its enumerator.
*
* Only the two documented spellings are accepted. A typo would otherwise pick
* the default silently, and the operator would never learn the setting had no
* effect. Matching is exact and case-sensitive, like every other value in this
* section.
*
* @param value Raw config value; empty means the key was absent.
* @return The matching strategy, or Deterministic when the key was absent.
* @throws std::runtime_error If the value is neither documented spelling.
*/
[[nodiscard]] ConsensusTraceStrategy
readConsensusTraceStrategy(std::string const& value)
{
if (value.empty() || value == strategyName(ConsensusTraceStrategy::Deterministic))
return ConsensusTraceStrategy::Deterministic;
if (value == strategyName(ConsensusTraceStrategy::Random))
return ConsensusTraceStrategy::Random;
Throw<std::runtime_error>(
std::string("Invalid value '") + key::consensusTraceStrategy + "' in " + kSectionLabel +
": must be '" + strategyName(ConsensusTraceStrategy::Deterministic) + "' or '" +
strategyName(ConsensusTraceStrategy::Random) + "'.");
}
} // namespace
Telemetry::Setup
@@ -322,7 +350,7 @@ makeTelemetrySetup(
setup.traceLedger = section.valueOr<int>(key::traceLedger, 1) != 0;
setup.consensusTraceStrategy =
section.valueOr<std::string>("consensus_trace_strategy", "deterministic");
readConsensusTraceStrategy(section.valueOr<std::string>(key::consensusTraceStrategy, ""));
return setup;
}

View File

@@ -168,14 +168,13 @@ public:
}
/**
* @return A fixed strategy label; the scope tests do not exercise
* deterministic trace-id correlation, so any stable value works.
* @return A fixed strategy; the scope tests do not exercise trace-id
* correlation, so either value works.
*/
[[nodiscard]] std::string const&
[[nodiscard]] ConsensusTraceStrategy
getConsensusTraceStrategy() const override
{
static std::string const kStrategy{"none"};
return kStrategy;
return ConsensusTraceStrategy::Deterministic;
}
opentelemetry::nostd::shared_ptr<opentelemetry::trace::Tracer>

View File

@@ -141,6 +141,7 @@ namespace key {
constexpr char const* batchSize = "batch_size";
constexpr char const* batchDelayMs = "batch_delay_ms";
constexpr char const* maxQueueSize = "max_queue_size";
constexpr char const* consensusTraceStrategy = "consensus_trace_strategy";
} // namespace key
/**
@@ -218,6 +219,7 @@ TEST(TelemetryConfig, setup_defaults)
EXPECT_TRUE(s.traceRpc);
EXPECT_TRUE(s.tracePeer);
EXPECT_TRUE(s.traceLedger);
EXPECT_EQ(s.consensusTraceStrategy, telemetry::ConsensusTraceStrategy::Deterministic);
}
TEST(TelemetryConfig, parse_empty_section)
@@ -763,6 +765,71 @@ TEST(TelemetryConfig, batch_size_equal_to_max_queue_size_is_accepted)
EXPECT_EQ(setup.maxQueueSize, 512u);
}
TEST(TelemetryConfig, consensus_trace_strategy_names_match_the_config_spellings)
{
// strategyName() feeds both the parser and the trace_strategy span
// attribute, so these two strings are the whole public vocabulary.
EXPECT_STREQ(
telemetry::strategyName(telemetry::ConsensusTraceStrategy::Deterministic), "deterministic");
EXPECT_STREQ(telemetry::strategyName(telemetry::ConsensusTraceStrategy::Random), "random");
}
TEST(TelemetryConfig, consensus_trace_strategy_defaults_to_deterministic)
{
// The key is absent, so the default applies. Deterministic is the only
// strategy in use, and a default of Random would break cross-node
// correlation on every node that omits the key.
EXPECT_EQ(
parseBatch({}).consensusTraceStrategy, telemetry::ConsensusTraceStrategy::Deterministic);
}
TEST(TelemetryConfig, consensus_trace_strategy_accepts_deterministic)
{
EXPECT_EQ(
parseBatch({{key::consensusTraceStrategy, "deterministic"}}).consensusTraceStrategy,
telemetry::ConsensusTraceStrategy::Deterministic);
}
TEST(TelemetryConfig, consensus_trace_strategy_accepts_random)
{
// Random is experimental and unused, but it is a documented spelling, so
// the parser must still map it to its own enumerator rather than reject it
// or fold it into the default.
EXPECT_EQ(
parseBatch({{key::consensusTraceStrategy, "random"}}).consensusTraceStrategy,
telemetry::ConsensusTraceStrategy::Random);
}
TEST(TelemetryConfig, consensus_trace_strategy_empty_value_is_the_default)
{
// `consensus_trace_strategy=` with nothing after it. An empty value means
// the operator wrote the key and no value, which is the default, not a typo.
EXPECT_EQ(
parseBatch({{key::consensusTraceStrategy, ""}}).consensusTraceStrategy,
telemetry::ConsensusTraceStrategy::Deterministic);
}
TEST(TelemetryConfig, consensus_trace_strategy_rejects_an_undocumented_value)
{
// "attribute" is not a spelling this parser accepts. Rejecting rather than
// defaulting is the point: a silent fallback would leave the operator
// believing a setting took effect.
EXPECT_EQ(
batchRejection({{key::consensusTraceStrategy, "attribute"}}),
"Invalid value 'consensus_trace_strategy' in [telemetry]: must be 'deterministic' or "
"'random'.");
}
TEST(TelemetryConfig, consensus_trace_strategy_matching_is_case_sensitive)
{
// Every other value in this section is matched exactly, so "Random" is a
// typo and must be reported as one.
EXPECT_EQ(
batchRejection({{key::consensusTraceStrategy, "Random"}}),
"Invalid value 'consensus_trace_strategy' in [telemetry]: must be 'deterministic' or "
"'random'.");
}
TEST(TelemetryConfig, null_telemetry_factory)
{
telemetry::Telemetry::Setup setup;

View File

@@ -695,6 +695,11 @@ RCLConsensus::Adaptor::doAccept(
JLOG(j_.debug()) << "Building canonical tx set: " << retriableTxs.key();
// One tx.included event per transaction of the agreed consensus set, which
// is not yet the accepted ledger: buildLCL() below applies these and some
// may fail, so the events are a superset of what the ledger ends up with. A
// transaction whose bytes cannot be parsed gets no event at all.
//
// txCount and the per-transaction event feed the span and nothing else, so
// both are guarded on the span being active. Unguarded, every accepted
// ledger builds one 64-character hash string per transaction that no one
@@ -1338,19 +1343,19 @@ RCLConsensus::Adaptor::startRoundTracing(RCLCxLedger const& prevLgr)
if (roundSpan_)
roundSpan_.reset();
auto const& strategy = app_.getTelemetry().getConsensusTraceStrategy();
auto const strategy = app_.getTelemetry().getConsensusTraceStrategy();
telemetry::SpanContext const* const link =
prevRoundSpanContext_.isValid() ? &prevRoundSpanContext_ : nullptr;
if (strategy == "attribute")
if (strategy == telemetry::ConsensusTraceStrategy::Random)
{
// Non-deterministic strategy: each node gets a random trace_id,
// correlated via the consensus_ledger_id attribute rather than a
// shared trace_id. Still attach a follows-from link to the prior
// round so consecutive rounds stay navigable. linkedSpan is not
// TraceCategory-aware, so gate it explicitly to match the gating
// of the hashSpan/span factories used below.
// Experimental strategy, not used on a live network: each node gets a
// random trace_id, so one round arrives as one trace per node, joinable
// only by the consensus_ledger_id attribute. Still attach a follows-from
// link to the prior round so consecutive rounds stay navigable.
// linkedSpan is not TraceCategory-aware, so gate it explicitly to match
// the gating of the hashSpan/span factories used below.
if (link != nullptr && app_.getTelemetry().shouldTraceConsensus())
{
roundSpan_.emplace(telemetry::SpanGuard::linkedSpan(cs::round, *link));
@@ -1364,7 +1369,7 @@ RCLConsensus::Adaptor::startRoundTracing(RCLCxLedger const& prevLgr)
}
else
{
// "deterministic" (the default): derive the trace_id from the previous
// Deterministic (the default): derive the trace_id from the previous
// ledger hash so all validators tracing the same round share one trace.
roundSpan_.emplace(
telemetry::SpanGuard::hashSpan(
@@ -1382,7 +1387,7 @@ RCLConsensus::Adaptor::startRoundTracing(RCLCxLedger const& prevLgr)
roundSpan_->setAttribute(cs::attr::ledgerId, to_string(prevLgr.id()).c_str());
roundSpan_->setAttribute(cs::attr::ledgerSeq, static_cast<int64_t>(prevLgr.seq()) + 1);
roundSpan_->setAttribute(cs::attr::traceStrategy, strategy.c_str());
roundSpan_->setAttribute(cs::attr::traceStrategy, telemetry::strategyName(strategy));
roundSpan_->setAttribute(cs::attr::roundId, static_cast<int64_t>(prevLgr.seq()) + 1);
roundSpan_->setAttribute(cs::attr::previousLedgerSeq, static_cast<int64_t>(prevLgr.seq()));
roundSpan_->setAttribute(cs::attr::previousProposers, static_cast<int64_t>(prevProposers_));

View File

@@ -92,8 +92,8 @@ class RCLConsensus
* Span for the current consensus round.
*
* Created in preStartRound(), ended (via reset()) when the next
* round begins. When consensusTraceStrategy is "deterministic",
* the trace_id is derived from previousLedger.id() so that all
* round begins. Under ConsensusTraceStrategy::Deterministic the
* trace_id is derived from previousLedger.id() so that all
* validators in the same round share the same trace_id.
*
* Thread-free: a SpanGuard owns no thread-local Scope, so it can be

View File

@@ -121,7 +121,10 @@ inline constexpr auto expiredCount = makeStr("expired_count");
*/
inline constexpr auto terCode = makeStr("ter_code");
/**
* "retries_remaining" — retries left before discard.
* "retries_remaining" — retries left as this attempt started, recorded before
* the transaction is applied and before any decrement. A span with
* txq_status="retried" therefore always shows a non-zero count; exhaustion
* shows up as txq_status="failed" with zero.
*/
inline constexpr auto retriesRemaining = makeStr("retries_remaining");
/**