diff --git a/OpenTelemetryPlan/02-design-decisions.md b/OpenTelemetryPlan/02-design-decisions.md index 4cc9441c74..1e5bb4ea8b 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 (redacted in prod) | -| `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. @@ -316,12 +317,14 @@ Establish-phase gap fill and cross-node correlation attributes (Phase 4a): #### 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 @@ -400,50 +403,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 a7d73700be..7ddc62bd9e 100644 --- a/OpenTelemetryPlan/05-configuration-reference.md +++ b/OpenTelemetryPlan/05-configuration-reference.md @@ -107,7 +107,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)** 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 f38f53ab25..6c0ab2f2b6 100644 --- a/cmake/XrplCore.cmake +++ b/cmake/XrplCore.cmake @@ -216,7 +216,8 @@ target_link_libraries( # each module's headers: a module can only include xrpl/telemetry/ headers if # it links this target, and the target must already exist at that point. # -# 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/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/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 3ecb2a4e1c..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 @@ -1561,10 +1562,28 @@ 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())); + // 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/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 1154109fc9..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,21 +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 asset renders as its issuance ID and carries no address, so it - // needs no redaction. - 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 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 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. */