mirror of
https://github.com/XRPLF/rippled.git
synced 2026-07-30 02:20:39 +00:00
fix(telemetry): make a failed validation run explain itself
Two gaps meant the last failure produced no evidence of its cause. The node-log upload was gated on `if: failure()`, but the validation step sets continue-on-error, so the job is not failing at that point and the condition never fired. Every failed run silently skipped the one artifact that records why a node did not reach consensus. It now keys on the validation step's own outcome, and also collects the harness logs. Transaction failures were logged at DEBUG, which CI does not enable, so a run where all 3052 submissions failed on a refused connection reported nothing about it. The first occurrence of each distinct failure kind is now a warning and repeats stay at DEBUG, so one refused connection says so once instead of 3052 times.
This commit is contained in:
12
.github/workflows/telemetry-validation.yml
vendored
12
.github/workflows/telemetry-validation.yml
vendored
@@ -229,13 +229,21 @@ jobs:
|
||||
path: /tmp/xrpld-validation/reports/
|
||||
retention-days: 30
|
||||
|
||||
# Keyed on the validation step's own outcome, not job status. The step
|
||||
# above sets continue-on-error, so the job is not failing at this point
|
||||
# and `if: failure()` never fires -- which silently skipped these logs on
|
||||
# every failed run, and they are the only record of why a node did not
|
||||
# reach consensus.
|
||||
- name: Upload node logs
|
||||
if: failure()
|
||||
if: always() && steps.validation.outcome != 'success'
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
|
||||
with:
|
||||
name: xrpld-node-logs
|
||||
path: /tmp/xrpld-validation/node*/debug.log
|
||||
path: |
|
||||
/tmp/xrpld-validation/node*/debug.log
|
||||
/tmp/xrpld-validation/*.log
|
||||
retention-days: 7
|
||||
if-no-files-found: warn
|
||||
|
||||
- name: Print validation summary
|
||||
if: always()
|
||||
|
||||
@@ -38,6 +38,31 @@ import websockets
|
||||
|
||||
logger = logging.getLogger("tx_submitter")
|
||||
|
||||
# Failure kinds already reported at WARNING, so a repeated failure is not
|
||||
# logged again at that level.
|
||||
_reported_failures: set[str] = set()
|
||||
|
||||
|
||||
def _log_first_failure(key: str, fmt: str, *args: object) -> None:
|
||||
"""Log a submission failure loudly the first time its kind is seen.
|
||||
|
||||
A run in which every transaction fails for one reason -- a refused
|
||||
connection, an unfunded account -- used to leave no trace, because these
|
||||
messages were DEBUG and CI does not enable DEBUG. Logging every failure
|
||||
instead would bury the run in thousands of identical lines, so the first
|
||||
of each distinct kind is a WARNING and the rest stay at DEBUG.
|
||||
|
||||
:param key: Identifies the failure kind, e.g. the engine result code.
|
||||
:param fmt: Logging format string.
|
||||
:param args: Format arguments.
|
||||
"""
|
||||
if key in _reported_failures:
|
||||
logger.debug(fmt, *args)
|
||||
return
|
||||
_reported_failures.add(key)
|
||||
logger.warning(fmt, *args)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Constants
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -631,7 +656,13 @@ async def submit_transaction(
|
||||
sender.sequence += 1
|
||||
|
||||
if not success:
|
||||
logger.debug(
|
||||
# First occurrence of each distinct result at WARNING, the rest at
|
||||
# DEBUG. A run where every transaction failed previously produced
|
||||
# no diagnostics at all, because DEBUG is off in CI; logging every
|
||||
# failure instead would bury the run in thousands of identical
|
||||
# lines.
|
||||
_log_first_failure(
|
||||
"result:%s" % engine_result,
|
||||
"%s result: %s (%s)",
|
||||
tx_type,
|
||||
engine_result,
|
||||
@@ -639,7 +670,9 @@ async def submit_transaction(
|
||||
)
|
||||
except Exception as exc:
|
||||
stats.record(tx_type, False)
|
||||
logger.debug("%s error: %s", tx_type, exc)
|
||||
# Same reasoning, keyed on the exception type: a connection that is
|
||||
# refused for the whole run says it once rather than per transaction.
|
||||
_log_first_failure("exc:%s" % type(exc).__name__, "%s error: %s", tx_type, exc)
|
||||
|
||||
|
||||
async def _refresh_sequences(
|
||||
|
||||
Reference in New Issue
Block a user