mirror of
https://github.com/XRPLF/rippled.git
synced 2026-08-28 17:50:55 +00:00
fix(telemetry): make the sync-diagnostics metric gate actually assert
The sync_diagnostics group asserted nothing. assert_sync_diagnostics_metrics called _check_prometheus_metric with five positional arguments against a six-parameter signature: `report` landed in `deadline` and `sem` was omitted entirely, so the call raised TypeError before a single metric was queried. Neither run_validation nor main catches anything, so the traceback propagated, run-full-validation.sh recorded the non-zero exit as a validation failure, and the four phases ordered after it -- dashboards, both parity checks and log-trace correlation -- never ran at all. Reproduced directly: TypeError, zero checks recorded. Even with the arity corrected the group would still have passed silently, because _check_prometheus_metric RETURNS its CheckResult rather than recording it and the value was discarded. Both halves are fixed by adopting the fan-out validate_metrics already uses: one shared deadline, a concurrency semaphore, gather, then report.add per result. The same call now records 55 checks where it previously recorded none. With the gate live, the inventory it guards had to be made honest. Two metrics could never have passed it. unl_fetch_total is emitted only from ValidatorSite::reportFetchOutcome, which indexes sites_[siteIdx]; sites_ comes from [validator_list_sites], and the harness writes a static [validators] file with no list site anywhere, so no fetch outcome is ever reported. handshake_negotiation_fail_total needs a rejected handshake, and no reject path was found to be reachable between identical localhost nodes. Both move to not_asserted.metrics_excluded, which is where the file's own description says workload-gated names belong. Eleven further conditional metrics -- the acquire, replay, disconnect, serve, jump and sweep counters -- were documented only inside free-text notes; they move to the same map. That matters beyond tidiness: _accounted_metric_names harvests metrics_excluded keys, so a name recorded only in prose is reported as unaccounted, and a prose note cannot be linted at all. Two metrics were wrongly excluded. rotation_state's callback gates only on dynamic_cast<DatabaseRotating*>, and online_delete=256 is set by both the cfg template and run-full-validation.sh, so SHAMapStoreImp builds a DatabaseRotatingImp, the cast succeeds, and both sub-series are observed on every collection tick. The note claiming the harness could not produce them conflated "no rotation runs" with "no series published"; the first is true and bounds the values, the second is false. Both are now asserted at value 0, where absence rather than the zero is the regression, and the note is corrected. The four new histograms listed only _bucket, or _bucket and _count. Each now lists _sum as well, matching the rpc_method_us and job_queued_us convention, so an exporter regression that drops one series cannot pass. On the span side, ledger.validate and ledger.store are the two ends of the per_ledger trace-join group, and the join is computed by hashing ledger_hash -- yet neither required it. Both spans take it unconditionally from makeLedgerTraceSpan, so requiring it is free, and without it a lost join key surfaces only as "spans landed in separate traces", naming the consequence instead of the cause. Deliberately unchanged: ledger.serve stays required and peer.dial keeps its current required attributes, though both look unsafe -- ledger.serve can only fire if an optional span fires first, and peer.dial's destructor exit sets neither outcome nor duration_ms. Those weaken assertions rather than add coverage, so they are reported rather than changed here. Verification: TypeError reproduced before the fix and absent after, with 55 checks recorded; both JSON files parse; no name is both asserted and excluded and none is duplicated; the declared span counters remain consistent at 48 and 74, proven by injecting an extra attribute and watching the check fail; check_otel_naming.py exits 0, and Rule K was proven to read these entries by injecting a bogus name in an owned family and observing exit 1; pre-commit passes on all three files; the levelization baseline is unchanged. NOT compiled -- no C++ changed.
This commit is contained in:
Pratik Mankawde
@@ -149,14 +149,14 @@
|
||||
"metrics": ["storage_detail{metric=\"stored_object_bytes\"}"]
|
||||
},
|
||||
"sync_diagnostics": {
|
||||
"description": "Fresh-node sync diagnostics (native metrics). Bootstrap (Domain 0) and acquire-pipeline signals rendered by the ledger-sync-health dashboard. Names are appended one per signal. Histograms are listed by their Prometheus _bucket series (the bare instrument name is not a series). The two observable gauges carry an inline metric= selector so the specific sub-series is asserted, matching the parity_* groups.",
|
||||
"description": "Fresh-node sync diagnostics (native metrics). Bootstrap (Domain 0) and acquire-pipeline signals rendered by the ledger-sync-health dashboard. Names are appended one per signal. Histograms are listed by all three of their Prometheus series -- _bucket, _count and _sum -- because the bare instrument name is not a series and an exporter regression can drop one without dropping the others; this matches the rpc_method_us / job_queued_us convention in the groups above. Observable gauges carry an inline metric= selector so the specific sub-series is asserted, matching the parity_* groups. Signals this workload cannot produce are NOT listed here: they belong in the not_asserted group, whose metrics_excluded map is the single machine-readable record of every emitted-but-unasserted name.",
|
||||
"metrics": [
|
||||
"dns_resolve_total",
|
||||
"dns_resolve_latency_ms_bucket",
|
||||
"dns_resolve_latency_ms_sum",
|
||||
"overlay_connect_total",
|
||||
"overlay_dial_latency_ms_bucket",
|
||||
"handshake_negotiation_fail_total",
|
||||
"unl_fetch_total",
|
||||
"overlay_dial_latency_ms_sum",
|
||||
"unl_quorum{metric=\"trusted_keys\"}",
|
||||
"unl_quorum{metric=\"quorum\"}",
|
||||
"clock_close_offset_seconds{metric=\"offset\"}",
|
||||
@@ -201,21 +201,25 @@
|
||||
"ledger_quorum_shortfall_total{stage=\"pre_accept\"}",
|
||||
"consensus_round_duration_ms_bucket",
|
||||
"consensus_round_duration_ms_count",
|
||||
"consensus_round_duration_ms_sum",
|
||||
"nodestore_state{metric=\"node_writes_duration_us\"}",
|
||||
"nodestore_state{metric=\"node_reads_duration_us\"}",
|
||||
"unl_quorum{metric=\"quorum_disabled\"}",
|
||||
"sweep_malloc_trim_us_bucket",
|
||||
"sweep_malloc_trim_us_count"
|
||||
"sweep_malloc_trim_us_count",
|
||||
"sweep_malloc_trim_us_sum",
|
||||
"rotation_state{metric=\"in_flight\"}",
|
||||
"rotation_state{metric=\"copy_forward\"}"
|
||||
],
|
||||
"_acquire_note": "The four sync_acquire sub-series and shamap_cache_hit_rate are unconditional: both are observable gauges whose callbacks observe every series on each collection tick, so each is present even when the value is 0 (an idle node reports in_flight=0 and missing_state_nodes_max=0, and a cold cache reports a 0.0 hit rate). Absence, not a zero, is the regression. The three WP-A3 counters (sync_acquire_source_total, sync_addnode_total, sync_acquire_no_progress_total) are deliberately NOT asserted here: all three are emitted only from InboundLedger, which runs only when a node must fetch a ledger it lacks. expected_spans.json already marks the ledger.acquire span optional for exactly this reason (\"A healthy local cluster rarely back-fills history\"), and the metric validator has no per-metric optional flag, so listing them would fail the harness red on a healthy run. They are covered by exact-value unit tests in src/tests/libxrpl/telemetry/MetricMacros.cpp and by the ledger-sync-health panels; add them here only alongside a harness step that forces a real acquire (e.g. starting a node against an existing ledger history).",
|
||||
"_jobq_note": "The three jobq_saturation series are unconditional: it is an observable gauge whose callback observes all three fields on every collection tick, so each series exists even when the value is 0, and absence rather than a zero is the regression. worker_threads is asserted because it is the denominator of the dashboard saturation ratio, and it is always at least 1 (the JobQueue ctor gives standalone mode exactly one worker), so a zero or missing reading there means the accessor regressed rather than the node being idle. The per-job-type waiting/running/deferred counts are published separately by JobQueue::collect() as the beast::insight gauges jobq_<type>_waiting / _running / _deferred, which the collector translates; they are covered by the StatsD-derived groups, not here.",
|
||||
"_conditional_note": "handshake_negotiation_fail_total and unl_fetch_total are conditional under the local harness: the first only exists once a handshake is rejected, and the second needs a [validator_list_sites] entry (run-full-validation.sh generates a static [validators] file instead). The validator has no per-metric optional flag, so if either reports 0 series in a harness run, move it out of this group rather than weakening the check.",
|
||||
"_conditional_note": "handshake_negotiation_fail_total and unl_fetch_total USED to be asserted here with a note saying to move them out if a run reported 0 series. No run could ever report that: assert_sync_diagnostics_metrics passed `report` where `deadline` belongs and omitted `sem`, so it raised TypeError before any check ran and this whole group asserted nothing. With that fixed, both would now fail a healthy run for the reasons the old note itself gave, so both have moved to not_asserted.metrics_excluded. Anything else added here must be reachable under run-full-validation.sh's own cluster and workload, not merely emitted somewhere in the code.",
|
||||
"_sync_state_note": "The four sync_state sub-series are unconditional: the gauge observes all four on every collection tick, so each is present as a series even when its value is 0 (a node that never reached FULL reports initial_full_duration_us=0, and a healthy node reports server_stall_seconds=0). The check asserts series presence, not a non-zero value, which is exactly right here \u2014 a zero is a meaningful reading for these signals, and absence is the regression. server_stall_events_total is likewise always present because the observable counter reports the tally (0 or more) every tick. state_changes_total is asserted here with a from!=\"\",to!=\"\" selector rather than bare (parity_counters already asserts the bare name): the selector is what proves the WP-A2 {from,to} label dimension actually reached Prometheus, so a regression to the old unlabelled counter fails this check instead of silently passing on the bare name. It needs at least one real mode transition, which any node reaching connected/syncing produces during startup.",
|
||||
"_a7_note": "WP-A7 adds three observable gauges and four counters. The 16 gauge sub-series (peer_ledger_supply, peerfinder_slot_census, amendment_block) are unconditional and asserted individually: each callback in MetricsRegistry.cpp calls observe() for every field on every collection tick with no early return between them, so the series exists whatever the value. That includes the two sentinel readings \u2014 a node whose peers have advertised nothing reports peer_ledger_supply{metric=\"supply_min_seq\"} = 0 meaning unknown, and a node with no pending amendment reports amendment_block{metric=\"seconds_to_block\"} = -1 meaning healthy. Absence, not the sentinel, is the regression. Of the four counters only peer_accept_total is asserted: run-full-validation.sh gives every node a [port_peer] on 0.0.0.0 and lists the other four nodes in [ips], so all 5 nodes dial each other and each one is also dialled, which means OverlayImpl::onHandoff runs and reports outcome=accepted (or slot_refused/no_slot on the duplicate half of each mutual dial) on every node. It is asserted bare rather than with an outcome= selector because which outcome a given node records depends on dial ordering, which the harness does not control. The other three counters are deliberately NOT asserted. peer_disconnect_total is emitted only from PeerImp::close, and a healthy 5-node localhost cluster holds its 4 fixed peers for the whole run: the timer-driven reasons need maxUnknownTime (600 s) or maxDivergedTime (300 s) to elapse (Config.h) while the full-validation profile totals well under that, and the shutdown reasons only fire during teardown, which happens in run-full-validation.sh after Step 5 has already scraped. serve_refused_total needs a peer to ask this node for a ledger, tx set or object it cannot serve \u2014 on a cluster where every node has the same complete history from genesis, getLedger()/getTxSet() succeed and the send queues never approach Tuning::kDropSendQueue. ledger_jump_total needs NetworkOPsImp::switchLastClosedLedger, reached only when consensus reports an LCL this node did not build on; a healthy 5-node cluster agrees every round, so it never jumps. The metric validator has no per-metric optional flag, so listing any of the three would fail the harness red on a healthy run \u2014 the same reasoning _acquire_note applies to the WP-A3 InboundLedger counters. All four counters are covered by exact-value unit tests in src/tests/libxrpl/telemetry/MetricMacros.cpp and rendered by the ledger-sync-health panels Peer Disconnects (Count By Reason & Direction), Ledger/Object Serve Refusals and Byzantine Ledger Jumps, and by the peer-quality panels Peer Disconnect Rate and Peer Disconnects By Reason & Direction. To make them assertable the harness would need a fault-injection step: kill one node mid-run and re-scrape before teardown (peer_disconnect_total, reason=read_error/graceful), request a ledger sequence outside the cluster's history or drive a node past its send-queue limit (serve_refused_total), and start a node on a divergent chain tip or partition the cluster and heal it (ledger_jump_total).",
|
||||
"_a6_note": "WP-A6 originally added a separate nodestore_latency gauge; it was retired because every one of its sub-metrics had an exact counterpart on nodestore_state computed from the same Database accessor, so the five entries above are the surviving equivalents (write_count -> node_writes, read_count -> node_reads_total, write_duration_us -> node_writes_duration_us, read_duration_us -> node_reads_duration_us, read_mean_us unchanged). All four totals are unconditional: MetricsRegistry::observeNodeStoreTotals observes them on every collection tick with no early return before them, so a series exists whatever the value and a node that has written nothing reports node_writes=0 rather than dropping the series. read_mean_us is safe because any node that has opened a ledger has already fetched objects, so the fetch count is non-zero. write_mean_us is NOT asserted, but only because the two means are the sub-series that scaledMean() omits when their denominator is zero, and this validator has no per-metric optional flag -- not because the numerator is missing. That older caveat is gone: all three concrete store paths now time themselves through Database::recordStoreDuration() (DatabaseNodeImp::store, DatabaseRotatingImp::store and Database::importInternal), so write_mean_us is live on an ordinary node and only a node that has performed literally zero stores would lack it. It can be promoted to an assertion once a harness run confirms it present. WP-A6's two replay counters (ledger_replay_fallback_total, ledger_replay_outcome_total) are likewise NOT asserted, for the same reason _acquire_note gives for the WP-A3 InboundLedger counters: both are emitted only from the ledger-replay path, which requires the [ledger_replay] config stanza AND a real historical back-fill against peers that support the LedgerReplay protocol feature. run-full-validation.sh starts a fresh local cluster with no history to back-fill, so no replay task is ever created and neither counter can produce a series. Both are covered by exact-value unit tests in src/tests/libxrpl/telemetry/MetricMacros.cpp and rendered by the ledger-sync-health panels Replay Fallback to Full Acquire and Replay Outcomes. To make them assertable the harness would need to enable [ledger_replay] and start a node against an existing ledger history so it back-fills through the replay path.",
|
||||
"_a5_note": "WP-A5 adds one observable gauge (ledger_quorum_publish) and one counter (ledger_quorum_shortfall_total). All four gauge sub-series are asserted and are unconditional: registerLedgerQuorumPublishGauge's callback in MetricsRegistry.cpp calls observe() for every field on every collection tick with no early return between them, and each accessor is a plain relaxed atomic load that always returns a value, so the series exists whatever the reading. That deliberately includes the three diagnostic zeros: a node that has never had a gate evaluated reports trusted_validation_tally=0 and quorum_target=0, one that has never fully validated reports time_to_first_validated_us=0, and one that is caught up reports publish_lag=0. Absence, not the zero, is the regression -- the same reasoning _sync_state_note gives for initial_full_duration_us. Note the sentinel: when the trusted list disables quorum entirely, getNeededValidations() returns SIZE_MAX and LedgerMaster reports quorum_target as int64 max rather than letting the cast wrap to -1, so the target reads far above any tally instead of inverting the comparison (the same fix as the unl_quorum gauge). ledger_quorum_shortfall_total IS asserted, which differs from the WP-A3/A6/A7 counters, and the reason is that this counter does not need a fault to fire. RCLConsensus::Adaptor::doAccept issues this node's own validation and then calls ledgerMaster_.consensusBuilt immediately (RCLConsensus.cpp), which calls checkAccept on the freshly built ledger (LedgerMaster.cpp) BEFORE the peers' validations for that same ledger have arrived. With the harness's 5 validators the quorum is max(ceil(5*0.8), ceil(5*0.6)) = 4 (ValidatorList::calculateQuorum), so that first evaluation of each round tallies short of 4 and takes the shortfall early return; the gate is then re-entered from RCLValidations handleNewValidation as each trusted validation arrives and eventually passes. A HEALTHY 5-node cluster therefore emits this counter every round, which is why it is safe to assert on a clean run -- unlike peer_disconnect_total or ledger_replay_fallback_total, it needs no fault injection, no [ledger_replay] stanza and no historical back-fill. The stage=\"pre_accept\" selector is asserted rather than the bare name so that the label dimension is proven to have reached Prometheus, matching the state_changes_total{from,to} pattern. Consequence for readers of the panels: a non-zero rate on Pre-Accept Quorum Shortfall Rate is NOT by itself a fault, and the panel description says so; the fault signature is that rate climbing well above the ledger-close rate while the tally on Trusted Validations vs Quorum Target stays flat below its target. If a future harness change makes the cluster single-node or standalone this assertion must move to a note: standalone_ short-circuits consensusBuilt before checkAccept, and getNeededValidations() returns 0 in standalone mode, so the gate can never report a shortfall.",
|
||||
"_b5_sweep_note": "WP-B5 Suspect 3 (per-sweep malloc_trim) adds one histogram and two counters. Only the histogram is asserted, by its Prometheus _bucket and _count series because the bare instrument name is not a series. It is unconditional on any running node: ApplicationImp::start arms the sweep timer before the workload begins, the interval is SizedItem::SweepInterval (Config.cpp: 10 s at nodeSize 0 through 120 s at nodeSize 4), and the full-validation profile runs 270 s of workload before Step 5 scrapes -- so at least two sweeps complete even in the slowest case, and each one records exactly one sample. The measurement itself is now unconditional too: it used to sit inside `if (journal.debug())` in MallocTrim.cpp, so a node at ordinary log level measured nothing; that gate now covers only the JLOG. The two counters are deliberately NOT asserted. sweep_malloc_trim_minor_faults_total is emitted only when the trim's minor-fault delta is greater than zero, and a trim on the small heap of a fresh localhost node routinely faults zero times -- the emit site publishes nothing rather than a zero, because a zero-valued series would claim the trim was measured as free when the honest statement is that there was nothing to fault on. sweep_malloc_trim_reclaimed_kb_total is emitted only when resident memory actually FELL across the trim, which on a node whose caches are still filling frequently does not happen (glibc has nothing above the top of the heap to release, and mmap-backed allocations are returned on free regardless of trimming). The validator has no per-metric optional flag, so asserting either would ship a permanently red CI check for a healthy run. Both are covered by exact-value unit tests in src/tests/libxrpl/telemetry/MetricMacros.cpp -- including the skip paths -- and rendered by the ledger-sync-health panel Sweep Heap-Trim Faults & Reclaim Rate. To make them assertable the harness would need a node with a large enough resident heap for a trim to reclaim, e.g. starting against an existing populated database rather than from genesis.",
|
||||
"_b5_rotation_note": "WP-B5 Suspect 4 (online_delete rotation extra writes) adds one observable gauge (rotation_state, sub-series in_flight and copy_forward) and one counter (rotation_copy_node_restore_total). NONE is asserted, because the 5-node localhost harness structurally CANNOT produce any of them -- this is a documented note rather than a check that would fail CI red. Two independent reasons. First, no rotation ever runs: xrpld-validator.cfg.template sets online_delete=256 and does not set advisory_delete, so SHAMapStoreImp's gate is validatedSeq >= lastRotated + 256 (SHAMapStoreImp.cpp), which needs 256 validated ledgers; at the network's several-seconds-per-ledger close rate that is on the order of 15-20 minutes, while the full-validation profile totals 270 s of workload before Step 5 scrapes. Second, even the in_flight flag needs a rotation to have started, and copy_forward additionally needs an ARCHIVE holding data that a fetch actually reads during the rotation window -- which requires a populated, already-rotated database, exactly the condition the hypothesis says is why this slowdown never appears on a fresh node. rotation_copy_node_restore_total is narrower still: it fires only for a clean tree node reachable from the validated state map whose sole on-disk copy was removed by an EARLIER rotation, so it needs at least two rotations plus real prior data loss. Note that rotation_state publishes no series at all when online_delete is not configured, by design: MetricsRegistry::registerRotationStateGauge dynamic_casts the node store to DatabaseRotating and returns early on failure, so an absent series means 'rotation is not configured' rather than the false 'rotation is free' a zero would report. All four signals are covered by exact-value unit tests in src/tests/libxrpl/telemetry/MetricMacros.cpp (including the not-configured and between-rotations cases) and rendered by the ledger-sync-health panels Online-Delete Rotation Window & Copy-Forward Writes and Rotation Node Re-Store Rate. To make them assertable the harness would need a step that starts a node against a pre-populated database that has already rotated at least once, or that lowers online_delete and advisory_delete far enough to force a rotation inside the run window and then re-scrapes before teardown.",
|
||||
"_b5_rotation_note": "WP-B5 Suspect 4 (online_delete rotation extra writes) adds one observable gauge (rotation_state, sub-series in_flight and copy_forward) and one counter (rotation_copy_node_restore_total). BOTH gauge sub-series ARE asserted; only the counter is excluded. An earlier version of this note claimed none of them could be produced by the harness, which conflated 'no rotation runs' with 'no series is published'. Those are different facts: registerRotationStateGauge's callback gates ONLY on dynamic_cast<DatabaseRotating*>, and both xrpld-validator.cfg.template:48 and run-full-validation.sh:300 set online_delete=256, so SHAMapStoreImp::makeNodeStore takes its deleteInterval_ != 0 branch and builds a DatabaseRotatingImp -- the cast succeeds and both observe() calls then run on every collection tick with no early return between them. So each series exists at value 0 (in_flight=0 meaning no rotation in progress, copy_forward=0 meaning none has yet copied anything), and absence rather than the zero is the regression, exactly as for the sync_state and peer_ledger_supply gauges. What the harness genuinely cannot produce is a rotation ACTUALLY RUNNING, for the reason below; that bounds the values, not the series. First, no rotation ever runs: xrpld-validator.cfg.template sets online_delete=256 and does not set advisory_delete, so SHAMapStoreImp's gate is validatedSeq >= lastRotated + 256 (SHAMapStoreImp.cpp), which needs 256 validated ledgers; at the network's several-seconds-per-ledger close rate that is on the order of 15-20 minutes, while the full-validation profile totals 270 s of workload before Step 5 scrapes. Second, even the in_flight flag needs a rotation to have started, and copy_forward additionally needs an ARCHIVE holding data that a fetch actually reads during the rotation window -- which requires a populated, already-rotated database, exactly the condition the hypothesis says is why this slowdown never appears on a fresh node. rotation_copy_node_restore_total is narrower still: it fires only for a clean tree node reachable from the validated state map whose sole on-disk copy was removed by an EARLIER rotation, so it needs at least two rotations plus real prior data loss. Note that rotation_state publishes no series at all when online_delete is not configured, by design: MetricsRegistry::registerRotationStateGauge dynamic_casts the node store to DatabaseRotating and returns early on failure, so an absent series means 'rotation is not configured' rather than the false 'rotation is free' a zero would report. All four signals are covered by exact-value unit tests in src/tests/libxrpl/telemetry/MetricMacros.cpp (including the not-configured and between-rotations cases) and rendered by the ledger-sync-health panels Online-Delete Rotation Window & Copy-Forward Writes and Rotation Node Re-Store Rate. To make rotation_copy_node_restore_total assertable -- and to give the two asserted gauge sub-series a NON-ZERO reading rather than merely a present one -- the harness would need a step that starts a node against a pre-populated database that has already rotated at least once, or that lowers online_delete and advisory_delete far enough to force a rotation inside the run window and then re-scrapes before teardown.",
|
||||
"_round_histogram_note": "consensus_round_duration_ms is a native OTel histogram recorded once per consensus round in RCLConsensus, so it needs no collector configuration -- it rides the existing OTLP -> Prometheus path. It is asserted by its Prometheus _bucket and _count series because the bare instrument name is not a series. Both are unconditional on any running cluster: every node closes ledgers continuously, so a round completes within the harness window and the histogram is populated. Absence means the record site or the explicit-bucket view regressed, not that the node was idle. The instrument carries NO labels, so exactly one series exists per node and per bucket boundary."
|
||||
},
|
||||
"node_health_gauges": {
|
||||
@@ -235,7 +239,7 @@
|
||||
"metrics": ["validation_agreements_total", "validation_missed_total"]
|
||||
},
|
||||
"not_asserted": {
|
||||
"description": "Emitted-and-dashboarded metrics deliberately left unasserted because they are workload-gated or defect-gated: the harness workload cannot guarantee they appear, and a check that fails on a healthy run is worse than no check. This group has no \"metrics\" key, so validate_telemetry.py skips it (validate_metrics iterates category_data.get(\"metrics\", [])). Promote an entry into an asserted group only after the workload is changed to guarantee it.",
|
||||
"description": "Emitted-and-dashboarded metrics deliberately left unasserted because they are workload-gated or defect-gated: the harness workload cannot guarantee they appear, and a check that fails on a healthy run is worse than no check. This group has no \"metrics\" key, so validate_telemetry.py skips it (validate_metrics iterates category_data.get(\"metrics\", [])). Promote an entry into an asserted group only after the workload is changed to guarantee it. This map is the ONLY machine-readable record of an emitted-but-unasserted name, so every such name belongs here rather than in a prose note inside an asserted group -- prose cannot be linted, and phase-10's incoming _unaccounted_metric_names warning reports anything absent from both. Caveat when adding: check_otel_naming.py Rule K harvests names only from `metrics` LISTS and from metric/name keys, so these dict KEYS are never validated against the MetricNames.h constants -- a typo here is silent, and must be checked by eye against the emit site cited in its own reason string.",
|
||||
"metrics_excluded": {
|
||||
"rpc_method_errored_total": "MetricsRegistry.cpp:354, push counter — needs an RPC that returns an error. rpc_load_generator.py issues only well-formed server_info / fee / ledger / ripple_path_find calls, so no series may ever be created.",
|
||||
"ledger_history_mismatch_total": "MetricsRegistry.cpp:377, incremented only from LedgerHistory.cpp:332 on a built-vs-validated ledger mismatch. On a healthy run it never fires — asserting it would mean asserting a defect.",
|
||||
@@ -245,7 +249,20 @@
|
||||
"getobject_request_objects": "GetObjectMetricNames.h:86, emitted from PeerImp.cpp:2926 only while serving an inbound TMGetObjectByHash. The XRPL_METRIC_* macros create their instrument lazily on first use (MetricMacros.h:174-285), so no series exists until a peer actually requests objects by hash — which a 5-node cluster started at genesis and already in sync may never do.",
|
||||
"getobject_lookup_us": "GetObjectMetricNames.h:95, PeerImp.cpp:2929. Same lazy-creation and same inbound-request gate as getobject_request_objects.",
|
||||
"getobject_lookups_total": "GetObjectMetricNames.h:100, PeerImp.cpp:2949/:2956. Same gate.",
|
||||
"getobject_charge": "GetObjectMetricNames.h:105, PeerImp.cpp:2931. Same gate."
|
||||
"getobject_charge": "GetObjectMetricNames.h:105, PeerImp.cpp:2931. Same gate.",
|
||||
"unl_fetch_total": "ValidatorSite.cpp:435, emitted only from reportFetchOutcome, which indexes sites_[siteIdx]. sites_ is populated from [validator_list_sites]; run-full-validation.sh writes a static [validators] file and no [validator_list_sites] exists anywhere under docker/telemetry/workload/, so sites_ is empty and no fetch outcome is ever reported. Needs a harness that serves a UNL over HTTP.",
|
||||
"handshake_negotiation_fail_total": "Handshake.cpp:265, emitted only when a handshake is REJECTED. Every node in the cluster shares one network id and one build, dials 127.0.0.1 and has no clock skew, so no reject path was found to be reachable. Not proven unreachable -- promote it if a run ever shows the series.",
|
||||
"sync_acquire_source_total": "InboundLedger.cpp:165, emitted from InboundLedger::init(), the same function that opens the ledger.acquire span. expected_spans.json already marks that span optional (\"A healthy local cluster rarely back-fills history\"), so the counter inherits exactly that condition. Best first candidate to promote if the harness ever forces an acquire: unlike its siblings it fires even when the store was already complete, via source=local.",
|
||||
"sync_acquire_no_progress_total": "InboundLedger.cpp:536. Strictly narrower than sync_acquire_source_total: needs an acquire AND a timer tick that made no progress.",
|
||||
"sync_addnode_total": "InboundLedger.cpp:1636 (recordBatchOutcome). Needs an acquire that actually receives SHAMap nodes over the wire; the emit site returns early when the batch count is <= 0, so a zero tally publishes nothing rather than a zero.",
|
||||
"ledger_replay_fallback_total": "SkipListAcquire.cpp:113 / LedgerDeltaAcquire.cpp:117. Structurally impossible here: reachable only from a LedgerReplayTask, created only via LedgerReplayer::replay(), whose sole production caller is LedgerMaster.cpp:1459 behind `if (app_.config().ledgerReplay)`. Config::ledgerReplay defaults false and is set only from [ledger_replay], which the harness never writes.",
|
||||
"ledger_replay_outcome_total": "LedgerReplayTask.cpp:237. Same [ledger_replay] gate as ledger_replay_fallback_total, plus a real historical back-fill against peers advertising the LedgerReplay feature.",
|
||||
"ledger_jump_total": "NetworkOPs.cpp:2307, in switchLastClosedLedger. Needs consensus to report an LCL this node did not build on. A 5-node cluster that agrees every round never jumps, and no divergence is injected.",
|
||||
"peer_disconnect_total": "PeerImp.cpp:655, inside PeerImp::close() behind the !socket_.is_open() early return. The timer-driven reasons need maxUnknownTime (600 s) or maxDivergedTime (300 s) to elapse, both longer than the profile's 270 s of workload; the shutdown reasons fire only during --cleanup teardown, after Step 5 has already scraped. A mutual-dial race could produce one non-deterministically, which is itself the argument against asserting it.",
|
||||
"serve_refused_total": "PeerImp.cpp:669 (reportServeRefusal). Needs a peer to ask for a ledger, tx set or object this node cannot serve, or a send queue near Tuning::kDropSendQueue. Every node holds the same complete history from genesis, so getLedger()/getTxSet() succeed.",
|
||||
"sweep_malloc_trim_minor_faults_total": "Application.cpp:1249, published only when the trim's minor-fault delta is > 0. A trim on the small heap of a fresh localhost node routinely faults zero times, and the site deliberately publishes nothing rather than a zero that would claim the trim was measured as free.",
|
||||
"sweep_malloc_trim_reclaimed_kb_total": "Application.cpp:1263, published only when resident memory actually FELL across the trim. On a node whose caches are still filling this frequently does not happen: glibc has nothing above the top of the heap to release, and mmap-backed allocations are returned on free regardless of trimming.",
|
||||
"rotation_copy_node_restore_total": "SHAMapStoreImp.cpp:283. Fires only for a clean tree node reachable from the validated state map whose sole on-disk copy an EARLIER rotation removed, so it needs at least two rotations (512 validated ledgers, ~15-20 min at the cluster's close rate) plus real prior data loss. The run window is 270 s. Note that the rotation_state gauge sub-series ARE asserted -- see sync_diagnostics._b5_rotation_note for why a series exists while a rotation never runs."
|
||||
}
|
||||
},
|
||||
"grafana_dashboards": {
|
||||
|
||||
@@ -313,15 +313,17 @@
|
||||
"name": "ledger.validate",
|
||||
"category": "ledger",
|
||||
"parent": null,
|
||||
"required_attributes": ["ledger_seq", "validations"],
|
||||
"config_flag": "trace_ledger"
|
||||
"required_attributes": ["ledger_hash", "ledger_seq", "validations"],
|
||||
"config_flag": "trace_ledger",
|
||||
"note": "ledger_hash is required because it is the trace-join key, not merely a descriptive attribute: this span is the anchor of the per_ledger group in trace_join_groups, and the join is computed by hashing that value. Both it and ledger_seq are stamped unconditionally by LedgerMaster::makeLedgerTraceSpan (LedgerMaster.cpp:186-190, called at :1089), so requiring it costs nothing on a healthy run. Without this the only symptom of a lost join key would be assert_trace_join_groups reporting that spans landed in separate traces, which names the consequence rather than the cause."
|
||||
},
|
||||
{
|
||||
"name": "ledger.store",
|
||||
"category": "ledger",
|
||||
"parent": null,
|
||||
"required_attributes": ["ledger_seq"],
|
||||
"config_flag": "trace_ledger"
|
||||
"required_attributes": ["ledger_hash", "ledger_seq"],
|
||||
"config_flag": "trace_ledger",
|
||||
"note": "ledger_hash is required for the same reason as on ledger.validate: it is the per_ledger trace-join key, stamped unconditionally by LedgerMaster::makeLedgerTraceSpan (called at LedgerMaster.cpp:511). This span is the required_member of that join group, so a missing key here breaks the join from the other end."
|
||||
},
|
||||
{
|
||||
"name": "ledger.acquire",
|
||||
|
||||
@@ -1077,10 +1077,34 @@ async def assert_sync_diagnostics_metrics(
|
||||
)
|
||||
return
|
||||
|
||||
for metric_name in metrics:
|
||||
await _check_prometheus_metric(
|
||||
session, prometheus_url, metric_name, SYNC_DIAGNOSTICS_GROUP, report
|
||||
# Same fan-out shape as validate_metrics(): ONE shared deadline for the
|
||||
# whole group, so the phase costs a single poll window instead of one per
|
||||
# metric, and a semaphore so the fan-out cannot exhaust the Prometheus
|
||||
# connection pool.
|
||||
#
|
||||
# _check_prometheus_metric RETURNS its CheckResult rather than recording it,
|
||||
# so each result must be added here. Discarding them would let the group
|
||||
# pass silently whatever Prometheus holds, which is the more dangerous half
|
||||
# of the defect this replaces: the original call also passed `report` where
|
||||
# `deadline` belongs and omitted `sem` entirely, raising TypeError before
|
||||
# any check ran and taking the four later validation phases down with it.
|
||||
deadline = time.monotonic() + METRIC_POLL_TIMEOUT_SEC
|
||||
sem = asyncio.Semaphore(METRIC_POLL_CONCURRENCY)
|
||||
checks = await asyncio.gather(
|
||||
*(
|
||||
_check_prometheus_metric(
|
||||
session,
|
||||
prometheus_url,
|
||||
metric_name,
|
||||
SYNC_DIAGNOSTICS_GROUP,
|
||||
deadline,
|
||||
sem,
|
||||
)
|
||||
for metric_name in metrics
|
||||
)
|
||||
)
|
||||
for check in checks:
|
||||
report.add(check)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user