Brings the MetricsRegistry split onto this branch. The pipeline half is now
xrpl::telemetry::MetricsRegistry in libxrpl; the observable gauges are
xrpl::telemetry::AppMetricGauges in xrpld.
This branch had added its own instrumentation to the pre-split class, so the
merge had to route each addition to the correct half:
- The thirteen gauges added here -- amendment block, cache hit-rate detail,
clock skew, job-queue saturation, ledger quorum publish, peer ledger supply,
rotation state, slot census, stall events, sync acquire, sync state, UNL
quorum, and the cache lock-hold observer -- all land on AppMetricGauges,
reading the core's meter and validation tracker through it.
- The pipeline additions stay in libxrpl: the consensus round-duration and
rotation-phase histogram views, the malloc-trim and dns/dial latency bucket
ladders, the job-stall counter, and the switch from literal metric names to
the MetricNames.h constants.
Git detected the pre-split MetricsRegistry.cpp and .h as renames of the gauge
files, so both sides' pipeline changes initially landed in the gauge half. They
were moved back, and the result was audited by inventory: every method
definition, instrument creation, view registration, and emitted string from
either side is present, with identical multiplicity.
MetricNames.h moves to include/xrpl/telemetry/ alongside the core. It has no
includes of its own and its two sibling name headers already live there, so
keeping it under src/ would leave an xrpld path in libxrpl's dependency
surface. Nineteen files follow it.
incrementStateChanges() stays removed. The labelled state_changes_total{from,to}
counter this branch introduced replaces it, and the test asserting the method is
absent is kept -- an unlabelled instrument alongside the labelled one would give
Prometheus two conflicting versions of one metric name.
Two tests that drove startAsyncGauges() against a mock ServiceRegistry are
dropped: xrpl_tests links only xrpl.libxrpl and cannot reach the gauge class.
Levelization regenerated. Both xrpld.telemetry loops become bidirectional
rather than one-way; neither is new.
MetricsRegistry did two jobs. It owned the OTel metrics pipeline, and it
registered the observable gauges whose callbacks read live application
services. The second job is what made the whole class xrpld-tier, so the
pipeline's lifecycle -- the recording() gate and the stop() teardown that
closes a use-after-free window -- could not be unit-tested in xrpl_tests.
Split it in two:
- xrpl::telemetry::MetricsRegistry (libxrpl) owns the exporter, provider,
meter, the 16 synchronous instruments, recording(), stop(), and the
record*/increment* methods.
- xrpl::telemetry::AppMetricGauges (xrpld) owns the 19 observable gauges
and their callbacks, holding a reference to the core and to the
ServiceRegistry.
MetricMacros.h and ValidationTracker move with the core. The macros need
only recording() and meter(), both core members; the core holds a tracker
by value, and a libxrpl header cannot include one from src/.
ApplicationImp owns both objects and sequences them. The core is built in
the member-init list, so every synchronous instrument exists before any
subsystem can record one. The gauges are armed once overlay_ exists, the
last service their callbacks read. Shutdown detaches the gauge callbacks
before the core drops the provider, and each shutdown step is isolated so
a failure in one cannot skip the others.
That detach call is new. detachCallbacks() had no callers, and the flag it
sets is read by the gauge callbacks but can no longer be written by the
core, so the caller now has to make the ordering explicit.
The telemetry module links xrpl.libxrpl.core and xrpl.libxrpl.protocol
PUBLIC: ValidationTracker.h takes a LedgerIndex and MetricMacros.h takes a
ServiceRegistry, both in interfaces a consumer compiles against.
Adds a MetricsRegistry gtest that drives an enabled core with telemetry on
and pins the recording() gate, stop() leaving the registry inert, and
stop() being idempotent. The libxrpl test tree no longer depends on
xrpld.telemetry at all, and the two CMake workarounds that compiled xrpld
sources into xrpl_tests are gone.
Documentation and dashboard source links follow the code to their new
paths, split between the two classes by which one now defines each metric.
The reference doc, span-harness notes and histogram-bucket comments
named the internal AWS dev box and dates while explaining why the
rotation phases are timed. Reword to the general mechanism (a
multi-second freeze at the copy-walk to freshen boundary on a populated
node); the specific hosts, dates and trace ids stay in the task notes.
Resolutions:
- MetricsRegistry.cpp: keep both <exception> and <limits>; drop
incrementStateChanges(), which this branch removed on purpose (the
labelled state_changes_total call site in NetworkOPsImp::setMode
replaces it, and a compile-time test guards that).
- tests/MetricsRegistry.cpp: constructor-built pipeline wording from
phase-10, this branch's test list and gauge paragraphs kept; the two
lifecycle tests now call startAsyncGauges() and pass kTestOptions.
- tests/MetricMacros.cpp: comments name the recording() gate.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Adds `consensus_view_change_total{consensus_mode}` and a `view.change` event
on the round span, both fired from `RCLConsensus::Adaptor::getPrevLedger`
on the same transition-into-WrongLedger edge that already calls
`consensusViewChange()`. The counter is the exact detector for
"consensus disagreed with this node's view this minute"; the event lands the
disagreement on the same trace that carries the round.
`net_ledger_prefix` (16 hex chars) joins `prev_ledger_prefix` on the event,
so a Tempo view of one flap shows both ledger identities on a single line.
`consensus_mode` labels the mode being left (never WrongLedger itself).
Together with the rotation-phase spans, this closes the proof chain a
rotation-driven `full`->`syncing` flap needs — the runbook's step 5
resolves now that the counter and event exist.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three conflicts, all from both branches editing the same passage:
- Main.cpp: kept phase-9's wording. The metrics registry only exists here, so
"unwinding destroys little: the metrics registry, whose destructor joins its
export thread" is the true statement on this branch.
- TESTING.md: kept both paragraphs. They document different things (the
private [network_id], and the log path plus log_level).
- 05-configuration-reference.md: composed both. The identity is now resolved
before construction and never empty, so every producer stamps the node key
from the start; the only divergence left is a wallet that already holds a
different key, which corrects the tracer alone. Rewrote the earlier
"three producers" blockquote too: its "no fallback", "first boot ... left
off" and "Known issue" claims are what this change removes.
One silent break the merge could not flag: makeMetricsRegistryOptions() took
the std::optional<std::string> node key that used to be a constructor
parameter, and that parameter is now the resolved keypair. It takes the base58
string directly, and the constructor derives it from nodeIdentity_, which is
declared before both telemetry_ and metricsRegistry_.
resolveNodePublicKey() returned std::nullopt in three real cases: a first boot
with no wallet database, a standalone run (its wallet is a private temporary
database), and --newnodeid. Telemetry's resources are built during
ApplicationImp's member-init list and are immutable, so on those runs the node
reported an empty service.instance.id and no xrpl.node.id for the whole run,
while setup() minted a key moments later and patched only the tracer.
Replace it with resolveNodeIdentity(), which always returns a keypair: derived
from a configured seed, else read from an existing wallet database, else
minted. Main.cpp passes that pair to makeApplication(), ApplicationImp stores
it in nodeIdentity_ -- now declared before telemetry_ and no longer an optional,
because it is always set -- and builds the telemetry resource from it.
setup() calls getNodeIdentity(), which now persists rather than mints: it
stores the resolved pair when the wallet holds no identity, adopts the stored
one when it does, and clears first for --newnodeid. The write stays in setup()
because that is where the database exists; a standalone run has no persistent
wallet to write to, which is why the pair has to be decided before
construction rather than read back afterwards. Wallet gains storeNodeIdentity()
for that write, and getNodeIdentity(session) now uses it instead of repeating
the insert.
The three-argument makeApplication() mints a keypair, so jtx::Env and any other
test Application behave as a standalone run always did.
Also fold the three hand-rolled "meter from a NoopMeterProvider" copies into
telemetry::noopMeter(): the base-pointer call and the kMeterVersion argument are
both easy to get wrong alone, and the meter identity has to match the one the
histogram views select on.
The new gtest covers the wallet half: store-then-read, store not replacing an
existing identity, clear-then-store, and that the mint path persists. It adds
the tests.libxrpl > xrpl.rdb levelization edge, regenerated here.
MetricsRegistry created its provider and synchronous instruments in start(),
called from setup() after the node identity was read. Every XRPL_METRIC_*
call site creates its instrument on first use, so any site that ran before
that point found no meter and never recorded again. The start was moved three
times to chase the newest early caller; nothing guaranteed the order.
Build the pipeline in the constructor instead. ApplicationImp declares
metricsRegistry_ right after telemetry_ and before every subsystem, so
declaration order now guarantees the instruments exist before any producer.
start() is gone and its config parsing moves to makeMetricsRegistryOptions().
The registry now guarantees a meter whenever it is enabled: the real one, or
the OTel no-op meter if the pipeline failed to build. disablePipeline() owns
that fallback and its one error log, and the constructor routes both
std::exception and a non-std throw through it, because the SDK is third-party
code. So the macros shrink to one function-local static built from meter()
plus the record call: no once-flag, no null check, and no path for a call
that arrives before the meter, because that state no longer exists. The three
observable macros drop the same now-dead meter check.
The lifecycle is three explicit phases with a Phase enum: Ready at
construction, GaugesArmed by startAsyncGauges() once overlay_ exists, and
Stopped by stop(). startAsyncGauges() checks the phase before the pipeline,
so a second call and a call after stop() are each reported as what they are.
run() stops both observers (insight collector, registry) before any service,
and ~ApplicationImp repeats the stop for the setup() failure paths that never
reach run(). stop() already detaches the callbacks, so it is the only call.
Meter name and version come from kMeterName/kMeterVersion, and the endpoint
default from Telemetry::Setup, so the two metric pipelines share one source
for both.
Grafana keys a dashboard by uid, so the Phase 9 and Phase 11 rows both
claiming `validator-health` meant one would silently overwrite the other.
Phase11_taskList.md already requires `validator-health-external`; the
reference table now agrees with it, and says why.
Grafana reaches the image renderer over the compose network at
http://renderer:8081, so the host publish gave nothing the stack needs.
AUTH_TOKEN is the only guard on the endpoint and its default is a fixed
string in this file.
Update the service table in the configuration reference to match.
Four doc conflicts, all where this branch had rewritten a passage that upstream
had only renamed. This branch's text is kept in seven of the eight hunks and
the spanmetrics -> span_metrics and otlp/tempo -> otlp_grpc/tempo spellings
carried into it, so the rewrite is not lost and the names stay current.
The exception is the TESTING.md span-call-count comment, where the incoming
side is the fuller text: it explains that the span_ prefix comes from the
connector's namespace setting. That side is taken.
Metric names are untouched — span_calls_total and traces_span_metrics_* are
produced by the connector's namespace, not by its component name.
Two conflicts, both where this branch's log-pipeline additions sat next to the
upstream spanmetrics -> span_metrics rename: the config header comment, which
this branch extended with a logs line, and the integration test, where the
log-correlation step precedes the span-metrics step. This branch's content is
kept in both and the rename carried into it.
Four conflicts, all where this branch's replacement of the StatsD path with
native OTLP met the upstream spanmetrics -> span_metrics rename. This branch's
design wins in every case; the rename is carried into its text rather than
reverting it, so the connector, its pipeline references, the header comment,
the TESTING.md summary and the runbook all use span_metrics while keeping the
native-OTLP wording.
One addition beyond a straight take-a-side: publish the collector's health
check port. This branch restored the health_check extension and its own
TESTING.md polls http://localhost:13133/ to decide the collector is ready, but
the port was never published on this side of docker-compose.yml, so that check
could not pass from the host. Verified the merged config loads with no
deprecation warnings and that 13133 is published exactly once.
No metric name changed: traces_span_metrics_* already read that way before the
rename, which only ever touched component names and prose.
Conflict in docker/telemetry/otel-collector-config.yaml, in the service
pipelines: this branch renamed the deprecated spanmetrics connector to
span_metrics and adds the statsd metrics pipeline, while upstream renamed the
deprecated otlp exporter to otlp_grpc. Both kept.
With both renames present the collector now starts with no deprecation
warnings at all, which was the point of the pair.
Conflict in docker/telemetry/otel-collector-config.yaml, in the traces
pipeline: this branch added the attributes/hash processor while upstream
renamed the deprecated otlp exporter to otlp_grpc. Both sides kept — the
processor list keeps attributes/hash and the exporter list takes the new name.
Checked that the exporter definition key was renamed to match the reference,
and that the collector still loads the merged config.
The three tracker files conflicted because both sides had rewritten them. Took
the incoming lock-free class, then put this branch's two monotonic accessors
back on top of it: totalAgreementsEver() and totalMissedEver(), backed by a
gross pair incremented at first classification and left alone by the repair
branch. MetricsRegistry reads both, so dropping them would not compile.
The test file kept the incoming suite, which renames every case, and gained
this branch's two gross-counter cases adapted to the injected clock.
The pinned collector warns on every start that "spanmetrics" is a deprecated
alias for "span_metrics". Rename the connector, its pipeline references and
the prose that names it.
The derived metric names are untouched. They come from the connector's
`namespace` setting rather than its component name, so occurrences inside a
metric name such as traces_spanmetrics_calls_total are deliberately left as
they are; renaming those would break every span panel. The rename is applied
only to the bare word, never where it is joined to a metric name by
underscores.
This branch introduces the connector, so the change belongs here.
The pinned collector warns on every start that "otlp" is a deprecated alias
for "otlp_grpc". Rename the trace exporter to otlp_grpc/tempo.
Only the exporter is affected. The otlp RECEIVER keeps its name: it serves
both gRPC and HTTP under one component and is not deprecated, verified by
renaming the exporter alone and seeing the warning stop.
This belongs on this branch because it introduces the exporter, and it is the
last of four deprecated aliases in the collector config; the other three are
owned by later branches in the chain.
Alloy carried no log pipeline at all: no loki.source, loki.write or
loki.process anywhere in the config, against a working metrics and OTLP path.
Any deployment fed through Alloy rather than the reference collector therefore
sent traces and metrics but no logs, so log-to-trace correlation was
unavailable there even though both ends of the link were configured.
Logs now leave through the same otlphttp exporter as the other two signals, so
all three carry one resource identity. Alloy's own filelog receiver would have
been the closest match to the reference collector, but it is still
public-preview and refuses to load unless the service is started with
--stability.level=public-preview, which would mean editing the unit on a box
reachable only by RunCommand. The Loki source components are generally
available, so they are used and bridged into OTLP by otelcol.receiver.loki;
the service needs no extra flag.
That bridge hands over an empty resource and puts everything on the log
record, so the transform sets the resource attributes in log context.
service.instance.id is concatenated in from XRPLD_HOST_LABEL because OTTL has
no env() converter, and it is a resource attribute rather than a record one
because only resource attributes are promoted to indexed Loki labels. It must
equal the node's own service_instance_id or the logs join nothing. devnet
writes one flat file rather than a per-node directory, so identity cannot be
read off the path the way the docker collector does it; XRPLD_LOG_GLOB
overrides the path for other layouts.
The line is parsed for its own timestamp, severity and trace context, and
trace_id/span_id are set on the first-class OTLP record fields so Grafana
links a log to its trace without re-parsing the body. Lines emitted outside a
sampled span keep an empty trace id rather than an invalid one.
Also carry the node-identity operators into the Grafana Cloud collector
variant, align this config's log directory with its service_instance_id, and
rename the deprecated otlphttp/filelog collector component names. Alloy's
otelcol.exporter.otlphttp is that product's own name and is unchanged.
Addresses the open review findings on this branch.
The log root was never delivered at all. Docker creates a missing bind-mount
source as root, Config::getDebugLogFile() only warns when it cannot create the
network subdirectory inside it, and Application carries on. The node therefore
looked healthy while writing no debug.log, and Loki stayed empty with no error
at any layer. docker/telemetry/data/logs has in fact been root-owned in a
working checkout since it was first created. A one-shot xrpld-logdir-init
service now creates the directory and hands it to XRPLD_UID/XRPLD_GID,
following the pattern the storage-init service already uses.
Ingested logs carried no node identity, so a multi-node stack collapsed into
one indistinguishable stream while every dashboard filters on
service_instance_id. The receiver now sets include_file_path and lifts the
per-node directory onto the resource attribute service.instance.id, which is
on the allow-list Loki promotes to an indexed stream label. A record attribute
would only become structured metadata and could not be used in a selector.
For that to join anything the directory name has to equal the emitter's
service_instance_id, so the node directories are renamed to match: node$i
becomes Node-$i, and the standalone config writes to logs/xrpld-standalone.
The integration test aborted before reporting. Under set -o pipefail the
grep | head -1 pipeline is killed by SIGPIPE once the log exceeds the pipe
buffer, so the run exited 141 somewhere past a few hundred matching lines and
read as a flaky test. grep -m1 stops on its own. The test also verified the
local file and Tempo but never that a line reached Loki, which is the one hop
this branch adds, so a bounded Loki assertion is added alongside a readiness
wait.
Documentation fixes: the Tempo cross-check counted .data, but Tempo returns
OTLP shape so the array is batches and one trace can span several; the Loki
step used the instant /query endpoint, which rejects a bare log selector with
HTTP 400 and a text/plain body, so jq could never parse it and the step never
printed a number even when ingestion worked. The filelog comment claimed six
fractional digits where the node always emits nine. The two flowcharts used
<br/>, carried no legend, and advertised GetSpan(), which Log.cpp deliberately
avoids in favour of reading the thread-local context directly.
Finally, rename the deprecated collector component names: the pinned
collector warns on every start that otlphttp and filelog are aliases for
otlp_http and file_log. Alloy's otelcol.exporter.otlphttp and
otelcol.receiver.filelog are that product's own component names and are not
deprecated, so they are left alone.
The workload README contradicted itself on --skip-loki: one bullet said CI always
passes it and so the two log-correlation checks are never exercised, another said
the workflow no longer passes it. The workflow mentions the flag nowhere, so the
first was the stale half.
Other claims checked against the tree and corrected:
- both the README and the plan doc described the push trigger as filtered on
branch names. The workflow has no branches filter, deliberately, because
GitHub ANDs branches with paths
- the plan doc printed 6 of the workflow's 12 paths globs, and claimed the
workflow was 367 lines against an actual 451. The glob block is now generated
from the workflow, and the line count dropped rather than restated
- rpcNOT_SUPPORTED does not exist anywhere in the tree. The symbol is
RpcNotSupported, and the refusal sites are RipplePathFind.cpp:59-60 and
PathFind.cpp:50-51, not :48-49 and :39
- RCLConsensus.cpp:666 and :663 are not log or event lines; the tx.included event
is at :720 and the per-transaction debug log at :715
- LedgerMaster.cpp:463 is fixIndex, not the ledger.store span, which is at :470
- ServerHandler.cpp:705 is inside makeJsonError; processRequest is at :718
- file counts: docker/telemetry/workload/ is 25 files, include/xrpl/telemetry/ 13
- the optional-span bullet named five causes covering 10 of 16 entries, omitting
the txq.* family and the WebSocket handshake
- the /api/v1/series choice was attributed to stale StatsD gauges; this harness
runs no StatsD
A line number in run-full-validation.sh was cited in five places and drifts on
every edit to that file, so those now name the file only. The keygen helper's
header records what production does instead -- validator-keys-tool create_keys
then create_token, keeping the master key off the node -- and why a disposable
cluster does not.
Every citation below was checked against the file it names:
- LedgerMaster.cpp:463 is fixIndex, not the ledger.store span; that guard is at
:470 and the insert it wraps at :476
- LedgerMaster.cpp:987 is the tvc assignment, which sits BEFORE the tvc < minVal
return at :988; the ledger.validate span opens at :1003
- ServerHandler.cpp:705 is inside makeJsonError; processRequest is at :718
- docker-compose.yml:71 and :75 are comments in the collector's volume block;
the loki service is at :112 and its config command at :116
Two claims were also wrong rather than merely stale. Log-trace correlation is
gated in CI, because the workflow passes no --skip-loki, and the separate check
in integration-test.sh is run by no workflow at all. The Loki label note
described the Grafana Cloud collector config rather than the local one: only the
cloud variant sets job=xrpld, and the local config's own comment says to select
on service_name. The dashboards carry 35 Loki queries, not 38.
One conflicted file, docs/telemetry-runbook.md, with three spots:
- Build section: both sides added different text at one point. Kept both,
incoming sentence first, then this branch's "Run against a live network".
- Disabling section, first spot: this branch's wording names the config
section and says no rebuild is needed, so it already covers the incoming
sentence.
- Disabling section, second spot: kept this branch's paragraph and folded in
the one point it lacked, that both flags have to be passed.
Three conflicts, all resolved by keeping this branch's rewrite and
re-applying the incoming change onto it:
- 09-data-collection-reference.md: phase-7 rewrote both attribute tables,
so the incoming table would have reverted them. Kept phase-7's and
re-applied the two "XRPL epoch" spellings.
- integration-test.sh: phase-7 moved these checks from StatsD to OTel and
no longer defines check_statsd_metric, so only this side compiles.
- TelemetryConfig.cpp: the incoming side carried networkTypeFromId(), which
this branch already has. Kept one definition and took the incoming
doc wording, which the auto-merged body below it already matches.
The rename script rewrites "Ripple epoch" to "XRPL epoch", so the old
spelling in a tracked .md makes the check-rename job fail on a dirty tree.
The attribute keys are left alone: the script's pattern needs a space, and
those keys are a cross-layer contract.
The rename script rewrites "Ripple epoch" to "XRPL epoch", so the old
spelling in a tracked .md makes the check-rename job fail on a dirty tree.
The attribute keys are left alone: the script's pattern needs a space, and
those keys are a cross-layer contract.
The rename script rewrites "Ripple epoch" to "XRPL epoch", so the old
spelling in a tracked .md makes the check-rename job fail on a dirty tree.
The attribute key close_time_ripple_epoch_s is left alone: the script's
pattern needs a space, and that key is a cross-layer contract.
The parser now maps the value onto ConsensusTraceStrategy and rejects any
other spelling, so the runbook note and the Phase 4 task-list bullet both
claimed a gap that no longer exists.
