fix(telemetry): make the consensus trace strategy an enum

consensus_trace_strategy was read as a std::string and compared against the
literal "attribute" in startRoundTracing(), while the runbook documented
"deterministic" and "random". The documented value "random" therefore fell
through to the default and did nothing.

Parse the setting once into ConsensusTraceStrategy, so the consensus code
branches on a type. The accepted spellings are now "deterministic" and
"random"; anything else fails at startup instead of silently defaulting.
The behaviour behind the old "attribute" name is unchanged and is now
reached by "random".

Document consensus_trace_strategy in xrpld-example.cfg, stating that
"random" is experimental and not used: it gives each node its own trace id,
so one round arrives as one trace per node.

Also state on the tx.included event that it covers the agreed consensus set
before the ledger is built, so it is a superset of the accepted ledger.

OpenTelemetryPlan/02-design-decisions.md

@@ -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                              |

cfg/xrpld-example.cfg

@@ -1775,6 +1775,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

include/xrpl/consensus/ConsensusSpanNames.h

@@ -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"));
 

include/xrpl/telemetry/Telemetry.h

@@ -122,6 +122,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
 {
     /**
@@ -267,12 +330,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;
@@ -345,9 +408,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

src/libxrpl/telemetry/NullTelemetry.cpp

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

src/libxrpl/telemetry/Telemetry.cpp

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

src/libxrpl/telemetry/TelemetryConfig.cpp

@@ -46,6 +46,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
 
 /**
@@ -150,6 +151,33 @@ networkTypeFromId(std::uint32_t networkId)
     }
 }
 
+/**
+ * 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
@@ -204,7 +232,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;
 }

src/tests/libxrpl/telemetry/SpanGuardScope.cpp

@@ -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>

src/tests/libxrpl/telemetry/TelemetryConfig.cpp

@@ -27,6 +27,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
 
 /**
@@ -104,6 +105,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)
@@ -286,6 +288,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;

src/xrpld/app/consensus/RCLConsensus.cpp

@@ -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_));

src/xrpld/app/consensus/RCLConsensus.h

@@ -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