mirror of
https://github.com/XRPLF/rippled.git
synced 2026-08-21 06:10:58 +00:00
fix(telemetry): emit valid JSON for sub-1 TPS, and stop double-reporting a span
Two defects reported against the harness, both confirmed. The TPS field was computed with `bc` at scale=2, and bc omits the leading zero: it prints ".25", not "0.25". A bare ".25" is not valid JSON, and this was the normal case rather than an edge case — ledgers close every few seconds, so ledger-advance over elapsed-seconds is well under 1 for any realistic window. It survived earlier checks because those piped the file through jq, which accepts the malformed form; Python's json rejects the whole file. awk's %.2f always pads, so the field is now produced with awk. Audited the other numeric fields at the same time: CPU average and memory peak already used awk, and the p99, sample count and consensus mean are integers, so TPS was the only one affected. Separately, a failing attribute fetch was reported under the span's own check name, which had already recorded the trace as found. That produced two entries for one name, one passing and one failing, inflating the check total and blaming the trace-existence check for a failure in a later network call. The fetch now carries its own error handling and reports under `span.attrs.<span>`, matching where its successful counterpart reports. It moved into a helper rather than growing `validate_spans`, which was already well over the line limit.
This commit is contained in:
@@ -282,7 +282,14 @@ LEDGER_ADVANCE=$((FINAL_SEQ - INITIAL_SEQ))
|
||||
if [ "$ELAPSED" -gt 0 ] && [ "$LEDGER_ADVANCE" -gt 0 ]; then
|
||||
# Rough TPS: assume ~avg_txs_per_ledger * ledgers / elapsed.
|
||||
# Without tx count, use ledger close rate as proxy.
|
||||
TPS=$(echo "scale=2; $LEDGER_ADVANCE / $ELAPSED" | bc 2>/dev/null || echo "0")
|
||||
#
|
||||
# awk rather than bc, because bc omits the leading zero: `scale=2` prints
|
||||
# ".25", not "0.25", and a bare ".25" is not valid JSON. A value below 1 is
|
||||
# the normal case here, not an edge case — ledgers close every few seconds,
|
||||
# so advance/elapsed is well under 1 for any realistic window. jq happens
|
||||
# to accept the malformed form, which is why it survived earlier checks,
|
||||
# but a strict parser rejects the whole file. awk's %.2f always pads.
|
||||
TPS=$(awk -v a="$LEDGER_ADVANCE" -v b="$ELAPSED" 'BEGIN { printf "%.2f", a / b }')
|
||||
else
|
||||
TPS="0"
|
||||
fi
|
||||
|
||||
@@ -399,10 +399,9 @@ async def validate_spans(
|
||||
|
||||
# Validate required attributes on first trace.
|
||||
if count > 0 and span_def.get("required_attributes"):
|
||||
trace_id = traces[0].get("traceID", "")
|
||||
if trace_id:
|
||||
spans = await _tempo_get_trace(session, tempo_url, trace_id)
|
||||
await _validate_span_attributes_otlp(spans, span_def, report)
|
||||
await _check_attributes_on_first_trace(
|
||||
session, tempo_url, traces, span_def, report
|
||||
)
|
||||
except Exception as exc:
|
||||
report.add(
|
||||
CheckResult(
|
||||
@@ -429,6 +428,47 @@ async def validate_spans(
|
||||
await _validate_parent_child(session, tempo_url, rel, report)
|
||||
|
||||
|
||||
async def _check_attributes_on_first_trace(
|
||||
session: aiohttp.ClientSession,
|
||||
tempo_url: str,
|
||||
traces: list[dict[str, Any]],
|
||||
span_def: dict[str, Any],
|
||||
report: ValidationReport,
|
||||
) -> None:
|
||||
"""Fetch the first trace and check the span's required attributes.
|
||||
|
||||
Fetching the trace is a second network call, so it carries its own error
|
||||
handling. Letting it fall through to the caller's handler would add a
|
||||
second result under the span's own check name, which has already recorded
|
||||
the trace as found -- one entry passing and one failing for the same name,
|
||||
inflating the check total and blaming the trace-existence check for an
|
||||
attribute-fetch failure.
|
||||
|
||||
Args:
|
||||
session: aiohttp client session.
|
||||
tempo_url: Base URL for the Tempo API.
|
||||
traces: Traces returned for this span, most recent first.
|
||||
span_def: The span's entry from expected_spans.json.
|
||||
report: ValidationReport to accumulate results.
|
||||
"""
|
||||
span_name = span_def["name"]
|
||||
try:
|
||||
trace_id = traces[0].get("traceID", "")
|
||||
if not trace_id:
|
||||
return
|
||||
spans = await _tempo_get_trace(session, tempo_url, trace_id)
|
||||
await _validate_span_attributes_otlp(spans, span_def, report)
|
||||
except Exception as exc:
|
||||
report.add(
|
||||
CheckResult(
|
||||
name=f"span.attrs.{span_name}",
|
||||
category="span",
|
||||
passed=False,
|
||||
message=f"{span_name}: attribute check failed ({exc})",
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
async def _validate_span_attributes_otlp(
|
||||
spans: list[dict[str, Any]],
|
||||
span_def: dict[str, Any],
|
||||
|
||||
Reference in New Issue
Block a user