Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill

# Conflicts:
#	OpenTelemetryPlan/08-appendix.md
#	OpenTelemetryPlan/presentation.md
#	docker/telemetry/xrpld-telemetry.cfg
#	docs/telemetry-runbook.md

src/libxrpl/telemetry/Redaction.cpp

@@ -0,0 +1,32 @@
+#include <xrpl/telemetry/Redaction.h>
+
+#include <xrpl/basics/Slice.h>
+#include <xrpl/basics/base_uint.h>
+#include <xrpl/protocol/digest.h>
+
+#include <algorithm>
+#include <cctype>
+#include <string>
+#include <string_view>
+
+namespace xrpl::telemetry {
+
+std::string
+redactAccount(std::string_view addr)
+{
+    // Empty in, empty out: nothing to hash and no token to emit.
+    if (addr.empty())
+        return {};
+
+    // sha512Half yields a uint256; to_string renders it as uppercase hex.
+    // Keep the first 16 chars (64 bits) — enough to correlate spans while
+    // staying non-reversible — and lowercase them for a stable token.
+    auto const digest = sha512Half(Slice(addr.data(), addr.size()));
+    std::string token = to_string(digest).substr(0, 16);
+    std::transform(token.begin(), token.end(), token.begin(), [](unsigned char c) {
+        return static_cast<char>(std::tolower(c));
+    });
+    return token;
+}
+
+}  // namespace xrpl::telemetry

src/libxrpl/telemetry/SpanGuard.cpp

@@ -238,6 +238,7 @@ SpanGuard::linkedSpan(std::string_view name) const
     return SpanGuard(
         std::make_unique<Impl>(tracer->StartSpan(
             std::string(name), {}, {{spanCtx, {{kLinkTypeKey, kLinkTypeFollowsFrom}}}}, opts)));
+    // LCOV_EXCL_STOP
 }
 
 SpanGuard

src/libxrpl/telemetry/TelemetryConfig.cpp

@@ -112,6 +112,16 @@ makeTelemetrySetup(
             "(set both for mutual TLS, or neither for one-way TLS).");
     }
 
+    // Mutual TLS only takes effect when TLS is on. Certificate paths set with
+    // use_tls=0 would be silently ignored and the exporter would connect in
+    // plaintext, so reject that contradiction instead of failing open.
+    if (!setup.tlsClientCertPath.empty() && !setup.useTls)
+    {
+        Throw<std::runtime_error>(
+            "[telemetry] tls_client_cert/tls_client_key require use_tls=1 "
+            "(set use_tls=1 to enable mutual TLS, or remove the cert paths).");
+    }
+
     // Head sampling is intentionally fixed at 1.0 (sample everything) and is
     // not read from config. A per-node ratio would let nodes make divergent
     // keep/drop decisions for the same distributed trace, producing broken

src/libxrpl/tx/Transactor.cpp

@@ -1294,10 +1294,15 @@ Transactor::checkInvariants(TER result, XRPAmount fee)
 ApplyResult
 Transactor::operator()()
 {
-    auto span = telemetry::SpanGuard::span(
+    // Derive the trace_id from the transaction id so this apply-stage span
+    // shares one trace with the preflight and preclaim spans (which also use
+    // hashSpan on the same id).
+    auto const txID = ctx_.tx.getTransactionID();
+    auto span = telemetry::SpanGuard::hashSpan(
         telemetry::TraceCategory::Transactions,
-        telemetry::seg::tx,
-        telemetry::tx_apply_span::op::transactor);
+        telemetry::tx_apply_span::transactor,
+        txID.data(),
+        txID.kBytes);
     // "apply" — the third apply-pipeline stage, after preflight and preclaim.
     span.setAttribute(telemetry::tx_apply_span::attr::stage, telemetry::tx_apply_span::val::apply);
     if (auto const* fmt = TxFormats::getInstance().findByType(ctx_.tx.getTxnType()))
@@ -1428,6 +1433,11 @@ Transactor::operator()()
 
     span.setAttribute(telemetry::tx_apply_span::attr::terResult, transToken(result).c_str());
     span.setAttribute(telemetry::tx_apply_span::attr::applied, applied);
+    // 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 (!applied || !isTesSuccess(result))
+        span.setError(transToken(result));
 
     return {result, applied, metadata};
 }

