MetricsRegistry::start() takes a StartOptions aggregate. The two telemetry-off
tests this branch adds still passed a string literal, which cannot bind to
StartOptions const&, so the #ifndef XRPL_ENABLE_TELEMETRY block did not
compile. Nothing in CI compiles it: telemetry defaults ON, and the block is
skipped whenever the macro is defined.
Add kTestStartOptions carrying just the endpoint, and pass it at both sites.
Its text is identical to the constant phase-9 introduces, so merging phase-9
forward is an identical addition rather than a second constant with the same
meaning. The six call sites phase-9 owns are left for that merge.
A StatsDCollector polls its metrics only after onCollectionReady(), so
neither test flushed anything: the gauge test timed out and the counter test
passed for the wrong reason. Call it in both, and drop the two includes
whose symbols the file never names.
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.
getConsensusTraceStrategy() returns ConsensusTraceStrategy, so comparing
it to a string literal does not compile.
No coverage is lost: strategyName()'s spelling has its own assertions in
the TelemetryConfig test, in this same binary.
readability-identifier-naming wants lower_case for a namespace, so
clang-tidy failed on this file under warnings-as-errors. All seven use
sites move with the declaration.
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.
requireReadableFile proved a path readable with getFileContents, which
loads the whole file into a std::string and then drops it. One of the
three paths it checks is tls_client_key, so a private key was loaded to
answer a question that does not need its contents. It now stats the
path, rejects anything that is not a regular file, and opens it without
reading. The message shape is unchanged:
"[telemetry] <key> cannot be read: <path> - <reason>".
A path naming a directory used to escape as an ios failure from the
stream buffer, naming neither the config key nor the path. It is now
rejected as "not a regular file" with both named. The new test covers
that case; it fails against the old implementation and against a copy
with the file-type branch removed.
The runbook's quick start and disable sections both told the reader to
run "cmake --preset default". No presets file is tracked, and the only
preset Conan generates is conan-release, so each of those steps failed
on its first command. Replaced with the flow BUILD.md documents, and
noted that telemetry is the current default while still passing the
flags.
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.
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.
One conflict, in docs/telemetry-runbook.md: both sides had independently
corrected the same consensus_round_id example. This branch kept the pipe form,
which Tempo rejects as a parse error; upstream moved the predicate inside the
braces, which parses and returns data. Upstream's query is kept, with this
branch's note that the value is the previous ledger sequence plus one.
Two conflicts, both additive.
TelemetryConfig.cpp: this branch added requireHttpsEndpoint next to
requireReadableFile; upstream added readConsensusTraceStrategy at the same spot.
Both kept.
05-configuration-reference.md: this branch added the two client-certificate rows
while upstream corrected the consensus strategy value from attribute to random.
Both kept. Also drops the stale "not yet implemented" row for
consensus_trace_strategy, which the merged table now contradicts twice over: the
option is parsed, and its value is no longer spelled attribute.
