The rename arrived from phase-1b by merge, which left this test naming a
member that no longer exists. Updates both assertions to tracesEndpoint
and the section key to traces_endpoint.
The key matters as much as the member: had only the member been renamed,
the parse would have fallen back to the default and the test would have
compared the collector URL against localhost.
[telemetry] endpoint carried one OTLP signal while its name implied it
covered every signal. That asymmetry is what let the metrics URL be
guessed later by rewriting this one's path suffix, so anything not
ending /v1/traces silently posted metrics to the traces path.
Renames the key to traces_endpoint and Setup::exporterEndpoint to
tracesEndpoint. The default value is unchanged and the URL is still used
verbatim, with no path derived from it. The startup log line and the
compose-file example name the new key, the latter being where an
operator copies it from.
No metrics_endpoint is added here: this branch has no metrics pipeline,
so the key would parse into a member nothing reads.
getInstance(), getTracer(), both startSpan() overloads and networkTypeFromId()
return values that a caller must use. A discarded startSpan() result destroys
the returned span immediately, so the span opens and closes with no content.
Six methods in Telemetry.h already carried the attribute, on the base and on
every override. The new attributes follow that: the overrides in Telemetry.cpp
and NullTelemetry.cpp get it too, because [[nodiscard]] is not inherited and a
call bound to the derived type would otherwise be unchecked.
No caller anywhere in the chain discards any of these results.
The default setServiceInstanceId() body ignores its argument. Use the
attribute rather than a (void) cast: the codebase already uses it 104 times
and the build is C++23.
The comment said per-asset timing 'is no longer split into individual spans'.
This change introduces the span, so per-asset spans never existed for it to be
split out of, and the comparison resolves against nothing once squash-merged.
Comments only, no behaviour change.
The conditional-compilation section promised zero overhead when telemetry is
not wanted. The span disappears, but the arguments passed to it do not: the
compiled-out guards are ordinary inline functions, so a to_string() or a hash
in an argument list still runs and its result is then discarded.
State that, show the guard that does remove the work, and name the opposite
case -- the metric macros, which discard their arguments and need no guard.
The comments claimed the guard skips work for a span that is "not being
recorded", which reads as sampling awareness. It has none: operator bool() is
impl_ != nullptr, and the span factories return an empty guard only when
telemetry is absent, disabled at runtime, or the trace category is off. A span
that exists but was sampled out still pays.
There is no isRecording() in the telemetry API, so the guard is still the
strongest available; only the justification was overstated.
updateAll's update_all span is wholly telemetry: the optional guard, the
empty-requests test that decides whether to emit at all, and the two
attributes have no reader outside the span. It runs on every ledger close, so
with telemetry compiled out the function still constructed a stub guard and
discarded pathfind_ledger_index and pathfind_num_requests once a close for
nothing. Everything the rest of updateAll depends on, including the
isNewPathRequest() flag reset, is outside the block and unchanged.
No span object exists to test before it is created, so the guard is an #ifdef
over the whole block. The three includes it was the sole user of --
PathFindSpanNames.h, SpanGuard.h and <optional> -- are gated the same way,
because otherwise they would be unused includes in that build and
clang-tidy's misc-include-cleaner would reject them.
Two pieces of pathfinding telemetry ran regardless of the build.
doUpdate fills pathfind_dest_currency by rendering the destination asset for
the pathfind.compute span. For a non-XRP issue that is a base58 check encode
of the issuer, two SHA-256 rounds, then a SHA-512Half over the result and
three string allocations. It is a call argument, so it ran even where
setAttribute's body is empty. doUpdate is not a cold path: besides once per
pathfinding RPC, PathRequestManager::updateAll calls it once per active
path_find subscription on every ledger close, so a node with N subscriptions
paid N times a close. It now sits inside "if (span)" with the cheap
pathfind_fast flag, so it is skipped with telemetry compiled out and also for
any span that is not being recorded.
findPaths keeps a totalPaths counter across its per-source-asset loop. Its
only reader is the pathfind_num_paths attribute at the end of the same
function, so the counter is maintained only when telemetry is compiled in.
That needs an #ifdef rather than "if (span)": the attribute cannot read a
variable that does not exist, and a counter kept up to date but never read is
an unused variable, which fails the build.
doPathFind and doRipplePathFind fill two span attributes from the request's
source and destination accounts. Both values are call arguments, so they are
built whatever the build: asString() copies the address out of the JSON and
redactAccount() takes a SHA-512Half over it and formats 16 hex characters.
That is two copies and two hashes on every pathfinding RPC, for
pathfind_source_account and pathfind_dest_account, which nothing outside the
span reads.
Wrapping the block in "if (span)" drops that work when telemetry is compiled
out, where the guard's operator bool() is a literal false, and also when
telemetry is on but this span is not being recorded. The const-reference read
of context.params moves inside the guard with the code that needs it, so a
telemetry read still never inserts a null into the request.
resolveCommandSpanName() and the error span in doCommand() exist only to
name and label a telemetry span. With telemetry compiled out the
resolver still ran on every RPC that fillHandler() rejects: up to five
json isMember lookups, up to three string copies, a virtual config()
call and a handler-table lookup, all to build a name nobody records. A
storm of malformed requests paid that cost once per request.
A runtime `if (span)` guard cannot work here. The resolver's result is
the span name itself, passed as the third argument of the
ScopedSpanGuard constructor, so no span object exists yet to test. That
leaves `#ifdef XRPL_ENABLE_TELEMETRY`, matching the house style used
elsewhere.
The guard covers the whole telemetry block at the call site and the
helper definition too, so the file-static helper does not become an
unreferenced function, which the build rejects because warnings are
errors. injectError() and the error return stay outside the guard, so
behaviour on the failure path is unchanged. No include is orphaned:
ErrorCodes.h, SpanGuard.h, RpcSpanNames.h and <string_view> all keep
uses outside the guards.
With telemetry on nothing changes: only comments and the four directives
were added.
The rpc.ws_message span's command attribute was resolved for every
inbound WebSocket message, whether or not anything was recording it.
The resolver does two JSON member tests plus two subscripts, copies the
command into a std::string, calls getAPIVersionNumber and then looks the
name up in the handler multimap. Because it is a call argument to
setAttribute, it ran even with telemetry compiled out, where
setAttribute's body is empty.
Wrapping the call in "if (span)" drops that work entirely when telemetry
is off, and also when telemetry is on but this span is not being
recorded. Nothing outside the attribute reads the resolved value, so no
other behaviour changes; the real request validation further down
computes its own api version, command string and handler role.
With telemetry compiled out, <string_view> in Telemetry.h and <memory> in
SpanGuard.h are named only by declarations that are themselves guarded, so
clang-tidy's include-cleaner reports them unused and warnings-as-errors fails
the build. NullTelemetry.cpp has the mirror problem: it names beast::Journal
only in the compiled-out makeTelemetry(), reaching the type transitively.
Guard each include to match the configuration that uses it, and correct three
comments that overstated what the empty destructors cost.
The compiled-out ScopedActivation, SpanGuard and ScopedSpanGuard each used a
defaulted destructor. A defaulted destructor on an empty class is trivial, so
compilers report any guard held only for its scope as an unused variable. With
telemetry compiled out that produced seven -Wunused-variable errors under
-Werror, across the ledger acquire, consensus, ledger master and overlay paths.
Write the three destructors by hand so destruction is not trivial, matching the
telemetry-enabled types, and assert that property so it cannot quietly regress
to `= default`. The bodies are empty, so no code is generated either way.
Also move <memory> inside the telemetry guard: only the telemetry-enabled types
hold a unique_ptr, so the include is unused when telemetry is compiled out.
The block was added on the StatsD branch, but telemetry sources start here.
Codecov config only flows child-ward, so every branch between this one and
that one kept reporting telemetry files as uncovered patch lines.
Also corrects the rationale. The old comment said telemetry is "not enabled
in coverage builds"; it is enabled — conanfile.py and CMakeLists.txt both
default it ON, and the coverage matrix leg does not turn it off. The reason
these files record no coverage is that the unit-test suite never starts an
exporter.
Adds *SpanLabels.h alongside *SpanNames.h: same compile-time-constant
category, and the existing glob did not match it.
This narrows the patch gap but does not close it — instrumentation added to
consensus and overlay files is still counted, so codecov/patch stays red on
the early phases.