src/libxrpl/tx/applySteps.cpp

@@ -190,6 +190,10 @@ invokePreflight(PreflightContext const& ctx)
         {
             span.setAttribute(
                 telemetry::tx_apply_span::attr::terResult, transToken(result.first).c_str());
+            // Mark the span as errored when preflight rejects the transaction so
+            // failed stages surface in span-status error counts.
+            if (!isTesSuccess(result.first))
+                span.setError(transToken(result.first));
         }
         return result;
     }

src/tests/libxrpl/telemetry/Redaction.cpp

@@ -0,0 +1,44 @@
+#include <xrpl/telemetry/Redaction.h>
+
+#include <gtest/gtest.h>
+
+#include <algorithm>
+#include <cctype>
+#include <string>
+
+using namespace xrpl;
+using namespace xrpl::telemetry;
+
+// Empty input must produce empty output (edge / negative path).
+TEST(Redaction, empty_input_returns_empty)
+{
+    EXPECT_EQ(redactAccount(""), "");
+}
+
+// Success path: assert the EXACT expected token. The value is the first
+// 16 chars of sha512Half(addr) rendered as lowercase hex, computed once
+// and hard-coded here so any change to the hashing recipe is caught.
+TEST(Redaction, known_account_exact_hash)
+{
+    EXPECT_EQ(redactAccount("rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"), "a513895f49311f54");
+}
+
+// Structural invariants: length, lowercase-hex alphabet, and stability.
+TEST(Redaction, token_is_16_lowercase_hex_and_stable)
+{
+    auto const token = redactAccount("rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh");
+    EXPECT_EQ(token.size(), 16u);
+    EXPECT_TRUE(std::all_of(token.begin(), token.end(), [](unsigned char c) {
+        return std::isdigit(c) || (c >= 'a' && c <= 'f');
+    }));
+    // Deterministic: hashing the same input twice yields the same token.
+    EXPECT_EQ(token, redactAccount("rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"));
+}
+
+// Distinct inputs must produce distinct tokens (no accidental constant).
+TEST(Redaction, distinct_inputs_distinct_tokens)
+{
+    EXPECT_NE(
+        redactAccount("rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"),
+        redactAccount("rDifferentAccount1234567890abcdefgh"));
+}

src/tests/libxrpl/telemetry/TraceContextPropagator.cpp

@@ -83,10 +83,8 @@ TEST(TraceContextPropagator, extract_empty_protobuf)
     protocol::TraceContext const proto;
     auto ctx = xrpl::telemetry::extractFromProtobuf(proto);
     auto span = trace::GetSpan(ctx);
-    if (span)
-    {
-        EXPECT_FALSE(span->GetContext().IsValid());
-    }
+    ASSERT_NE(span, nullptr);
+    EXPECT_FALSE(span->GetContext().IsValid());
 }
 
 TEST(TraceContextPropagator, extract_wrong_size_trace_id)
@@ -97,10 +95,8 @@ TEST(TraceContextPropagator, extract_wrong_size_trace_id)
 
     auto ctx = xrpl::telemetry::extractFromProtobuf(proto);
     auto span = trace::GetSpan(ctx);
-    if (span)
-    {
-        EXPECT_FALSE(span->GetContext().IsValid());
-    }
+    ASSERT_NE(span, nullptr);
+    EXPECT_FALSE(span->GetContext().IsValid());
 }
 
 TEST(TraceContextPropagator, extract_wrong_size_span_id)
@@ -111,10 +107,8 @@ TEST(TraceContextPropagator, extract_wrong_size_span_id)
 
     auto ctx = xrpl::telemetry::extractFromProtobuf(proto);
     auto span = trace::GetSpan(ctx);
-    if (span)
-    {
-        EXPECT_FALSE(span->GetContext().IsValid());
-    }
+    ASSERT_NE(span, nullptr);
+    EXPECT_FALSE(span->GetContext().IsValid());
 }
 
 TEST(TraceContextPropagator, inject_invalid_span)

src/xrpld/app/consensus/RCLConsensus.cpp

@@ -1274,8 +1274,25 @@ RCLConsensus::Adaptor::startRoundTracing(RCLCxLedger const& prevLgr)
     telemetry::SpanContext const* const link =
         prevRoundSpanContext_.isValid() ? &prevRoundSpanContext_ : nullptr;
 
