From 652b0c0dbf5efea633f39df1f222752a366718ae Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 23 Sep 2026 14:29:18 +0100 Subject: [PATCH 1/6] 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. --- src/xrpld/app/misc/NetworkOPs.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index 3ecb2a4e1c..e4c16bd85c 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -1561,10 +1561,16 @@ NetworkOPsImp::processTransaction( { if (auto const* fmt = TxFormats::getInstance().findByType(stx->getTxnType())) span->setAttribute(tx_span::attr::txType, fmt->getName().c_str()); + // xrp() throws on a non-XRP fee. preflight rejects such a + // transaction with temBAD_FEE, so leave the attribute out rather + // than let tracing turn that into an internal error. + if (auto const& fee = stx->getFieldAmount(sfFee); fee.native()) + { + span->setAttribute( + tx_span::attr::fee, static_cast(fee.xrp().drops())); + } span->setAttribute( - tx_span::attr::fee, static_cast(stx->getFieldAmount(sfFee).xrp().drops())); - span->setAttribute( - tx_span::attr::sequence, static_cast(stx->getSeqProxy().value())); + tx_span::attr::sequence, static_cast(stx->getSeqProxy().value())); } } From adfd9900a7b2697bbe786c8739894e42cc76aeb9 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 23 Sep 2026 14:29:19 +0100 Subject: [PATCH 2/6] 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. --- src/xrpld/rpc/detail/PathRequest.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/xrpld/rpc/detail/PathRequest.cpp b/src/xrpld/rpc/detail/PathRequest.cpp index 1154109fc9..4baf6567d6 100644 --- a/src/xrpld/rpc/detail/PathRequest.cpp +++ b/src/xrpld/rpc/detail/PathRequest.cpp @@ -784,8 +784,9 @@ PathRequest::doUpdate( // the issuer as a plaintext Base58 address, so it cannot be emitted // as-is: every account reaching a span is hashed first. Redact just the // issuer and keep the currency, which is what this attribute is for. An - // MPT asset renders as its issuance ID and carries no address, so it - // needs no redaction. + // MPT issuance id ends with the issuer's account id, so hash the whole + // id: that still gives one stable token per asset, without publishing + // the issuer. span.setAttribute( pathfind_span::attr::destCurrency, saDstAmount_.asset().visit( @@ -794,7 +795,7 @@ PathRequest::doUpdate( ? to_string(issue.currency) : redactAccount(toBase58(issue.account)) + "/" + to_string(issue.currency); }, - [](MPTIssue const& mpt) { return to_string(mpt.getMptID()); })); + [](MPTIssue const& mpt) { return redactAccount(to_string(mpt.getMptID())); })); } JLOG(journal_.debug()) << iIdentifier_ << " update " << (fast ? "fast" : "normal"); From 91b440820a92d23726d181562b397c04f6d58eb4 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 23 Sep 2026 18:00:32 +0100 Subject: [PATCH 3/6] 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", "/", 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. --- docker/telemetry/otel-collector-config.yaml | 15 ++--- include/xrpl/telemetry/Redaction.h | 50 +++++++------- src/xrpld/rpc/detail/PathFindSpanAttributes.h | 66 +++++++++++++++++++ src/xrpld/rpc/detail/PathFindSpanNames.h | 15 ++++- src/xrpld/rpc/detail/PathRequest.cpp | 22 ++----- src/xrpld/rpc/handlers/orderbook/PathFind.cpp | 33 +++++----- .../rpc/handlers/orderbook/RipplePathFind.cpp | 33 +++++----- 7 files changed, 143 insertions(+), 91 deletions(-) create mode 100644 src/xrpld/rpc/detail/PathFindSpanAttributes.h diff --git a/docker/telemetry/otel-collector-config.yaml b/docker/telemetry/otel-collector-config.yaml index 6a38155f71..49b94fd18a 100644 --- a/docker/telemetry/otel-collector-config.yaml +++ b/docker/telemetry/otel-collector-config.yaml @@ -48,15 +48,10 @@ processors: action: delete - key: telemetry.sdk.version action: delete - # Defense-in-depth: hash path-finding account attributes. The xrpld SDK - # already hashes these before export, but a node that emitted raw values - # is caught here so raw addresses never reach the backend. - attributes/hash: - actions: - - key: pathfind_source_account - action: hash - - key: pathfind_dest_account - action: hash + # No attribute hashing or redaction. Account addresses in span attributes + # (pathfind_source_account, pathfind_dest_account) are public ledger + # identifiers and are stored as emitted so they join against explorers and + # logs. Do not add an attributes/hash processor for them. exporters: debug: @@ -75,5 +70,5 @@ service: pipelines: traces: receivers: [otlp] - processors: [resource/tier, resource/stripsdk, attributes/hash, batch] + processors: [resource/tier, resource/stripsdk, batch] exporters: [debug, otlp_grpc/tempo] diff --git a/include/xrpl/telemetry/Redaction.h b/include/xrpl/telemetry/Redaction.h index 3b1e8f67ef..c3738ddbb7 100644 --- a/include/xrpl/telemetry/Redaction.h +++ b/include/xrpl/telemetry/Redaction.h @@ -1,34 +1,30 @@ #pragma once /** - * Account-address redaction for telemetry span attributes. + * Account-address redaction helper for telemetry span attributes. * - * Path-finding RPC handlers would otherwise emit the caller's raw - * account addresses as span attributes. To keep plaintext addresses out - * of the telemetry backend, they are hashed at the point of emission. - * This header exposes a single pure helper that turns an address into a - * short, stable, obfuscated token. + * A single pure helper that turns a string into a short, stable, + * obfuscated token, for a span attribute whose value should not be stored + * in the clear and is hard to guess. * - * Data flow: - * - * handler -> redactAccount(addr) -> span attribute -> OTLP export + * Not applied to any span today. Account addresses are public ledger + * identifiers, so the path-finding spans emit them raw (see + * PathFindSpanNames.h) and no collector processor hashes them. Use this + * helper only for a value that is genuinely private, and document the + * reason at the attribute constant. * * The returned token is the first 16 hex characters (lowercase) of the - * SHA-512Half digest of the address. It is deterministic (same address - * always maps to the same token) so operators can still correlate spans - * for a given account across nodes and restarts. + * SHA-512Half digest of the input. It is deterministic (same input + * always maps to the same token) so spans for one value still correlate + * across nodes and restarts. * - * The hash is unsalted, so it is obfuscation, not a secrecy guarantee: - * XRP account addresses are a public, enumerable set, so a determined - * observer with the telemetry stream could rebuild the address->token - * mapping. The goal here is to keep plaintext addresses out of traces - * and dashboards, not to defend against a precomputation attack. A salt - * is intentionally omitted because it would break cross-node/restart - * correlation, which is the reason for hashing rather than dropping. - * - * A second, independent hashing layer runs in the OpenTelemetry - * Collector (an `attributes/hash` processor) as defense-in-depth for - * any node that emits a raw value. + * The hash is unsalted, so it is obfuscation, not a secrecy guarantee. + * It hides a value only when that value is hard to guess: for an input + * drawn from a small or enumerable set, such as an account address, an + * observer can rebuild the value->token mapping by lookup, which is why + * account addresses are emitted raw instead. A salt is intentionally + * omitted because it would break cross-node/restart correlation, which is + * the reason for hashing rather than dropping. * * @note This function is pure and reentrant: it holds no global state, * performs no I/O, and is safe to call concurrently from any thread. @@ -38,8 +34,7 @@ * #include * using namespace xrpl::telemetry; * - * span.setAttribute( - * pathfind_span::attr::sourceAccount, redactAccount(src.asString())); + * auto const token = redactAccount(value); // 16 lowercase hex chars * @endcode * * Edge case (empty input yields empty output): @@ -54,9 +49,10 @@ namespace xrpl::telemetry { /** - * Hash an account address into a short, stable, obfuscated token. + * Hash a value into a short, stable, obfuscated token. * - * @param addr The account address to redact (e.g. an r-address). + * @param addr The value to redact. Named for its original use on account + * addresses; any string can be passed. * @return The first 16 lowercase hex characters of sha512Half(addr), * or an empty string when @p addr is empty. */ diff --git a/src/xrpld/rpc/detail/PathFindSpanAttributes.h b/src/xrpld/rpc/detail/PathFindSpanAttributes.h new file mode 100644 index 0000000000..52d1996439 --- /dev/null +++ b/src/xrpld/rpc/detail/PathFindSpanAttributes.h @@ -0,0 +1,66 @@ +#pragma once + +/** + * Helpers that set path-finding span attributes from RPC request fields. + * + * The path_find and ripple_path_find handlers both record the request's + * source and destination accounts on the pathfind.request span. The value + * comes from client input, so it is emitted only when it parses as an + * r-address; a malformed or mistaken value never reaches the span. The + * address itself is a public ledger identifier and is emitted raw (see the + * attribute docs in PathFindSpanNames.h). + * + * doPathFind() / doRipplePathFind() + * │ params[jss::source_account], read through a const json::Value + * ▼ + * setAccountAttribute(span, key, field) (this header) + * │ parseBase58: nullopt -> nothing emitted + * ▼ + * span.setAttribute(key, toBase58(account)) + * + * @code + * // Primary use, inside the span-live guard of a handler: + * auto const& params = std::as_const(context.params); + * pathfind_span::setAccountAttribute( + * span, pathfind_span::attr::sourceAccount, params[jss::source_account]); + * @endcode + * + * @code + * // Edge cases: a missing field (null), a non-string, or a string that is + * // not an r-address all leave the span untouched. + * pathfind_span::setAccountAttribute(span, key, json::Value{}); + * pathfind_span::setAccountAttribute(span, key, json::Value{"not an address"}); + * @endcode + * + * @note Not a hot path: one Base58 decode per account per RPC call, and only + * while the span is live. Thread-safe; it holds no state. + */ + +#include +#include +#include + +#include + +namespace xrpl::telemetry::pathfind_span { + +/** + * Set an account attribute on a path-finding span when the request field + * holds an r-address. + * + * @param span The live pathfind.request span. + * @param key The attribute key, pathfind_source_account or + * pathfind_dest_account. + * @param field The request parameter. Read it through a const json::Value so + * a missing key is not inserted into the request. + */ +inline void +setAccountAttribute(ScopedSpanGuard& span, std::string_view key, json::Value const& field) +{ + if (!field.isString()) + return; + if (auto const account = parseBase58(field.asString())) + span.setAttribute(key, toBase58(*account)); +} + +} // namespace xrpl::telemetry::pathfind_span diff --git a/src/xrpld/rpc/detail/PathFindSpanNames.h b/src/xrpld/rpc/detail/PathFindSpanNames.h index 47376f7e8b..5e5a71f811 100644 --- a/src/xrpld/rpc/detail/PathFindSpanNames.h +++ b/src/xrpld/rpc/detail/PathFindSpanNames.h @@ -80,10 +80,20 @@ inline constexpr auto discover = makeStr("discover"); namespace attr { /** * "pathfind_source_account" — originating account for path search. + * + * Emitted as the raw r-address, not hashed. An account address is a public + * ledger identifier drawn from an enumerable set, so an unsalted hash of it + * is reversible by lookup and protects nothing; it only breaks the join + * against explorers, RPC responses and logs that show the same address. + * Only a value that parses as an r-address is emitted, so a malformed or + * mistaken request value never reaches the span. Do not add redaction here + * or in a collector processor. */ inline constexpr auto sourceAccount = makeStr("pathfind_source_account"); /** * "pathfind_dest_account" — destination account. + * + * Raw r-address, for the same reason as pathfind_source_account. */ inline constexpr auto destAccount = makeStr("pathfind_dest_account"); /** @@ -109,7 +119,10 @@ inline constexpr auto numRequests = makeStr("pathfind_num_requests"); */ inline constexpr auto ledgerIndex = makeStr("pathfind_ledger_index"); /** - * "pathfind_dest_currency" — destination currency code. + * "pathfind_dest_currency" — destination asset as rendered by to_string(Asset): + * "XRP", "/" for an IOU, or the 48-hex-char + * issuance id for an MPT (its last 20 bytes are the issuer's account id). + * The issuer is a public identifier and is not hashed. */ inline constexpr auto destCurrency = makeStr("pathfind_dest_currency"); /** diff --git a/src/xrpld/rpc/detail/PathRequest.cpp b/src/xrpld/rpc/detail/PathRequest.cpp index 4baf6567d6..a7fb54801c 100644 --- a/src/xrpld/rpc/detail/PathRequest.cpp +++ b/src/xrpld/rpc/detail/PathRequest.cpp @@ -35,7 +35,6 @@ #include #include #include -#include #include #include @@ -780,22 +779,11 @@ PathRequest::doUpdate( if (span) { span.setAttribute(pathfind_span::attr::fast, fast); - // to_string(Issue) renders a non-XRP asset as "/" with - // the issuer as a plaintext Base58 address, so it cannot be emitted - // as-is: every account reaching a span is hashed first. Redact just the - // issuer and keep the currency, which is what this attribute is for. An - // MPT issuance id ends with the issuer's account id, so hash the whole - // id: that still gives one stable token per asset, without publishing - // the issuer. - span.setAttribute( - pathfind_span::attr::destCurrency, - saDstAmount_.asset().visit( - [](Issue const& issue) { - return isXRP(issue.account) - ? to_string(issue.currency) - : redactAccount(toBase58(issue.account)) + "/" + to_string(issue.currency); - }, - [](MPTIssue const& mpt) { return redactAccount(to_string(mpt.getMptID())); })); + // to_string(Asset) renders XRP as "XRP", an IOU as "/" + // with the issuer's Base58 address, and an MPT as its issuance id. The + // issuer is a public ledger identifier, so the asset is emitted as + // rendered (see the attribute docs in PathFindSpanNames.h). + span.setAttribute(pathfind_span::attr::destCurrency, to_string(saDstAmount_.asset())); } JLOG(journal_.debug()) << iIdentifier_ << " update " << (fast ? "fast" : "normal"); diff --git a/src/xrpld/rpc/handlers/orderbook/PathFind.cpp b/src/xrpld/rpc/handlers/orderbook/PathFind.cpp index af15403030..a4323b9e00 100644 --- a/src/xrpld/rpc/handlers/orderbook/PathFind.cpp +++ b/src/xrpld/rpc/handlers/orderbook/PathFind.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include @@ -10,7 +11,6 @@ #include #include #include -#include #include #include @@ -25,26 +25,23 @@ doPathFind(rpc::JsonContext& context) // thread) nest under it. doPathFind does not yield, so scoping is safe. auto span = ScopedSpanGuard( TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::request); - // Guarded on the span being live because setAttribute's arguments are - // evaluated whatever the build, and neither is free: asString() copies the - // address out of the JSON and redactAccount() takes a SHA-512Half over it. - // That is two copies and two hashes on every path_find call. The - // compiled-out guard's operator bool() is a literal false, so the block - // disappears entirely in that build; with telemetry compiled in it is - // skipped when telemetry is disabled at runtime or the category is off. + // Guarded on the span being live because the account parse below is not + // free and runs on every path_find call otherwise. The compiled-out + // guard's operator bool() is a literal false, so the block disappears + // entirely in that build; with telemetry compiled in it is skipped when + // telemetry is disabled at runtime or the category is off. if (span) { - // Addresses are hashed before emission for privacy. Read through a - // const reference: the non-const json::Value::operator[] inserts a null - // for a missing key, which would make PathRequest::parseJson's - // isMember() checks see an absent field as present and return Malformed - // instead of Missing. Reading for telemetry must not alter what the - // request looks like. + // Read through a const reference: the non-const json::Value::operator[] + // inserts a null for a missing key, which would make + // PathRequest::parseJson's isMember() checks see an absent field as + // present and return Malformed instead of Missing. Reading for + // telemetry must not alter what the request looks like. auto const& params = std::as_const(context.params); - if (auto const& src = params[jss::source_account]; src.isString()) - span.setAttribute(pathfind_span::attr::sourceAccount, redactAccount(src.asString())); - if (auto const& dst = params[jss::destination_account]; dst.isString()) - span.setAttribute(pathfind_span::attr::destAccount, redactAccount(dst.asString())); + pathfind_span::setAccountAttribute( + span, pathfind_span::attr::sourceAccount, params[jss::source_account]); + pathfind_span::setAccountAttribute( + span, pathfind_span::attr::destAccount, params[jss::destination_account]); } // A failed reply carries the rpc error token, so reading the status off the diff --git a/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp b/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp index 9abe40cdcc..944b442299 100644 --- a/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp +++ b/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -14,7 +15,6 @@ #include #include #include -#include #include #include @@ -34,26 +34,23 @@ doRipplePathFind(rpc::JsonContext& context) // span's log lines stay trace-correlated. auto span = ScopedSpanGuard( TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::request); - // Guarded on the span being live because setAttribute's arguments are - // evaluated whatever the build, and neither is free: asString() copies the - // address out of the JSON and redactAccount() takes a SHA-512Half over it. - // That is two copies and two hashes on every ripple_path_find call. The - // compiled-out guard's operator bool() is a literal false, so the block - // disappears entirely in that build; with telemetry compiled in it is - // skipped when telemetry is disabled at runtime or the category is off. + // Guarded on the span being live because the account parse below is not + // free and runs on every ripple_path_find call otherwise. The compiled-out + // guard's operator bool() is a literal false, so the block disappears + // entirely in that build; with telemetry compiled in it is skipped when + // telemetry is disabled at runtime or the category is off. if (span) { - // Addresses are hashed before emission for privacy. Read through a - // const reference: the non-const json::Value::operator[] inserts a null - // for a missing key, which would make PathRequest::parseJson's - // isMember() checks see an absent field as present and return Malformed - // instead of Missing. Reading for telemetry must not alter what the - // request looks like. + // Read through a const reference: the non-const json::Value::operator[] + // inserts a null for a missing key, which would make + // PathRequest::parseJson's isMember() checks see an absent field as + // present and return Malformed instead of Missing. Reading for + // telemetry must not alter what the request looks like. auto const& params = std::as_const(context.params); - if (auto const& src = params[jss::source_account]; src.isString()) - span.setAttribute(pathfind_span::attr::sourceAccount, redactAccount(src.asString())); - if (auto const& dst = params[jss::destination_account]; dst.isString()) - span.setAttribute(pathfind_span::attr::destAccount, redactAccount(dst.asString())); + pathfind_span::setAccountAttribute( + span, pathfind_span::attr::sourceAccount, params[jss::source_account]); + pathfind_span::setAccountAttribute( + span, pathfind_span::attr::destAccount, params[jss::destination_account]); } // A failed reply carries the rpc error token, so reading the status off the From 9bfa3cc32cf73934d74c4526ef243a4f74e22836 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 23 Sep 2026 18:00:34 +0100 Subject: [PATCH 4/6] 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. --- OpenTelemetryPlan/02-design-decisions.md | 78 ++++++++++--------- .../05-configuration-reference.md | 2 +- OpenTelemetryPlan/OpenTelemetryPlan.md | 2 +- 3 files changed, 42 insertions(+), 40 deletions(-) diff --git a/OpenTelemetryPlan/02-design-decisions.md b/OpenTelemetryPlan/02-design-decisions.md index cb435951f4..e39e0bc4db 100644 --- a/OpenTelemetryPlan/02-design-decisions.md +++ b/OpenTelemetryPlan/02-design-decisions.md @@ -240,7 +240,7 @@ keys (the dotted form is reserved for resource scope per §2.3.3). | -------------------- | ------ | ------------------------------------- | | `tx_hash` | string | Transaction hash (hex) | | `tx_type` | string | `"Payment"`, `"OfferCreate"`, etc. | -| `tx_account` | string | Source account (redacted in prod) | +| `tx_account` | string | Source account, raw r-address | | `tx_sequence` | int64 | Account sequence number | | `tx_fee` | int64 | Fee in drops | | `tx_result` | string | `"tesSUCCESS"`, `"tecPATH_DRY"`, etc. | @@ -299,12 +299,14 @@ keys (the dotted form is reserved for resource scope per §2.3.3). #### PathFinding Attributes -| Key | Type | Description | -| -------------------------- | ------ | ------------------------- | -| `pathfind_source_currency` | string | Source currency code | -| `pathfind_dest_currency` | string | Destination currency code | -| `pathfind_path_count` | int64 | Number of paths found | -| `pathfind_cache_hit` | bool | RippleLineCache hit | +| Key | Type | Description | +| -------------------------- | ------ | ---------------------------------------------------------------------- | +| `pathfind_source_account` | string | Source r-address, raw | +| `pathfind_dest_account` | string | Destination r-address, raw | +| `pathfind_source_currency` | string | Source currency code | +| `pathfind_dest_currency` | string | Destination asset: `XRP`, `/`, or an MPT issuance id | +| `pathfind_path_count` | int64 | Number of paths found | +| `pathfind_cache_hit` | bool | RippleLineCache hit | #### TxQ Attributes @@ -383,50 +385,50 @@ The following data is explicitly **excluded** from telemetry collection: #### Privacy Protection Mechanisms -| Mechanism | Description | -| ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| **Account Hashing** | Account addresses are hashed both SDK-side (`pathfind_source_account`, `pathfind_dest_account` — always hashed before emission) and again at the collector level, so raw addresses never reach storage | -| **Configurable Redaction** | Sensitive fields can be excluded via `[telemetry]` config section | -| **Collector Tail Sampling** | xrpld head sampling is fixed at 1.0 (every span emitted); the collector retains ~10% of non-error traces, reducing stored data exposure | -| **Sampling** | Only 10% of traces recorded by default, reducing data exposure | -| **Local Control** | Node operators have full control over what gets exported | -| **No Raw Payloads** | Transaction content is never recorded, only metadata (hash, type, result) | -| **Collector-Level Filtering** | Additional redaction/hashing can be configured at OTel Collector | +| Mechanism | Description | +| ----------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Account Addresses** | Emitted raw (`pathfind_source_account`, `pathfind_dest_account`). An account address is a public ledger identifier; hashing it protects nothing and breaks the join against explorers, RPC and logs | +| **Collector Tail Sampling** | xrpld head sampling is fixed at 1.0 (every span emitted); the collector retains ~10% of non-error traces, reducing stored data exposure | +| **Sampling** | Only 10% of traces recorded by default, reducing data exposure | +| **Local Control** | Node operators have full control over what gets exported | +| **No Raw Payloads** | Transaction content is never recorded, only metadata (hash, type, result) | +| **Collector-Level Filtering** | Available for a future genuinely sensitive attribute via an `attributes` processor. None is shipped, and none must be added for account addresses | -#### Account Address Hashing +#### Account Addresses -Account addresses are **always** hashed before they reach the telemetry -backend — there is no opt-out flag and therefore no insecure-by-default -failure mode. Protection is applied in two independent layers: +Account addresses are emitted **raw**, at every layer: -1. **SDK-side** (this node): the path-finding RPC handlers call - `redactAccount()` (`xrpl::telemetry`, `Redaction.h`) before setting the - `pathfind_source_account` / `pathfind_dest_account` span attributes. The - helper emits the first 16 characters of `sha512Half(address)` as - lowercase hex — deterministic (spans for one account still correlate) - but non-reversible. -2. **Collector-side** (defense-in-depth): an `attributes/hash` processor in - the OpenTelemetry Collector re-hashes those same attributes, so any node - that emitted a raw value is still redacted before storage. +1. **SDK-side** (this node): the path-finding RPC handlers set + `pathfind_source_account` / `pathfind_dest_account` to the request's + r-address, only when it parses as one, and `pathfind_dest_currency` to `to_string(Asset)`, + which carries the IOU issuer's r-address. The rationale sits on the attribute + constants in `PathFindSpanNames.h`. +2. **Collector-side**: no collector configuration in this repository hashes + or deletes these attributes. + +Why raw: an r-address is a public, enumerable identifier on the ledger. An +unsalted hash of it is reversible by table lookup, so it protects nothing, and +it breaks the one thing the attribute is for: joining a span to the account as +explorers, RPC responses and logs show it. The helper `redactAccount()` +(`xrpl::telemetry`, `Redaction.h`) remains available for a value that is +genuinely private, but it is applied to no span. #### Collector-Level Data Protection -The OpenTelemetry Collector can be configured (via an `attributes` processor) -to hash or redact sensitive attributes before export — for example, hashing -`pathfind_source_account` / `pathfind_dest_account`, deleting `peer_address` -to drop IP addresses, and deleting `params` to redact request parameters. +No hashing or redaction processor is shipped. If a future span introduces a +genuinely sensitive attribute, an `attributes` processor in the collector is the +place to strip it, and the attribute is added to the §2.4 catalogue with that +note in the same change. Account addresses are not such an attribute. #### Configuration Options for Privacy In `xrpld.cfg`, operators control data collection granularity through the `[telemetry]` section. Besides `enabled`, per-component toggles (`trace_transactions`, `trace_consensus`, `trace_rpc`, `trace_peer` — the last -often disabled due to high volume) select which spans are emitted. Account -address hashing is not configurable: addresses are hashed unconditionally by -the SDK helper described above, with collector-level hashing as a second -layer. +often disabled due to high volume) select which spans are emitted. There is no +redaction setting: account addresses are public and are emitted raw. -> **Key Principle**: Telemetry collects **operational metadata** (timing, counts, hashes) — never **sensitive content** (keys, balances, amounts, raw payloads). +> **Key Principle**: Telemetry collects **operational metadata** (timing, counts, hashes, public identifiers) — never **sensitive content** (keys, balances, amounts, raw payloads). --- diff --git a/OpenTelemetryPlan/05-configuration-reference.md b/OpenTelemetryPlan/05-configuration-reference.md index c97d47d6e4..aad0ec27d0 100644 --- a/OpenTelemetryPlan/05-configuration-reference.md +++ b/OpenTelemetryPlan/05-configuration-reference.md @@ -105,7 +105,7 @@ The development collector enables an OTLP receiver on both gRPC (`0.0.0.0:4317`) ### 5.5.2 Production Configuration -The production collector adds TLS on the OTLP gRPC receiver and a richer processor chain: a `memory_limiter` (OOM guard), `batch` (5s timeout, size 512), `tail_sampling`, and an `attributes` processor that hashes sensitive fields (e.g. `tx_account`) and stamps `deployment.environment`. Tail sampling keeps all `ERROR` traces, slow consensus rounds (>5s) and slow RPC requests (>1s), and probabilistically samples the remainder at 10%. Exporters target Grafana Tempo (TLS) and Elastic APM; `health_check` and `zpages` extensions are enabled for operability. +The production collector adds TLS on the OTLP gRPC receiver and a richer processor chain: a `memory_limiter` (OOM guard), `batch` (5s timeout, size 512), `tail_sampling`, and an `attributes` processor that stamps `deployment.environment`. Account addresses are public identifiers and are not hashed at any layer. Tail sampling keeps all `ERROR` traces, slow consensus rounds (>5s) and slow RPC requests (>1s), and probabilistically samples the remainder at 10%. Exporters target Grafana Tempo (TLS) and Elastic APM; `health_check` and `zpages` extensions are enabled for operability. --- diff --git a/OpenTelemetryPlan/OpenTelemetryPlan.md b/OpenTelemetryPlan/OpenTelemetryPlan.md index 51e83b6d65..687225f03a 100644 --- a/OpenTelemetryPlan/OpenTelemetryPlan.md +++ b/OpenTelemetryPlan/OpenTelemetryPlan.md @@ -130,7 +130,7 @@ The OpenTelemetry C++ SDK is selected for its CNCF backing, active development, Span naming follows a hierarchical `.` convention (e.g., `rpc.submit`, `tx.relay`, `consensus.round`). Context propagation uses W3C Trace Context headers for HTTP and embedded Protocol Buffer fields for P2P messages. The implementation coexists with existing PerfLog and Insight observability systems through correlation IDs. -**Data Collection & Privacy**: Telemetry collects only operational metadata (timing, counts, hashes) — never sensitive content (private keys, balances, amounts, raw payloads). Privacy protection includes account hashing, configurable redaction, sampling, and collector-level filtering. Node operators retain full control over telemetry configuration. +**Data Collection & Privacy**: Telemetry collects only operational metadata (timing, counts, hashes) — never sensitive content (private keys, balances, amounts, raw payloads). Account addresses are public ledger identifiers and are emitted raw; there is no redaction setting. Trace volume is reduced, where wanted, by collector-side sampling. Node operators retain full control over telemetry configuration. ➡️ **[Read full Design Decisions](./02-design-decisions.md)** From 509fc7853e7e812f8c505ece997bbc1ff222b557 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 23 Sep 2026 18:01:14 +0100 Subject: [PATCH 5/6] 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_ 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. --- OpenTelemetryPlan/Phase3_taskList.md | 37 ++-- cmake/XrplCore.cmake | 3 +- include/xrpl/telemetry/TxAccountSpanNames.h | 172 ++++++++++++++++++ src/libxrpl/telemetry/TxAccountSpanNames.cpp | 45 +++++ .../libxrpl/telemetry/TxAccountSpanNames.cpp | 163 +++++++++++++++++ src/xrpld/app/misc/NetworkOPs.cpp | 13 ++ src/xrpld/telemetry/TxSpanNames.h | 3 + 7 files changed, 417 insertions(+), 19 deletions(-) create mode 100644 include/xrpl/telemetry/TxAccountSpanNames.h create mode 100644 src/libxrpl/telemetry/TxAccountSpanNames.cpp create mode 100644 src/tests/libxrpl/telemetry/TxAccountSpanNames.cpp diff --git a/OpenTelemetryPlan/Phase3_taskList.md b/OpenTelemetryPlan/Phase3_taskList.md index 601e0bd047..1652d3c89b 100644 --- a/OpenTelemetryPlan/Phase3_taskList.md +++ b/OpenTelemetryPlan/Phase3_taskList.md @@ -487,24 +487,25 @@ This gives the best of both worlds: guaranteed cross-node correlation via determ **Attributes added**: -| Span | Attribute | Type | Source | -| ----------------- | -------------------- | ------ | ------------------------------------------------------------------- | -| `tx.process` | `tx_type` | string | `TxFormats::getInstance().findByType(stx->getTxnType())->getName()` | -| `tx.process` | `fee` | int64 | `stx->getFieldAmount(sfFee).xrp().drops()` | -| `tx.process` | `sequence` | int64 | `stx->getSeqProxy().value()` | -| `tx.process` | `ter_result` | string | `transToken(e.result)` (set after batch application) | -| `tx.process` | `applied` | bool | `e.applied` (set after batch application) | -| `tx.receive` | `tx_type` | string | `TxFormats::getInstance().findByType(stx->getTxnType())->getName()` | -| `txq.enqueue` | `tx_type` | string | same pattern as above | -| `txq.enqueue` | `txq_status` | string | `queued` / `applied_direct` / `applied` / `failed` / `rejected` | -| `txq.enqueue` | `ter_code` | string | `transToken(directApplied->ter)` (set on the direct-apply path) | -| `txq.enqueue` | `fee_level_paid` | int64 | `getFeeLevelPaid(view, *tx).value()` | -| `txq.enqueue` | `required_fee_level` | int64 | `getRequiredFeeLevel(...).value()` | -| `txq.batch_clear` | `num_cleared` | int64 | queued txs cleared ahead of the applying tx | -| `txq.cleanup` | `expired_count` | int64 | entries dropped for passed `LastLedgerSequence` | -| `txq.accept_tx` | `txq_status` | string | `applied` / `failed` / `retried` | -| `txq.accept_tx` | `ter_code` | string | `transToken(txnResult)` (set before branching on the outcome) | -| `txq.accept` | `ledger_changed` | bool | set at end of accept loop | +| Span | Attribute | Type | Source | +| ----------------- | -------------------- | ------ | -------------------------------------------------------------------------------------------------------------------------- | +| `tx.process` | `tx_type` | string | `TxFormats::getInstance().findByType(stx->getTxnType())->getName()` | +| `tx.process` | `fee` | int64 | `stx->getFieldAmount(sfFee).xrp().drops()` | +| `tx.process` | `sequence` | int64 | `stx->getSeqProxy().value()` | +| `tx.process` | `tx_` | string | one per top-level `STI_ACCOUNT` field, raw r-address (`tx_account`, `tx_destination`, ...); keys in `TxAccountSpanNames.h` | +| `tx.process` | `ter_result` | string | `transToken(e.result)` (set after batch application) | +| `tx.process` | `applied` | bool | `e.applied` (set after batch application) | +| `tx.receive` | `tx_type` | string | `TxFormats::getInstance().findByType(stx->getTxnType())->getName()` | +| `txq.enqueue` | `tx_type` | string | same pattern as above | +| `txq.enqueue` | `txq_status` | string | `queued` / `applied_direct` / `applied` / `failed` / `rejected` | +| `txq.enqueue` | `ter_code` | string | `transToken(directApplied->ter)` (set on the direct-apply path) | +| `txq.enqueue` | `fee_level_paid` | int64 | `getFeeLevelPaid(view, *tx).value()` | +| `txq.enqueue` | `required_fee_level` | int64 | `getRequiredFeeLevel(...).value()` | +| `txq.batch_clear` | `num_cleared` | int64 | queued txs cleared ahead of the applying tx | +| `txq.cleanup` | `expired_count` | int64 | entries dropped for passed `LastLedgerSequence` | +| `txq.accept_tx` | `txq_status` | string | `applied` / `failed` / `retried` | +| `txq.accept_tx` | `ter_code` | string | `transToken(txnResult)` (set before branching on the outcome) | +| `txq.accept` | `ledger_changed` | bool | set at end of accept loop | **New attr keys**: `TxSpanNames.h` (`txType`, `fee`, `sequence`, `terResult`, `applied`), `TxQSpanNames.h` (`txType`). diff --git a/cmake/XrplCore.cmake b/cmake/XrplCore.cmake index c9d392cfcc..ed619c286f 100644 --- a/cmake/XrplCore.cmake +++ b/cmake/XrplCore.cmake @@ -222,7 +222,8 @@ target_link_libraries( # opentelemetry-cpp::opentelemetry-cpp (individual component targets like # ::api, ::sdk are not available in the Conan package). # -# Links xrpl.libxrpl.protocol PRIVATELY for sha512Half (digest.h) +# Links xrpl.libxrpl.protocol PRIVATELY for sha512Half (digest.h) and the +# SField table behind TxAccountSpanNames.cpp add_module(xrpl telemetry) target_link_libraries( xrpl.libxrpl.telemetry diff --git a/include/xrpl/telemetry/TxAccountSpanNames.h b/include/xrpl/telemetry/TxAccountSpanNames.h new file mode 100644 index 0000000000..fd6560a9f7 --- /dev/null +++ b/include/xrpl/telemetry/TxAccountSpanNames.h @@ -0,0 +1,172 @@ +#pragma once + +/** + * Span attribute keys for the account-typed fields of a transaction. + * + * A transaction names one or more accounts: the sender in `Account`, and + * depending on the type a `Destination`, `Owner`, `Issuer`, `Holder` and so + * on. The tx.process span emits every one it finds as its own attribute, so + * an account can be searched for in traces whatever role it played. An + * account address is a public ledger identifier, so each is emitted as the + * raw r-address and never hashed. + * + * One key per protocol field: `tx_` followed by the field's JSON name in + * lower snake case. The full table is the initializer in + * src/libxrpl/telemetry/TxAccountSpanNames.cpp; the common ones are + * + * STTx field span attribute key + * ----------------- ------------------ + * Account tx_account + * Destination tx_destination + * Owner tx_owner + * Issuer tx_issuer + * RegularKey tx_regular_key + * NFTokenMinter tx_nftoken_minter + * + * Only fields that some transaction format carries at top level have a key. + * Account-typed fields that appear only in ledger entries or inner objects + * (LowSponsor, LockingChainDoor, ...) map to nullopt. A library test walks + * TxFormats and fails when a format gains an account field with no key. + * + * Why this header lives in libxrpl rather than beside TxSpanNames.h: the + * mapping is keyed by protocol fields and its completeness is checked from + * TxFormats, which a library test can reach and a daemon header cannot. + * + * Data flow: + * + * NetworkOPs::processTransaction (src/xrpld) + * │ for each top-level field with getSType() == STI_ACCOUNT + * ▼ + * accountFieldAttributeKey(field.getFName()) (this header) + * │ the key, or nullopt for a field with no key + * ▼ + * span->setAttribute(key, field.getText()) + * + * @code + * // Primary use: emit every account the transaction names. An empty + * // account field is skipped so it is not rendered as the zero address. + * for (auto const& field : stx) + * { + * if (field.getSType() != STI_ACCOUNT || field.isDefault()) + * continue; + * if (auto const key = telemetry::accountFieldAttributeKey(field.getFName())) + * span.setAttribute(*key, toBase58(stx.getAccountID(field.getFName()))); + * } + * @endcode + * + * @code + * // Edge case: a field that is not a top-level transaction account has + * // no key, so a caller must test the optional before using it. + * accountFieldAttributeKey(sfFee); // == std::nullopt + * accountFieldAttributeKey(sfLowSponsor); // == std::nullopt + * @endcode + * + * @note Only top-level fields are covered. Accounts nested in Signers, in a + * Batch's inner transactions, or as the issuer inside an Amount are not + * emitted. + * @note accountFieldAttributeKey() is thread-safe. Its table is built once + * on first use and is read-only afterwards. + */ + +#include + +#include +#include + +namespace xrpl { +class SField; +} // namespace xrpl + +namespace xrpl::telemetry { + +namespace tx_account_span::attr { +/** + * "tx_account" — the sending account (`Account`). Every transaction has one. + */ +inline constexpr auto account = makeStr("tx_account"); +/** + * "tx_destination" — the receiving account (`Destination`). + */ +inline constexpr auto destination = makeStr("tx_destination"); +/** + * "tx_owner" — the owner of the object acted on (`Owner`). + */ +inline constexpr auto owner = makeStr("tx_owner"); +/** + * "tx_issuer" — the issuer named by the transaction (`Issuer`). + */ +inline constexpr auto issuer = makeStr("tx_issuer"); +/** + * "tx_authorize" — the account being authorised (`Authorize`). + */ +inline constexpr auto authorize = makeStr("tx_authorize"); +/** + * "tx_unauthorize" — the account whose authorisation is removed (`Unauthorize`). + */ +inline constexpr auto unauthorize = makeStr("tx_unauthorize"); +/** + * "tx_regular_key" — the regular key being set (`RegularKey`). + */ +inline constexpr auto regularKey = makeStr("tx_regular_key"); +/** + * "tx_nftoken_minter" — the authorised NFToken minter (`NFTokenMinter`). + */ +inline constexpr auto nftokenMinter = makeStr("tx_nftoken_minter"); +/** + * "tx_holder" — the token holder acted on (`Holder`). + */ +inline constexpr auto holder = makeStr("tx_holder"); +/** + * "tx_delegate" — the delegate signing on the sender's behalf (`Delegate`). + */ +inline constexpr auto delegate = makeStr("tx_delegate"); +/** + * "tx_sponsor" — the account paying the fee or reserve (`Sponsor`). + */ +inline constexpr auto sponsor = makeStr("tx_sponsor"); +/** + * "tx_sponsee" — the account being sponsored (`Sponsee`). + */ +inline constexpr auto sponsee = makeStr("tx_sponsee"); +/** + * "tx_counterparty" — the other party to a loan (`Counterparty`). + */ +inline constexpr auto counterparty = makeStr("tx_counterparty"); +/** + * "tx_counterparty_sponsor" — the counterparty's sponsor (`CounterpartySponsor`). + */ +inline constexpr auto counterpartySponsor = makeStr("tx_counterparty_sponsor"); +/** + * "tx_subject" — the subject of a credential (`Subject`). + */ +inline constexpr auto subject = makeStr("tx_subject"); +/** + * "tx_other_chain_source" — the source account on the other chain (`OtherChainSource`). + */ +inline constexpr auto otherChainSource = makeStr("tx_other_chain_source"); +/** + * "tx_other_chain_destination" — destination on the other chain (`OtherChainDestination`). + */ +inline constexpr auto otherChainDestination = makeStr("tx_other_chain_destination"); +/** + * "tx_attestation_signer_account" — the attestation signer (`AttestationSignerAccount`). + */ +inline constexpr auto attestationSignerAccount = makeStr("tx_attestation_signer_account"); +/** + * "tx_attestation_reward_account" — attestation reward account (`AttestationRewardAccount`). + */ +inline constexpr auto attestationRewardAccount = makeStr("tx_attestation_reward_account"); +} // namespace tx_account_span::attr + +/** + * Look up the span attribute key for an account-typed transaction field. + * + * @param field The protocol field, as returned by STBase::getFName(). + * @return The `tx_*` key for a top-level transaction account field, or + * nullopt when the field is not account-typed or is carried only by ledger + * entries and inner objects. + */ +[[nodiscard]] std::optional +accountFieldAttributeKey(SField const& field); + +} // namespace xrpl::telemetry diff --git a/src/libxrpl/telemetry/TxAccountSpanNames.cpp b/src/libxrpl/telemetry/TxAccountSpanNames.cpp new file mode 100644 index 0000000000..71318f4195 --- /dev/null +++ b/src/libxrpl/telemetry/TxAccountSpanNames.cpp @@ -0,0 +1,45 @@ +#include + +#include + +#include +#include +#include + +namespace xrpl::telemetry { + +std::optional +accountFieldAttributeKey(SField const& field) +{ + // Built on first call, not at static initialisation: the sf* objects are + // globals in another translation unit, so their codes are only safe to + // read once main() has started. + static std::unordered_map const kTable = { + {sfAccount.getCode(), tx_account_span::attr::account}, + {sfDestination.getCode(), tx_account_span::attr::destination}, + {sfOwner.getCode(), tx_account_span::attr::owner}, + {sfIssuer.getCode(), tx_account_span::attr::issuer}, + {sfAuthorize.getCode(), tx_account_span::attr::authorize}, + {sfUnauthorize.getCode(), tx_account_span::attr::unauthorize}, + {sfRegularKey.getCode(), tx_account_span::attr::regularKey}, + {sfNFTokenMinter.getCode(), tx_account_span::attr::nftokenMinter}, + {sfHolder.getCode(), tx_account_span::attr::holder}, + {sfDelegate.getCode(), tx_account_span::attr::delegate}, + {sfSponsor.getCode(), tx_account_span::attr::sponsor}, + {sfSponsee.getCode(), tx_account_span::attr::sponsee}, + {sfCounterparty.getCode(), tx_account_span::attr::counterparty}, + {sfCounterpartySponsor.getCode(), tx_account_span::attr::counterpartySponsor}, + {sfSubject.getCode(), tx_account_span::attr::subject}, + {sfOtherChainSource.getCode(), tx_account_span::attr::otherChainSource}, + {sfOtherChainDestination.getCode(), tx_account_span::attr::otherChainDestination}, + {sfAttestationSignerAccount.getCode(), tx_account_span::attr::attestationSignerAccount}, + {sfAttestationRewardAccount.getCode(), tx_account_span::attr::attestationRewardAccount}, + }; + + auto const it = kTable.find(field.getCode()); + if (it == kTable.end()) + return std::nullopt; + return it->second; +} + +} // namespace xrpl::telemetry diff --git a/src/tests/libxrpl/telemetry/TxAccountSpanNames.cpp b/src/tests/libxrpl/telemetry/TxAccountSpanNames.cpp new file mode 100644 index 0000000000..0b51776647 --- /dev/null +++ b/src/tests/libxrpl/telemetry/TxAccountSpanNames.cpp @@ -0,0 +1,163 @@ +#include + +#include +#include + +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +/** + * Contract tests for the account-field attribute keys of the tx.process span. + * + * The key set is a cross-component contract: Tempo span-filter tags, the + * naming CI check and dashboards read these exact strings. A transaction type + * that gains an account-typed field without a key would silently emit nothing + * for it, so the completeness tests derive the required set from TxFormats + * and from the SField registry rather than from a copied list. + */ + +using namespace xrpl; +using namespace xrpl::telemetry; + +namespace { + +// Every account-typed field that any transaction format carries at top level, +// including the common fields shared by all formats. +std::set +accountFieldsInTransactionFormats() +{ + std::set fields; + for (auto const& format : TxFormats::getInstance()) + { + for (auto const& element : format.getSOTemplate()) + { + if (element.sField().fieldType == STI_ACCOUNT) + fields.insert(&element.sField()); + } + } + return fields; +} + +// Every account-typed field the protocol defines, whether or not a transaction +// carries it. +std::set +allAccountFields() +{ + std::set fields; + for (auto const& [code, field] : SField::getKnownCodeToField()) + { + if (field->fieldType == STI_ACCOUNT) + fields.insert(field); + } + return fields; +} + +} // namespace + +TEST(TxAccountSpanNames, every_account_field_a_transaction_can_carry_has_a_key) +{ + auto const fields = accountFieldsInTransactionFormats(); + // Setup: the walk over TxFormats found the one field every transaction has. + ASSERT_TRUE(fields.contains(&sfAccount)); + + for (auto const* field : fields) + { + EXPECT_TRUE(accountFieldAttributeKey(*field).has_value()) + << "no attribute key for " << field->getName(); + } +} + +TEST(TxAccountSpanNames, account_fields_no_transaction_carries_have_no_key) +{ + auto const carried = accountFieldsInTransactionFormats(); + auto const all = allAccountFields(); + // Setup: the registry holds more account fields than transactions carry. + ASSERT_TRUE(all.contains(&sfLowSponsor)); + ASSERT_FALSE(carried.contains(&sfLowSponsor)); + + for (auto const* field : all) + { + if (!carried.contains(field)) + EXPECT_EQ(accountFieldAttributeKey(*field), std::nullopt) << field->getName(); + } +} + +TEST(TxAccountSpanNames, every_key_is_tx_plus_the_field_name_in_lower_snake_case) +{ + for (auto const* field : accountFieldsInTransactionFormats()) + { + auto const maybeKey = accountFieldAttributeKey(*field); + ASSERT_TRUE(maybeKey.has_value()) << field->getName(); + auto const key = maybeKey.value_or(std::string_view{}); + + // Shape: tx_ prefix, then lower_snake_case with no empty segment. + EXPECT_TRUE(key.starts_with("tx_")) << key; + EXPECT_FALSE(key.ends_with('_')) << key; + EXPECT_EQ(key.find("__"), std::string_view::npos) << key; + EXPECT_TRUE(std::ranges::all_of(key, [](unsigned char c) { + return std::islower(c) != 0 || std::isdigit(c) != 0 || c == '_'; + })) << key; + + // Content: the key with prefix and underscores removed is the field's + // JSON name lowercased. Catches a misspelt or swapped key. + std::string flattened(key.substr(3)); + std::erase(flattened, '_'); + std::string lowered = field->getName(); + std::ranges::transform(lowered, lowered.begin(), [](unsigned char c) { + return static_cast(std::tolower(c)); + }); + EXPECT_EQ(flattened, lowered) << key; + } +} + +// The published contract: every carried field and its exact key. Literals are +// deliberate here; the point is to pin underscore placement, which the shape +// test above cannot see. +TEST(TxAccountSpanNames, exact_key_for_every_carried_field) +{ + std::vector> const expected = { + {&sfAccount, "tx_account"}, + {&sfDestination, "tx_destination"}, + {&sfOwner, "tx_owner"}, + {&sfIssuer, "tx_issuer"}, + {&sfAuthorize, "tx_authorize"}, + {&sfUnauthorize, "tx_unauthorize"}, + {&sfRegularKey, "tx_regular_key"}, + {&sfNFTokenMinter, "tx_nftoken_minter"}, + {&sfHolder, "tx_holder"}, + {&sfDelegate, "tx_delegate"}, + {&sfSponsor, "tx_sponsor"}, + {&sfSponsee, "tx_sponsee"}, + {&sfCounterparty, "tx_counterparty"}, + {&sfCounterpartySponsor, "tx_counterparty_sponsor"}, + {&sfSubject, "tx_subject"}, + {&sfOtherChainSource, "tx_other_chain_source"}, + {&sfOtherChainDestination, "tx_other_chain_destination"}, + {&sfAttestationSignerAccount, "tx_attestation_signer_account"}, + {&sfAttestationRewardAccount, "tx_attestation_reward_account"}, + }; + // Setup: this list and the TxFormats walk must name the same fields, or a + // row is missing here. + std::set listed; + for (auto const& [field, key] : expected) + listed.insert(field); + ASSERT_EQ(listed, accountFieldsInTransactionFormats()); + + for (auto const& [field, key] : expected) + EXPECT_EQ(accountFieldAttributeKey(*field).value_or(""), key) << field->getName(); +} + +TEST(TxAccountSpanNames, non_account_fields_have_no_key) +{ + EXPECT_EQ(accountFieldAttributeKey(sfFee), std::nullopt); + EXPECT_EQ(accountFieldAttributeKey(sfSequence), std::nullopt); + EXPECT_EQ(accountFieldAttributeKey(sfInvalid), std::nullopt); +} diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index e4c16bd85c..17fead873e 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -119,6 +119,7 @@ #include #include #include +#include #include #include @@ -1571,6 +1572,18 @@ NetworkOPsImp::processTransaction( } span->setAttribute( tx_span::attr::sequence, static_cast(stx->getSeqProxy().value())); + // Every account the transaction names, keyed by its role + // (tx_account, tx_destination, ...). Addresses are public ledger + // identifiers and go out raw; see TxAccountSpanNames.h. A present + // but empty account field is skipped rather than rendered as the + // all-zero address. + for (auto const& field : *stx) + { + if (field.getSType() != STI_ACCOUNT || field.isDefault()) + continue; + if (auto const key = accountFieldAttributeKey(field.getFName())) + span->setAttribute(*key, field.getText()); + } } } diff --git a/src/xrpld/telemetry/TxSpanNames.h b/src/xrpld/telemetry/TxSpanNames.h index 1b54f3371b..64049da6c9 100644 --- a/src/xrpld/telemetry/TxSpanNames.h +++ b/src/xrpld/telemetry/TxSpanNames.h @@ -85,6 +85,9 @@ inline constexpr auto fee = makeStr("fee"); * "sequence" — transaction sequence number. */ inline constexpr auto sequence = makeStr("sequence"); +// The per-role account keys (tx_account, tx_destination, ...) that tx.process +// also carries live in , in libxrpl, so a +// library test can check them against TxFormats. /** * "ter_result" — engine result code after application. */ From dd8a35ec36eb378445fbd74daad46c23dca52d4a Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 23 Sep 2026 18:02:15 +0100 Subject: [PATCH 6/6] docs(telemetry): List the per-role account attributes of tx.process --- OpenTelemetryPlan/02-design-decisions.md | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/OpenTelemetryPlan/02-design-decisions.md b/OpenTelemetryPlan/02-design-decisions.md index b67ede46b5..3ab48809e7 100644 --- a/OpenTelemetryPlan/02-design-decisions.md +++ b/OpenTelemetryPlan/02-design-decisions.md @@ -236,16 +236,17 @@ keys (the dotted form is reserved for resource scope per §2.3.3). #### Transaction Attributes -| Key | Type | Description | -| -------------------- | ------ | ------------------------------------- | -| `tx_hash` | string | Transaction hash (hex) | -| `tx_type` | string | `"Payment"`, `"OfferCreate"`, etc. | -| `tx_account` | string | Source account, raw r-address | -| `tx_sequence` | int64 | Account sequence number | -| `tx_fee` | int64 | Fee in drops | -| `tx_result` | string | `"tesSUCCESS"`, `"tecPATH_DRY"`, etc. | -| `current_ledger_seq` | int64 | Open ledger the transaction targeted | -| `relay_count` | int64 | Peers the transaction was relayed to | +| Key | Type | Description | +| -------------------- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `tx_hash` | string | Transaction hash (hex) | +| `tx_type` | string | `"Payment"`, `"OfferCreate"`, etc. | +| `tx_account` | string | Sending account, raw r-address | +| `tx_` | string | One per account-typed top-level field the transaction carries (`tx_destination`, `tx_owner`, `tx_issuer`, ...), raw r-address; keys in `TxAccountSpanNames.h` | +| `tx_sequence` | int64 | Account sequence number | +| `tx_fee` | int64 | Fee in drops | +| `tx_result` | string | `"tesSUCCESS"`, `"tecPATH_DRY"`, etc. | +| `current_ledger_seq` | int64 | Open ledger the transaction targeted | +| `relay_count` | int64 | Peers the transaction was relayed to | > **Note:** `current_ledger_seq` and `ledger_seq` are the same concept — a ledger's sequence number — but they name different ledgers, so the design keeps two keys rather than one. `current_ledger_seq` is the open or in-flight ledger a transaction's work was applied into; it is named after the RPC field `ledger_current_index`. `ledger_seq` (see [Ledger & Job Attributes](#ledger--job-attributes)) is a closed or validated ledger, set by the ledger and consensus spans. Neither is spelled `ledger_index`: per rule 2 of [Telemetry span attribute naming](../CONTRIBUTING.md#telemetry-span-attribute-naming), one concept gets one key reused verbatim, and a different referent is disambiguated with a prefix rather than a synonym.