From 9aebdc292c248d8342f7e318c02586270ead59b7 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Mon, 7 Sep 2026 13:16:32 +0100 Subject: [PATCH] docs(telemetry): fix stale symbols, attribute keys and TraceQL in the plan docs Review feedback on the plan documents. Four kinds of error: - Symbols that do not exist: ConsensusProposal::prevLedger_ (it is previousLedger_), RCLConsensusAdaptor (it is RCLConsensus::Adaptor, and startRound() is on RCLConsensus itself), and RPCHandler::doCommand (a free function, xrpl::rpc::doCommand). - Attribute keys: the tables used ledger_index, which no telemetry code emits. Same concept as ledger_seq but a different referent, so the code disambiguates by prefix: current_ledger_seq for the open ledger a transaction targeted, ledger_seq for a closed or validated one. A note now states which is which. - TraceQL that does not parse: span-field predicates need braces, status.code is not an intrinsic (status = error), and avg(duration) does not take a by clause (avg_over_time does). All five re-tested against Tempo. - The StatsD comparison omitted the Histogram instrument, which aggregates at the point of measure, and the when-to-use table had no row for a metric that spans cannot afford to carry. --- OpenTelemetryPlan/00-tracing-fundamentals.md | 6 +-- OpenTelemetryPlan/02-design-decisions.md | 52 +++++++++++++------ .../05-configuration-reference.md | 14 ++--- OpenTelemetryPlan/06-implementation-phases.md | 28 +++++----- .../07-observability-backends.md | 2 +- 5 files changed, 62 insertions(+), 40 deletions(-) diff --git a/OpenTelemetryPlan/00-tracing-fundamentals.md b/OpenTelemetryPlan/00-tracing-fundamentals.md index 9c6f96d7af..1c7675243a 100644 --- a/OpenTelemetryPlan/00-tracing-fundamentals.md +++ b/OpenTelemetryPlan/00-tracing-fundamentals.md @@ -297,9 +297,9 @@ XRPL has a unique advantage: its core workflows produce **globally unique 256-bi Transaction: STTx::getTransactionID() → uint256 tid_ TMTransaction::rawTransaction → recompute hash from bytes -Consensus: ConsensusProposal::prevLedger_ → uint256 (previous ledger hash) - ConsensusProposal::position_ → uint256 (TxSet hash) - LedgerHeader::seq → uint32_t (ledger sequence) +Consensus: ConsensusProposal::previousLedger_ → uint256 (previous ledger hash) + ConsensusProposal::position_ → uint256 (TxSet hash) + LedgerHeader::seq → uint32_t (ledger sequence) Validation: STValidation::getLedgerHash() → uint256 STValidation::getNodeID() → NodeID (160-bit) diff --git a/OpenTelemetryPlan/02-design-decisions.md b/OpenTelemetryPlan/02-design-decisions.md index c7930454e5..dae5e8fe82 100644 --- a/OpenTelemetryPlan/02-design-decisions.md +++ b/OpenTelemetryPlan/02-design-decisions.md @@ -236,17 +236,19 @@ keys (the dotted form is reserved for resource scope per §2.3.3). #### Transaction Attributes -| Key | Type | Description | -| -------------- | ------ | ------------------------------------- | -| `tx_hash` | string | Transaction hash (hex) | -| `tx_type` | string | `"Payment"`, `"OfferCreate"`, etc. | -| `tx_account` | string | Source account (redacted in prod) | -| `tx_sequence` | int64 | Account sequence number | -| `tx_fee` | int64 | Fee in drops | -| `tx_result` | string | `"tesSUCCESS"`, `"tecPATH_DRY"`, etc. | -| `ledger_index` | int64 | Ledger containing transaction | -| `relay_count` | int64 | Peers the transaction was relayed to | -| `suppressed` | bool | `true` when HashRouter dropped a dup | +| Key | Type | Description | +| -------------------- | ------ | ------------------------------------- | +| `tx_hash` | string | Transaction hash (hex) | +| `tx_type` | string | `"Payment"`, `"OfferCreate"`, etc. | +| `tx_account` | string | Source account (redacted in prod) | +| `tx_sequence` | int64 | Account sequence number | +| `tx_fee` | int64 | Fee in drops | +| `tx_result` | string | `"tesSUCCESS"`, `"tecPATH_DRY"`, etc. | +| `current_ledger_seq` | int64 | Open ledger the transaction targeted | +| `relay_count` | int64 | Peers the transaction was relayed to | +| `suppressed` | bool | `true` when HashRouter dropped a dup | + +> **Note:** `current_ledger_seq` and `ledger_seq` are the same concept — a ledger's sequence number — but they name different ledgers, so the design keeps two keys rather than one. `current_ledger_seq` is the open or in-flight ledger a transaction's work was applied into; it is named after the RPC field `ledger_current_index`. `ledger_seq` (see [Ledger & Job Attributes](#ledger--job-attributes)) is a closed or validated ledger, set by the ledger and consensus spans. Neither is spelled `ledger_index`: per rule 2 of [Telemetry span attribute naming](../CONTRIBUTING.md#telemetry-span-attribute-naming), one concept gets one key reused verbatim, and a different referent is disambiguated with a prefix rather than a synonym. #### Consensus Attributes @@ -289,7 +291,7 @@ keys (the dotted form is reserved for resource scope per §2.3.3). | Key | Type | Description | | --------------------------- | ------- | --------------------------------- | | `ledger_hash` | string | Ledger hash | -| `ledger_index` | int64 | Ledger sequence/index | +| `ledger_seq` | int64 | Closed/validated ledger sequence | | `close_time_ripple_epoch_s` | int64 | Close time (Ripple epoch seconds) | | `ledger_tx_count` | int64 | Transaction count | | `job_type` | string | Job type name | @@ -348,11 +350,11 @@ The following table summarizes what data is collected by category: | Category | Attributes Collected | Purpose | | --------------- | ------------------------------------------------------------------------------------------------- | ---------------------------- | -| **Transaction** | `tx_hash`, `tx_type`, `tx_result`, `tx_fee`, `ledger_index` | Trace transaction lifecycle | +| **Transaction** | `tx_hash`, `tx_type`, `tx_result`, `tx_fee`, `current_ledger_seq` | Trace transaction lifecycle | | **Consensus** | `consensus_round`, `consensus_phase`, `consensus_mode`, `proposers`, `round_time_ms` | Analyze consensus timing | | **RPC** | `command`, `version`, `rpc_status`, `duration_ms` | Monitor RPC performance | | **Peer** | `peer_id` (public key), `peer_latency_ms`, `message_type`, `message_size_bytes` | Network topology analysis | -| **Ledger** | `ledger_hash`, `ledger_index`, `close_time`, `ledger_tx_count` | Ledger progression tracking | +| **Ledger** | `ledger_hash`, `ledger_seq`, `close_time`, `ledger_tx_count` | Ledger progression tracking | | **Job** | `job_type`, `job_queue_ms`, `job_worker` | JobQueue performance | | **PathFinding** | `pathfind_source_currency`, `pathfind_dest_currency`, `pathfind_path_count`, `pathfind_cache_hit` | Payment path analysis | | **TxQ** | `txq_queue_depth`, `txq_fee_level`, `txq_eviction_reason` | Queue depth and fee tracking | @@ -492,9 +494,12 @@ A PerfLog entry is a JSON object with fields such as `time`, `method`, - No request-level detail - No causal relationships - Single-node perspective + - Aggregation happens on the StatsD server, not in the process In xrpld, Beast Insight is used through `increment` (counters), `gauge` -(point-in-time values), and `timing` (durations) calls. +(point-in-time values), and `timing` (durations) calls. A `timing` call sends +each measured value as its own raw `|ms` sample, so the histogram a dashboard +reads is built by the StatsD server from that stream of values. #### OpenTelemetry (NEW) @@ -503,6 +508,7 @@ In xrpld, Beast Insight is used through `increment` (counters), `gauge` - **Cross-node correlation** via `trace_id` - Parent-child span relationships - Rich attributes per span + - A `Histogram` instrument that aggregates **at the point of measure** - Industry standard (CNCF) - **Limitations**: - Requires collector infrastructure @@ -512,6 +518,13 @@ A span is created via `startSpan` (e.g. `"tx.relay"`), annotated with attributes such as `tx_hash` and `peer_id`, and is automatically linked to its parent through the active context. +OpenTelemetry is not only spans. The same SDK offers a `Histogram` instrument, +and a `Record()` call folds the value straight into bucket counts inside the +process — no per-event record is shipped and no server-side aggregation step is +needed. That is what makes it affordable in a hot loop where one span per event +would not be, and it is the one thing Beast Insight cannot do, because its +`timing` path ships raw values and aggregates them on the StatsD server. + ### 2.6.3 When to Use Each | Scenario | PerfLog | StatsD | OpenTelemetry | @@ -522,6 +535,15 @@ parent through the active context. | "Which node delayed consensus?" | ❌ | ❌ | ✅ | | "What happened on node X at time T?" | ✅ | ❌ | ✅ | | "Show me the TX journey across 5 nodes" | ❌ | ❌ | ✅ | +| "p99 NodeStore fetch latency?" | ❌ | ❌ | ✅ | + +The last row is the case a span cannot answer. One `TMGetObjectByHash` message +requests up to `tuning::kHardMaxReplyNodes` objects, so a span per NodeStore +fetch is not affordable in that loop. Instead the fetch loop's wall time is +recorded once per message into an OpenTelemetry `Histogram` +(`getobject_lookup_us`), and the quantile is read off its buckets. StatsD is +marked ❌ because that instrument is recorded on the native OpenTelemetry metrics +path, not through Beast Insight. ### 2.6.4 Coexistence Strategy diff --git a/OpenTelemetryPlan/05-configuration-reference.md b/OpenTelemetryPlan/05-configuration-reference.md index cf0f993dda..15b594f0e1 100644 --- a/OpenTelemetryPlan/05-configuration-reference.md +++ b/OpenTelemetryPlan/05-configuration-reference.md @@ -75,7 +75,7 @@ The parser `setup_Telemetry()` in `src/libxrpl/telemetry/TelemetryConfig.cpp` re > available on both. Components that hold a `ServiceRegistry&` (e.g. > `NetworkOPsImp`) call `registry_.get().getTelemetry()`. Components that > still hold an `Application&` (e.g. `ServerHandler`, `PeerImp`, -> `RCLConsensusAdaptor`) call `app_.getTelemetry()` directly. +> `RCLConsensus::Adaptor`) call `app_.getTelemetry()` directly. --- @@ -196,7 +196,7 @@ An example `xrpld RPC Performance` dashboard (uid `xrpld-rpc-performance`) sourc ### 5.8.4 Example Dashboard: Transaction Tracing -An example `xrpld Transaction Tracing` dashboard (uid `xrpld-tx-tracing`) over Tempo provides three panels: transaction throughput (`tx.receive` rate, stat), cross-node relay count (average `span.relay_count` on `tx.relay`, timeseries), and a table of transaction validation errors (`tx.validate` with `status.code=error`). +An example `xrpld Transaction Tracing` dashboard (uid `xrpld-tx-tracing`) over Tempo provides three panels: transaction throughput (`tx.receive` rate, stat), cross-node relay count (average `span.relay_count` on `tx.relay`, timeseries), and a table of transaction validation errors (`tx.validate` with `status = error`). ### 5.8.5 TraceQL Query Examples @@ -207,19 +207,19 @@ Common queries for xrpld traces: {resource.service.name="xrpld" && span.tx_hash="ABC123..."} # Find slow RPC commands (>100ms) -{resource.service.name="xrpld" && name=~"rpc.command.*"} | duration > 100ms +{resource.service.name="xrpld" && name=~"rpc.command.*"} | { duration > 100ms } # Find consensus rounds taking >5 seconds -{resource.service.name="xrpld" && name="consensus.round"} | duration > 5s +{resource.service.name="xrpld" && name="consensus.round"} | { duration > 5s } # Find failed transactions with error details -{resource.service.name="xrpld" && name="tx.validate" && status.code=error} +{resource.service.name="xrpld" && name="tx.validate" && status = error} # Find transactions relayed to many peers -{resource.service.name="xrpld" && name="tx.relay"} | span.relay_count > 10 +{resource.service.name="xrpld" && name="tx.relay"} | { span.relay_count > 10 } # Compare latency across nodes -{resource.service.name="xrpld" && name="rpc.command.account_info"} | avg(duration) by (resource.service.instance.id) +{resource.service.name="xrpld" && name="rpc.command.account_info"} | avg_over_time(duration) by (resource.service.instance.id) ``` ### 5.8.6 Correlation with PerfLog diff --git a/OpenTelemetryPlan/06-implementation-phases.md b/OpenTelemetryPlan/06-implementation-phases.md index 12eea9c67b..cf1e58f779 100644 --- a/OpenTelemetryPlan/06-implementation-phases.md +++ b/OpenTelemetryPlan/06-implementation-phases.md @@ -97,7 +97,7 @@ gantt | ---- | -------------------------------------------------------------------------- | | 2.1 | Implement W3C Trace Context HTTP header extraction | | 2.2 | Instrument `ServerHandler::onRequest()` | -| 2.3 | Instrument `RPCHandler::doCommand()` | +| 2.3 | Instrument `xrpl::rpc::doCommand()` | | 2.4 | Add RPC-specific attributes | | 2.5 | Instrument WebSocket handler | | 2.6 | PathFinding instrumentation (`pathfind.request`, `pathfind.compute` spans) | @@ -150,19 +150,19 @@ gantt ### Tasks -| Task | Description | -| ---- | ---------------------------------------------- | -| 4.1 | Instrument `RCLConsensusAdaptor::startRound()` | -| 4.2 | Instrument phase transitions | -| 4.3 | Instrument proposal handling | -| 4.4 | Instrument validation handling | -| 4.5 | Add consensus-specific attributes | -| 4.6 | Correlate with transaction traces | -| 4.7 | Validator list and manifest tracing | -| 4.8 | Amendment voting tracing | -| 4.9 | SHAMap sync tracing | -| 4.10 | Multi-validator integration tests | -| 4.11 | Performance validation | +| Task | Description | +| ---- | --------------------------------------- | +| 4.1 | Instrument `RCLConsensus::startRound()` | +| 4.2 | Instrument phase transitions | +| 4.3 | Instrument proposal handling | +| 4.4 | Instrument validation handling | +| 4.5 | Add consensus-specific attributes | +| 4.6 | Correlate with transaction traces | +| 4.7 | Validator list and manifest tracing | +| 4.8 | Amendment voting tracing | +| 4.9 | SHAMap sync tracing | +| 4.10 | Multi-validator integration tests | +| 4.11 | Performance validation | ### Exit Criteria diff --git a/OpenTelemetryPlan/07-observability-backends.md b/OpenTelemetryPlan/07-observability-backends.md index 4ebb6028fd..ecca8320d3 100644 --- a/OpenTelemetryPlan/07-observability-backends.md +++ b/OpenTelemetryPlan/07-observability-backends.md @@ -245,7 +245,7 @@ A Tempo-backed dashboard (uid `xrpld-node-overview`) with four panels: - **Active Nodes** (stat): count of distinct `resource.service.instance.id` values seen for the `xrpld` service. - **Total Transactions (1h)** (stat): count of `tx.receive` spans. -- **Error Rate** (gauge, percent): ratio of `status.code=error` spans to all spans, with yellow/red thresholds at 1%/5%. +- **Error Rate** (gauge, percent): ratio of `status = error` spans to all spans, with yellow/red thresholds at 1%/5%. - **Service Map** (nodeGraph): Tempo-generated service dependency graph. ### 7.6.3 Alert Rules