Commit Graph

167 Commits

Author SHA1 Message Date
Pratik Mankawde
2969965d8c Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
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.
2026-09-23 18:08:07 +01:00
Pratik Mankawde
1381b8f684 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-09-23 18:06:19 +01:00
Pratik Mankawde
b98b4e0c5a docs(telemetry): Account attributes in the data-collection reference 2026-09-23 18:06:17 +01:00
Pratik Mankawde
964b0d64bf docs(telemetry): drop the example ticket id from the work-item filter
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.
2026-09-23 12:00:37 +01:00
Pratik Mankawde
2d3eb7a981 fix(telemetry): Stop unl_expiry_days wrapping, and count only unfinished sweep evictions
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.
2026-09-22 19:07:41 +01:00
Pratik Mankawde
8fef4b3cc6 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-09-18 10:09:50 +01:00
Pratik Mankawde
8a5e831448 docs(telemetry): drop the suppressed row from the second attribute table
The same attribute was listed in two tables in this file. tx.receive does not
set it, because the span starts after the duplicate check.
2026-09-18 10:09:26 +01:00
Pratik Mankawde
95cc50d43f Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-09-18 10:03:14 +01:00
Pratik Mankawde
4e2b8ce536 merge: bring the plan-doc suppressed cleanup forward from phase-6
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.
2026-09-18 10:02:54 +01:00
Pratik Mankawde
ff44c3c851 docs(telemetry): drop suppressed from the attribute and label references
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.
2026-09-18 09:56:57 +01:00
Pratik Mankawde
073ff1a93f docs(telemetry): drop the suppressed row from the attribute reference
tx.receive does not set it. The span starts after the duplicate check, so a
dropped copy produces no span for the attribute to sit on.
2026-09-18 09:55:45 +01:00
Pratik Mankawde
ec0bfe521d refactor(telemetry): move the metrics pipeline core into libxrpl
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.
2026-09-16 13:45:52 +01:00
Pratik Mankawde
ddff6019f2 docs(telemetry): give the Phase 11 validator board its own dashboard uid
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.
2026-09-10 15:42:16 +01:00
Pratik Mankawde
d141405ff3 merge: bring the lock-free ValidationTracker forward from phase8-log-correlation
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.
2026-09-09 15:59:55 +01:00
Pratik Mankawde
cbb8581997 fix(telemetry): make the log pipeline actually deliver, and fix its docs
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.
2026-09-09 15:11:49 +01:00
Pratik Mankawde
bae31955db merge: bring the CI fixes forward from phase8-log-correlation
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.
2026-09-08 17:22:04 +01:00
Pratik Mankawde
75f2d042f4 merge: bring the CI fixes forward from phase7-native-metrics 2026-09-08 17:19:40 +01:00
Pratik Mankawde
ff1a905ede merge: bring the CI fixes forward from phase6-statsd
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.
2026-09-08 17:17:50 +01:00
Pratik Mankawde
23c1a6f5dd fix(docs): spell it "XRPL epoch" in the data-collection reference
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.
2026-09-08 17:12:55 +01:00
Pratik Mankawde
623e3d8ad3 feat(telemetry): add batch-size and discovered-path histograms
Review feedback asked for a Histogram rather than a span attribute at these two
places. Both, not either: the attribute answers how big one sampled request was,
which an aggregate cannot, and the histogram answers the distribution across all
requests, which an unsampled trace never reveals. Both attributes stay.

The metrics land here rather than with the attributes because neither
HistogramBuckets.h nor the metric macro exists on the branch that added them.

Both use kObjectCountBuckets. The argument is the floor, not the ceiling: the SDK
default edges start 0,5,10,25, so an ordinary batch of one to five falls in a
single bucket and every quantile becomes an interpolation on one edge. The object
ladder puts five edges over the mass of both distributions. Path count is bounded
at 352 by kMaxPaths times kMaxAutoSrcCur and cannot saturate. Batch size can, at
roughly 333k, but no measured traffic goes near it, so the ladder is not widened
for a range nothing occupies; the runbook records the overflow query and a test
asserts it stays readable.
2026-09-07 13:51:44 +01:00
Pratik Mankawde
2b61e763f7 merge: bring the close-time attr doc fixes forward from phase8-log-correlation
The runbook span table, the phase-4 task list and the data reference all
conflicted: this branch had already expanded them with the open-phase,
avalanche and proposal-prefix attributes. Keep this branch's tables and
apply the close-time rename to their rows.