-    if (strategy == "deterministic")
+    if (strategy == "attribute")
     {
+        // Non-deterministic strategy: each node gets a random trace_id,
+        // correlated via the consensus_ledger_id attribute rather than a
+        // shared trace_id. Still attach a follows-from link to the prior
+        // round so consecutive rounds stay navigable. linkedSpan is not
+        // TraceCategory-aware, so gate it explicitly to match the gating
+        // of the hashSpan/span factories used below.
+        if (link != nullptr && app_.getTelemetry().shouldTraceConsensus())
+            roundSpan_.emplace(telemetry::SpanGuard::linkedSpan(cs::round, *link));
+        else
+            roundSpan_.emplace(
+                telemetry::SpanGuard::span(
+                    telemetry::TraceCategory::Consensus, telemetry::seg::consensus, cs::op::round));
+    }
+    else
+    {
+        // "deterministic" (the default): derive the trace_id from the previous
+        // ledger hash so all validators tracing the same round share one trace.
         roundSpan_.emplace(
             telemetry::SpanGuard::hashSpan(
                 telemetry::TraceCategory::Consensus,
@@ -1284,12 +1301,6 @@ RCLConsensus::Adaptor::startRoundTracing(RCLCxLedger const& prevLgr)
                 prevLgr.id().kBytes,
                 link));
     }
-    else
-    {
-        roundSpan_.emplace(
-            telemetry::SpanGuard::span(
-                telemetry::TraceCategory::Consensus, telemetry::seg::consensus, cs::op::round));
-    }
 
     if (!*roundSpan_)
         return;

src/xrpld/app/misc/NetworkOPs.cpp

@@ -17,7 +17,6 @@
 #include <xrpld/app/misc/FeeVote.h>
 #include <xrpld/app/misc/Transaction.h>
 #include <xrpld/app/misc/TxQ.h>
-#include <xrpld/app/misc/TxSpanNames.h>
 #include <xrpld/app/misc/ValidatorKeys.h>
 #include <xrpld/app/misc/ValidatorList.h>
 #include <xrpld/app/misc/make_NetworkOPs.h>
@@ -36,6 +35,7 @@
 #include <xrpld/rpc/ServerHandler.h>
 #include <xrpld/telemetry/MetricsRegistry.h>
 #include <xrpld/telemetry/PropagationHelpers.h>
+#include <xrpld/telemetry/TxSpanNames.h>
 #include <xrpld/telemetry/TxTracing.h>
 
 #include <xrpl/basics/Log.h>

src/xrpld/app/misc/detail/TxQ.cpp

