initMetrics() held its own copy of the suffix-swap: take tracesEndpoint,
replace a trailing /v1/traces with /v1/metrics. Any other URL shape sent
metrics to the traces path. metrics_endpoint is now a config key of its
own, so use it as given.
Application::startTelemetry() still reads the key itself for
MetricsRegistry, which builds its own exporter. Telemetry exposes no
accessor for the Setup it parsed, and re-parsing would re-run the mTLS
validation and cert-file checks at a later point in startup, so the
second read stays. The comment above it no longer claims the URL is
derived from the traces one.
Also corrects 05-configuration-reference.md: the sample configs carry
metrics_endpoint where exporter used to sit, not a comment.
Telemetry.cpp conflicted. Phase-9 rewrote the metrics pipeline into
makeTracerResource()/makeMetricsResource()/initMetrics() further down the
class, so its side of the region is empty and phase-8's private helper
block does not apply. Resolved to phase-9's structure; phase-8's own
hunks outside the region (the deleted kTracesPath/kMetricsPath, the
verbatim traces URL, the two-endpoint startup log) merged in.
Phase-9's initMetrics() still derives the metrics URL by suffix-swap.
That is fixed in the next commit, not here.
One [telemetry] key served both OTLP signals, and the metrics URL was
derived from it by suffix-swap: strip a trailing slash, strip a known
signal path if present, append the wanted one. Anything not ending
/v1/traces therefore posted metrics to the traces path, and the OTLP
version was pinned in code where an operator could not reach it.
Adds metrics_endpoint alongside traces_endpoint. Both are full URLs used
verbatim, so traces and metrics can go to different collectors, or to one
whose OTLP paths are not the defaults. signalEndpoint(), kTracesPath and
kMetricsPath are gone; nothing derives an endpoint from another.
The startup log names both URLs, since with two independent endpoints
there was otherwise no way to see where metrics were going.
Also drops exporter=otlp_http from the shipped config and the test
fixture. No branch in the chain reads an `exporter` key: it was a real
Setup member in the first phase-1b implementation, removed when only
OTLP/HTTP was wired up, and already deleted from TESTING.md once on the
same grounds.
Three conflicts, all between this branch's own sync-diagnostics work and
phase-10's older versions. Resolved to this branch in each case, since it
owns the newer content:
- InboundLedger.h keeps the missing-node and receive-depth gauges and the
fuller acquire-span contract.
- MetricsRegistry.cpp keeps the namespaced label:: constants.
- LedgerMaster.cpp keeps makeLedgerTraceSpan(), which joins the store and
validate spans into one per-ledger trace by hash.
LedgerMaster.cpp needed a second pass. The automatic merge had kept both
sides outside the conflict markers, nesting phase-10's older promotion
block inside this branch's `if (!pubLedger_)` — so setValidated,
setFull and setValidLedger would each have run twice. Taking this
branch's file wholesale removes the duplicate; brace balance and a single
"Advancing accepted ledger" confirm it.
That resolution drops two things phase-10 was carrying into this file:
the storeSpan/validateSpan guard names, and the explicit scope that keeps
the one-in-256 flag-ledger check outside the ledger.validate measurement.
Both are re-applied on this branch in the next commit; the scope needs a
variable-lifetime check that does not belong in a merge.
Eighteen conflict regions across nine files. Resolved by asking, per
region, which side is the better final state rather than by taking a
branch wholesale.
Telemetry.cpp keeps phase-9's two resource builders. phase-8 offered a
single makeResource() with no node identity; phase-9 splits it into
makeTracerResource() and makeMetricsResource() because the metrics
provider is built in the constructor, before setNodeId() runs, so
xrpl.node.id can only be stamped unconditionally on the tracer side.
Collapsing them would have dropped that attribute, which is what keeps
per-node traces from folding into one identity.
Telemetry.h and the config test compose both sides: phase-9's nodeId
member and its assertion, plus the renamed endpoint.
xrpld-telemetry.cfg keeps phase-9's devnet identity and its
metrics_endpoint, renames the traces key, and drops exporter=otlp_http.
Nothing reads an `exporter` key on any branch in the chain: it was a real
Setup member in the first phase-1b implementation, removed when only
OTLP/HTTP was wired up, and already deleted from TESTING.md once on the
same grounds. The cfg line was the last carrier.
The docs keep phase-9's versions, which are both fuller and more
accurate: the incoming runbook listed the consensus strategy values as
"random" where the code compares against "attribute".
OTelCollector.cpp had five comment-only regions in a file phase-7 owns,
so those take the upstream side.
MetricsRegistry.h's usage example named a member that no longer exists
and the wrong arity; it now matches the real three-argument call and says
where the endpoint comes from.
Ten conflict regions in four files, none of them caused by the rename.
phase-8's own commit had rewritten comments in files phase-7 owns
(Unit.h, HistogramBuckets.h, OTelCollector.cpp) and in Telemetry.cpp,
while the same sweep ran independently on phase-7.
Resolved every region to phase-7's side on ownership grounds: those files
belong to phase-7 or earlier, so a downstream branch should not carry
divergent copies. phase-8 changed comments only in all four, verified
against the merge base, so no code was dropped.
The one structural region: phase-7 had refactored addUnitView from an
inline lambda into a member function, and phase-8 still held the lambda.
Keeping phase-8's would have shadowed the member.
Wording phase-8 had that is worth restoring on phase-7 -- the "legacy"
qualifier on the prefix parameter, and the rejected-alternative note on
the bucket ladders -- is recorded outside the tree for a follow-up.
Three conflicts, all composed rather than resolved by taking a side:
- TelemetryConfig.cpp: phase-6 kept networkTypeFromId file-local with
[[nodiscard]]; phase-7 had relocated it to public scope for
Application.cpp. Kept phase-7's relocation, so one definition remains.
The [[nodiscard]] survives on the declaration in Telemetry.h.
- Telemetry.cpp x2: phase-7 added getMeter overrides, phase-6 added
[[nodiscard]] to the startSpan below them. Kept both, and put
[[nodiscard]] on getMeter too.
- TESTING.md: phase-7 had the right metric name (span_calls_total, which
the spanmetrics namespace produces) but the wrong label. Its
xrpl.rpc.command appears nowhere else in the branch; the attribute is
bare `command`, which is what the dashboards query. Took phase-7's
metric with the correct label.
Both signalEndpoint call sites follow the renamed member. signalEndpoint
itself is left in place: removing it and adding metrics_endpoint is a
design change, not part of propagating a rename.
The rename arrived from phase-1b by merge. Four files still wrote the old
key, which the parser no longer reads, so each would have silently
fallen back to the default collector URL.
integration-test.sh is the load-bearing one: it generates the node config
the test harness starts, so the stale key would have pointed the node at
localhost regardless of the compose network. xrpld-telemetry.cfg is the
standalone node config; the other two document the key.
Note this cfg has a second, divergent variant on the devnet branches that
needs the same fix there.
Conflict in OpenTelemetryPlan/05-configuration-reference.md: phase-5 had
corrected the enabled and use_tls types to 0 or 1 and added the
tls_client_cert and tls_client_key rows, while the incoming side renamed
the endpoint option.
Composed both — phase-5's type corrections and its two mTLS rows are
kept, with the endpoint row renamed to traces_endpoint.
Conflict in OpenTelemetryPlan/05-configuration-reference.md: phase-3 had
widened the options table and added the tx_trace_strategy and
consensus_trace_strategy rows, while the incoming side renamed the
endpoint option.
Composed both — phase-3's wider layout and its two extra rows are kept,
with the endpoint row renamed to traces_endpoint.
The [telemetry] option table documents the config key operators copy.
The key is now traces_endpoint, named for the one OTLP signal it
carries, so the old row pointed at a key the parser no longer reads.
Two prose mentions of "endpoint" further down describe the concept
rather than naming the key, and are left alone.
Conflict in docker/telemetry/integration-test.sh: the incoming side removes
the inert [insight] prefix, and this branch had added service_instance_id
to the same block. Resolved by taking both -- prefix=rippled dropped,
service_instance_id=Node-${i} kept.
OTelCollector routes every instrument name through a static formatName()
that only lowercases the name and maps '.' and space to '_'. The sole read
of prefix_ is the startup log line at OTelCollector.cpp:802, and all four
instrument factories go through formatName(), so no prefix can ever reach
an exported name. StatsDCollector does prepend it, so the StatsD example
keeps the key and now states why.
Covers the three server=otel blocks in the 09 reference and the config
integration-test.sh generates. This branch introduces OTelCollector, so it
is where the inert examples first appear; phase-6's examples are all
server=statsd and stay as they are.
Two conflicts.
RCLConsensus.cpp: upstream restructured makeAcceptSpan so the accept span's
attributes sit behind if (*span). This branch's own contribution there is the
consensus round-duration histogram, which is kept -- placed inside the
telemetry guard but OUTSIDE the span-liveness test, because a metric must
still record when the trace category is disabled or the span was not created.
The duplicated attribute lines on this side are dropped; the guarded block
upstream added supersedes them.
MetricsRegistry.cpp: kept this branch's JobQueue.h include, which it uses.
Its Journal.h include was dropped as a duplicate -- the file already includes
that header higher up, with a comment explaining why it is unguarded, and
readability-duplicate-include is fatal under WarningsAsErrors.
Four conflicts. None was a take-a-side.
xrpld-telemetry.cfg: dropped the incoming [insight] block. This branch already
has one, and duplicate ini sections do not replace each other -- parseIniFile
appends onto the same section, so keys merge last-wins and the effective config
is one that appears nowhere in the file.
src/tests/libxrpl/CMakeLists.txt: kept this branch's else() branch, which
compiles MetricsRegistry.cpp for the telemetry-off build, and dropped only the
ValidationTracker target_sources inside if(telemetry). Upstream relocated that
one out of the guard, so keeping both would have compiled it twice. The
else() branch is this branch's own: the MetricsRegistry test exists only here,
and without its implementation the off build would not link.
RCLConsensus.cpp: union. The metric macros and registry come from this branch,
PropagationHelpers from upstream; all three are used.
PeerImp.cpp: MetricMacros.h stays unguarded, because all seven XRPL_METRIC_*
uses in this file are on unconditional paths and the header is what defines
them. ConsensusReceiveTracing.h takes upstream's guarded placement, and this
branch's guarded GetObjectMetricNames.h is kept beside it.
formatName() never reads prefix, so setting it here does nothing and the
exported names are bare and lowercase. Leaving it invites queries written
against xrpld_jobq_job_count, which match no series.
The StatsD examples keep it, because that path does apply it to the name.
Brings phase-10 up to 3836078a78 (74 commits). Seven conflicts, resolved
per-hunk; no side was taken wholesale.
validate_telemetry.py, four hunks. The module docstring keeps both category
lists, renumbered. _log_prometheus_metric_names takes phase-10's version: it
returns the family list that the new reverse-coverage check consumes, and
_log_name_list prints every family sorted one per line, which supersedes the
hand-maintained prefix filter this branch had been extending -- that filter
existed only to keep the log readable and listed strictly less. validate_metrics
takes phase-10's _metric_check_targets call. assert_sync_diagnostics_metrics and
phase-10's _check_metric_label both landed at the same place; both are kept.
That third hunk carried the hazard this branch had flagged in advance.
_metric_check_targets selects every group satisfying isinstance(dict) and had no
equivalent of SKIPPED_METRIC_GROUPS, because on phase-10 there was no group that
needed excluding. Merged as-is it would have walked sync_diagnostics while
assert_sync_diagnostics_metrics also walks it, polling and reporting all 61
metrics twice. The exclusion is reinstated inside that function, and its
docstring claim that the isinstance test "selects exactly the same groups the
previous name-based exclusion list did" is corrected -- true on phase-10, false
here, and the reason is ownership, which no structural test can express.
expected_metrics.json: both sides appended to metrics_excluded, so both sets are
kept, 29 entries. expected_spans.json: phase-10's fuller pathfind.compute
skip_reason replaces this branch's, and this branch's ledger.acquire ->
ledger.acquire.astree relationship is kept.
Both docs carried stale counts, and the two sides disagreed with each other --
15 dashboards against 16, and both claiming 41 span types when the contract holds
48. Rather than pick a stale side, every figure is recomputed from the resolved
contract: 48 span types as 28 required and 20 optional, 145 metric checks across
26 asserting categories as 140 names plus 5 required_labels, of which 61 are the
sync_diagnostics names, and 16 dashboards, which matches both the uid list and
the files on disk. The runbook keeps phase-10's table, which adds the reverse-
coverage row.
ConsensusSpanNames.h: both sides added different constants to namespace val;
both kept. Confirmed no identifier is redefined -- the merged file's duplicate
set is identical to this branch's, and those duplicates are distinct namespaces
(op::round against the enclosing span::round), not redefinitions.
ConsensusSpanNames.cpp was an add/add: both branches wrote this file
independently, 9 tests here and 8 on phase-10, with no name in common. All 17
are kept. The guard is dropped rather than applied to the union: SpanNames.h
documents that its constants are deliberately NOT guarded by
XRPL_ENABLE_TELEMETRY, ConsensusSpanNames.h has no guard, and the tests this
branch contributed reference ValStatus only in comments while calling
validationStatusValue with plain ints. So they compile without telemetry, and
unguarding them gains coverage in a -Dtelemetry=OFF build rather than losing it.
One defect belongs to the merge itself, appearing on neither parent. phase-10
added a job_queue_per_type_gauges group holding six jobq_<type>_running/_waiting
names; this branch declared jobq_saturation in MetricNames.h. Rule K checks a
name only when its family is owned, so declaring that constant made jobq_ owned
and turned phase-10's six entries into violations. They are beast::insight gauges
created per job type by JobTypeData's constructor, so no constant can exist for
them -- one triple per job type, minted at runtime. The group joins
NON_OTEL_METRIC_GROUPS alongside statsd_gauges for the same reason.
Verification: no conflict markers repo-wide and no unmerged index entries; both
JSON contracts parse; validate_telemetry.py compiles; every count written into
the docs re-derived from the resolved files and matching; span counters still 48
and 74; check_otel_naming.py exits 0, and Rule K proven still able to fail by
injecting a bogus name in an owned family; levelization baseline clean after
regeneration, with 17 incoming include changes; doxygen style clean across all
tracked C++ at CI scope; pre-commit --all-files clean except cargo-fmt, which
reports "Executable `cargo` not found" and touches none of the 0 Rust files here.
NOT compiled -- no approval to build, so the incoming C++ is unverified by a
compiler on this branch.
The two log-correlation checks have never executed in CI: the workflow
hardcoded --skip-loki, so validate_telemetry.py never constructed
log.trace_id_present or log.trace_id_cross_reference. A green Telemetry
Validation therefore carried no evidence that a log line reaches Loki with
trace context. Drop the flag so both checks run and can fail the job.
Correlation spans four independent legs and a failed check names none of
them, so run-full-validation.sh now prints a per-leg diagnostic after the
suite whenever the checks are enabled:
node per-node debug.log line count, the count matching the injected
trace_id/span_id shape, one sample line, and the severity mix,
so "no log at all", "log level too high" and "no active sampled
span" are distinguishable
mount the container-side listing of /var/log/xrpld, taken with the
collector's own mounts and uid. That image is built from
scratch and carries no shell, so the listing runs in a
throwaway container with --volumes-from, not via docker exec
collector the receiver's watched files, logs-pipeline warnings, and the
internal log-record counters, read from inside the container's
network namespace because that endpoint binds to the
container's own localhost and its port is not published
loki the exact query used, the label inventory, and entry counts for
the stream selector with and without the line filter, so "Loki
has nothing" and "Loki has lines but none carry a trace id" are
distinguishable
The diagnostics are non-fatal by construction: every leg runs in its own
subshell with errexit off, each docker and curl call is guarded, and the
coordinator always returns success. Verified with no containers and no Loki
reachable, with an emptied PATH, and with a leg forced to exit non-zero.
validate_telemetry.py gains a matching diagnostic beside the checks,
following _log_prometheus_metric_names: warnings only, never a check
result. Its stream selector and line filter move into module constants
that the shell diagnostic reads back, so the two cannot drift into
describing different queries.
No check was widened or auto-passed, and LOG_QUERY_WINDOW_SECONDS stays at
four hours; a wider window would let a check pass on a previous run's logs.
validate_metrics and validate_spans only ever run one direction: read the
contract, ask the backend whether each listed name exists. Nothing looked the
other way, so a metric family or span name the contract omitted was invisible
by construction. Both emitted inventories were already being fetched for the
CI log and neither was compared back, which is how a 345 family metric gap and
7 unknown span names went unnoticed.
Add two reverse checks, metric.reverse_coverage and span.reverse_coverage.
Each names every emitted family the contract never mentions, sorted, one per
line, with counts in the report details.
Warn only, by design. passed is hardcoded True in a single shared builder, so
an unaccounted name cannot turn CI red: downstream branches legitimately add
telemetry an upstream contract has not seen yet, and a hard failure would
redden all of them for doing the right thing.
Bulk families are accounted for declaratively. A new top level
accounted_patterns list in expected_metrics.json holds anchored regexes with a
written reason each, covering the 105 per job type queue gauges, the 70 per job
type histogram families, the 228 overlay per category traffic families, and the
Prometheus scrape plumbing that is not xrpld telemetry. Job type shapes are
reduced structurally because every job type name lowercases to letters only;
traffic categories are enumerated instead, because they contain underscores and
a structural pattern there would swallow unrelated names. Anything outside
these shapes still surfaces.
Exporter shapes are folded before matching, so a histogram triple is accounted
for by an entry written for its base family and is never reported as three
separate gaps. Spans need no pattern list: the reverse check reuses the same
matcher the forward check uses, so a glob such as rpc.command.* covers every
command it expands to, and an optional entry still counts as known.
Also fix the diagnostic these checks feed on: both emitted lists were logged as
a single Python list repr, about 15 kB on one line for 422 families, unreadable
and impossible to compare between runs. Both now print one name per line.
_metric_check_targets now selects groups by testing that the value is an
object, rather than by excluding two key names, so a non group top level key
cannot break it. Output is byte identical: 79 metric plus 5 label checks, same
names in the same order.
The assert / do-not-assert decisions in expected_metrics.json and
expected_spans.json were all correct, but several recorded reasons were not.
Pathfinding is disabled outright on every harness node: Config.cpp:725-726
zeroes pathSearchMax whenever a [validation_seed] or [validator_token] section
is present, run-full-validation.sh writes [validation_seed] for every node and
has no [path_search] override, and both handlers return rpcNOT_SUPPORTED
before constructing a PathRequest.
- pathfind_full_milliseconds no longer claims a probabilistic path, nor
prescribes an explicit ledger index, which cannot help: the config gate
fires before the ledger parameter is read.
- pathfind_fast_milliseconds keeps its hasCompletion argument but now leads
with the config gate, which is the operative blocker.
- The pathfind.compute and pathfind.discover notes and the
pathfind.request to pathfind.compute skip reason no longer blame missing
liquidity. pathfind.update_all now records why its request list stays empty.
- statsd_gauges states the arming precondition: a beast gauge is only as safe
as an observable gauge when its object exists before Application.cpp:1570,
where onCollectionReady arms the registered gauges exactly once.
- Alert wiring claims softened: every rule in rules.yaml is paused.
- The per job type gauge group loses its bogus poll bandwidth reason, and its
regex claim is corrected: there is no running state regex, so 30 of those
gauges have no consumer at all.
- overlay_peer_disconnects has one query consumer, not two.
- Cloud dashboard copies dropped from consumer counts: that tree is ignored by
git and has no tracked files.
- rpc_method_errored_total explains that a refused RPC is a normal return, not
a throw, so the refusals above do not make it fire.
- 09-data-collection-reference.md no longer claims a Prometheus name query in
expected_metrics.json.
No behaviour change: the flattened check name list is byte identical.
ff8629bb11 dropped prefix=xrpld as inert and misleading, but four OTel-path
sites still set it, so the branch contradicted itself.
OTelCollector routes every instrument name through a static formatName()
that only lowercases and maps '.'/space to '_'; the sole read of prefix_ is
the startup log line at OTelCollector.cpp:810. All four instrument factories
funnel through formatName(), so no prefix can reach an exported name.
StatsDCollector does prepend it (StatsDCollector.cpp:551/592/640/715), so the
StatsD example legitimately keeps it.
Removed from the 09 reference's OTel config block and from both
quick-reference setups, and from the cfg integration-test.sh generates. The
StatsD example is unchanged and now states why it keeps the key.
Also corrected run-full-validation.sh: [insight] endpoint was described as
"already matches the built-in default", implying it would matter if it
differed. CollectorManager reads it and hands it to OTelCollector, which also
only logs it; the exporter URL is built in Telemetry::initMetrics() from
[telemetry] endpoint. It is as inert as prefix was.
Eight attributes added on phase 4 were missing from the attribute catalogue.
Seven are on consensus.phase.open, which had no entries at all; the eighth is
the terminal regime on consensus.establish.
Does not touch the neighbouring proposers_agreed row, which names an attribute
the code never sets -- pre-existing and outside this change.
The Phase 4a span table had no consensus.phase.open row at all and listed
only three attributes for consensus.establish. Add the row, the eight
attributes added on this branch, and the two label-valued ones to the
attribute inventory.
Leaves the pre-existing dotted names in the consensus.round row alone; they
predate the underscore convention and are not part of this change.
A tx-set fetch carried no key tying it to the consensus round that needed
the set, so attributing a stalled fetch to a round meant guessing from
timestamps. One fetch is wanted by many rounds -- it is keyed by set hash,
survives the round sweep, and the round never blocks on it -- so a single
parent, link or attribute cannot describe the relationship.
Instead the fetch span records one timestamped event per requesting round,
carrying the round's parent-ledger hash and the ledger it is building. Both
attribute keys already existed in the shared telemetry namespace with
exactly this meaning, and the existing addEvent API is used as-is, so no
new telemetry surface is added and the whole feature compiles out with
telemetry disabled.
The event fires once per round rather than once per peer proposal, keyed on
the round's parent-ledger hash: that distinguishes rounds started on
different forks at the same height, which a ledger-height compare cannot.
A mid-round wrong-ledger recovery re-enters consensus without re-caching the
round identity, so a fetch begun after that switch is attributed to the
pre-switch round; the limitation is documented where the values are cached.
Also fixes the fetch span's end time, which depended on when the C++ object
was destroyed. Three of the four exits that stop pursuing a fetch -- the set
arriving from elsewhere, the round sweep, and shutdown -- ended the span
only via the destructor, so the recorded duration included however long any
reference happened to be held. Each exit now ends the span itself, plus
cancel() and container teardown, and the destructor asserts the span is
already closed rather than closing it: a fallback that can never legitimately
fire should fail loudly instead of hiding a missed exit. An abandoned fetch
also no longer asks peers for a set nobody wants, which previously led to
charging those peers for answering our own request.
Verification: pre-commit and TIDY=1 clang-tidy pass; levelization is
unchanged. NOT compiled -- the branch is blocked by a gcc-15 internal
compiler error in the unrelated xrpl.libxrpl.rdb unity translation unit.
Runtime behaviour is unasserted: xrpl_tests links only xrpl.libxrpl, so
TransactionAcquire is unreachable from GTest; the added tests cover the new
span-name and attribute constants only.