The close_time entry in ledger_history_mismatch_total{reason} is a metric
label value, not the span attribute, and is deliberately unchanged.
2026-09-04 12:43:38 +01:00
Pratik Mankawde
1bee0b1c63 merge: bring the close-time attr doc fixes forward from phase7-native-metrics 2026-09-04 12:42:22 +01:00
Pratik Mankawde
87270ef642 merge: bring the close-time attr doc fixes forward from phase6-statsd
The consensus and ledger attribute tables conflicted: this branch had
already rewritten both, adding the open-phase and avalanche attributes and
correcting tx_count/tx_failed to sit on tx.apply alone. Keep this branch's
tables and apply the close-time rename to their rows, rather than taking
either side whole.
2026-09-04 12:42:08 +01:00
Pratik Mankawde
ca22f57919 docs(telemetry): follow the close-time attr rename in the data reference
The emitted keys carry the unit and epoch suffix. Update the consensus
and ledger attribute tables and the ledger.build span row to match.

The Close Time Drift panel row is left alone: phase-7 removes that whole
table, so editing it here would only conflict on the way forward.
2026-09-04 12:37:19 +01:00
Pratik Mankawde
637f9ae3d0 merge: bring telemetry config and doc fixes forward from phase8-log-correlation
Nine files conflicted. Resolutions, and why:

Telemetry.cpp - upstream carried its own initMetrics(), makeResource() and
makeMetricExporter(); this branch already has an initMetrics() that builds the
exporter inline and uses makeMetricsResource(), which stamps xrpl.node.id only
when it is already known. Keeping both would have defined initMetrics() twice.
Kept this branch's, then pointed its reader at setup_.metricExportInterval and
setup_.metricExportTimeout: upstream turned those constants into [telemetry]
keys and removed the old ones, so the previous spelling no longer resolves.

OTelCollector.h - kept upstream's parameter docs. This branch's text promised
instanceId, serviceName and networkType become resource attributes; the
constructor marks all three [[maybe_unused]] and the .cpp already says they are
not read.

CollectorManager.cpp - kept upstream's comment for the same reason.

node-health.json - kept this branch's 60 panels. Upstream's only change to the
file was job_count to jobq_job_count, which this branch already had.

cfg/xrpld-example.cfg - composed. Kept this branch's warning that
service_instance_id must be set explicitly for the metrics pipeline, took
upstream's traces_endpoint rename, and removed a duplicate metrics_endpoint
entry along with the claim that metrics derive from the traces URL by rewriting
the signal path. Nothing derives it; both metric exporters read
metrics_endpoint. 17 keys, one entry each.

05-configuration-reference.md - both sides misdescribed the parser. Kept this
branch's fuller text, corrected the endpoint default to traces_endpoint, and
replaced the "resolve their URL differently" table with what the code does now.

09-data-collection-reference.md, 06-implementation-phases.md,
Phase7_taskList.md - kept this branch's versions, which drop a metric that was
never implemented, correct the state encoding to 0-6, and rename nudb_bytes to
stored_object_bytes. Re-applied upstream's rpc_requests_total fix, which taking
this side had reverted.
2026-09-03 20:37:19 +01:00
Pratik Mankawde
771ea277a5 merge: bring telemetry config and doc fixes forward from phase7-native-metrics 2026-09-03 20:30:33 +01:00
Pratik Mankawde
aa12aab146 fix(telemetry): honour TLS and configured resource attributes in MetricsRegistry
The native-metrics pipeline built its own OTLP/HTTP exporter and set only the
URL, so an operator who enabled TLS got mutual TLS on the trace exporter and a
plaintext-configured exporter for metrics. cfg/xrpld-example.cfg promises TLS for
"the OTLP exporter connection" with no carve-out, and two exporters exist. The
exporter now reads the same four [telemetry] TLS keys the trace exporter does.

Its resource was also thinner than the trace resource: service.name was
hardcoded, and service.version, xrpl.network.id and xrpl.network.type were
absent. Because the collector promotes resource attributes to labels, an operator
setting service_name split their fleet - spans carried the configured name while
every XRPL_METRIC_ series still said xrpld, blanking native-metric panels in
every dashboard that filters on it. All four now come from config, and
xrpl.network.type is derived inside from network_id through the shared
networkTypeFromId so a caller cannot supply a mismatched pair.

