diff --git a/OpenTelemetryPlan/02-design-decisions.md b/OpenTelemetryPlan/02-design-decisions.md index 4b9dcf993e..ff71e44a80 100644 --- a/OpenTelemetryPlan/02-design-decisions.md +++ b/OpenTelemetryPlan/02-design-decisions.md @@ -123,44 +123,56 @@ path in Phase 1b through Phase 5. ### 2.3.2 Complete Span Catalog -| Span name | Description | -| ------------------------------ | --------------------------------------- | -| `tx.receive` | Transaction received from network | -| `tx.validate` | Transaction signature/format validation | -| `tx.process` | Full transaction processing | -| `tx.relay` | Transaction relay to peers | -| `tx.apply` | Apply transaction to ledger | -| `consensus.round` | Complete consensus round | -| `consensus.phase.open` | Open phase - collecting transactions | -| `consensus.phase.establish` | Establish phase - reaching agreement | -| `consensus.phase.accept` | Accept phase - applying consensus | -| `consensus.proposal.receive` | Receive peer proposal | -| `consensus.proposal.send` | Send our proposal | -| `consensus.validation.receive` | Receive peer validation | -| `consensus.validation.send` | Send our validation | -| `rpc.request` | HTTP/WebSocket request handling | -| `rpc.command.*` | Specific RPC command (dynamic) | -| `peer.connect` | Peer connection establishment | -| `peer.disconnect` | Peer disconnection | -| `peer.message.send` | Send protocol message | -| `peer.message.receive` | Receive protocol message | -| `ledger.acquire` | Ledger acquisition from network | -| `ledger.build` | Build new ledger | -| `ledger.validate` | Ledger validation | -| `ledger.close` | Close ledger | -| `ledger.replay` | Ledger replay executed | -| `ledger.delta` | Delta-based ledger acquired | -| `pathfind.request` | Path request initiated | -| `pathfind.compute` | Path computation executed | -| `txq.enqueue` | Transaction queued | -| `txq.apply` | Queued transaction applied | -| `fee.escalate` | Fee escalation triggered | -| `validator.list.fetch` | UNL list fetched | -| `validator.manifest` | Manifest update processed | -| `amendment.vote` | Amendment voting executed | -| `shamap.sync` | State tree synchronization | -| `job.enqueue` | Job added to queue | -| `job.execute` | Job execution | +> **Status column.** This catalog is the design inventory; it is not a +> statement of what currently emits. `Live` means the span is present in the +> implemented inventory ([09-data-collection-reference.md §1.1](./09-data-collection-reference.md#11-complete-span-inventory-37-spans)), +> which is the authoritative list. `Renamed`/`Split` means the concept shipped +> under a different name than planned here. **Not built** means no span is +> emitted for it today. +> +> The four `peer.*` entries are the peer-span coverage gap: only +> `peer.proposal.receive` and `peer.validation.receive` exist, so protocol +> message send/receive and connection lifecycle are untraced. See +> [09 §6.4](./09-data-collection-reference.md#64-peer-span-coverage-gap-not-implemented). + +| Span name | Description | Status | +| ------------------------------ | --------------------------------------- | ------------------------------------------------ | +| `tx.receive` | Transaction received from network | Live | +| `tx.validate` | Transaction signature/format validation | **Not built** | +| `tx.process` | Full transaction processing | Live | +| `tx.relay` | Transaction relay to peers | **Not built** | +| `tx.apply` | Apply transaction to ledger | Live | +| `consensus.round` | Complete consensus round | Live | +| `consensus.phase.open` | Open phase - collecting transactions | Live | +| `consensus.phase.establish` | Establish phase - reaching agreement | Renamed `consensus.establish` | +| `consensus.phase.accept` | Accept phase - applying consensus | Renamed `consensus.accept` | +| `consensus.proposal.receive` | Receive peer proposal | Live | +| `consensus.proposal.send` | Send our proposal | Live | +| `consensus.validation.receive` | Receive peer validation | Live | +| `consensus.validation.send` | Send our validation | Live | +| `rpc.request` | HTTP/WebSocket request handling | Split into `rpc.http_request` / `rpc.ws_message` | +| `rpc.command.*` | Specific RPC command (dynamic) | Live | +| `peer.connect` | Peer connection establishment | **Not built** | +| `peer.disconnect` | Peer disconnection | **Not built** | +| `peer.message.send` | Send protocol message | **Not built** | +| `peer.message.receive` | Receive protocol message | **Not built** | +| `ledger.acquire` | Ledger acquisition from network | Live | +| `ledger.build` | Build new ledger | Live | +| `ledger.validate` | Ledger validation | Live | +| `ledger.close` | Close ledger | Renamed `consensus.ledger_close` | +| `ledger.replay` | Ledger replay executed | **Not built** | +| `ledger.delta` | Delta-based ledger acquired | **Not built** | +| `pathfind.request` | Path request initiated | Live | +| `pathfind.compute` | Path computation executed | Live | +| `txq.enqueue` | Transaction queued | Live | +| `txq.apply` | Queued transaction applied | Renamed `txq.apply_direct` / `txq.accept_tx` | +| `fee.escalate` | Fee escalation triggered | **Not built** | +| `validator.list.fetch` | UNL list fetched | **Not built** | +| `validator.manifest` | Manifest update processed | **Not built** | +| `amendment.vote` | Amendment voting executed | **Not built** | +| `shamap.sync` | State tree synchronization | **Not built** | +| `job.enqueue` | Job added to queue | **Not built** | +| `job.execute` | Job execution | **Not built** | ### 2.3.3 Attribute Naming Conventions diff --git a/OpenTelemetryPlan/09-data-collection-reference.md b/OpenTelemetryPlan/09-data-collection-reference.md index 4c1d5c0120..8e2f51c172 100644 --- a/OpenTelemetryPlan/09-data-collection-reference.md +++ b/OpenTelemetryPlan/09-data-collection-reference.md @@ -1707,13 +1707,158 @@ counters), observed from an existing cumulative source each collection cycle: ## 6. Known Issues -| Issue | Impact | Status | -| ------------------------------------------------------------------ | ------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------- | -| `warn` and `drop` metrics use non-standard StatsD `\|m` meter type | Metrics silently dropped by OTel StatsD receiver | Phase 6 Task 6.1 — needs `\|m` → `\|c` change in StatsDCollector.cpp | -| `jobq_job_count` may not emit in standalone mode | Missing from Prometheus in some test configs | Requires active job queue activity | -| `rpc_requests` depends on `[insight]` config | Zero series if StatsD not configured | Requires `[insight] server=statsd` in xrpld.cfg | -| Peer tracing enabled by default | `peer.*` spans emit unless `trace_peer=0` | High volume — set `trace_peer=0` to opt out on busy mainnet nodes | -| `handler="other"` mixes several producers | Cannot separate `GetConsL1` from `GetConsL2` | By design — the cardinality bound; see [§Per-Job-Type Metrics](#per-job-type-metrics-synchronous-countershistogram) | +| Issue | Impact | Status | +| --------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------- | +| `warn` and `drop` metrics use non-standard StatsD `\|m` meter type | Metrics silently dropped by OTel StatsD receiver | Phase 6 Task 6.1 — needs `\|m` → `\|c` change in StatsDCollector.cpp | +| `jobq_job_count` may not emit in standalone mode | Missing from Prometheus in some test configs | Requires active job queue activity | +| `rpc_requests` depends on `[insight]` config | Zero series if StatsD not configured | Requires `[insight] server=statsd` in xrpld.cfg | +| Peer tracing enabled by default | `peer.*` spans emit unless `trace_peer=0` | High volume — set `trace_peer=0` to opt out on busy mainnet nodes | +| `handler="other"` mixes several producers | Cannot separate `GetConsL1` from `GetConsL2` | By design — the cardinality bound; see [§Per-Job-Type Metrics](#per-job-type-metrics-synchronous-countershistogram) | +| `overhead_cluster_*` is always zero | 8 dashboard panel references are flatlines by construction; cluster traffic is counted as `unknown` | **NOT IMPLEMENTED** — see [§6.0](#60-mtcluster-is-counted-as-unknown-not-implemented) | +| `squelch_ignored_bytes_in/out` always read zero | Only the `_messages_*` pair carries signal for this category | **NOT IMPLEMENTED** — see [§6.1](#61-squelch_ignored-byte-counts-not-implemented) | +| `total_bytes_in` and `total_bytes_out` use different size bases | In/out byte totals are not directly comparable when compression is on | **NOT IMPLEMENTED** — see [§6.2](#62-inboundoutbound-byte-basis-asymmetry-not-implemented) | +| `overhead` conflates `mtPING` with `mtSTATUS_CHANGE` | Keepalive traffic cannot be isolated from status-change traffic | **NOT IMPLEMENTED** — needs a new category; see [§6.3](#63-peer-keepalive-and-discovery-traffic-gaps-not-implemented) | +| No metrics for ping RTT distribution, ping timeouts, or `mtENDPOINTS` | Peer keepalive and discovery health are not observable | **NOT IMPLEMENTED** — see [§6.3](#63-peer-keepalive-and-discovery-traffic-gaps-not-implemented) | +| 11 of 13 peer message families have no spans | `02` §2.3.2 catalogs `peer.message.*`, `peer.connect`, `peer.disconnect` that were never built | **NOT IMPLEMENTED** — see [§6.4](#64-peer-span-coverage-gap-not-implemented) | +| PeerFinder exports 2 of ~17 available slot/cache readings | Slot pressure, connection churn and discovery-cache health are not observable | **NOT IMPLEMENTED** — see [§6.5](#65-peerfinder-slot-and-cache-metrics-not-implemented) | + +### 6.0 `mtCLUSTER` is counted as `unknown`: NOT IMPLEMENTED + +`mtCLUSTER` is absent from `kTypeLookup` +(`src/xrpld/overlay/detail/TrafficCount.cpp:11-27`), and `categorize()`'s +fallback chain only inspects `TMLedgerData`, `TMGetLedger` and +`TMGetObjectByHash` before returning `Category::Unknown` (`:135`). No call site +ever passes `Category::Cluster`. Cluster traffic is therefore counted as +`unknown`, and `overhead_cluster_bytes_in/out` and +`overhead_cluster_messages_in/out` are always zero — including the 8 panel +references across `network-traffic` and `overlay-traffic-detail` (both the local +and grafanacloud copies). + +This also degrades `unknown_*` as an anomaly signal: on a clustered node it mixes +genuinely unrecognized wire types with routine `mtCLUSTER` traffic. + +**Status**: Planned, not yet implemented. The fix is a one-line addition to +`kTypeLookup`, but `TrafficCount.cpp` is shared overlay code rather than a +telemetry-owned file, so it is scoped as a separate overlay change. Note that +landing it moves volume out of `unknown_bytes_in`, so any threshold measured +against that series needs re-baselining. Until then, treat `overhead_cluster_*` +as "no data" rather than "no cluster traffic", and read the +[Cluster](../docs/telemetry-glossary.md#cluster) glossary entry's guidance on +sustained cluster overhead as not yet observable. + +### 6.1 `squelch_ignored` byte counts: NOT IMPLEMENTED + +`OverlayImpl::updateSlotAndSquelch` reports the `SquelchIgnored` category with a +hardcoded size of `0` (`src/xrpld/overlay/detail/OverlayImpl.cpp:1460` and +`:1489`), so `squelch_ignored_bytes_in` and `squelch_ignored_bytes_out` are +always zero. Only `squelch_ignored_messages_in/out` carry signal. This is +inconsistent with `SquelchSuppressed`, which passes the real wire size +(`src/xrpld/overlay/detail/PeerImp.cpp:302`) — so the two squelch categories are +not comparable on bytes. + +The message size is available at all four call sites (each holds the protobuf +message and could call `Message::messageSize()`), but plumbing it through would +require widening the two `OverlayImpl::updateSlotAndSquelch` overloads. + +**Status**: Deferred as a separate change — a public signature change on +`OverlayImpl` is out of scope for the telemetry chain, since `OverlayImpl.h` is +shared overlay code rather than a telemetry-owned file. Until it lands, read +`squelch_ignored` on the `_messages_*` series only and do not build a +bytes-per-message ratio from this category. + +### 6.2 Inbound/outbound byte-basis asymmetry: NOT IMPLEMENTED + +Inbound traffic is counted with the raw wire size as received +(`src/xrpld/overlay/detail/PeerImp.cpp:1079`), while outbound traffic is counted +from the possibly-compressed send buffer +(`getBuffer(compressionEnabled_).size()`, `PeerImp.cpp:313`). When compression is +enabled the two directions measure different things, so `total_bytes_in` versus +`total_bytes_out` is not a like-for-like comparison, and neither is any +`{category}_bytes_in` / `_bytes_out` pair. + +A related documentation defect sits in the same class: the `TrafficCount` header +comment states that "messages whose category is not in `TrafficCount::categorize` +are not included in the total" (`src/xrpld/overlay/detail/TrafficCount.h:28-31`), +but `Category::Total` is incremented unconditionally at +`src/xrpld/overlay/detail/PeerImp.cpp:1079`, _before_ the per-category split. The +total does include uncategorized traffic; the comment is stale. + +**Status**: Planned, not yet implemented — neither the metric change nor the +header-comment correction has landed, because `TrafficCount.h` is shared overlay +code rather than a telemetry-owned file. Normalizing one direction would in any +case silently redefine an existing series, so the likely resolution is to document +the asymmetry at the class and leave both readings intact. Until then, compare +`_bytes_in` against `_bytes_out` only when compression is known to be off. + +### 6.3 Peer keepalive and discovery traffic gaps: NOT IMPLEMENTED + +Three related gaps on the peer keepalive and discovery paths. All are byte/message +counters only — none has a dedicated instrument, and none is traced. + +| Gap | Current state | What is missing | +| --------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------- | +| `mtPING` / `mtSTATUS_CHANGE` share `Category::Base` (`TrafficCount.cpp:12-13`) → `overhead_*` | Both message types land in one bucket | A distinct category for one of them, plus name-map, `counts_` and dashboard entries | +| Ping RTT | An 8-sample EWMA per peer (`PeerImp.cpp:1150-1163`), exported only as the single `peer_quality{metric="peer_latency_p90_ms"}` gauge | A histogram; the lone p90 hides a bimodal peer set | +| Ping failures | `fail("Ping Timeout")` (`PeerImp.cpp:762`) logs only; a wrong-cookie PONG (`PeerImp.cpp:1146`) is silently ignored | A counter for each | +| `mtENDPOINTS` | `overhead_overlay_*` bytes only | Counters for endpoints received / handed out / malformed (`PeerImp.cpp:1265-1270` charges a fee but records no metric) | + +**Status**: Planned, not yet implemented. Adding these means a new metric family +plus matching rows in this document, in +[docs/telemetry-runbook.md](../docs/telemetry-runbook.md) § Metric Reference, and +in `docker/telemetry/workload/expected_metrics.json` (Phase 10 branch — see the +Cross-Phase Dependency Chain in +[06-implementation-phases.md](./06-implementation-phases.md)), and dashboard +panels following the conventions in `06` § Branch-to-Change Mapping. + +### 6.4 Peer span coverage gap: NOT IMPLEMENTED + +[02-design-decisions.md §2.3.2](./02-design-decisions.md#232-complete-span-catalog) +catalogs `peer.connect`, `peer.disconnect`, `peer.message.send` and +`peer.message.receive`. None was ever built: the implemented peer surface is the +two spans in [§Peer Spans](#peer-spans) above (`peer.proposal.receive`, +`peer.validation.receive`). Of the 13 protocol message families, only +`mtGET_OBJECTS` has native instrumentation, and only transactions and consensus +messages are traced. + +**Status**: NOT IMPLEMENTED. The span catalog in `02` §2.3.2 is a design +inventory, not a statement of what emits; §2.3.2 now marks which entries are +live. Instrumenting the remaining families would change the "~37 spans" count +asserted in [§1.1](#11-complete-span-inventory-37-spans) and in +`docker/telemetry/workload/expected_spans.json`, so it is scoped as its own +change rather than folded into a metric task. + +### 6.5 PeerFinder slot and cache metrics: NOT IMPLEMENTED + +`peer_finder::Manager` registers exactly two instruments — +`peer_finder_active_inbound_peers` and `peer_finder_active_outbound_peers` +(`src/libxrpl/peerfinder/PeerfinderManager.cpp:229-230`), listed in +[§2.1](#21-gauges). The `Counts` class exposes roughly fifteen further readings +that are never exported (`include/xrpl/peerfinder/detail/Counts.h`), and neither +discovery cache has any instrument at all. + +| Reading | Source | Why it matters | +| --------------------------------------------------- | ---------------------- | ---------------------------------------------------------------------------------------------------------------------- | +| `attempts()`, `attemptsNeeded()` | `Counts.h:68,79` | Outbound connection churn; distinguishes "not trying" from "trying and failing" | +| `outMax()`, `outActive()`, `outboundSlotsFree()` | `Counts.h:88,98,205` | Outbound slot saturation | +| `inMax()`, `inboundActive()`, `inboundSlotsFree()` | `Counts.h:165,174,193` | Inbound slot saturation — the two exported gauges give the actives but not the caps, so utilization cannot be computed | +| `acceptCount()`, `connectCount()`, `closingCount()` | `Counts.h:138,147,156` | Handshake pipeline depth; `closingCount()` rising is teardown backpressure | +| `fixed()`, `fixedActive()` | `Counts.h:107,116` | Whether configured fixed peers are actually connected | +| `isConnectedToNetwork()` | `Counts.h:218` | Binary reachability | +| `Livecache::size()` | `Livecache.h:365` | Size of the live endpoint pool used to answer `mtENDPOINTS` | +| `Bootcache::size()` | `Bootcache.h:121` | Bootstrap-address pool; an empty bootcache is why a fresh node cannot find peers | + +The two exported actives are also the only inputs to the "Inbound vs Outbound" +panel specified for the Peer Quality dashboard +([06 § Branch-to-Change Mapping, Task 9.12](./06-implementation-phases.md)), so +that panel cannot show slot utilization as a percentage. + +**Status**: Planned, not yet implemented. These would extend the existing +`beast::insight` registration in `PeerfinderManager.cpp` (arrow **B** in the +[Data Flow Overview](#data-flow-overview)) rather than use the `XRPL_METRIC_*` +macros, because `libxrpl` code cannot use those macros — see the pipeline note in +[§2.5](#25-per-job-type-queue-gauges). `Livecache`/`Bootcache` currently receive +no collector reference, so exporting their sizes needs one plumbed in or the +values read via the existing `Manager` hook. --- diff --git a/OpenTelemetryPlan/Phase9_taskList.md b/OpenTelemetryPlan/Phase9_taskList.md index 818530ef93..5c67ec1097 100644 --- a/OpenTelemetryPlan/Phase9_taskList.md +++ b/OpenTelemetryPlan/Phase9_taskList.md @@ -439,6 +439,137 @@ These metrics serve multiple external consumer categories identified during rese --- +## Task 9.14: Overlay Traffic Accounting Defects (Documentation Only) + +> **Status**: DOCUMENTED, NOT FIXED. Reference: [09 §6.0-§6.2](./09-data-collection-reference.md#6-known-issues) + +**Objective**: Record four pre-existing overlay traffic-accounting defects so +dashboard readers are not misled. All four originate in `develop`-owned overlay +files, so **no code fix lands on this branch**. + +| # | Defect | Effect | Fix location (NOT this branch) | +| --- | -------------------------------------- | ----------------------------------------------------------------------------------------- | ------------------------------------------------ | +| 1 | `mtCLUSTER` missing from `kTypeLookup` | `overhead_cluster_*` always zero; 8 panels flatline; cluster traffic counted as `unknown` | `TrafficCount.cpp:11-27` | +| 2 | Stale `Total` header comment | Claims uncategorized traffic is excluded; it is included | `TrafficCount.h:28-31` | +| 3 | `SquelchIgnored` reported with size 0 | `squelch_ignored_bytes_*` always zero, inconsistent with `SquelchSuppressed` | `OverlayImpl.cpp:1460,1489` (+ signature change) | +| 4 | In/out byte-basis asymmetry | `_bytes_in` vs `_bytes_out` not comparable under compression | `PeerImp.cpp:1079` vs `:313` | + +**Why deferred**: Defect 3 requires widening the two +`OverlayImpl::updateSlotAndSquelch` overloads — a public signature change on +shared overlay code. Defects 1, 2 and 4 sit in `TrafficCount.{h,cpp}`, likewise +not telemetry-owned. Routing them through the telemetry chain would hide overlay +changes from overlay reviewers and couple them to a 12-PR merge timeline. + +**Key modified files**: `OpenTelemetryPlan/09-data-collection-reference.md` only. + +**Exit Criteria**: + +- [x] Each defect documented with file:line evidence in `09` §6 +- [x] `overhead_cluster_*` documented as "no data", not "no cluster traffic" +- [ ] Follow-up overlay-owned branch raised for the four code fixes +- [ ] Re-baseline any threshold keyed on `unknown_bytes_in` when defect 1 lands + +--- + +## Task 9.15: Peer Keepalive and Discovery Instrumentation + +> **Status**: NOT IMPLEMENTED — awaiting a decision on whether `XRPL_METRIC_*` +> call sites may be added to `src/xrpld/overlay/detail/PeerImp.cpp` from this +> branch. Reference: [09 §6.3](./09-data-collection-reference.md#63-peer-keepalive-and-discovery-traffic-gaps-not-implemented) + +**Objective**: Make peer keepalive and peer-discovery health observable. Today +`mtPING`, `mtSTATUS_CHANGE` and `mtENDPOINTS` are byte counters only. + +| Proposed metric | Type | Labels | Record site | +| ------------------------------- | --------- | -------------------------------- | --------------------------------------------------- | +| `peer_ping_rtt_ms` | Histogram | none (see note) | `PeerImp.cpp:1150-1163`, where the EWMA is computed | +| `peer_ping_timeouts_total` | Counter | `reason="timeout"\|"bad_cookie"` | `PeerImp.cpp:762` and `:1146` | +| `peer_endpoints_received_total` | Counter | `result="accepted"\|"malformed"` | `PeerImp.cpp:1265-1270` | + +**Design notes / open questions**: + +- A histogram needs an explicit bucket view: the SDK default tops out at 10000, + and these are milliseconds. Follow the µs-ladder precedent in + `MetricsRegistry.cpp` (see [09 § GetObject Request Path](./09-data-collection-reference.md#getobject-request-path-synchronous-countershistograms)). +- `peer_id` as a label is unbounded cardinality — rejected. A bounded + `peer_role`-style label is the alternative if per-peer attribution is needed. +- Splitting `mtPING` out of `Category::Base` is a `TrafficCount.cpp` change and + therefore blocked with Task 9.14. +- Per the runbook's "Adding a New Metric" contract, `_total` is reserved for + monotonic counters; a histogram takes no suffix. + +**Key files (if approved)**: `src/xrpld/overlay/detail/PeerImp.cpp`, +`09-data-collection-reference.md`, `docs/telemetry-runbook.md` § Metric Reference, +`docker/telemetry/grafana/dashboards/peer-quality.json`, and +`docker/telemetry/workload/expected_metrics.json` (**Phase 10 branch**). + +**Exit Criteria**: + +- [ ] Decision recorded on editing `PeerImp.cpp` from the telemetry chain +- [ ] Three instruments emitting, with an explicit histogram bucket view +- [ ] Rows added to `09` §5b, runbook § Metric Reference, and `expected_metrics.json` +- [ ] Peer Quality dashboard panels follow the Task 9.12 conventions (`$node`, Title Case, legend dimensions) +- [ ] `check_otel_naming.py` passes (Rules D and E cover the new labels) + +--- + +## Task 9.16: PeerFinder Slot and Cache Metrics + +> **Status**: NOT IMPLEMENTED. Reference: [09 §6.5](./09-data-collection-reference.md#65-peerfinder-slot-and-cache-metrics-not-implemented) + +**Objective**: Export the PeerFinder slot counts and discovery-cache sizes. +Only 2 of ~17 available readings are exported today. + +**What to do**: Extend the existing `Stats` struct in +`src/libxrpl/peerfinder/PeerfinderManager.cpp:227-236` with gauges for the +`Counts` accessors listed in [09 §6.5](./09-data-collection-reference.md#65-peerfinder-slot-and-cache-metrics-not-implemented) +(slot caps and frees, attempt counts, handshake pipeline depth, fixed-peer state, +network reachability), plus `Livecache::size()` and `Bootcache::size()`. + +**Pipeline constraint**: `PeerfinderManager.cpp` is in `libxrpl`, which **cannot** +use the `XRPL_METRIC_*` macros. These must go through `beast::insight` — +arrow **B**, not **C**. Naming follows `GroupImp::makeName()` + +`OTelCollectorImp::formatName()`, so the `"Peer_Finder"` group yields +`peer_finder_` lowercased. + +**Known obstacle**: `Livecache` and `Bootcache` hold no collector reference, so +their sizes must either be read through the existing `Manager` hook or have a +collector plumbed in. + +**Exit Criteria**: + +- [ ] Slot caps exported so utilization (`active / max`) is computable +- [ ] Both cache sizes exported +- [ ] "Inbound vs Outbound" panel on `peer-quality` extended to show utilization % +- [ ] Rows added to `09` §2.1, runbook § Metric Reference, `expected_metrics.json` (Phase 10) + +--- + +## Task 9.17: Peer Span Coverage (Deferred to Phase 11) + +> **Status**: NOT IMPLEMENTED — design only, pending approval. Reference: +> [09 §6.4](./09-data-collection-reference.md#64-peer-span-coverage-gap-not-implemented) +> and [02 §2.3.2](./02-design-decisions.md#232-complete-span-catalog) + +**Objective**: Close the gap between the `02` §2.3.2 span catalog and what +actually emits. `peer.connect`, `peer.disconnect`, `peer.message.send` and +`peer.message.receive` were catalogued from the start and never built; 11 of 13 +protocol message families have no spans. + +**Scope warning**: This is larger than Tasks 9.14-9.16 combined and changes the +"~37 spans" figure asserted in `09` §1.1 and in +`docker/telemetry/workload/expected_spans.json`. `trace_peer` is also **on by +default** and already flagged as high-volume, so adding per-message spans has a +volume cost that needs measuring before commitment. + +**Exit Criteria**: + +- [x] `02` §2.3.2 marked Live / Not built / Renamed against the real inventory +- [ ] User approval to proceed with span implementation +- [ ] Volume impact measured under `trace_peer=1` before any span is added + +--- + ## Exit Criteria - [ ] All ~50 new metrics visible in Prometheus via OTLP pipeline diff --git a/docker/telemetry/grafana/dashboards/log-derived-insights.json b/docker/telemetry/grafana/dashboards/log-derived-insights.json index dc2d967c1d..6acda66c69 100644 --- a/docker/telemetry/grafana/dashboards/log-derived-insights.json +++ b/docker/telemetry/grafana/dashboards/log-derived-insights.json @@ -1858,7 +1858,7 @@ { "type": "timeseries", "title": "Peer Disconnect Rate By Reason", - "description": "###### What this is:\n*Rate of peer connection endings, split by the reason recorded in the log.*\n\n###### How it's computed:\n*Per-second count of Peer log lines matching Timeout, Closed, or a refused connection attempt.*\n\n###### Reading it:\n*Distinguishes clean teardown from failure. Closed is a normal ending; Timeout and Connection refused are not.*\n\n###### Healthy range:\n*Closed dominant with a low, steady background of the others.*\n\n###### Watch for:\n*A Timeout rate approaching the Closed rate, which points at network trouble or unresponsive peers.*\n\n###### Keywords:\n- **Closed** *(per peer)* — a clean connection teardown; the normal ending.\n- **Timeout** *(per peer)* — the peer stopped responding.\n- **Connection refused** *(per attempt)* — an outbound attempt the remote rejected.\n\n###### Computation boundary:\n*Result: Per node per outcome — a count of peer lifecycle events.*\n*Derived in the Grafana query. Note `overlay_peer_disconnects` exists as a metric but carries no reason breakdown, which is what this panel adds.*\n\n###### Source:\n[PeerImp.cpp](https://github.com/XRPLF/rippled/blob/develop/src/xrpld/overlay/detail/PeerImp.cpp)\n\n###### Function:\n`PeerImp::close / onTimer`\n\n###### References:\n[Peer protocol](https://xrpl.org/docs/concepts/networks-and-servers/peer-protocol)", + "description": "###### What this is:\n*Rate of peer connection endings, split by the reason recorded in the log.*\n\n###### How it's computed:\n*Per-second count of Peer log lines matching a clean close, a ping-keepalive timeout, a connect timeout, or a refused connection attempt.*\n\n###### Reading it:\n*Distinguishes clean teardown from failure. Closed is a normal ending; Timeout and Connection refused are not.*\n\n###### Healthy range:\n*Closed dominant with a low, steady background of the others.*\n\n###### Watch for:\n*A Ping Timeout rate approaching the Closed rate, which points at network trouble or unresponsive peers.*\n\n###### Keywords:\n- **Closed** *(per peer)* — a clean connection teardown; the normal ending.\n- **Connect Timeout** *(per attempt)* — an outbound connect attempt timed out.\n- **Ping Timeout** *(per peer)* — an established peer missed its keepalive PONG.\n- **Connection refused** *(per attempt)* — an outbound attempt the remote rejected.\n\n###### Computation boundary:\n*Result: Per node per outcome — a count of peer lifecycle events.*\n*Derived in the Grafana query. Note `overlay_peer_disconnects` exists as a metric but carries no reason breakdown, which is what this panel adds.*\n\n###### Source:\n[PeerImp.cpp](https://github.com/XRPLF/rippled/blob/develop/src/xrpld/overlay/detail/PeerImp.cpp)\n\n###### Function:\n`PeerImp::close / onTimer`\n\n###### References:\n[Peer protocol](https://xrpl.org/docs/concepts/networks-and-servers/peer-protocol)", "gridPos": { "h": 10, "w": 12, @@ -1906,7 +1906,7 @@ "uid": "${DS_LOKI}" }, "refId": "A", - "expr": "sum by (outcome, service_instance_id) (rate({service_name=~\"$service_name\", service_instance_id=~\"$node\", deployment_environment=~\"$deployment_environment\"} | xrpl_network_type =~ \"$xrpl_network_type\" | partition = `Peer` | severity =~ \"$severity\" | regexp `\\] (?PTimeout|Closed|onConnect: Connection refused)` | outcome != `` | label_format outcome=`{{if eq .outcome \"Closed\"}}Closed{{else if eq .outcome \"Timeout\"}}Timeout{{else if eq .outcome \"onConnect: Connection refused\"}}Connection Refused{{else}}{{.outcome}}{{end}}` [$__auto]))" + "expr": "sum by (outcome, service_instance_id) (rate({service_name=~\"$service_name\", service_instance_id=~\"$node\", deployment_environment=~\"$deployment_environment\"} | xrpl_network_type =~ \"$xrpl_network_type\" | partition = `Peer` | severity =~ \"$severity\" | regexp `(?:\\] |failed: |close: )(?PPing Timeout|Timeout|Closed|onConnect: Connection refused)` | outcome != `` | label_format outcome=`{{if eq .outcome \"Closed\"}}Closed{{else if eq .outcome \"Ping Timeout\"}}Ping Timeout{{else if eq .outcome \"Timeout\"}}Connect Timeout{{else if eq .outcome \"onConnect: Connection refused\"}}Connection Refused{{else}}{{.outcome}}{{end}}` [$__auto]))" } ], "id": 29, diff --git a/docs/telemetry-glossary.md b/docs/telemetry-glossary.md index b3df34ccdc..b5a5a64559 100644 --- a/docs/telemetry-glossary.md +++ b/docs/telemetry-glossary.md @@ -9,7 +9,12 @@ documentation. > **Related docs**: > [docs/telemetry-runbook.md](./telemetry-runbook.md) (operator runbook). - + ## Contents @@ -1016,7 +1021,9 @@ A cluster is a set of servers run by the same operator that trust each other, ex **Scope:** cluster-wide — shared across a co-operated cluster of nodes run by one operator. -**See also:** [Cluster on xrpl.org](https://xrpl.org/docs/concepts/networks-and-servers/clustering) +**What is observable:** cluster overhead is **not** measurable today. Cluster messages are counted under `unknown` rather than `overhead_cluster`, so the `overhead_cluster_*` series read zero on a clustered node — treat them as "no data", not "no cluster traffic". The churn guidance above cannot yet be acted on. + +**See also:** [Cluster on xrpl.org](https://xrpl.org/docs/concepts/networks-and-servers/clustering) · [Data collection reference §6.0](../OpenTelemetryPlan/09-data-collection-reference.md#60-mtcluster-is-counted-as-unknown-not-implemented) @@ -1086,6 +1093,18 @@ The overlay is xrpld's peer-to-peer messaging layer connecting nodes. All inter- **See also:** [Overlay on xrpl.org](https://xrpl.org/docs/concepts/networks-and-servers/peer-protocol) + + +### Ping / pong keepalive + +Each peer connection is probed on a timer: the node sends a ping carrying a random cookie and expects a pong echoing it back. The round-trip is smoothed into a per-peer latency estimate that feeds peer scoring, and a peer that leaves a ping unanswered before the next probe is dropped as a ping timeout. A pong bearing the wrong cookie is ignored, so a peer answering incorrectly eventually times out too. Ping timeouts are distinct from connect timeouts, which happen while an outbound connection is still being established and so involve no established peer. + +**Scope:** per node — measured on and specific to this individual server. + +**What is observable:** only the p90 of the smoothed per-peer latency (`peer_quality{metric="peer_latency_p90_ms"}`) — there is no distribution, ping timeouts and wrong-cookie pongs have no counter, and ping bytes are not separable from status-change bytes because both share the `overhead` traffic category. + +**See also:** [Data collection reference §6.3](../OpenTelemetryPlan/09-data-collection-reference.md#63-peer-keepalive-and-discovery-traffic-gaps-not-implemented) + ### Proof path @@ -1162,6 +1181,10 @@ Squelching is a relay-control mechanism: a node tells peers to stop sending it a **Scope:** per node — measured on and specific to this individual server. +**What is observable:** read ignored directives on `squelch_ignored_messages_in/out` only. The paired `squelch_ignored_bytes_*` series are always zero because the ignored-squelch callback records no size, so bandwidth wasted by peers ignoring squelch cannot be quantified — and `squelch_ignored` is therefore not comparable on bytes against `squelch_suppressed`, which does record real sizes. + +**See also:** [Data collection reference §6.1](../OpenTelemetryPlan/09-data-collection-reference.md#61-squelch_ignored-byte-counts-not-implemented) + ### Trusted / untrusted / duplicate diff --git a/docs/telemetry-runbook.md b/docs/telemetry-runbook.md index 1a3019f6c9..64376ff7eb 100644 --- a/docs/telemetry-runbook.md +++ b/docs/telemetry-runbook.md @@ -2507,7 +2507,7 @@ timeseries, 2 table, 1 state-timeline, 1 logs, 1 text, across 35 queries. | Manifests — Disposition & Producers | `[DBG]` | Disposition rate; accept-vs-reject; top-N master keys | | Resource Fee Charges — Load Attribution | `[DBG]` | Charge rate by reason; fee-weighted load; top-N peers by IP and public key | | Ledger Acquisition Efficiency | `[DBG]` | Duplicate ratio; good vs duplicate vs timeout | -| Peer Lifecycle & Disconnects | `[DBG]` | Disconnect reason breakdown; handshake and accept rate | +| Peer Lifecycle & Disconnects | `[DBG]` | Disconnect reason breakdown (Closed / Ping Timeout / Connect Timeout / Connection Refused); handshake and accept rate | | Consensus Phase & Mode | `[DEFAULT OK]` | Phase transitions; operating-mode proxy; quorum and trusted-set size | | Slow Job Latency Breaches | `[DEFAULT OK]` | Run p99, wait p99, breach rate by job (`LoadMonitor`, >500ms only) | | Error & Warning Stream | `[DEFAULT OK]` | WRN/ERR/FTL rate by partition; live log tail | @@ -2577,6 +2577,20 @@ Stream labels are only `service_name`, `service_instance_id`, target as a range query even with `instant: true`, so `lastNotNull` reads only the final bucket — a window total shows as a single-bucket count. Aggregate over `$__range` and reduce with `max`. +11. **A `regexp` anchored on the log prefix silently drops most matches.** The + _Peer Disconnect Rate By Reason_ panel anchored its capture on `\] `, which + only matches a reason emitted immediately after the `[NNN] ` peer-id prefix. + `PeerImp` does not log that way: `PeerImp::fail` emits + `[NNN] failed: ` (`src/xrpld/overlay/detail/PeerImp.cpp:645`) + and the clean teardown emits `close: Closed` (`:635`). Only + `ConnectAttempt::fail`, which logs the bare reason + (`src/xrpld/overlay/detail/ConnectAttempt.cpp:136`), ever matched — so the + panel's `Timeout` series was connect-attempt timeouts only, `Ping Timeout` + (`PeerImp.cpp:762`) was invisible, and `PeerImp`'s own `Closed` was + uncounted. The panel now matches all three prefixes + (`(?:\] |failed: |close: )`) and distinguishes `Ping Timeout` from + `Connect Timeout`. When adding a log-derived panel, enumerate every producer + of the string being captured rather than sampling one. Also worth knowing: the **Grafana Cloud image renderer cannot query Loki** in this stack. A minimal probe dashboard with a hardcoded datasource uid, a literal