ARCHIVE/rippled
1
0 Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-10-22 03:27:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'pratik/otel-sync-diagnostics' into pratik/otel-sync-diagnostics-freshen-fix

Browse Source
This commit is contained in:
Pratik Mankawde committed 2026-10-01 15:10:59 +01:00
2 parents 73d18a1577 495e648d8b
commit 08e92f96df
4 files changed
+28 -43
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

OpenTelemetryPlan/06-implementation-phases.md
+1 -1
View File
@@ -906,7 +906,7 @@ flowchart TB
| 9.11 | Validator Health dashboard | ✅ Done |
| 9.12 | Peer Quality dashboard | ✅ Done |
| 9.13 | Ledger Economy row on `node-health` | ✅ Done |
| 9.14 | Overlay traffic accounting defects (documentation only) | 📄 Documented, not fixed |
| 9.14 | Overlay traffic accounting defects (documentation only) | 📄 Documented, 1 of 4 fixed |
| 9.15 | Peer keepalive / discovery instrumentation | ❌ Not implemented |
| 9.16 | PeerFinder slot and cache metrics | ❌ Not implemented |
| 9.17 | Peer span coverage (`peer.connect` / `peer.message.*`) | ❌ Not implemented (deferred) |
OpenTelemetryPlan/09-data-collection-reference.md
+8 -21
View File
@@ -2091,7 +2091,7 @@ Grafana keys a dashboard by its UID, so two dashboards sharing one UID overwrite
| `rpc_requests_total` depends on `[insight]` config | Zero series if `[insight]` is absent or unset | Requires `[insight] server=otel` 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) |
| `overhead_cluster_*` counts cluster members only | Zero on a node with no `[cluster_nodes]`; cluster messages from non-member peers count as `unknown` | By design — see [§6.0](#60-cluster-traffic-is-counted-for-cluster-members-only) |
| `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) |
@@ -2102,29 +2102,16 @@ Grafana keys a dashboard by its UID, so two dashboards sharing one UID overwrite
| Nine dotted `xrpl.<domain>.*` span attributes never shipped | TraceQL filters and harness assertions on the dotted keys match nothing | **NOT IMPLEMENTED** — renamed to bare keys; see [§Span Attribute Enrichments](#span-attribute-enrichments-phases-2-4-removed) |
| `node_writes_duration_us` has no dashboard panel | Cumulative write latency is exported and linted, but never charted | Open follow-up — see [§Extended NodeStore Metrics](#extended-nodestore-metrics-additions-to-existing-nodestore_state) |
### 6.0 `mtCLUSTER` is counted as `unknown`: NOT IMPLEMENTED
### 6.0 Cluster traffic is counted for cluster members only
`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).
`kTypeLookup` in `src/xrpld/overlay/detail/TrafficCount.cpp` maps `mtCLUSTER` to `Category::Cluster`. That category is exported as `overhead_cluster_bytes_in/out` and `overhead_cluster_messages_in/out`.
This also degrades `unknown_*` as an anomaly signal: on a clustered node it mixes
genuinely unrecognized wire types with routine `mtCLUSTER` traffic.
- **Outbound:** the `Message` constructor sets the category. `NetworkOPsImp::processClusterTimer` sends cluster reports only to cluster members (`PeerInCluster`), so all outbound cluster traffic is member traffic.
- **Inbound:** `PeerImp::onMessageBegin` passes the category through `TrafficCount::attribute`. A cluster message from a cluster member stays in the cluster category. From any other peer it is counted as `unknown`, and the node ignores it.
**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.
A cluster member is a peer in this node's cluster list, which `[cluster_nodes]` sets up. A node with no `[cluster_nodes]` has no members, so its `overhead_cluster_*` series read zero. That zero is the true value, not missing data. The local telemetry stack configures no cluster, so these series read zero there.
Routine cluster traffic does not land in `unknown_*`. On a clustered node, a sustained non-zero `unknown_*` therefore points at an unrecognized wire type or at a cluster message from a peer outside the cluster. See the [Cluster](../docs/telemetry-glossary.md#cluster) glossary entry.
### 6.1 `squelch_ignored` byte counts: NOT IMPLEMENTED
OpenTelemetryPlan/Phase9_taskList.md
+18 -20
View File
@@ -596,22 +596,22 @@ dropped the `xrpld-` prefix).
## 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)
> **Status**: Defect 1 FIXED in `develop`. Defects 2-4 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` |
| # | Defect | Effect | Fix location (NOT this branch) |
| --- | -------------------------------------- | ------------------------------------------------------------------------------------------------- | ------------------------------------------------ |
| 1 | `mtCLUSTER` missing from `kTypeLookup` | **Fixed in `develop`.** Cluster messages from members count as cluster; others count as `unknown` | `TrafficCount.cpp`: `kTypeLookup`, `attribute()` |
| 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 and 4 need `TrafficCount.cpp` and `PeerImp.cpp`
shared overlay code. Defect 4 needs `PeerImp.cpp`
edits that are 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.
@@ -620,10 +620,10 @@ merge timeline.
> `TrafficCount.{h,cpp}`" no longer holds for the header: the telemetry chain
> already edits `TrafficCount.h` — Phase 6's `77f35c03db` fixed the
> `Category::GetFetchPack` label from `"getobject_Fetch Pack_get"` to
> `"getobject_Fetch_Pack_get"` at `TrafficCount.h:285`, the sole difference from
> `"getobject_Fetch_Pack_get"` in its category-name table, the sole difference from
> `develop`. Defect 2 (the stale `Total` header comment, `TrafficCount.h:28-31`)
> is therefore **unblocked** and can land here. Defects **1, 3 and 4** stay
> blocked: defect 1 needs `TrafficCount.cpp`'s `kTypeLookup`, defect 3 needs the
> is therefore **unblocked** and can land here. Defect 1 is fixed in `develop`.
> Defects **3 and 4** stay blocked: defect 3 needs the
> `OverlayImpl` signature change, and defect 4 needs `PeerImp.cpp:1079` vs `:313`
> to agree on a byte basis (compressed vs uncompressed) — a change to overlay
> accounting semantics, not telemetry.
@@ -633,13 +633,13 @@ merge timeline.
**Exit Criteria**:
- [x] Each defect documented with file:line evidence in `09` §6
- [x] `overhead_cluster_*` documented as "no data", not "no cluster traffic"
- [x] Defect 1 fixed in `develop`'s `TrafficCount` code; `09` §6.0 says what `overhead_cluster_*` counts
- [ ] Defect 2 (stale `Total` header comment, `TrafficCount.h:28-31`) fixed on
this branch — it is **unblocked** (the chain already edits
`TrafficCount.h`) but the comment is still uncorrected
- [ ] Follow-up overlay-owned branch raised for the three still-blocked code
fixes (defects 1, 3, 4)
- [ ] Re-baseline any threshold keyed on `unknown_bytes_in` when defect 1 lands
- [ ] Follow-up overlay-owned branch raised for the two still-blocked code
fixes (defects 3, 4)
- [x] No re-baseline needed for defect 1: no alert rule in this repo reads `unknown_*`
---
@@ -676,9 +676,7 @@ merge timeline.
`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 still blocked with Task 9.14 defect 1. (The `.h` half of that
constraint no longer applies — see Task 9.14.)
- Splitting `mtPING` out of `Category::Base` is a `TrafficCount.cpp` change, so it needs its own overlay change, like the one in `develop` that fixed Task 9.14 defect 1. (The `.h` half of that constraint no longer applies — see Task 9.14.)
- Per the runbook's "Adding a New Metric" contract, `_total` is reserved for
monotonic counters; a histogram takes no suffix.
@@ -789,8 +787,8 @@ before commitment.
"Ledger Economy" row)
- [x] Provisioned Grafana alerting: 14 rules / 5 groups, 2 contact points,
nested notification policy
- [ ] Tasks 9.14-9.17 closed — **open by design**: 9.14 documented-not-fixed
(defects 1, 3 and 4 still blocked; defect 2 unblocked but not yet fixed),
- [ ] Tasks 9.14-9.17 closed — **open by design**: 9.14 partly fixed
(defect 1 fixed in `develop`; defects 3 and 4 still blocked; defect 2 unblocked but not yet fixed),
9.15 and 9.16 not implemented, 9.17 deferred pending approval and volume
measurement
docs/telemetry-glossary.md
+1 -1
View File
@@ -1061,7 +1061,7 @@ 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.
**What is observable:** cluster overhead is **not** measurable today, and this is a gap in the instrumentation rather than a display problem. The cluster message type is not in the overlay's message-to-category lookup table and none of the fallback branches match it, so every cluster message falls through to the `unknown` category (`src/xrpld/overlay/detail/TrafficCount.cpp`); no code path ever reports the cluster category, even though the `overhead_cluster` name is defined. Consequences: 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, and `unknown_*` is a weaker anomaly signal on a clustered node because it mixes genuinely unrecognized wire types with routine cluster traffic. If this is ever instrumented, volume moves out of `unknown_bytes_in`, so any alert threshold set against that series will need re-baselining.
**What is observable:** the `overhead_cluster_*` series measure cluster overhead in bytes and messages, in and out. The overlay's message-to-category lookup table maps the cluster message type to the cluster category (`src/xrpld/overlay/detail/TrafficCount.cpp`). Only traffic with cluster members counts. A node sends cluster messages only to its members. A cluster message from any other peer is counted as `unknown` instead. A node with no `[cluster_nodes]` has no members, so these series read zero there, and that zero is the true value.
**See also:** [Cluster on xrpl.org](https://xrpl.org/docs/concepts/networks-and-servers/clustering)