start() and initExporterAndProvider() take a StartOptions aggregate rather than
growing to eleven positional parameters, seven of them same-typed strings where a
swap would compile silently and stamp the wrong label - the defect class this
change exists to remove. It is constructed at one production site, so replacing
it with Telemetry::Setup once that is exposed stays a single-site edit.

service.version, service.instance.id and xrpl.node.id are stamped only when
non-empty, keeping this pipeline's existing behaviour of omitting an attribute
rather than writing it blank.

Also corrects three plan-doc references that cited line numbers rather than
symbols; the reader line moved and the numbers differ per branch.
2026-09-03 20:20:37 +01:00
Pratik Mankawde
f180b70640 fix(telemetry): correct drain, init and naming defects; add export cadence keys
Addresses review findings on the native-metrics work.

StatsDCollector::onTimer drained the send buffer inside the polling_ gate. That
gate holds back hook handlers until the application's services are built, but
sendBuffers() is socket I/O. StatsDEventImpl derives only from EventImpl, so it
never enters metrics_ and posts straight to the buffer; its |ms timings piled up
before onCollectionReady and were dropped after onCollectionStopping. The drain
now runs every tick, and outside metricsLock_, so onCollectionStopping no longer
waits on a UDP flush.

TelemetryImpl's constructor left meterProvider_ set when initMetrics() threw.
initMetrics publishes globally as its last step, so a throw left getMeter()
callers holding a provider nothing else could reach. Reset it in the catch.

~ApplicationImp caught only std::exception around telemetry shutdown while the
callees reach third-party SDK code, so a foreign exception would have terminated
the process. Added a logging catch-all.

ValidationTracker's hard trim evicted by unordered_map bucket order. It now
evicts oldest-first, so the entry dropped under pressure is the one least likely
to still reconcile.

The GetMeter test restored the global meter provider only on the success path,
and ASSERT_TRUE early-returns past it. Uses xrpl::ScopeExit instead.

The hook debounce window is a named constant rather than a bare 500 in a
comparison, and the metric export cadence becomes operator-configurable through
metric_export_interval_ms and metric_export_timeout_ms. Both are range-checked:
the SDK warns and silently substitutes its own 60s/30s defaults when the timeout
is not below the interval, so an unchecked value would slow export rather than
speed it up. Parsing uses a signed representation because lexical_cast<uint32_t>
accepts a leading minus and wraps it.

Naming corrections: CollectorManager documented exported_instance, which no OTel
dashboard uses; node-health queried job_count where the exported name is
jobq_job_count; network-traffic and overlay-traffic-detail referenced an
undeclared DS_PROMETHEUS variable; the counter table omitted the _total suffix
the Prometheus exporter appends; the plan docs and task list carried an xrpld_
prefix formatName never applies; and OTelCollector::New()'s contract promised its
instanceId, serviceName and networkType arguments were read, contradicting the
definition that marks them unused.
2026-09-03 20:20:13 +01:00
Pratik Mankawde
9f55acef38 docs(telemetry): name the label the consensus dashboard actually filters on
The Consensus Health template-variable table documented $node as resolving via
exported_instance. That dashboard defines $node as
label_values(target_info, service_instance_id) and its panels filter on
service_instance_id; exported_instance appears in it zero times.

