Conflict in docker/telemetry/docker-compose.yml, on the collector service's
command key. This branch layers a second --config for file_log offset
persistence; upstream added a comment above the ports block explaining the
loopback binding.
Kept both. The two changes are unrelated: one is the command list, the other is
a comment. The layered --config and all nine loopback-bound ports are present,
and the file still parses.
Conflict in docker/telemetry/docker-compose.yml, in the collector's ports
block. Upstream bound every port to the host loopback, including the StatsD
UDP port. This branch had already retired that port, because beast::insight
moved to native OTLP, leaving it commented out as a fallback example.
Not resolved as a union, which would have resurrected the retired port.
Composed instead: this branch's shape is kept, so StatsD stays commented, and
the loopback binding is applied to the active ports and to the commented
example, so uncommenting it yields the safe form.
Conflict in docker/telemetry/docker-compose.yml, in the collector's ports
block. Both sides bound ports to the host loopback: upstream did 4317, 4318
and 13133, this branch did 8125, 8889 and 9090.
Resolved as the union. All eight published ports are loopback-bound, and both
explanatory comments are kept: upstream's, which covers the whole block and
cites the upstream guidance, and this branch's, which is specific to the
unauthenticated StatsD receiver.
Grafana reaches Loki as loki:3100 over the compose network and the collector
exports to it in-network, so the published port was reachable off-host for no
consumer.
The StatsD port beside them was already loopback-bound with a comment saying
why; the Prometheus exporter and Prometheus itself were not. Prometheus
scrapes the collector as otel-collector:8889 over the compose network, so the
published port had no consumer.
The compose ports published on every host interface, so the collector's
unauthenticated OTLP receivers, Tempo, and the anonymous-admin Grafana were
reachable by anything that could route to the host.
Nothing consumed the published ports from off-host: containers address each
other by compose service name, and every doc and script uses localhost.
Resolved consensus-health.json. This branch re-exported that dashboard, so
phase-6's unit changes land in lines this branch had already rewritten. Took
this branch's file from the clean baseline merge, then applied si: and
decimals 0 to every count-unit panel left in it. The panel set matches that
baseline merge exactly.
Panels whose unit counts whole things switch from a custom `suffix:` unit to
`si:`, so a large value reads 15 k nodes/s instead of 15000 nodes/s. A custom
suffix unit is formatted by toFixedUnit, which never scales; si: goes through
SIPrefix and does.
decimals is set to 0 on the same panels, because a fraction of a ledger or a
node says nothing. The two settings need each other: on its own decimals 0
would round a slow rate down to zero, and si: shifts it to the milli step
instead.
Left alone: the one panel whose unit is a seconds-per-second ratio.
26 panels across 9 dashboards, plus 3 legends and one name placement.
Panels whose unit counts whole things switch from a custom `suffix:` unit to
`si:`, so a large value reads 15 k nodes/s instead of 15000 nodes/s. A custom
suffix unit is formatted by toFixedUnit, which never scales; si: goes through
SIPrefix and does.
decimals is set to 0 on the same panels, because a fraction of a ledger or a
node says nothing. The two settings need each other: on its own decimals 0
would round a slow rate down to zero, and si: shifts it to the milli step
instead.
Left alone: the one panel whose unit is a seconds-per-second ratio.
20 panels across 5 dashboards.
A panel legend drops ${__field.labels.series} where the panel title already
carries that name, and the query drops the label_replace that set the label,
since nothing reads it any more.
Horizontal bar gauges put the series name above the bar.
3 legends across 2 dashboards, and one bar gauge name placement.
Resolved three dashboards. phase-7 rewrote consensus-health, rpc-performance
and transaction-overview, so both sides added the annotations block next to
lines that had already diverged. Rebuilt each file from the clean pre-change
merge of the two branches, then set the option on every panel, which is what
both sides intended. Panel sets match that baseline merge exactly.
All 107 timeseries and state-timeline panels on this branch now set multiLane,
and both state timelines set perPage.
Every timeseries and state-timeline panel sets
options.annotations.multiLane. The state-timeline panels set
options.perPage to 5.
62 panels across 5 dashboards.
Every timeseries and state-timeline panel sets
options.annotations.multiLane. The state-timeline panels set
options.perPage to 5.
46 panels across 5 dashboards.
Resolved transaction-overview.json by composing both sides: this branch's
renamed span_calls_total metric and its 15s interval, with phase-6's removal of
the suppressed grouping and its new legend.
tx.receive is created after the duplicate check, so it never carries a
suppressed attribute. The Tempo tag filter and the dashboard panel still
referenced it, and check_otel_naming rules C and D fail on a reference with no
constant behind it.
The panel keeps its remaining signal as a plain receive rate. How many relayed
copies were dropped is the transactions_duplicate traffic category, which does
not depend on a span.
The spanmetrics dimension goes too; a dimension for an attribute nothing sets
only widens the label set.
Merges pratik/otel-phase7-native-metrics into pratik/otel-phase8-log-correlation.
Conflict was one TESTING.md hunk under "Nodes not reaching proposing state":
this branch renamed the node directories to Node-N in integration-test.sh,
phase-7 kept nodeN and expanded the [peer_private] explanation. Resolution
keeps this branch's Node-1 path (its own script uses that naming) and
phase-7's fuller prose citing peerfinder/Config.cpp.
Non-conflicting phase-7 changes come through: OTelCollector's gauges_ list
becomes weak_ptr, matching the earlier hooks_ change; the phase-6 revert of
the StatsD-test onCollectionReady() calls resolved against phase-7's version
that keeps them.
Merges pratik/otel-phase6-statsd into pratik/otel-phase7-native-metrics.
Phase-6 dropped the three onCollectionReady() calls that had been added to
its StatsD test, because that method is only declared here on phase-7.
This branch's own copy of the file was unchanged from the merge base, so
the default merge would have silently deleted the calls from here too —
where they are needed, because this branch gates polling behind
onCollectionReady() in OTelCollectorImp::onTimer.
Resolution keeps both sides: phase-6's two new include lines
(Counter.h, Gauge.h) and phase-7's three onCollectionReady() calls plus
their doxygen and inline explanations. The merged file is exactly
phase-7's tip plus those two includes.
TESTING.md auto-merged cleanly; both sides added text under Test 1 in
different regions.
The config template wrote each node's log to a lowercase node{N} directory
while setting service_instance_id=Node-{N}. The collector takes the node name
from the log file's parent directory and stamps it as the Loki
service_instance_id label, so the logs carried a name no trace or metric
shared and nothing joined. Use Node-{N} and state the rule.
The template also set log_level to warning. Nothing in the pipeline filters on
severity; the constraint is that a log line carries trace context only when it
is emitted inside an active span. At warning the only such statements in the
consensus accept span are a catch path a healthy round never takes and a
periodic censorship warning. At info the CNF Val / CNF buildLCL pair writes one
line per accepted ledger, which is what makes this test's Step 1 findable.
Grafana 13 offers the link per span, labelled "Logs for this span", in the
span's Links row — not per trace. Fix the step and the expected-results row.
The example log line quoted a message that does not exist. The real in-span
RPC statement logs at debug, so the severity code is DBG; say which line to
look for under each test, since Test 2 now logs at info.
Drop the reference to workload/validate_telemetry.py: that file is not part of
this branch, and its instant-endpoint call uses seconds, so the nanoseconds
claim applied only to query_range.
On the OTel path only [insight] server is load-bearing. CollectorManager reads
endpoint and hands it to OTelCollector, which logs it at startup and routes
nothing with it; the real export endpoint is [telemetry] metrics_endpoint,
which the template already sets. service_instance_id and service_name in that
section are read and discarded.
Leaving the line invited an operator to reconcile a mismatch that has no
effect. integration-test.sh already emits only server=otel with the same
explanation, so the two now agree.
The Payment destination was not a valid XRPL address — its base58 checksum
does not match — so Test 1 Step 4 and Test 2 Step 7 could never have returned
the tesSUCCESS they claim. Use a valid one and note that the destination does
not need to exist.
The Tempo search loop had no -G, so curl posted the parameters as a body,
Tempo answered 200 while ignoring the query, and every span name came back
non-zero. It also had no time bound, and Tempo keeps blocks for an hour, so a
re-run was answered by the previous run's traces. Add -G, RUN_START, and
start/end, matching what integration-test.sh already does.
Split the query list in two: 35 names that should be present, and 8 that need
a trigger neither test performs, where zero is the expected answer. Previously
two of the latter sat in the pass/fail list and read as failures.
Correct the standalone span table. consensus.mode_change fires once per round
start whether or not the mode changes, ledger.validate cannot fire because
checkAccept is unreachable in standalone, and the apply-stage, TxQ and ledger
families were missing rows. Give each "No" row the reason that actually
applies: the establish phase, a missing validator key, or no peers.
Also: ledger_accept is not required before submit, the teardown pgrep matched
more than this node, [peer_private] also disables the inbound listener, and
the 15-second wait covers Tempo but not Prometheus.
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.
`docker compose down` keeps the `tempo-data` named volume, so a previous
run's traces stay in Tempo and can satisfy this run's span searches. Pass
-v in cleanup(), and tear the stack down before starting it: a run that
reaches the summary deliberately leaves the stack up, so nothing else
clears it.
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.
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 health_check extension was present on the previous branch and dropped
here with no replacement, while this branch's own TESTING.md still polls
http://localhost:13133/ to decide the collector is ready. That check has had
no listener since, so the documented readiness step cannot pass.
Also add batch to the metrics pipeline. Without it the OTLP metric path
exports one request per instrument; the added delay is bounded by the batch
timeout, well under the Prometheus scrape interval.
Both belong here rather than downstream: this branch owns the metrics
pipeline and is the one that regressed the extension.
Two files conflicted and both were composed rather than taken from one side.
integration-test.sh: kept this branch's spanmetrics names, since the collector
sets namespace: "span" here and traces_span_metrics_* matches nothing, and took
phase-6's --max-time on every probe. The Tempo time bound needed restoring by
hand: RUN_START, the check_span guard and the start/end parameters are on
phase-6 and absent here, so a plain resolution kept phase-6's comment about
bounding the search while shipping no bound. All four pieces are back.
TESTING.md: kept server=otel with the metrics endpoint. Phase-6's template sets
server=statsd and documents prefix, which this branch's OTel path ignores.
This branch switches integration-test.sh to [insight] server=otel and adds an
assertion that no StatsD listener is needed, but TESTING.md still described the
metrics it verifies as StatsD-derived and its manual node-config template had no
[insight] stanza at all.
CollectorManagerImp falls through to NullCollector when server is neither statsd
nor otel, so a reader building configs from that template got zero
beast::insight metrics. The template also omitted service_instance_id and
metrics_endpoint, which the script writes; without the former every node is
indistinguishable in the $node dashboard filter.
curl applies no overall timeout of its own, so a server that accepts the
connection and then stops answering parks a poll loop for the rest of the run
and the loop's attempt count stops bounding anything. Add a CURL_MAX_TIME
ceiling and apply it to all 18 executable probes in integration-test.sh.
TESTING.md's manual node-config template also disagreed with what the script
writes, so a reader following it could not reproduce the automated path:
- no [insight] stanza, so no beast::insight metric leaves the node at all and
Step 10b's ten rippled_* assertions cannot pass
- [ips_fixed] listed all six peer ports including the node's own
The log level is deliberately untouched: the template and the script agree on
warning here.
check_statsd_metric queried rippled_rpc_requests, which no pipeline
produces: the collector's statsd receiver runs with is_monotonic_counter,
so the Prometheus exporter appends _total. A wrong name returns zero
series rather than an error, so the assertion could not be told apart
from a broken pipeline. All eight assertions were re-derived from how
each metric is created in code; this was the only counter.
Tempo searches carried no start/end, and tempo-data is a named volume
that `docker compose down` preserves under a one-hour block retention, so
the 17 span assertions could pass on an earlier local run's traces. Bound
every search to this run, and tear the stack down with -v before starting
so no earlier data is present to match. The service-name check now
matches a whole line, because the tag-values endpoint ignores start/end.
Add a gtest for the StatsD gauge that publishes its initial zero and for
the counter that must publish nothing. Assert two metrics the harness
never checked: a traffic-category gauge no message reaches, and
io_context latency.
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.
