Conflicts, all resolved by keeping phase-9's rewritten text and applying
the upstream change to it:
- PathRequest.cpp: drop the Redaction.h include, keep MetricMacros.h.
- XrplCore.cmake: keep the PUBLIC-link comment, note the SField table use.
- OpenTelemetryPlan.md, 02-design-decisions.md, 05-configuration-reference.md,
telemetry-runbook.md: account addresses are emitted raw; no hashing at
any layer; tx.process lists the per-role account attributes.
Conflicts in src/libxrpl/telemetry/TelemetryConfig.cpp and its test:
phase-5's "require an https endpoint whenever use_tls=1" met phase-7's
version of the same guard, which already covers both the traces and the
metrics endpoint. Kept phase-7's side.
A transaction names one or more accounts: the sender in Account, and by
type a Destination, Owner, Issuer, Holder and so on. tx.process now
carries each top-level account-typed field as its own attribute, keyed
tx_<field> in lower snake case (tx_account, tx_destination, ...), so an
account can be searched for in traces whatever role it played.
Addresses are public ledger identifiers and are emitted raw.
The keys live in TxAccountSpanNames.h in libxrpl, with a field-to-key
table in TxAccountSpanNames.cpp. A library test walks TxFormats and the
SField registry: every account field a transaction can carry has a key,
and no field that only ledger entries carry has one. An empty account
field is skipped rather than rendered as the zero address.
Rewrite the privacy policy (design decisions section 2.4.4) and the two
plan summaries that still described account hashing and a configurable
redaction. Add the two account attributes to the path-finding attribute
table and describe pathfind_dest_currency as the rendered asset.
The $xrpl_work_item variable's description carried a real ticket id as its
example. The filter needs no example, so the id is gone and the wording stays.
StatsDCollector test kept both sides: this branch's onCollectionReady() call,
which enables polling and only exists from here on, followed by the upstream
branch's control assertion that reads the resulting datagram.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both doc indexes kept this branch's 09-data-collection-reference.md rows,
which only exist here, and dropped every secure-OTel.md reference because
the upstream branch removed that file. No dangling link remains.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SpanGuardScope.cpp kept both includes: each side added one and both symbols
are used in the merged test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
unl_expiry_days subtracted two NetClock time points, whose rep is uint32_t, so
the subtraction wrapped before the duration_cast ran. A list expired by one day
read about +49709 days. The panel is green above 30 while its own description
promises red at expiry, so an expired validator list rendered healthy.
daysUntil() widens both endpoints to int64_t first, which makes the wrap
impossible rather than checked for. It deliberately does not clamp at zero: a
negative reading is the signal that expiry has passed. A config-listed list,
which uses time_point::max(), now reports positive infinity, because any finite
sentinel could not be told apart from the wrap this removes. -1 keeps its
existing meaning of no published list fetched.
The sweep counter told a second story it could not support. It counted every
entry the 1-minute sweep evicted, including acquisitions that had already
completed or failed and were merely still in the map. Those were counted when
they ended, so the metric buried the wasteful case in ordinary cleanup while
the runbook, the reference doc and the panel description all described only the
unfinished population. It now counts what those three already claimed.
isComplete()/isFailed() are used rather than isDone(), which is protected on
TimeoutCounter and not callable here.
The document is internal and is kept outside the repo instead.
This also removes every link to it: the document-index rows in
OpenTelemetryPlan.md and 08-appendix.md, its node, edge and style lines in the
plan's Mermaid map, its own section in OpenTelemetryPlan.md, and the two
cross-reference notes in 02-design-decisions.md and 05-configuration-reference.md.
The steps described setting a suppressed attribute on a dropped duplicate, in
the pre-rename dotted spelling. The span is now created after the duplicate
check, so there is no span on that path in either spelling.
The attribute table conflicted because this branch had added the fee and
sequence rows and widened tx_type's set-on list. Kept those, and dropped the
suppressed row the incoming side removed.
tx.receive no longer sets it, and it is no longer a spanmetrics dimension, so
the attribute tables, the span-scope list and the Prometheus label list all
named something that is not emitted. The reduce-relay suppressed_peers gauge is
a different thing and stays.
The steps described setting a suppressed attribute on a dropped duplicate. The
span is now created after the duplicate check, so there is no span on that path.
The tx.receive span is created after the duplicate check, so it carries no
suppressed attribute and a dropped copy produces no span at all. How many were
dropped is the transactions_duplicate traffic category.
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.
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.