exported_instance is a real label, but it belongs to the StatsD boards shipped
alongside, where Prometheus renames a scraped instance label that collides with
the target's own. Documenting it against an OTel dashboard pointed readers at
the wrong pipeline's label.
2026-09-03 20:19:27 +01:00
Pratik Mankawde
a5d5d0911f merge: bring the metrics_endpoint key forward from phase-8
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.
2026-09-03 16:45:09 +01:00
Pratik Mankawde
4587986a93 merge: bring the metrics_endpoint key forward from phase-7 2026-09-03 16:40:59 +01:00
Pratik Mankawde
7b845392d4 feat(telemetry): give metrics its own endpoint key
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.
2026-09-03 16:39:04 +01:00
Pratik Mankawde
3aff6d4d64 merge: bring the traces_endpoint rename forward from phase-8
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.
2026-09-03 16:06:38 +01:00
Pratik Mankawde
9fd5414fe6 merge: bring the traces_endpoint rename forward from phase-7
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.
2026-09-03 15:43:24 +01:00
Pratik Mankawde
8d1eacc5fc merge: bring the traces_endpoint rename forward from phase-6
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.
2026-09-03 15:38:47 +01:00
Pratik Mankawde
8c695b69a5 fix(telemetry): follow the traces_endpoint rename in the node configs
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.
2026-09-03 15:21:01 +01:00
Pratik Mankawde
566a734f13 Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
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.
2026-08-27 13:00:11 +01:00
Pratik Mankawde
0fcd690cf9 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-27 12:59:13 +01:00
Pratik Mankawde
95fb21b5d5 docs(telemetry): drop the inert insight prefix from the OTel examples
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.
2026-08-27 12:58:45 +01:00
Pratik Mankawde
f71357d826 Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
# Conflicts:
#	OpenTelemetryPlan/05-configuration-reference.md
#	OpenTelemetryPlan/09-data-collection-reference.md
#	OpenTelemetryPlan/Phase4_taskList.md
#	docker/telemetry/grafana/dashboards/ledger-data-sync.json
#	docs/telemetry-runbook.md
2026-08-25 15:14:56 +01:00
Pratik Mankawde
520e8e5895 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-25 15:07:36 +01:00
Pratik Mankawde
a2cc3abba5 Merge branch 'pratik/otel-phase6-statsd' into pratik/otel-phase7-native-metrics
# Conflicts:
#	OpenTelemetryPlan/09-data-collection-reference.md
2026-08-25 15:07:21 +01:00
Pratik Mankawde
a10a4e98db docs(telemetry): add the consensus phase-span attributes to the reference
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.
2026-08-24 20:49:37 +01:00
Pratik Mankawde
6e2b2da772 fix(telemetry): resolve microsecond latencies below 100us
The microsecond ladder's first edge was 100us, which sat ABOVE the mass of
every instrument using it. Measured on devnet: 99.3% of job_queued_us
samples, 92.5% of job_running_us and 90.4% of getobject_lookup_us fell in
that first bucket. histogram_quantile then interpolated inside bucket 0 and
returned `quantile / fraction_in_bucket_0 x first_edge` -- p75/p95/p99 of
job_queued_us read 75.52/95.66/99.69us against a prediction of
75.53/95.67/99.70. Three-decimal agreement: those panels were reporting
arithmetic on the bucket edge, not latency.

The fix was already half-written. kSubMillisecondBoundaries had been parked
in MetricsRegistry.cpp as [[maybe_unused]] with a comment noting exactly this
problem for nodestore reads. Its edges are now folded into kMicrosecondBuckets
rather than deleted, so the parked intent is carried forward: 1..1000us
resolution where the mass is, upper edges unchanged so multi-second stalls
stay measurable.

Also moves the GetObject count and charge ladders into HistogramBuckets.h, so
all five ladders have one owner and one set of invariant tests (29 now).

Adds check_bucket_parity.py, wired into the existing OTel naming workflow.
The C++ millisecond ladder and the collector's spanmetrics ladder are
specified to agree over their shared range; they were identical when shipped,
then the collector side alone was extended and nothing noticed for eleven
phases. The check asserts containment rather than equality, because jobs
outlive spans -- jobq_updatepaths averages ~60s, which no span approaches, so
demanding equality would force a ceiling that censors it. Verified it rejects
a missing collector edge, a bogus in-range edge, and a return to the 5s
ceiling.

ledger-data-sync's "Job Queue Wait p95 By Type" moves off the beast
jobq_*_q_milliseconds pair onto job_queued_us filtered by job_type. Those
beast metrics are ms-quantised at the source (Event rounds up to a whole
millisecond), so 94-100% of their samples sat in the first bucket and no
ladder change could fix them. Note the label values are camelCase
(job_type="ledgerData"), not the lowercase metric-name fragments.

Both histogram-fed alert thresholds re-validated and left unchanged, with the
measured basis recorded so neither gets tuned against the old artefact: only
0.0022% of job_queued_us samples exceed the 1s threshold, and every edge
bracketing the 1000ms ios_latency threshold survived the ladder change.

