Commit Graph

15153 Commits

Author SHA1 Message Date
Pratik Mankawde
dd8a35ec36 docs(telemetry): List the per-role account attributes of tx.process 2026-09-23 18:02:15 +01:00
Pratik Mankawde
3761dee234 Merge branch 'pratik/otel-phase2-rpc-tracing' into pratik/otel-phase3-tx-tracing 2026-09-23 18:01:54 +01:00
Pratik Mankawde
509fc7853e feat(telemetry): Emit every account a transaction names on tx.process
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.
2026-09-23 18:01:14 +01:00
Pratik Mankawde
9bfa3cc32c docs(telemetry): Account addresses are public and not hashed
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.
2026-09-23 18:00:34 +01:00
Pratik Mankawde
91b440820a fix(telemetry): Emit path-finding accounts as raw r-addresses
An XRP account address is a public ledger identifier drawn from an
enumerable set. An unsalted hash of it is reversible by lookup, so it
protected nothing and only broke the join against explorers, RPC
responses and logs that show the same address.

- pathfind_source_account and pathfind_dest_account carry the request's
  r-address. A value that does not parse as an r-address is not emitted,
  so a malformed or mistaken request value never reaches a span. Both
  handlers share setAccountAttribute() in PathFindSpanAttributes.h.
- pathfind_dest_currency is to_string(Asset): "XRP", "<issuer>/<CUR>",
  or the MPT issuance id.
- The collector's attributes/hash processor is removed. No layer hashes.
- redactAccount() stays available; its header no longer claims to sit in
  the emit path.
2026-09-23 18:00:32 +01:00
Pratik Mankawde
adfd9900a7 fix(telemetry): Hash the MPT issuance id in the path-find span
An MPT issuance id is a sequence concatenated with the issuer's account id, so
emitting it whole put an account on a span in the clear, while the Issue arm
beside it redacts its issuer. The collector hashes only the two account
attributes, so nothing downstream caught it.

Hash the whole id: one stable token per asset, no issuer. The comment claiming
the id carries no address was wrong and is corrected.
2026-09-23 14:29:19 +01:00
Pratik Mankawde
652b0c0dbf fix(telemetry): Skip the fee span attribute for a non-XRP fee
The tx.process span read the fee with STAmount::xrp(), which throws on a
non-native amount. preflight1 rejects such a transaction with temBAD_FEE, but
the attribute runs first, so with tracing on the submit RPC returned
internalSubmit instead. Turning tracing on must not change a response.

Guard on native() and leave the attribute out otherwise. The peer path was
already contained by its own catch.
2026-09-23 14:29:18 +01:00
Pratik Mankawde
c23646c279 Merge branch 'pratik/otel-phase2-rpc-tracing' into pratik/otel-phase3-tx-tracing 2026-09-22 21:30:34 +01:00
Pratik Mankawde
a7b3a0df6e fix(tests): Locate the in-memory exporter by build config
The find_library hint was pinned to the _RELEASE variable CMakeDeps
generates, so in any other configuration it expanded to nothing. The
archive was then found only via CMAKE_PREFIX_PATH, which can hand a Debug
build the Release archive instead of failing. Derive the suffix from
CMAKE_BUILD_TYPE and ask for the package's lib directory directly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-22 21:30:22 +01:00
Pratik Mankawde
bba92767a6 Merge branch 'pratik/otel-phase2-rpc-tracing' into pratik/otel-phase3-tx-tracing
tempo.yaml kept both sides' filter blocks: phase-2's six path-finding
filters ahead of this branch's three transaction filters, matching chain
order, and both header comment lines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-22 21:14:20 +01:00
Pratik Mankawde
d69bd5d19c Merge branch 'pratik/otel-phase1c-rpc-integration' into pratik/otel-phase2-rpc-tracing
RPCHandler.cpp composed both sides: phase-1c's null-guard and reply-aware
status logic, keeping this branch's load_type attribute inside that guard
because its argument allocates.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-22 21:13:17 +01:00
Pratik Mankawde
a293fb66e5 fix(telemetry): Report pathfind span status from the reply
Both pathfind handlers returned on many paths without recording a status,
so a failed request produced a span that read as success. Route every exit
through one helper that reads the rpc error token off the reply, which also
covers the replies built further down the call chain.

