diff --git a/include/xrpl/consensus/Consensus.h b/include/xrpl/consensus/Consensus.h index 78750656a4..aced7555ea 100644 --- a/include/xrpl/consensus/Consensus.h +++ b/include/xrpl/consensus/Consensus.h @@ -1689,8 +1689,9 @@ Consensus::updateOurPositions(std::unique_ptr const& // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above using namespace telemetry; // Child of the establish span via its captured context (establishSpan_ is - // a thread-free SpanGuard, so parent explicitly via its context). Null - // context (establish not started) yields a null guard, same as before. + // a thread-free SpanGuard, so parent explicitly via its context). A null + // context — the establish phase has not started — yields a null guard, so + // the setAttribute calls below are no-ops. auto span = SpanGuard::childSpan(consensus::span::updatePositions, establishSpanContext_); span.setAttribute( consensus::span::attr::convergePercent, static_cast(convergePercent_)); diff --git a/include/xrpl/consensus/ConsensusSpanLabels.h b/include/xrpl/consensus/ConsensusSpanLabels.h index cc66639c41..21b867d128 100644 --- a/include/xrpl/consensus/ConsensusSpanLabels.h +++ b/include/xrpl/consensus/ConsensusSpanLabels.h @@ -3,10 +3,11 @@ /** * Enum-to-label mappings for consensus span attribute values. * - * Split from ConsensusSpanNames.h so that header stays dependency-free like - * its siblings: the span-name and attribute-key constants are included by - * overlay and app translation units that have no use for the consensus - * enums, while these mappings are needed only by Consensus.h. + * These mappings live in their own header so ConsensusSpanNames.h stays + * dependency-free like its siblings: the span-name and attribute-key + * constants are included by overlay and app translation units that have no + * use for the consensus enums, while these mappings are needed only by + * Consensus.h. * * ConsensusSpanNames.h (constants only, no domain deps) * ^ diff --git a/include/xrpl/consensus/ConsensusSpanNames.h b/include/xrpl/consensus/ConsensusSpanNames.h index fc56adcd44..f71b332457 100644 --- a/include/xrpl/consensus/ConsensusSpanNames.h +++ b/include/xrpl/consensus/ConsensusSpanNames.h @@ -59,10 +59,10 @@ * | | * | +-- consensus.accept.apply [jtACCEPT thread, child of accept] * | Created: Adaptor::doAccept() - * | Attrs: ledger_seq, close_time, close_time_correct, + * | Attrs: ledger_seq, close_time_ripple_epoch_s, close_time_correct, * | close_resolution_ms, consensus_state, proposing, round_time_ms, - * | parent_close_time, close_time_self, close_time_vote_bins, - * | resolution_direction, tx_count + * | parent_close_time_ripple_epoch_s, close_time_self_ripple_epoch_s, + * | close_time_vote_bins, resolution_direction, tx_count * | Events: tx.included (per tx, attrs: tx_id) * | * +~~~ consensus.validation.send [jtACCEPT thread, linked] @@ -140,8 +140,8 @@ namespace attr { * concept, same key, distinguished by span name (not an emitter prefix). */ using ::xrpl::telemetry::attr::closeResolutionMs; -using ::xrpl::telemetry::attr::closeTime; using ::xrpl::telemetry::attr::closeTimeCorrect; +using ::xrpl::telemetry::attr::closeTimeRippleEpochS; using ::xrpl::telemetry::attr::fullValidation; using ::xrpl::telemetry::attr::ledgerHash; using ::xrpl::telemetry::attr::ledgerSeq; @@ -232,8 +232,20 @@ inline constexpr auto positionHashPrefix = makeStr("position_hash_prefix"); * "consensus_state" — domain-qualified (collides with other domains' state). */ inline constexpr auto consensusState = makeStr("consensus_state"); -inline constexpr auto parentCloseTime = makeStr("parent_close_time"); -inline constexpr auto closeTimeSelf = makeStr("close_time_self"); +/** + * Close-time instants, both NetClock readings in whole seconds since the XRP + * Ledger epoch (2000-01-01T00:00:00Z) — see `closeTimeRippleEpochS` in + * SpanNames.h for why the epoch is spelled into the key. + * + * `parentCloseTimeRippleEpochS` is the previous ledger's close time; + * `closeTimeSelfRippleEpochS` is this node's own close-time vote for the round, + * so the pair shows how far the node's position sat from the ledger it built on. + * + * `closeTimeVoteBins` is not a time: it holds the number of distinct close-time + * positions seen from peers this round. + */ +inline constexpr auto parentCloseTimeRippleEpochS = makeStr("parent_close_time_ripple_epoch_s"); +inline constexpr auto closeTimeSelfRippleEpochS = makeStr("close_time_self_ripple_epoch_s"); inline constexpr auto closeTimeVoteBins = makeStr("close_time_vote_bins"); inline constexpr auto resolutionDirection = makeStr("resolution_direction"); inline constexpr auto convergePercent = makeStr("converge_percent"); @@ -275,6 +287,14 @@ inline constexpr auto disputesCount = makeStr("disputes_count"); */ inline constexpr auto proposalTrusted = makeStr("proposal_trusted"); inline constexpr auto validationTrusted = makeStr("validation_trusted"); + +/** + * "validation_status" — which exit the inbound validation took. Set once per + * exit, so a dropped validation (microseconds) is separable from a queued one + * (job wait plus checkValidation). Without it the span name reports two + * unrelated latency distributions and every quantile over it is meaningless. + */ +inline constexpr auto validationStatus = makeStr("validation_status"); } // namespace attr // ===== Event names =========================================================== @@ -340,6 +360,10 @@ inline constexpr auto closeAnomaly = makeStr("anomaly"); inline constexpr auto closeOthersClosed = makeStr("others_closed"); inline constexpr auto closeIdle = makeStr("idle"); inline constexpr auto closeNormal = makeStr("normal"); +// validation_status values, one per exit of the inbound validation path. +inline constexpr auto validationQueued = makeStr("queued"); +inline constexpr auto validationDroppedDiverged = makeStr("dropped_diverged"); +inline constexpr auto validationDroppedLoad = makeStr("dropped_load"); } // namespace val } // namespace xrpl::telemetry::consensus::span diff --git a/include/xrpl/telemetry/SpanGuard.h b/include/xrpl/telemetry/SpanGuard.h index 95c471f677..9ccfcca607 100644 --- a/include/xrpl/telemetry/SpanGuard.h +++ b/include/xrpl/telemetry/SpanGuard.h @@ -410,8 +410,10 @@ public: * follows-from link. Use to stitch sequential * top-level spans (e.g. consecutive consensus * rounds). Ignored if nullptr or invalid. + * @return An active guard, or a null guard when the category is + * disabled or hashSize is under 16. */ - static SpanGuard + [[nodiscard]] static SpanGuard hashSpan( TraceCategory const cat, std::string_view const name, @@ -431,8 +433,10 @@ public: * @param parentSpanId Pointer to 8 bytes of parent span ID. * @param parentSpanSize Size of parent span ID buffer (must be 8). * @param traceFlags Trace flags from remote context. + * @return An active guard, or a null guard when the category is + * disabled, hashSize is under 16, or parentSpanSize is not 8. */ - static SpanGuard + [[nodiscard]] static SpanGuard hashSpan( TraceCategory const cat, std::string_view const name, diff --git a/include/xrpl/telemetry/SpanNames.h b/include/xrpl/telemetry/SpanNames.h index b848f96c03..c24f0b254b 100644 --- a/include/xrpl/telemetry/SpanNames.h +++ b/include/xrpl/telemetry/SpanNames.h @@ -133,8 +133,19 @@ inline constexpr auto ledgerSeq = makeStr("ledger_seq"); /** * Shared close-time attrs — bare names, reused by consensus and ledger. + * + * `closeTimeRippleEpochS` carries a NetClock reading: whole seconds since the + * XRP Ledger epoch (2000-01-01T00:00:00Z), never the Unix epoch. The key names + * both the unit and the epoch because neither is recoverable from the value. + * A consumer rendering it as wall-clock time must first add kEpochOffset + * (946684800 seconds, see basics/chrono.h); read as a Unix timestamp instead, + * it lands roughly 30 years early. + * + * `closeResolutionMs` is a duration, not an instant — the granularity the + * close time is rounded to. NetClock resolution is whole seconds, so this + * value is always a multiple of 1000. */ -inline constexpr auto closeTime = makeStr("close_time"); +inline constexpr auto closeTimeRippleEpochS = makeStr("close_time_ripple_epoch_s"); inline constexpr auto closeTimeCorrect = makeStr("close_time_correct"); inline constexpr auto closeResolutionMs = makeStr("close_resolution_ms"); /** diff --git a/include/xrpl/telemetry/Telemetry.h b/include/xrpl/telemetry/Telemetry.h index bffe0477bc..caaf3c744d 100644 --- a/include/xrpl/telemetry/Telemetry.h +++ b/include/xrpl/telemetry/Telemetry.h @@ -136,7 +136,7 @@ public: * Get the global Telemetry instance. * @return Pointer to the active instance, or nullptr if not started. */ - static Telemetry* + [[nodiscard]] static Telemetry* getInstance() { return instance.load(std::memory_order_acquire); @@ -297,10 +297,9 @@ public: * @param id The node's base58-encoded public key or custom identifier. */ virtual void - setServiceInstanceId(std::string const& id) + setServiceInstanceId([[maybe_unused]] std::string const& id) { // Default no-op for NullTelemetry implementations. - (void)id; } /** @@ -366,7 +365,7 @@ public: * @param name Tracer name used to identify the instrumentation library. * @return A shared pointer to the Tracer. */ - virtual opentelemetry::nostd::shared_ptr + [[nodiscard]] virtual opentelemetry::nostd::shared_ptr getTracer(std::string_view name = kTracerName) = 0; /** @@ -384,7 +383,7 @@ public: * - kConsumer: async message receive * @return A shared pointer to the new Span. */ - virtual opentelemetry::nostd::shared_ptr + [[nodiscard]] virtual opentelemetry::nostd::shared_ptr startSpan( std::string_view name, opentelemetry::trace::SpanKind kind = opentelemetry::trace::SpanKind::kInternal) = 0; @@ -400,7 +399,7 @@ public: * @param kind The span kind (defaults to kInternal). * @return A shared pointer to the new Span. */ - virtual opentelemetry::nostd::shared_ptr + [[nodiscard]] virtual opentelemetry::nostd::shared_ptr startSpan( std::string_view name, opentelemetry::context::Context const& parentContext, diff --git a/include/xrpl/telemetry/TraceContextPropagator.h b/include/xrpl/telemetry/TraceContextPropagator.h index 9933e79292..54305eb355 100644 --- a/include/xrpl/telemetry/TraceContextPropagator.h +++ b/include/xrpl/telemetry/TraceContextPropagator.h @@ -43,7 +43,7 @@ namespace xrpl::telemetry { * @return An OTel Context with the extracted parent span, or an empty * context if the protobuf fields are missing or invalid. */ -inline opentelemetry::context::Context +[[nodiscard]] inline opentelemetry::context::Context extractFromProtobuf(protocol::TraceContext const& proto) { namespace trace = opentelemetry::trace; diff --git a/include/xrpl/telemetry/TraceContextValidation.h b/include/xrpl/telemetry/TraceContextValidation.h index e69b2d17ca..c299ba6cf9 100644 --- a/include/xrpl/telemetry/TraceContextValidation.h +++ b/include/xrpl/telemetry/TraceContextValidation.h @@ -57,7 +57,7 @@ namespace xrpl::telemetry { * @param traceId The raw trace_id bytes from a protobuf TraceContext. * @return true if usable as a trace identifier, false otherwise. */ -inline bool +[[nodiscard]] inline bool isValidTraceId(std::string const& traceId) { return traceId.size() == 16 && std::ranges::any_of(traceId, [](char c) { return c != 0; }); @@ -69,7 +69,7 @@ isValidTraceId(std::string const& traceId) * @param spanId The raw span_id bytes from a protobuf TraceContext. * @return true if usable as a span identifier, false otherwise. */ -inline bool +[[nodiscard]] inline bool isValidSpanId(std::string const& spanId) { return spanId.size() == 8 && std::ranges::any_of(spanId, [](char c) { return c != 0; }); @@ -86,7 +86,7 @@ isValidSpanId(std::string const& spanId) * @param tc The protobuf TraceContext received from a peer. * @return true if both ids are present and valid, false otherwise. */ -inline bool +[[nodiscard]] inline bool isValidTraceContext(protocol::TraceContext const& tc) { return tc.has_trace_id() && isValidTraceId(tc.trace_id()) && tc.has_span_id() && diff --git a/src/libxrpl/telemetry/NullTelemetry.cpp b/src/libxrpl/telemetry/NullTelemetry.cpp index df3ef140a3..f8c3e4eb67 100644 --- a/src/libxrpl/telemetry/NullTelemetry.cpp +++ b/src/libxrpl/telemetry/NullTelemetry.cpp @@ -111,7 +111,7 @@ public: } #ifdef XRPL_ENABLE_TELEMETRY - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr getTracer(std::string_view) override { static auto noopTracer = opentelemetry::nostd::shared_ptr( @@ -119,14 +119,14 @@ public: return noopTracer; } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr startSpan(std::string_view, opentelemetry::trace::SpanKind) override { return opentelemetry::nostd::shared_ptr( new opentelemetry::trace::NoopSpan(nullptr)); } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr startSpan( std::string_view, opentelemetry::context::Context const&, diff --git a/src/libxrpl/telemetry/Telemetry.cpp b/src/libxrpl/telemetry/Telemetry.cpp index de4bb16246..98ebe06588 100644 --- a/src/libxrpl/telemetry/Telemetry.cpp +++ b/src/libxrpl/telemetry/Telemetry.cpp @@ -222,7 +222,7 @@ public: return setup_.consensusTraceStrategy; } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr getTracer(std::string_view) override { static auto noopTracer = @@ -230,13 +230,13 @@ public: return noopTracer; } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr startSpan(std::string_view, trace_api::SpanKind) override { return opentelemetry::nostd::shared_ptr(new trace_api::NoopSpan(nullptr)); } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr startSpan(std::string_view, opentelemetry::context::Context const&, trace_api::SpanKind) override { @@ -446,7 +446,7 @@ public: return setup_.consensusTraceStrategy; } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr getTracer(std::string_view name = kTracerName) override { if (!sdkProvider_) @@ -456,7 +456,7 @@ public: return sdkProvider_->GetTracer(std::string(name)); } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr startSpan(std::string_view name, trace_api::SpanKind kind) override { auto tracer = getTracer(); @@ -465,7 +465,7 @@ public: return tracer->StartSpan(std::string(name), opts); } - opentelemetry::nostd::shared_ptr + [[nodiscard]] opentelemetry::nostd::shared_ptr startSpan( std::string_view name, opentelemetry::context::Context const& parentContext, diff --git a/src/libxrpl/telemetry/TelemetryConfig.cpp b/src/libxrpl/telemetry/TelemetryConfig.cpp index f0cc61517f..a8f1c0b1a5 100644 --- a/src/libxrpl/telemetry/TelemetryConfig.cpp +++ b/src/libxrpl/telemetry/TelemetryConfig.cpp @@ -71,7 +71,7 @@ constexpr std::uint32_t maxQueueSize = 2048u; * @param networkId The network identifier from [network_id] config. * @return "mainnet", "testnet", "devnet", or "unknown" for other values. */ -std::string +[[nodiscard]] std::string networkTypeFromId(std::uint32_t networkId) { switch (networkId) diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index 4eb43d1596..4e6dbf33b2 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include @@ -1624,126 +1625,140 @@ Transactor::operator()() trapTransaction(*trap); } - auto result = ctx_.preclaimResult; - if (isTesSuccess(result)) - result = apply(); - - // No transaction can return temUNKNOWN from apply, - // and it can't be passed in from a preclaim. - XRPL_ASSERT(result != temUNKNOWN, "xrpl::Transactor::operator() : result is not temUNKNOWN"); - - if (auto stream = j_.trace()) - stream << "preclaim result: " << transToken(result); - - auto fee = ctx_.tx.getFieldAmount(sfFee).xrp(); - bool const canApply = std::invoke([&result, &fee, this] { - bool canApplyTmp = isTesSuccess(result); - - if (ctx_.size() > kOversizeMetaDataCap) - result = tecOVERSIZE; - - if (isTecClaim(result) && ((view().flags() & TapFailHard) != 0u)) - { - // If the TapFailHard flag is set, a tec result - // must not do anything - ctx_.discard(); - canApplyTmp = false; - } - else if ( - (result == tecOVERSIZE) || (result == tecKILLED) || (result == tecINCOMPLETE) || - (result == tecEXPIRED) || (isTecClaimHardFail(result, view().flags()))) - { - // This is and must remain the only place where `canApplyTmp` can change from false to - // true. Changing from true to false is no problem. - std::tie(result, fee, canApplyTmp) = processPersistentChanges(result, fee); - } - return canApplyTmp; - }); - - // Every exit from this function funnels through here, so this is also where - // the apply span records its outcome: each return path reports the engine - // result and whether the transaction was applied. - auto const logger = [this, &span]( - TER result, - bool canApply, - std::optional&& metadata = std::nullopt) -> ApplyResult { - JLOG(j_.trace()) << (canApply ? "applied " : "not applied ") << transToken(result); - - // Also guarded: transToken() is a lookup returning a string, and this - // funnel runs on every exit path. - if (span) - { - span.setAttribute( - telemetry::tx_apply_span::attr::terResult, transToken(result).c_str()); - span.setAttribute(telemetry::tx_apply_span::attr::applied, canApply); - // Mark the span as errored when the transaction was not applied or - // the engine result is not a success, so failed applies surface in - // span-status error counts alongside preflight and preclaim. - if (!canApply || !isTesSuccess(result)) - span.setError(transToken(result)); - } - - return {result, canApply, std::move(metadata)}; - }; - - if (!canApply) - return logger(result, canApply); - - // First invariant pass: both protocol and transaction-specific - // checks run against the transaction's tentative outcome. If it - // does not return tecINVARIANT_FAILED, we can proceed to apply the - // tx. - result = checkInvariants(result, fee, InvariantScope::Full); - if (result == tecINVARIANT_FAILED) + try { - // Fee-claim reset: roll the transaction's effects back so that - // only the fee deduction remains. This is the reset referenced - // by InvariantScope::ProtocolOnly. - auto const resetResult = reset(fee); - if (!isTesSuccess(resetResult.first)) - result = resetResult.first; + auto result = ctx_.preclaimResult; + if (isTesSuccess(result)) + result = apply(); - fee = resetResult.second; + // No transaction can return temUNKNOWN from apply, + // and it can't be passed in from a preclaim. + XRPL_ASSERT( + result != temUNKNOWN, "xrpl::Transactor::operator() : result is not temUNKNOWN"); - // Re-check invariants against the post-reset (fee-claim only) - // state. The transaction's effects are gone, so the - // transaction-specific invariants no longer apply and only the - // protocol invariants are re-run. A failure here escalates to - // tefINVARIANT_FAILED and excludes the tx from the ledger. - if (isTesSuccess(result) || isTecClaim(result)) - result = checkInvariants(result, fee, InvariantScope::ProtocolOnly); + if (auto stream = j_.trace()) + stream << "preclaim result: " << transToken(result); + + auto fee = ctx_.tx.getFieldAmount(sfFee).xrp(); + bool const canApply = std::invoke([&result, &fee, this] { + bool canApplyTmp = isTesSuccess(result); + + if (ctx_.size() > kOversizeMetaDataCap) + result = tecOVERSIZE; + + if (isTecClaim(result) && ((view().flags() & TapFailHard) != 0u)) + { + // If the TapFailHard flag is set, a tec result + // must not do anything + ctx_.discard(); + canApplyTmp = false; + } + else if ( + (result == tecOVERSIZE) || (result == tecKILLED) || (result == tecINCOMPLETE) || + (result == tecEXPIRED) || (isTecClaimHardFail(result, view().flags()))) + { + // This is and must remain the only place where `canApplyTmp` can change from false + // to true. Changing from true to false is no problem. + std::tie(result, fee, canApplyTmp) = processPersistentChanges(result, fee); + } + return canApplyTmp; + }); + + // Each return path funnels through here, so this is also where the apply + // span records its outcome: the engine result and whether the transaction + // was applied. A throw bypasses it and ends the span with no outcome. + auto const logger = [this, &span]( + TER result, + bool canApply, + std::optional&& metadata = std::nullopt) -> ApplyResult { + JLOG(j_.trace()) << (canApply ? "applied " : "not applied ") << transToken(result); + + // Also guarded: transToken() is a lookup returning a string, and this + // funnel runs on every return path. + if (span) + { + span.setAttribute( + telemetry::tx_apply_span::attr::terResult, transToken(result).c_str()); + span.setAttribute(telemetry::tx_apply_span::attr::applied, canApply); + // Mark the span as errored when the engine result is not a success, + // so failed applies surface alongside preflight and preclaim. Not + // keyed on `canApply`: a dry run reports tesSUCCESS with canApply + // false, and that is not a failure. + if (!isTesSuccess(result)) + span.setError(transToken(result)); + } + + return {result, canApply, std::move(metadata)}; + }; + + if (!canApply) + return logger(result, canApply); + + // First invariant pass: both protocol and transaction-specific + // checks run against the transaction's tentative outcome. If it + // does not return tecINVARIANT_FAILED, we can proceed to apply the + // tx. + result = checkInvariants(result, fee, InvariantScope::Full); + if (result == tecINVARIANT_FAILED) + { + // Fee-claim reset: roll the transaction's effects back so that + // only the fee deduction remains. This is the reset referenced + // by InvariantScope::ProtocolOnly. + auto const resetResult = reset(fee); + if (!isTesSuccess(resetResult.first)) + result = resetResult.first; + + fee = resetResult.second; + + // Re-check invariants against the post-reset (fee-claim only) + // state. The transaction's effects are gone, so the + // transaction-specific invariants no longer apply and only the + // protocol invariants are re-run. A failure here escalates to + // tefINVARIANT_FAILED and excludes the tx from the ledger. + if (isTesSuccess(result) || isTecClaim(result)) + result = checkInvariants(result, fee, InvariantScope::ProtocolOnly); + } + + // We ran through the invariant checker, which can, in some cases, + // return a tef error code. Don't apply the transaction in that case. + if (!isTecClaim(result) && !isTesSuccess(result)) + return logger(result, false); + + std::optional metadata; + + // Transaction succeeded fully or (retries are not allowed and the + // transaction could claim a fee) + + // The transactor and invariant checkers guarantee that this will + // *never* trigger but if it, somehow, happens, don't allow a tx + // that charges a negative fee. + if (fee < beast::kZero) + Throw("fee charged is negative!"); + + // Charge whatever fee they specified. The fee has already been + // deducted from the balance of the account that issued the + // transaction. We just need to account for it in the ledger + // header. + if (!view().open() && fee != beast::kZero) + ctx_.destroyXRP(fee); + + // Once we call apply, we will no longer be able to look at view() + metadata = ctx_.apply(result); + + if ((ctx_.flags() & TapDryRun) != 0u) + return logger(result, false, std::move(metadata)); + + return logger(result, canApply, std::move(metadata)); + } + catch (std::exception const& e) + { + // The caller's doApply() maps this to tefEXCEPTION. Record it on the + // span before unwinding so per-stage error counts include exceptions. + span.setAttribute( + telemetry::tx_apply_span::attr::terResult, transToken(tefEXCEPTION).c_str()); + span.recordException(e); + throw; } - - // We ran through the invariant checker, which can, in some cases, - // return a tef error code. Don't apply the transaction in that case. - if (!isTecClaim(result) && !isTesSuccess(result)) - return logger(result, false); - - std::optional metadata; - - // Transaction succeeded fully or (retries are not allowed and the - // transaction could claim a fee) - - // The transactor and invariant checkers guarantee that this will - // *never* trigger but if it, somehow, happens, don't allow a tx - // that charges a negative fee. - if (fee < beast::kZero) - Throw("fee charged is negative!"); - - // Charge whatever fee they specified. The fee has already been - // deducted from the balance of the account that issued the - // transaction. We just need to account for it in the ledger - // header. - if (!view().open() && fee != beast::kZero) - ctx_.destroyXRP(fee); - - // Once we call apply, we will no longer be able to look at view() - metadata = ctx_.apply(result); - - if ((ctx_.flags() & TapDryRun) != 0u) - return logger(result, false, std::move(metadata)); - - return logger(result, canApply, std::move(metadata)); } } // namespace xrpl diff --git a/src/libxrpl/tx/applySteps.cpp b/src/libxrpl/tx/applySteps.cpp index 45758bf616..3ec6c25aa8 100644 --- a/src/libxrpl/tx/applySteps.cpp +++ b/src/libxrpl/tx/applySteps.cpp @@ -314,6 +314,10 @@ invokePreclaim(PreclaimContext const& ctx) { span.setAttribute( telemetry::tx_apply_span::attr::terResult, transToken(preclaimTer).c_str()); + // Mark the span as errored when preclaim rejects the transaction so + // failed stages surface in span-status error counts. + if (!isTesSuccess(preclaimTer)) + span.setError(transToken(preclaimTer)); } return preclaimTer; } diff --git a/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp b/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp index 36ab40b1b6..6cec7a5c86 100644 --- a/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp +++ b/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp @@ -99,7 +99,7 @@ TEST(SpanGuardFactory, consensus_close_time_attributes) auto span = telemetry::SpanGuard::span( telemetry::TraceCategory::Consensus, telemetry::seg::consensus, "accept.apply"); span.setAttribute("ledger_seq", static_cast(42)); - span.setAttribute("close_time", static_cast(780000000)); + span.setAttribute("close_time_ripple_epoch_s", static_cast(780000000)); span.setAttribute("close_time_correct", true); span.setAttribute("close_resolution_ms", static_cast(30000)); span.setAttribute("consensus_state", std::string("finished")); diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index a0e2f6f6d3..460a447631 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -635,7 +635,8 @@ RCLConsensus::Adaptor::doAccept( : telemetry::SpanGuard::childSpan(cs::acceptApply, roundSpanContext_); doAcceptSpan.setAttribute(cs::attr::ledgerSeq, static_cast(prevLedger.seq()) + 1); doAcceptSpan.setAttribute( - cs::attr::closeTime, static_cast(consensusCloseTime.time_since_epoch().count())); + cs::attr::closeTimeRippleEpochS, + static_cast(consensusCloseTime.time_since_epoch().count())); doAcceptSpan.setAttribute(cs::attr::closeTimeCorrect, closeTimeCorrect); doAcceptSpan.setAttribute( cs::attr::closeResolutionMs, @@ -648,10 +649,10 @@ RCLConsensus::Adaptor::doAccept( doAcceptSpan.setAttribute( cs::attr::roundTimeMs, static_cast(result.roundTime.read().count())); doAcceptSpan.setAttribute( - cs::attr::parentCloseTime, + cs::attr::parentCloseTimeRippleEpochS, static_cast(prevLedger.closeTime().time_since_epoch().count())); doAcceptSpan.setAttribute( - cs::attr::closeTimeSelf, + cs::attr::closeTimeSelfRippleEpochS, static_cast(rawCloseTimes.self.time_since_epoch().count())); doAcceptSpan.setAttribute( cs::attr::closeTimeVoteBins, static_cast(rawCloseTimes.peers.size())); diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 0859e251ca..0bce9a252c 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -2706,12 +2706,29 @@ PeerImp::onMessage(std::shared_ptr const& m) static_cast(val->getSignTime().time_since_epoch().count())); } + // validation_status is set once on each exit below, not as a default + // here, to avoid OTel SDK attribute duplication. It is what separates + // the microsecond drop paths from the queued path, which also covers + // job wait and checkValidation. if (!isTrusted && (tracking_.load() == Tracking::Diverged)) { + if (span && *span) + { + span->setAttribute( + telemetry::consensus::span::attr::validationStatus, + telemetry::consensus::span::val::validationDroppedDiverged); + } JLOG(pJournal_.debug()) << "Dropping untrusted validation from diverged peer"; } else if (isTrusted || !app_.getFeeTrack().isLoadedLocal()) { + // Set before the handle is moved into the job below. + if (span && *span) + { + span->setAttribute( + telemetry::consensus::span::attr::validationStatus, + telemetry::consensus::span::val::validationQueued); + } std::string const name = isTrusted ? "ChkTrust" : "ChkUntrust"; std::weak_ptr const weak = shared_from_this(); @@ -2725,6 +2742,12 @@ PeerImp::onMessage(std::shared_ptr const& m) } else { + if (span && *span) + { + span->setAttribute( + telemetry::consensus::span::attr::validationStatus, + telemetry::consensus::span::val::validationDroppedLoad); + } JLOG(pJournal_.debug()) << "Dropping untrusted validation for load"; } } diff --git a/src/xrpld/rpc/detail/PathRequest.cpp b/src/xrpld/rpc/detail/PathRequest.cpp index 827393b071..1154109fc9 100644 --- a/src/xrpld/rpc/detail/PathRequest.cpp +++ b/src/xrpld/rpc/detail/PathRequest.cpp @@ -596,8 +596,8 @@ PathRequest::findPaths( // One `pathfind.discover` span wraps the entire per-source-asset loop so // that a single RPC call produces one discover span instead of N (one per // candidate source asset). Trade-off: per-asset discovery/ranking timing - // is no longer split into individual spans — span count and Tempo storage - // are bounded per RPC at the cost of per-asset visibility. + // is not measured separately — span count and Tempo storage are bounded + // per RPC at the cost of per-asset visibility. // // This is an unscoped guard: it takes the ambient span as its own parent, // but does not itself become the ambient parent. Adding per-asset child diff --git a/src/xrpld/telemetry/TxTracing.h b/src/xrpld/telemetry/TxTracing.h index 5baf01df2d..682f482f1b 100644 --- a/src/xrpld/telemetry/TxTracing.h +++ b/src/xrpld/telemetry/TxTracing.h @@ -32,8 +32,12 @@ namespace xrpl::telemetry { * trace_id is derived from txID[0:16]. If the incoming message carries * a protobuf TraceContext with a valid span_id, it is used as the * parent to preserve relay ordering. + * @param txID Transaction id; its first 16 bytes become the trace_id. + * @param msg The received message, read only for its trace context. + * @return An active guard, or a null guard when the Transactions category + * is disabled. Bind it: a discarded guard ends the span immediately. */ -inline SpanGuard +[[nodiscard]] inline SpanGuard txReceiveSpan(uint256 const& txID, [[maybe_unused]] protocol::TMTransaction const& msg) { #ifdef XRPL_ENABLE_TELEMETRY @@ -63,8 +67,11 @@ txReceiveSpan(uint256 const& txID, [[maybe_unused]] protocol::TMTransaction cons /** * Create a "tx.process" span for transaction processing in NetworkOPs. * trace_id is derived from txID[0:16]. + * @param txID Transaction id; its first 16 bytes become the trace_id. + * @return An active guard, or a null guard when the Transactions category + * is disabled. Bind it: a discarded guard ends the span immediately. */ -inline SpanGuard +[[nodiscard]] inline SpanGuard txProcessSpan(uint256 const& txID) { return SpanGuard::hashSpan(