diff --git a/OpenTelemetryPlan/02-design-decisions.md b/OpenTelemetryPlan/02-design-decisions.md index a9bb7a3c71..77e0fe7197 100644 --- a/OpenTelemetryPlan/02-design-decisions.md +++ b/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 | @@ -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? diff --git a/OpenTelemetryPlan/05-configuration-reference.md b/OpenTelemetryPlan/05-configuration-reference.md index 08222a7a77..3182b8dfd2 100644 --- a/OpenTelemetryPlan/05-configuration-reference.md +++ b/OpenTelemetryPlan/05-configuration-reference.md @@ -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 | `` | 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 | `` | 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 | --- diff --git a/OpenTelemetryPlan/06-implementation-phases.md b/OpenTelemetryPlan/06-implementation-phases.md index 12d996457d..8ff34c8101 100644 --- a/OpenTelemetryPlan/06-implementation-phases.md +++ b/OpenTelemetryPlan/06-implementation-phases.md @@ -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. diff --git a/cfg/xrpld-example.cfg b/cfg/xrpld-example.cfg index 3b52ad8eae..8893d89c99 100644 --- a/cfg/xrpld-example.cfg +++ b/cfg/xrpld-example.cfg @@ -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 diff --git a/include/xrpl/consensus/ConsensusSpanNames.h b/include/xrpl/consensus/ConsensusSpanNames.h index 3c09dded41..e9d073b5e9 100644 --- a/include/xrpl/consensus/ConsensusSpanNames.h +++ b/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")); diff --git a/include/xrpl/telemetry/Telemetry.h b/include/xrpl/telemetry/Telemetry.h index b996279d3a..c8873da0f6 100644 --- a/include/xrpl/telemetry/Telemetry.h +++ b/include/xrpl/telemetry/Telemetry.h @@ -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 diff --git a/src/libxrpl/telemetry/NullTelemetry.cpp b/src/libxrpl/telemetry/NullTelemetry.cpp index f8c3e4eb67..64030b02d7 100644 --- a/src/libxrpl/telemetry/NullTelemetry.cpp +++ b/src/libxrpl/telemetry/NullTelemetry.cpp @@ -104,7 +104,7 @@ public: return false; } - [[nodiscard]] std::string const& + [[nodiscard]] ConsensusTraceStrategy getConsensusTraceStrategy() const override { return setup_.consensusTraceStrategy; diff --git a/src/libxrpl/telemetry/Telemetry.cpp b/src/libxrpl/telemetry/Telemetry.cpp index b6cd988eec..bea8632d1a 100644 --- a/src/libxrpl/telemetry/Telemetry.cpp +++ b/src/libxrpl/telemetry/Telemetry.cpp @@ -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; diff --git a/src/libxrpl/telemetry/TelemetryConfig.cpp b/src/libxrpl/telemetry/TelemetryConfig.cpp index ef7d430e21..a5994a173c 100644 --- a/src/libxrpl/telemetry/TelemetryConfig.cpp +++ b/src/libxrpl/telemetry/TelemetryConfig.cpp @@ -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::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(key::traceLedger, 1) != 0; setup.consensusTraceStrategy = - section.valueOr("consensus_trace_strategy", "deterministic"); + readConsensusTraceStrategy(section.valueOr(key::consensusTraceStrategy, "")); return setup; } diff --git a/src/tests/libxrpl/telemetry/SpanGuardScope.cpp b/src/tests/libxrpl/telemetry/SpanGuardScope.cpp index b14102f96f..5c70f806ed 100644 --- a/src/tests/libxrpl/telemetry/SpanGuardScope.cpp +++ b/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 diff --git a/src/tests/libxrpl/telemetry/TelemetryConfig.cpp b/src/tests/libxrpl/telemetry/TelemetryConfig.cpp index 15cb52a562..1f258bfcdc 100644 --- a/src/tests/libxrpl/telemetry/TelemetryConfig.cpp +++ b/src/tests/libxrpl/telemetry/TelemetryConfig.cpp @@ -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; diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index ac627e2519..5b216dcce5 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/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(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(prevLgr.seq()) + 1); roundSpan_->setAttribute(cs::attr::previousLedgerSeq, static_cast(prevLgr.seq())); roundSpan_->setAttribute(cs::attr::previousProposers, static_cast(prevProposers_)); diff --git a/src/xrpld/app/consensus/RCLConsensus.h b/src/xrpld/app/consensus/RCLConsensus.h index 7830622fa8..af85f2bcc7 100644 --- a/src/xrpld/app/consensus/RCLConsensus.h +++ b/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 diff --git a/src/xrpld/app/misc/detail/TxQSpanNames.h b/src/xrpld/app/misc/detail/TxQSpanNames.h index 6b64b76a57..703553e7b9 100644 --- a/src/xrpld/app/misc/detail/TxQSpanNames.h +++ b/src/xrpld/app/misc/detail/TxQSpanNames.h @@ -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"); /**