The token set is fixed by the error registry, so it is safe as a span
label; raw request text would not be.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-22 21:11:30 +01:00
Pratik Mankawde
35db8610cc fix(telemetry): Set rpc_status on the invalid-JSON websocket span
rpc_status is a span-metrics dimension, so leaving it unset on this path
emitted a series with a blank label. Any query selecting on error missed
the failure entirely.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-22 21:11:21 +01:00
Pratik Mankawde
87560c5157 test(telemetry): Restore the thread's LocalValue store from a scope guard
The store swap was undone by trailing statements, which a fatal assertion skips.
The thread store is a function-local static, so it outlives every test rather
than being reset between them: a skipped restore left it owning a stack object
from a dead frame, and the crash then landed in whichever test ran next.

The onCoro flag did not protect it either, because the cleanup function reads
that flag out of the freed object to decide not to delete it.

The guard is declared after both stack stores so it is destroyed before them.
2026-09-22 20:31:15 +01:00
Pratik Mankawde
3cfe8d139e feat(telemetry): Add Tempo search filters for the path-finding attributes
This branch emits nine pathfind attributes and the datasource offered a dropdown
for none of them, so the signal was there but not searchable in Explore. The
file's own header states that each phase adds filters for what it introduces.

Six filters, for the attributes that select a request. The three counts are
measurements read off a span rather than things an operator searches by, so they
get none. ledger_index is dynamic because it takes a new value every ledger.
2026-09-22 20:31:14 +01:00
Pratik Mankawde
89fc3ba450 fix(telemetry): Name the reason an RPC failed in the span status
The status description was the fixed string error, so a trace recorded that a
request failed but not why.

It now carries the error token from the reply, falling back to the status's own
error code and then to the old string. Every source is a compile-time literal
from the error registry, so no request text reaches it. The rpc_status attribute
stays at success and error: that one is a span-metrics dimension, and widening
it would mint a series per token per command per node.

The work is gated on the span being live, so a build with telemetry compiled out
or a disabled guard pays nothing, matching how the neighbouring helper is
handled. asCString() is null-checked as well as type-checked, because it returns
the raw pointer where asString() guards it.

The comment above resolveCommandSpanName now states the invariant rather than
how it could be abused.
2026-09-22 20:29:58 +01:00
Pratik Mankawde
a2ee20b88f fix(telemetry): Read the RPC span status from the reply, not the Status
callMethod decided the rpc.command span's status from the Status the handler
returned. Handler.cpp registers 70 of its 72 methods through byRef(), which
returns a default Status whatever happened, because an old-style handler
reports its error in the reply body instead. So a failed account_info or a
refused path_find came out with rpc_status=success and span status Ok.

That is the opposite of what the comment above the code claimed it did, and it
left a {status.code=error} query blind to every non-throwing RPC error.

The status now comes from the reply as well as the Status, which covers both
handler styles. byRef() is unchanged and identical to develop: it behaves
correctly for its own purpose, and no RPC reply changes. Only telemetry was
reading the wrong signal.

setOk() is dropped rather than moved. The specification reserves Ok for an
operator asserting verified success and warns that a tool may treat it as
suppressing errors, so a successful call now leaves the status Unset.

No test accompanies this: the Beast tree has no telemetry fixture and the
in-memory span exporter is linked only into xrpl_tests, which cannot reach
daemon code. Asserting the exported status needs that infrastructure first.
2026-09-22 20:29:57 +01:00
Pratik Mankawde
c587cf5edf fix(rpc): Do not let span naming change the RPC error a client sees
resolveCommandSpanName() converted command/method to a string with no type
check. json::Value::asString() throws for an array or an object, so a request
whose nested method is [] reached that conversion and the throw replaced a
clean tooBusy reply with internal.