Docs: the rpc_size "known issue -- tracked separately" notes in the runbook
and 09-data-collection-reference are now resolved notes, the stale 10-edge
span_duration bucket list is corrected to the collector's real 20, and the
runbook gains a "Reading A Histogram Percentile" section covering both
saturation traps and the expected discontinuity after a ladder change.
2026-08-21 12:46:56 +01:00
Pratik Mankawde
bc99a4edcd Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
This branch had already made the same corrections independently, and in
richer form, so the resolution keeps this branch's version nearly throughout:

- 09-data-collection-reference.md: this branch already documents the
  state-accounting gauges as cumulative **microseconds** with an explanatory
  callout, and already names `jobq_job_count` with its `jobq` group. Kept.
- telemetry-runbook.md: already carries `jobq_job_count` in both tables. Kept,
  along with this branch's larger additions.
- OpenTelemetryPlan.md: kept this branch's rewritten section 9 blurb, which
  describes the inventory without hardcoding counts and so cannot drift.
- consensus-health.json: kept this branch's rewrite. It deliberately removed
  the four TraceQL close-time detail panels and renamed the agreement panel;
  the incoming side would have resurrected them. Panel count unchanged at 26.
- integration-test.sh: this branch's unprefixed native metric names were kept,
  but it still asserted `job_count`, so the `jobq_job_count` correction was
  carried over. That check would otherwise always fail.
2026-08-17 12:07:56 +01:00
Pratik Mankawde
f3681ad920 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation
Carries phase-6's integration-test and telemetry-doc fixes forward. Merged
cleanly with no conflicts.
2026-08-17 12:06:12 +01:00
Pratik Mankawde
b77f516f95 Merge branch 'pratik/otel-phase6-statsd' into pratik/otel-phase7-native-metrics
Conflict resolution kept this branch's evolution and re-applied phase-6's
fixes on top of it, rather than taking either side wholesale:

- consensus-health.json: kept the native `span_calls_total` metric name and
  the `interval: 15s` and point styling from this branch; added phase-6's
  `close_time_correct` PromQL filter and the NetClock axis labels. The
  TraceQL boolean-regex filter stays removed and the `byRegexp` overrides
  carry over. Panel count unchanged at 27.
- 09-data-collection-reference.md: kept this branch's headings, its more
  detailed consensus attribute table (which already types
  `consensus_round_id` as int64) and its section numbering, including the
  deliberate removal of the SpanNames inventory. Carried over only the
  correction that the state-accounting duration gauges are cumulative
  microseconds, not seconds.
- telemetry-runbook.md: kept this branch's native metric names
  (`span_calls_total`, `span_duration_milliseconds_bucket`); carried the
  `rpc.request` -> `rpc.http_request` span-name fix and the `jobq_` segment
  on the job-queue depth metric.
- integration-test.sh: kept this branch's `check_otel_metric` form and
  carried the `jobq_job_count` correction.
2026-08-17 12:05:10 +01:00
Pratik Mankawde
57f0c81d75 Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
Conflict resolution, OpenTelemetryPlan/09-data-collection-reference.md
Known Issues table: kept phase-9's expanded 17-row table rather than the
incoming 4-row version, and re-applied the incoming correction to the one
row it changed. The other three incoming rows are already present in
phase-9's table, and phase-9's version is more current (jobq_job_count,
not job_count).

The rpc_requests row now says [insight] server=otel; server=statsd
reaches a collector with no statsd receiver on this branch. The section 8
Quick Reference and the fallback caveat auto-merged cleanly.
2026-08-17 11:44:04 +01:00
Pratik Mankawde
de0fefc95b Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-17 11:42:31 +01:00
Pratik Mankawde
77f338c0d0 docs(telemetry): correct statsd config guidance for the native OTel path
09-data-collection-reference.md contradicted itself. Section 2 presents
server=otel as the recommended transport with StatsD as a fallback, but
the Known Issues table and both Configuration Quick Reference examples
still prescribed server=statsd, which on this branch reaches a collector
with no statsd receiver and an unpublished 8125/udp.

Switch the Known Issues row and the Minimal and Production examples to
server=otel with the OTLP metrics endpoint. Keep the labelled fallback
block, and state what it actually requires: re-adding the statsd
receiver and republishing the port. Also record that StatsDCollector
applies prefix to metric names while OTelCollector does not, so the two
transports do not produce the same series.

The phase-6 copy is left alone; server=statsd is correct there.
2026-08-17 11:42:21 +01:00