From 3df7e9cba680c0a5812c8b5f3fe2d8be10b5946a Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Fri, 5 Jun 2026 11:42:33 +0100 Subject: [PATCH] code review changes and wire unused attributes Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> --- OpenTelemetryPlan/Phase3_taskList.md | 27 ++++++++++++++---------- src/xrpld/app/misc/detail/TxQ.cpp | 18 +++++++++++++++- src/xrpld/app/misc/detail/TxQSpanNames.h | 6 ++++-- 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/OpenTelemetryPlan/Phase3_taskList.md b/OpenTelemetryPlan/Phase3_taskList.md index 55b00690ea..5cb723d878 100644 --- a/OpenTelemetryPlan/Phase3_taskList.md +++ b/OpenTelemetryPlan/Phase3_taskList.md @@ -474,17 +474,22 @@ 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.accept.tx` | `txq_status` | string | `applied` / `failed` / `retried` | -| `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` | `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` / `rejected` | +| `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` | `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/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index 352bef6bd9..2a6f00385f 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -607,7 +607,8 @@ TxQ::tryClearAccountQueueUpThruTx( if (txResult.applied) { // All of the queued transactions applied, so remove them from the - // queue. + // queue. `dist` queued txs preceded the current one in the batch. + span.setAttribute(txq_span::attr::numCleared, static_cast(dist)); endTxIter = erase(accountIter->second, beginTxIter, endTxIter); // If `tx` is replacing a queued tx, delete that one, too. if (endTxIter != accountIter->second.transactions.end() && endTxIter->first == tSeqProx) @@ -744,6 +745,9 @@ TxQ::apply( span.setAttribute(txq_span::attr::txHash, to_string(tx->getTransactionID()).c_str()); if (auto const* fmt = TxFormats::getInstance().findByType(tx->getTxnType())) span.setAttribute(txq_span::attr::txType, fmt->getName().c_str()); + // Default outcome; overridden below on the direct-apply and queued paths. + // Every other early return leaves the tx rejected from the queue. + span.setAttribute(txq_span::attr::txqStatus, txq_span::val::rejected); NumberSO const stNumberSO{view.rules().enabled(fixUniversalNumber)}; @@ -757,7 +761,10 @@ TxQ::apply( // See if the transaction paid a high enough fee that it can go straight // into the ledger. if (auto directApplied = tryDirectApply(app, view, tx, flags, j)) + { + span.setAttribute(txq_span::attr::txqStatus, txq_span::val::appliedDirect); return *directApplied; + } if ((flags & TapDryRun) != 0u) return {telCAN_NOT_QUEUE, false}; @@ -884,6 +891,10 @@ TxQ::apply( auto const metricsSnapshot = feeMetrics_.getSnapshot(); auto const feeLevelPaid = getFeeLevelPaid(view, *tx); auto const requiredFeeLevel = getRequiredFeeLevel(view, flags, metricsSnapshot, lock); + span.setAttribute( + txq_span::attr::feeLevelPaid, static_cast(feeLevelPaid.value())); + span.setAttribute( + txq_span::attr::requiredFeeLevel, static_cast(requiredFeeLevel.value())); // Is there a blocker already in the account's queue? If so, don't // allow additional transactions in the queue. @@ -1217,6 +1228,7 @@ TxQ::apply( /* Can't erase (*replacedTxIter) here because success implies that it has already been deleted. */ + span.setAttribute(txq_span::attr::txqStatus, txq_span::val::applied); return result; } } @@ -1332,6 +1344,7 @@ TxQ::apply( << " to queue." << " Flags: " << flags; + span.setAttribute(txq_span::attr::txqStatus, txq_span::val::queued); return {terQUEUED, false}; } @@ -1366,18 +1379,21 @@ TxQ::processClosedLedger(Application& app, ReadView const& view, bool timeLeap) maxSize_ = std::max(snapshot.txnsExpected * setup_.ledgersInQueue, setup_.queueSizeMin); // Remove any queued candidates whose LastLedgerSequence has gone by. + std::int64_t expiredCount = 0; for (auto candidateIter = byFee_.begin(); candidateIter != byFee_.end();) { if (candidateIter->lastValid && *candidateIter->lastValid <= ledgerSeq) { byAccount_.at(candidateIter->account).dropPenalty = true; candidateIter = erase(candidateIter); + ++expiredCount; } else { ++candidateIter; } } + span.setAttribute(txq_span::attr::expiredCount, expiredCount); // Remove any TxQAccounts that don't have candidates // under them diff --git a/src/xrpld/app/misc/detail/TxQSpanNames.h b/src/xrpld/app/misc/detail/TxQSpanNames.h index 9292ba1e7c..3f8f86aa30 100644 --- a/src/xrpld/app/misc/detail/TxQSpanNames.h +++ b/src/xrpld/app/misc/detail/TxQSpanNames.h @@ -15,12 +15,14 @@ * | +--------------------------------------------------+ | * | | txq.enqueue | | * | | TxQ::apply() | | - * | | attrs: tx_hash, status, fee_level | | + * | | attrs: tx_hash, tx_type, txq_status, | | + * | | fee_level_paid, required_fee_level | | * | | | | * | | +-------------------+ +----------------------+ | | * | | | txq.apply_direct | | txq.batch_clear | | | * | | | tryDirectApply() | | tryClearAccount...() | | | - * | | +-------------------+ +----------------------+ | | + * | | +-------------------+ | attrs: num_cleared | | | + * | | +----------------------+ | | * | +--------------------------------------------------+ | * +-------------------------------------------------------+ *