@@ -1459,9 +1459,6 @@ bool
 TxQ::accept(Application& app, OpenView& view)
 {
     using namespace telemetry;
-    auto span =
-        SpanGuard::span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::accept);
-    span.setAttribute(txq_span::attr::queueSize, static_cast<int64_t>(byFee_.size()));
 
     /* Move transactions from the queue from largest fee level to smallest.
        As we add more transactions, the required fee level will increase.
@@ -1473,6 +1470,12 @@ TxQ::accept(Application& app, OpenView& view)
 
     std::scoped_lock const lock(mutex_);
 
+    // Create the span and read byFee_.size() only after taking the lock, since
+    // byFee_ is guarded by mutex_.
+    auto span =
+        SpanGuard::span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::accept);
+    span.setAttribute(txq_span::attr::queueSize, static_cast<int64_t>(byFee_.size()));
+
     auto const metricsSnapshot = feeMetrics_.getSnapshot();
 
     for (auto candidateIter = byFee_.begin(); candidateIter != byFee_.end();)

src/xrpld/overlay/detail/PeerImp.cpp

@@ -7,7 +7,6 @@
 #include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/ledger/TransactionMaster.h>
 #include <xrpld/app/misc/Transaction.h>
-#include <xrpld/app/misc/TxSpanNames.h>
 #include <xrpld/app/misc/ValidatorList.h>
 #include <xrpld/consensus/ConsensusSpanNames.h>
 #include <xrpld/consensus/Validations.h>
@@ -25,6 +24,7 @@
 #include <xrpld/peerfinder/PeerfinderManager.h>
 #include <xrpld/peerfinder/Slot.h>
 #include <xrpld/telemetry/ConsensusReceiveTracing.h>
+#include <xrpld/telemetry/TxSpanNames.h>
 #include <xrpld/telemetry/TxTracing.h>
 
 #include <xrpl/basics/Blob.h>

src/xrpld/rpc/detail/PathFindSpanNames.h

@@ -84,8 +84,6 @@ inline constexpr auto numPaths = makeStr("pathfind_num_paths");
 inline constexpr auto numRequests = makeStr("pathfind_num_requests");
 /// "pathfind_ledger_index" — pathfind target ledger index.
 inline constexpr auto ledgerIndex = makeStr("pathfind_ledger_index");
-/// "pathfind_dest_amount" — requested destination amount as string.
-inline constexpr auto destAmount = makeStr("pathfind_dest_amount");
 /// "pathfind_dest_currency" — destination currency code.
 inline constexpr auto destCurrency = makeStr("pathfind_dest_currency");
 /// "pathfind_num_source_assets" — candidate source assets count.

src/xrpld/rpc/detail/PathRequest.cpp

@@ -742,7 +742,6 @@ PathRequest::doUpdate(
     auto span = SpanGuard::span(
         TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::compute);
     span.setAttribute(pathfind_span::attr::fast, fast);
-    span.setAttribute(pathfind_span::attr::destAmount, saDstAmount_.getFullText().c_str());
     span.setAttribute(pathfind_span::attr::destCurrency, to_string(saDstAmount_.asset()).c_str());
 
     JLOG(journal_.debug()) << iIdentifier_ << " update " << (fast ? "fast" : "normal");

src/xrpld/rpc/detail/PathRequestManager.cpp

@@ -73,15 +73,16 @@ PathRequestManager::updateAll(std::shared_ptr<ReadView const> const& inLedger)
         cache = getAssetCache(inLedger, true);
     }
 
-    // updateAll runs on every ledger close; skip span emission entirely when
-    // there are no active path subscriptions to avoid a steady stream of empty
-    // spans at mainnet close cadence.
-    if (requests.empty())
-        return;
-
     using namespace telemetry;
-    auto span = SpanGuard::span(
-        TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::updateAll);
+    // updateAll runs on every ledger close. Skip span emission when there are
+    // no active path subscriptions, to avoid a steady stream of empty spans at
+    // mainnet close cadence. A null guard is used in that case; all other work
+    // still runs unchanged (notably the isNewPathRequest() flag reset below),
+    // so behaviour matches the pre-span code path.
+    auto span = requests.empty()
+        ? SpanGuard{}
+        : SpanGuard::span(
+              TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::updateAll);
     span.setAttribute(pathfind_span::attr::ledgerIndex, static_cast<int64_t>(inLedger->seq()));
     span.setAttribute(pathfind_span::attr::numRequests, static_cast<int64_t>(requests.size()));
 

src/xrpld/rpc/detail/RPCHandler.cpp

@@ -221,6 +221,31 @@ callMethod(JsonContext& context, Method method, std::string const& name, Object&
     }
 }
 
+// Resolve the span suffix / command attribute for a request that failed in
+// fillHandler. Returns the canonical handler name for a recognized command
+// (a finite, bounded set) or the literal "unknown" for a request that omits
+// both fields or names an unregistered command. The raw request value is
+// deliberately NOT used: the command attribute is promoted to a Prometheus
+// label by the spanmetrics connector, so an attacker-controlled string would
+// let arbitrary request input drive unbounded span-name / label cardinality.
+// Resolving against the registry keeps per-command error attribution for real
+// commands (e.g. a submit rejected with rpcTOO_BUSY stays rpc.command.submit)
+// while collapsing garbage input to a single series.
+std::string_view
+resolveCommandSpanName(JsonContext const& context)
+{
+    if (!context.params.isMember(jss::command) && !context.params.isMember(jss::method))
+        return rpc_span::val::unknownCommand;
+
+    std::string const cmd = context.params.isMember(jss::command)
+        ? context.params[jss::command].asString()
+        : context.params[jss::method].asString();
+
+    auto const* handler = getHandler(context.apiVersion, context.app.config().betaRpcApi, cmd);
+    return (handler != nullptr) ? std::string_view{handler->name}
+                                : std::string_view{rpc_span::val::unknownCommand};
+}
+
 }  // namespace
 
 Status
@@ -229,25 +254,12 @@ doCommand(RPC::JsonContext& context, json::Value& result)
     Handler const* handler = nullptr;
     if (auto error = fillHandler(context, handler))
     {
-        std::string cmdName;
-        if (context.params.isMember(jss::command))
-        {
-            cmdName = context.params[jss::command].asString();
-        }
-        else if (context.params.isMember(jss::method))
-        {
-            cmdName = context.params[jss::method].asString();
-        }
-        else
-        {
-            cmdName = "unknown";  // LCOV_EXCL_LINE
-        }
-        // Use the resolved command name as the span suffix so dashboards
-        // can break out per-command error rates (e.g. rpc.command.submit
-        // for a submit that hit rpcTOO_BUSY). Falling back to a single
-        // "unknown" name only when the request truly omits both fields.
+        // Bound the span name and command attribute to the finite set of
+        // registered handler names (plus "unknown") — see the helper for why
+        // raw request input must not reach the telemetry pipeline.
+        auto const cmdName = resolveCommandSpanName(context);
         auto span = SpanGuard::span(TraceCategory::Rpc, rpc_span::prefix::command, cmdName);
-        span.setAttribute(rpc_span::attr::command, cmdName.c_str());
+        span.setAttribute(rpc_span::attr::command, cmdName);
         span.setAttribute(rpc_span::attr::rpcStatus, rpc_span::val::error);
         span.setError(getErrorInfo(error).token.cStr());
 

src/xrpld/rpc/handlers/orderbook/PathFind.cpp

@@ -10,6 +10,7 @@
 #include <xrpl/protocol/jss.h>
 #include <xrpl/resource/Fees.h>
 #include <xrpl/server/InfoSub.h>
+#include <xrpl/telemetry/Redaction.h>
 #include <xrpl/telemetry/SpanGuard.h>
 
 namespace xrpl {
@@ -20,10 +21,11 @@ doPathFind(RPC::JsonContext& context)
     using namespace telemetry;
     auto span = SpanGuard::span(
         TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::request);
+    // Addresses are hashed before emission for privacy.
     if (auto const& src = context.params[jss::source_account]; src.isString())
-        span.setAttribute(pathfind_span::attr::sourceAccount, src.asString());
+        span.setAttribute(pathfind_span::attr::sourceAccount, redactAccount(src.asString()));
     if (auto const& dst = context.params[jss::destination_account]; dst.isString())
-        span.setAttribute(pathfind_span::attr::destAccount, dst.asString());
+        span.setAttribute(pathfind_span::attr::destAccount, redactAccount(dst.asString()));
 
     if (context.app.config().pathSearchMax == 0)
         return rpcError(RpcNotSupported);

src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp

@@ -14,6 +14,7 @@
 #include <xrpl/protocol/RPCErr.h>
 #include <xrpl/protocol/jss.h>
 #include <xrpl/resource/Fees.h>
+#include <xrpl/telemetry/Redaction.h>
 #include <xrpl/telemetry/SpanGuard.h>
 
 #include <memory>
@@ -28,10 +29,11 @@ doRipplePathFind(RPC::JsonContext& context)
     using namespace telemetry;
     auto span = SpanGuard::span(
         TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::request);
+    // Addresses are hashed before emission for privacy.
     if (auto const& src = context.params[jss::source_account]; src.isString())
-        span.setAttribute(pathfind_span::attr::sourceAccount, src.asString());
+        span.setAttribute(pathfind_span::attr::sourceAccount, redactAccount(src.asString()));
     if (auto const& dst = context.params[jss::destination_account]; dst.isString())
-        span.setAttribute(pathfind_span::attr::destAccount, dst.asString());
+        span.setAttribute(pathfind_span::attr::destAccount, redactAccount(dst.asString()));
 
     if (context.app.config().pathSearchMax == 0)
         return rpcError(RpcNotSupported);

src/xrpld/telemetry/TxTracing.h

@@ -10,7 +10,7 @@
  *  no-op SpanGuard instances (zero overhead, zero dependencies).
  */
 
-#include <xrpld/app/misc/TxSpanNames.h>
+#include <xrpld/telemetry/TxSpanNames.h>
 
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/proto/xrpl.pb.h>