The overloaded path is the only way in: fillHandler() returns tooBusy before
anything has read those fields, and every other exit either converted them
itself or means neither field is present. The effect is that the error code a
client receives depends on whether telemetry was compiled in, which telemetry
must never do.

The span now falls back to its existing unknown-command label when either
present field is not a string. The WebSocket path already validates both
fields before dispatch, so it is left alone.

The test drives a genuinely overloaded job queue, reading the threshold from
the production constant rather than copying it, and asserts the client still
gets tooBusy. It lives in the Beast tree because doCommand is daemon code and
needs jtx, which the gtest binary cannot reach.
2026-09-22 19:06:07 +01:00
Pratik Mankawde
be1e4e3c67 docs(telemetry): remove an internal plan doc from the tree
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.
2026-09-22 19:04:01 +01:00
Pratik Mankawde
7b275f6c0d Merge branch 'pratik/otel-phase2-rpc-tracing' into pratik/otel-phase3-tx-tracing 2026-09-22 15:38:17 +01:00
Pratik Mankawde
dad8838c31 Merge branch 'pratik/otel-phase1c-rpc-integration' into pratik/otel-phase2-rpc-tracing 2026-09-22 15:38:12 +01:00
Pratik Mankawde
3b2192a48b Merge branch 'pratik/otel-phase1b-telemetry-infra' into pratik/otel-phase1c-rpc-integration 2026-09-22 15:38:06 +01:00
Pratik Mankawde
359ae64170 Merge branch 'pratik/otel-phase1a-plan-docs' into pratik/otel-phase1b-telemetry-infra 2026-09-22 15:38:01 +01:00
Pratik Mankawde
94f131b165 Merge remote-tracking branch 'origin/develop' into pratik/otel-phase1a-plan-docs 2026-09-22 15:37:53 +01:00
Pratik Mankawde
0e1e335f42 Merge branch 'pratik/otel-phase2-rpc-tracing' into pratik/otel-phase3-tx-tracing 2026-09-22 15:11:47 +01:00
Pratik Mankawde
5a148ff55e Merge branch 'pratik/otel-phase1c-rpc-integration' into pratik/otel-phase2-rpc-tracing 2026-09-22 15:11:47 +01:00
Pratik Mankawde
76c2aa8518 Merge branch 'pratik/otel-phase1b-telemetry-infra' into pratik/otel-phase1c-rpc-integration 2026-09-22 15:11:47 +01:00
Bart
00606bec1a fix: Derive traversal node IDs from the branch actually descended (#7942)
Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
2026-09-22 14:02:11 +00:00
Pratik Mankawde
3f510ecf44 fix(telemetry): publish local stack ports on the host loopback
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.
2026-09-22 13:58:17 +01:00
Mayukha Vadari
2bc17c3e73 refactor: Add initial wrapper classes for all SLEs (#7886) 2026-09-21 19:47:23 +00:00
dependabot[bot]
0229c294a9 ci: [DEPENDABOT] bump codecov/codecov-action from 7.0.0 to 7.1.1 in the github-actions group across 1 directory (#8251)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-09-21 12:09:57 +00:00
Pratik Mankawde
870367d71e merge: bring develop forward into phase-3 tx tracing
Resolved src/libxrpl/tx/applySteps.cpp. calculateBaseFee now returns
std::expected<XRPAmount, TER>, so invokePreclaim rejects the transaction when
the fee cannot be computed. Kept that inside this branch's preclaimTer wrapper,
so the returned error also reaches the span's ter_result attribute.
2026-09-21 12:45:45 +01:00
Pratik Mankawde
c030880a36 Merge branch 'pratik/otel-phase1c-rpc-integration' into pratik/otel-phase2-rpc-tracing 2026-09-21 12:44:43 +01:00
Pratik Mankawde
806473a9e9 fix(rpc): pass the command name to the span as a string_view
callMethod now takes its name as a std::string_view, which has no c_str().
setAttribute already accepts a string_view, so hand it the name directly.
2026-09-21 12:44:34 +01:00
Pratik Mankawde
d16f2c4181 Merge branch 'pratik/otel-phase1b-telemetry-infra' into pratik/otel-phase1c-rpc-integration 2026-09-21 12:43:48 +01:00
Pratik Mankawde
f48a2b4871 Merge branch 'pratik/otel-phase1a-plan-docs' into pratik/otel-phase1b-telemetry-infra 2026-09-21 12:43:44 +01:00
Pratik Mankawde
e25c1a5920 Merge remote-tracking branch 'origin/develop' into pratik/otel-phase1a-plan-docs 2026-09-21 12:43:23 +01:00
Bart
184fe173fb refactor: Remove a dead unwrap from the WebSocket success path (#8252)
Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
2026-09-21 11:23:31 +00:00
Bart
16f7b263fd docs: Correct three comments about null-terminated views (#8253)
Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
2026-09-21 11:12:09 +00:00
Bart
f6b51f0b8b ci: Say when a package publish is a dry run (#8247)
Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
2026-09-18 18:31:07 +00:00
Pratik Mankawde
49ce63c843 merge: bring the task-list span-placement wording forward from phase-2
Conflicted only on the two adjacent lines this branch had renamed from dotted
to bare attribute keys. Kept the bare keys, which are the convention the naming
check enforces.
2026-09-18 10:07:32 +01:00
Pratik Mankawde
2f61ce6851 docs(telemetry): match the task list to where the receive span now starts
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.
2026-09-18 10:06:36 +01:00
Pratik Mankawde
660535c4d8 merge: bring the plan-doc suppressed cleanup forward from phase-2
The acceptance table conflicted because this branch had added four criteria to
it. Kept them, and took the incoming HashRouter row.
2026-09-18 10:01:30 +01:00
Pratik Mankawde
bab53ef8d0 Merge branch 'pratik/otel-phase1b-telemetry-infra' into pratik/otel-phase1c-rpc-integration 2026-09-18 10:00:51 +01:00
Pratik Mankawde
cf2d412554 Merge branch 'pratik/otel-phase1a-plan-docs' into pratik/otel-phase1b-telemetry-infra 2026-09-18 10:00:51 +01:00
Pratik Mankawde
3d21e7e13f Merge branch 'pratik/otel-phase1c-rpc-integration' into pratik/otel-phase2-rpc-tracing 2026-09-18 10:00:51 +01:00
Pratik Mankawde
a0a23f0b61 docs(telemetry): match the task list to where the receive span now starts
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.
2026-09-18 09:55:14 +01:00
Pratik Mankawde
811579b615 docs(telemetry): drop the suppressed attribute from the design tables
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.
2026-09-18 09:54:39 +01:00
Pratik Mankawde
fe4a778fed perf(telemetry): trace only the transactions this node will process
A peer relays every transaction it hears, so most inbound copies are ones
handleTransaction() drops. The tx.receive span was created before those checks
ran, so every dropped copy paid for a span, a hex transaction id, and an
open-ledger index read that takes the mutex the apply path needs.

Measured over a four-hour payment run: 83.2M of 101.1M tx.receive spans
described a copy the node dropped. That is 82% of this span and 23% of every
span the node emitted.

Move the span and its attributes below the duplicate check. How many copies
were dropped is already reported as the transactions_duplicate traffic
category, which costs no span. Why a copy was dropped is no longer recorded;
a labelled counter restores it on the branch that carries the metric registry.

The suppressed attribute and the suppressed and rejected_inner_batch status
values go with it. This function was their only user.
2026-09-17 23:00:59 +01:00
Denis Angell
07b36871c7 ci: Publish packages through packages-upload.xrplf.org (#8241) 2026-09-17 18:05:51 +00:00