diff --git a/OpenTelemetryPlan/02-design-decisions.md b/OpenTelemetryPlan/02-design-decisions.md index 1ccf7c9285..a1cc4642c1 100644 --- a/OpenTelemetryPlan/02-design-decisions.md +++ b/OpenTelemetryPlan/02-design-decisions.md @@ -439,7 +439,7 @@ this table disagrees with either, they win. | Category | Attributes emitted (from `*SpanNames.h`) | Named here but NOT emitted | Purpose | | --------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------- | -| **Transaction** | `tx_hash`, `tx_type`, `ter_result`, `fee`, `sequence`, `current_ledger_seq`, `current_ledger_hash`, `local`, `path`, `suppressed`, `tx_status`, `peer_version`, `peer_id`, `stage`, `applied` | `tx_result` (renamed → `ter_result`), `tx_fee` (→ `fee`), `ledger_index` (→ `current_ledger_seq`), `relay_count`. **`ledger_seq` is not a `tx.*` key**: no `tx.*` span sets it — the receive and apply-stage spans stamp `current_ledger_seq` (`NetworkOPs.cpp:1422`, `PeerImp.cpp:1337`, `Transactor.cpp:1613`, `applySteps.cpp:115`) and, where a view exists, `current_ledger_hash` (`Transactor.cpp:1615`, `applySteps.cpp:121`) | Trace transaction lifecycle | +| **Transaction** | `tx_hash`, `tx_type`, `ter_result`, `fee`, `sequence`, `current_ledger_seq`, `current_ledger_hash`, `local`, `path`, `tx_status`, `peer_version`, `peer_id`, `stage`, `applied` | `tx_result` (renamed → `ter_result`), `tx_fee` (→ `fee`), `ledger_index` (→ `current_ledger_seq`), `relay_count`. **`ledger_seq` is not a `tx.*` key**: no `tx.*` span sets it — the receive and apply-stage spans stamp `current_ledger_seq` (`NetworkOPs.cpp:1422`, `PeerImp.cpp:1337`, `Transactor.cpp:1613`, `applySteps.cpp:115`) and, where a view exists, `current_ledger_hash` (`Transactor.cpp:1615`, `applySteps.cpp:121`) | Trace transaction lifecycle | | **Consensus** | All 54 keys in `ConsensusSpanNames.h`'s `attr` namespace (48 own constants + 6 `using` re-exports), each with a set-site: `consensus_ledger_id`, `consensus_round`, `consensus_round_id`, `consensus_phase`, `consensus_mode`, `consensus_state`, `consensus_result`, `consensus_stalled`, `proposers`, `proposers_finished`, `previous_proposers`, `previous_ledger_seq`, `previous_round_time_ms`, `round_time_ms`, `open_duration_ms`, `quorum`, `proposing`, `is_bow_out`, `trace_strategy`, `converge_percent`, `establish_count`, `tx_count`, `tx_count_open`, `tx_id`, `disputes_count`, `disputes_resolved_count`, `dispute_our_vote`, `dispute_yays`, `dispute_nays`, `agree_count`, `disagree_count`, `threshold_percent`, `avalanche_threshold`, `close_time_threshold`, `have_close_time_consensus`, `close_time_resolution_ms`, `close_time_self_ripple_epoch_s`, `close_time_vote_bins`, `resolution_direction`, `parent_close_time_ripple_epoch_s`, `peer_positions_at_close`, `prev_ledger_prefix`, `position_hash_prefix`, `mode_old`, `mode_new`, `validation_sign_time`, `proposal_trusted`, `validation_trusted`; re-exported shared keys `ledger_seq`, `ledger_hash`, `full_validation`, `close_time_ripple_epoch_s`, `close_time_correct`, `close_resolution_ms` | — | Analyze consensus timing | | **RPC** | `command`, `version`, `rpc_role`, `rpc_status`, `request_payload_size`, `is_batch`, `batch_size`, `load_type` | `duration_ms` (span duration is a TraceQL intrinsic — query `duration`), `params` | Monitor RPC performance | | **Peer** | `peer_id` (**int64**, the process-local `Peer::id_` slot number — not a key of any kind; also set on `tx.receive`), `proposal_trusted`, `validation_trusted`, `ledger_hash`, `full_validation`. (`peer_version` is **not** a peer-span key: the constant lives in `TxSpanNames.h:79` and its only set-site is `PeerImp.cpp:1342` on the `tx.receive` span — see the Transaction row) | `peer_address`, `peer_latency_ms`, `peer_cluster`, `message_type`, `message_size_bytes`, `message_compressed` — the peer-span coverage gap (§2.3.2) | Network topology analysis | diff --git a/OpenTelemetryPlan/05-configuration-reference.md b/OpenTelemetryPlan/05-configuration-reference.md index 967f79a96c..ba92196349 100644 --- a/OpenTelemetryPlan/05-configuration-reference.md +++ b/OpenTelemetryPlan/05-configuration-reference.md @@ -441,7 +441,7 @@ The search-filter list is the practical index of queryable span attributes: resource scope (`service.name`, `service.instance.id`, `service.version`, `xrpl.network.id`, `xrpl.network.type`), intrinsics (`name`, `status`, `duration`), and span scope (`command`, `rpc_status`, `rpc_role`, `tx_hash`, -`tx_type`, `tx_status`, `local`, `path`, `suppressed`, `peer_version`, +`tx_type`, `tx_status`, `local`, `path`, `peer_version`, `consensus_*`, `ledger_seq`, `ledger_hash`, `close_time_correct`, `close_resolution_ms`, `proposers`, `mode_old`, `mode_new`, `txq_status`, `ter_code`). diff --git a/OpenTelemetryPlan/06-implementation-phases.md b/OpenTelemetryPlan/06-implementation-phases.md index 0d94e61d24..479ca85ea8 100644 --- a/OpenTelemetryPlan/06-implementation-phases.md +++ b/OpenTelemetryPlan/06-implementation-phases.md @@ -182,7 +182,7 @@ and [Phase3_taskList.md Task 3.9](./Phase3_taskList.md) for the full implementat - [x] Trace context in Protocol Buffer messages — `message TraceContext` (`include/xrpl/proto/xrpl.proto:101`), carried as optional field `1001` on three message types (`:130`, `:181`, `:229`) -- [x] HashRouter deduplication visible in traces — `suppressed` attribute +- [x] HashRouter deduplication visible — a dropped duplicate produces no span (`TxSpanNames.h:71`) - [ ] Multi-node integration tests passing — Phase 10 harness - [ ] <5% overhead on transaction throughput — needs the Phase 10 benchmark suite @@ -1053,7 +1053,7 @@ flowchart LR - **Metric validation** uses the Prometheus `/api/v1/series` endpoint (not instant queries) which polls for late-populating series and ignores Prometheus's staleness horizon. Every metric in `expected_metrics.json` must have > 0 series. - **Gauge visibility**: the harness sets `[insight] server=otel` (`run-full-validation.sh`), so `beast::insight` gauges become OTel observable gauges whose callback is invoked on every collection cycle. A gauge that sits at 0 and never changes (e.g. `jobq_job_count`) therefore still reports, and `/api/v1/series` sees it. - **I/O latency fix**: `io_latency_sampler` emits unconditionally on first sample, then applies the 10 ms threshold. This ensures `ios_latency` is registered in Prometheus even in low-load CI environments. -- **tx.receive span**: attribute keys are bare, not dotted — `suppressed` and `tx_status` (`TxSpanNames.h:71,75`). `suppressed` is set on both outcomes (`false` on the accepted path, `true` when the HashRouter suppresses), but `tx_status` is set **only** on the reject/known-bad/dropped paths, so it is absent on a successful receive. Assert on the attribute, not on span status. +- **tx.receive span**: attribute keys are bare, not dotted — `tx_status` (`TxSpanNames.h`). The span is created only after the node decides to process the transaction, so a relayed duplicate produces no span; how many were dropped is the `transactions_duplicate` traffic category. `tx_status` is set **only** on the paths that drop a transaction after that point, so it is absent on a successful receive. Assert on the attribute, not on span status. ### Tasks diff --git a/OpenTelemetryPlan/09-data-collection-reference.md b/OpenTelemetryPlan/09-data-collection-reference.md index 1394a9a91e..3a0cb7608e 100644 --- a/OpenTelemetryPlan/09-data-collection-reference.md +++ b/OpenTelemetryPlan/09-data-collection-reference.md @@ -353,7 +353,6 @@ The tables below list one row per attribute per subsystem, so a key shared by tw | `tx_type` | string | `tx.process`, `tx.preflight`, `tx.preclaim`, `tx.transactor` | Transaction type name (e.g., `Payment`) | | `fee` | int64 | `tx.process` | Transaction fee in drops | | `sequence` | int64 | `tx.process` | Transaction sequence number | -| `suppressed` | boolean | `tx.receive` | `true` if transaction was suppressed (duplicate) | | `tx_status` | string | `tx.receive` | Transaction status (e.g., `"known_bad"`) | | `peer_id` | int64 | `tx.receive` | Peer identifier (also set on peer spans) | | `peer_version` | string | `tx.receive` | Peer protocol version string | @@ -366,7 +365,7 @@ The tables below list one row per attribute per subsystem, so a key shared by tw **Tempo query**: `{span.tx_hash=""}` to trace a specific transaction across nodes. Join a transaction's work to its ledger with `{span.current_ledger_seq=}`. -**Prometheus labels**: `local`, `suppressed`, `tx_type`, `ter_result`, `stage` (SpanMetrics dimensions). +**Prometheus labels**: `local`, `tx_type`, `ter_result`, `stage` (SpanMetrics dimensions). #### Transaction Queue (TxQ) Attributes @@ -559,7 +558,6 @@ SpanMetrics connector does not rewrite or prefix it: | `consensus_mode` | string | `consensus.round`, `consensus.ledger_close` | | `close_time_correct` | boolean | `consensus.accept.apply` | | `local` | boolean | `tx.process` | -| `suppressed` | boolean | `tx.receive` | | `proposal_trusted` | boolean | `peer.proposal.receive` | | `validation_trusted` | boolean | `peer.validation.receive` | | `tx_type` | string | `tx.*`, `txq.enqueue` | diff --git a/OpenTelemetryPlan/Phase3_taskList.md b/OpenTelemetryPlan/Phase3_taskList.md index 3b73809b98..96d54870a5 100644 --- a/OpenTelemetryPlan/Phase3_taskList.md +++ b/OpenTelemetryPlan/Phase3_taskList.md @@ -98,7 +98,7 @@ - Extract parent trace context from incoming `TMTransaction::trace_context` field (if present) - Create `tx.receive` span as child of extracted context (or new root if none) - Set attributes: `tx_hash`, `peer_id`, `tx_status` - - On HashRouter suppression (duplicate): set `suppressed=true`, add `tx.duplicate` event + - Create the span only after `HashRouter::shouldProcess()` accepts, so a dropped duplicate produces no span - Wrap validation call with child span `tx.validate` - Wrap relay with `tx.relay` span - When relaying to peers: @@ -169,7 +169,7 @@ - Edit `src/xrpld/overlay/detail/PeerImp.cpp` (in handleTransaction): - After calling `HashRouter::shouldProcess()` or `addSuppressionPeer()`: - - Record `suppressed` attribute (true/false) + - Start the span here, not before, so only transactions this node will process are traced - Record `tx_flags` showing current HashRouter state (SAVED, TRUSTED, etc.) - Add `tx.first_seen` or `tx.duplicate` event @@ -386,7 +386,7 @@ This gives the best of both worlds: guaranteed cross-node correlation via determ **combine** with deterministic trace_id — use the protobuf span_id as parent to preserve relay ordering, but override trace_id with the deterministic one. 4. If no protobuf context: create span under `detCtx` directly. - 5. Set all existing attributes (`hash`, `peerId`, `peerVersion`, `suppressed`, etc.). + 5. Set all existing attributes (`hash`, `peerId`, `peerVersion`, etc.). - **Combining deterministic trace_id with protobuf parent span_id**: When both are available, construct a synthetic `SpanContext` with: