tracing: dedup TRACE macros & rename to TRACEPOINT

This deduplicates the TRACEx macros by using systemtaps STAP_PROBEV[0]
variadic macro instead of the DTrace compability DTRACE_PROBE[1] macros.
Bitcoin Core never had DTrace tracepoints, so we don't need to use the
drop-in replacement for it. As noted in pr25541[2], these macros aren't
compatibile with DTrace on macOS anyway.

This also renames the TRACEx macro to TRACEPOINT to clarify what the
macro does: inserting a tracepoint vs tracing (logging) something.

[0]: https://sourceware.org/git/?p=systemtap.git;a=blob;f=includes/sys/sdt.h;h=24d5e01c37805e55c36f7202e5d4e821b85167a1;hb=ecab2afea46099b4e7dfd551462689224afdbe3a#l407
[1]: https://sourceware.org/git/?p=systemtap.git;a=blob;f=includes/sys/sdt.h;h=24d5e01c37805e55c36f7202e5d4e821b85167a1;hb=ecab2afea46099b4e7dfd551462689224afdbe3a#l490
[2]: https://github.com/bitcoin/bitcoin/pull/25541/files#diff-553886c5f808e01e3452c7b21e879cc355da388ef7680bf310f6acb926d43266R30-R31

Co-authored-by: Martin Leitner-Ankerl <[email protected]>

Author: 0xB10C

cmake/module/FindUSDT.cmake

@@ -36,13 +36,16 @@ if(USDT_INCLUDE_DIR)
   include(CheckCXXSourceCompiles)
   set(CMAKE_REQUIRED_INCLUDES ${USDT_INCLUDE_DIR})
   check_cxx_source_compiles("
+    // Setting SDT_USE_VARIADIC lets systemtap (sys/sdt.h) know that we want to use
+    // the optional variadic macros to define tracepoints.
+    #define SDT_USE_VARIADIC 1
     #include <sys/sdt.h>
 
     int main()
     {
-      DTRACE_PROBE(context, event);
+      STAP_PROBEV(context, event);
       int a, b, c, d, e, f, g;
-      DTRACE_PROBE7(context, event, a, b, c, d, e, f, g);
+      STAP_PROBEV(context, event, a, b, c, d, e, f, g);
     }
     " HAVE_USDT_H
   )

doc/tracing.md

@@ -265,35 +265,19 @@ Arguments passed:
 
 ## Adding tracepoints to Bitcoin Core
 
-To add a new tracepoint, `#include <util/trace.h>` in the compilation unit where
-the tracepoint is inserted. Use one of the `TRACEx` macros listed below
-depending on the number of arguments passed to the tracepoint. Up to 12
-arguments can be provided. The `context` and `event` specify the names by which
-the tracepoint is referred to. Please use `snake_case` and try to make sure that
-the tracepoint names make sense even without detailed knowledge of the
-implementation details. Do not forget to update the tracepoint list in this
+Use the `TRACEPOINT` macro to add a new tracepoint. If not yet included, include
+`util/trace.h` (defines the tracepoint macros) with `#include <util/trace.h>`.
+Each tracepoint needs a `context` and an `event`. Please use `snake_case` and
+try to make sure that the tracepoint names make sense even without detailed
+knowledge of the implementation details. You can pass zero to twelve arguments
+to the tracepoint. Do not forget to update the tracepoint list in this
 document.
 
-```c
-#define TRACE(context, event)
-#define TRACE1(context, event, a)
-#define TRACE2(context, event, a, b)
-#define TRACE3(context, event, a, b, c)
-#define TRACE4(context, event, a, b, c, d)
-#define TRACE5(context, event, a, b, c, d, e)
-#define TRACE6(context, event, a, b, c, d, e, f)
-#define TRACE7(context, event, a, b, c, d, e, f, g)
-#define TRACE8(context, event, a, b, c, d, e, f, g, h)
-#define TRACE9(context, event, a, b, c, d, e, f, g, h, i)
-#define TRACE10(context, event, a, b, c, d, e, f, g, h, i, j)
-#define TRACE11(context, event, a, b, c, d, e, f, g, h, i, j, k)
-#define TRACE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l)
-```
-
-For example:
+For example, a tracepoint in the `net` context for the event `inbound_message`
+and six arguments:
 
 ```C++
-TRACE6(net, inbound_message,
+TRACEPOINT(net, inbound_message,
     pnode->GetId(),
     pnode->m_addr_name.c_str(),
     pnode->ConnectionTypeAsString().c_str(),
@@ -347,7 +331,7 @@ first six argument fields. Binary data can be placed in later arguments. The BCC
 supports reading from all 12 arguments.
 
 #### Strings as C-style String
-Generally, strings should be passed into the `TRACEx` macros as pointers to
+Generally, strings should be passed into the `TRACEPOINT` macros as pointers to
 C-style strings (a null-terminated sequence of characters). For C++
 `std::strings`, [`c_str()`]  can be used. It's recommended to document the
 maximum expected string size if known.

src/coins.cpp

@@ -97,7 +97,7 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
     it->second.coin = std::move(coin);
     it->second.AddFlags(CCoinsCacheEntry::DIRTY | (fresh ? CCoinsCacheEntry::FRESH : 0), *it, m_sentinel);
     cachedCoinsUsage += it->second.coin.DynamicMemoryUsage();
-    TRACE5(utxocache, add,
+    TRACEPOINT(utxocache, add,
            outpoint.hash.data(),
            (uint32_t)outpoint.n,
            (uint32_t)it->second.coin.nHeight,
@@ -131,7 +131,7 @@ bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) {
     CCoinsMap::iterator it = FetchCoin(outpoint);
     if (it == cacheCoins.end()) return false;
     cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
-    TRACE5(utxocache, spent,
+    TRACEPOINT(utxocache, spent,
            outpoint.hash.data(),
            (uint32_t)outpoint.n,
            (uint32_t)it->second.coin.nHeight,
@@ -278,7 +278,7 @@ void CCoinsViewCache::Uncache(const COutPoint& hash)
     CCoinsMap::iterator it = cacheCoins.find(hash);
     if (it != cacheCoins.end() && !it->second.IsDirty() && !it->second.IsFresh()) {
         cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
-        TRACE5(utxocache, uncache,
+        TRACEPOINT(utxocache, uncache,
                hash.hash.data(),
                (uint32_t)hash.n,
                (uint32_t)it->second.coin.nHeight,

src/net.cpp

@@ -3809,7 +3809,7 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg)
         CaptureMessage(pnode->addr, msg.m_type, msg.data, /*is_incoming=*/false);
     }
 
-    TRACE6(net, outbound_message,
+    TRACEPOINT(net, outbound_message,
         pnode->GetId(),
         pnode->m_addr_name.c_str(),
         pnode->ConnectionTypeAsString().c_str(),

src/net_processing.cpp

@@ -5421,7 +5421,7 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt
     CNetMessage& msg{poll_result->first};
     bool fMoreWork = poll_result->second;
 
-    TRACE6(net, inbound_message,
+    TRACEPOINT(net, inbound_message,
         pfrom->GetId(),
         pfrom->m_addr_name.c_str(),
         pfrom->ConnectionTypeAsString().c_str(),

src/test/CMakeLists.txt

@@ -135,6 +135,7 @@ add_executable(test_bitcoin
   util_string_tests.cpp
   util_tests.cpp
   util_threadnames_tests.cpp
+  util_trace_tests.cpp
   validation_block_tests.cpp
   validation_chainstate_tests.cpp
   validation_chainstatemanager_tests.cpp

src/test/util_trace_tests.cpp

@@ -0,0 +1,23 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <test/util/setup_common.h>
+
+#include <boost/test/unit_test.hpp>
+
+#include <util/trace.h>
+
+BOOST_FIXTURE_TEST_SUITE(util_trace_tests, BasicTestingSetup)
+
+// Tests the TRACEPOINT macro and that we can compile tracepoints with 0 to 12 args.
+BOOST_AUTO_TEST_CASE(test_tracepoints)
+{
+    TRACEPOINT(test, zero_args);
+    TRACEPOINT(test, one_arg, 1);
+    TRACEPOINT(test, six_args, 1, 2, 3, 4, 5, 6);
+    TRACEPOINT(test, twelve_args, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12);
+    BOOST_CHECK(true);
+}
+
+BOOST_AUTO_TEST_SUITE_END()

src/txmempool.cpp

@@ -477,7 +477,7 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces
     txns_randomized.emplace_back(newit->GetSharedTx());
     newit->idx_randomized = txns_randomized.size() - 1;
 
-    TRACE3(mempool, added,
+    TRACEPOINT(mempool, added,
         entry.GetTx().GetHash().data(),
         entry.GetTxSize(),
         entry.GetFee()
@@ -497,7 +497,7 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
         // notification.
         m_opts.signals->TransactionRemovedFromMempool(it->GetSharedTx(), reason, mempool_sequence);
     }
-    TRACE5(mempool, removed,
+    TRACEPOINT(mempool, removed,
         it->GetTx().GetHash().data(),
         RemovalReasonToString(reason).c_str(),
         it->GetTxSize(),

src/util/trace.h

@@ -9,37 +9,18 @@
 
 #ifdef ENABLE_TRACING
 
-#include <sys/sdt.h>
+// Setting SDT_USE_VARIADIC lets systemtap (sys/sdt.h) know that we want to use
+// the optional variadic macros to define tracepoints.
+#define SDT_USE_VARIADIC 1
 
-#define TRACE(context, event) DTRACE_PROBE(context, event)
-#define TRACE1(context, event, a) DTRACE_PROBE1(context, event, a)
-#define TRACE2(context, event, a, b) DTRACE_PROBE2(context, event, a, b)
-#define TRACE3(context, event, a, b, c) DTRACE_PROBE3(context, event, a, b, c)
-#define TRACE4(context, event, a, b, c, d) DTRACE_PROBE4(context, event, a, b, c, d)
-#define TRACE5(context, event, a, b, c, d, e) DTRACE_PROBE5(context, event, a, b, c, d, e)
-#define TRACE6(context, event, a, b, c, d, e, f) DTRACE_PROBE6(context, event, a, b, c, d, e, f)
-#define TRACE7(context, event, a, b, c, d, e, f, g) DTRACE_PROBE7(context, event, a, b, c, d, e, f, g)
-#define TRACE8(context, event, a, b, c, d, e, f, g, h) DTRACE_PROBE8(context, event, a, b, c, d, e, f, g, h)
-#define TRACE9(context, event, a, b, c, d, e, f, g, h, i) DTRACE_PROBE9(context, event, a, b, c, d, e, f, g, h, i)
-#define TRACE10(context, event, a, b, c, d, e, f, g, h, i, j) DTRACE_PROBE10(context, event, a, b, c, d, e, f, g, h, i, j)
-#define TRACE11(context, event, a, b, c, d, e, f, g, h, i, j, k) DTRACE_PROBE11(context, event, a, b, c, d, e, f, g, h, i, j, k)
-#define TRACE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l) DTRACE_PROBE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l)
+#include <sys/sdt.h>
 
+// A USDT tracepoint with zero to twelve arguments.
+#define TRACEPOINT(context, ...) \
+    STAP_PROBEV(context, __VA_ARGS__);
 #else
 
-#define TRACE(context, event)
-#define TRACE1(context, event, a)
-#define TRACE2(context, event, a, b)
-#define TRACE3(context, event, a, b, c)
-#define TRACE4(context, event, a, b, c, d)
-#define TRACE5(context, event, a, b, c, d, e)
-#define TRACE6(context, event, a, b, c, d, e, f)
-#define TRACE7(context, event, a, b, c, d, e, f, g)
-#define TRACE8(context, event, a, b, c, d, e, f, g, h)
-#define TRACE9(context, event, a, b, c, d, e, f, g, h, i)
-#define TRACE10(context, event, a, b, c, d, e, f, g, h, i, j)
-#define TRACE11(context, event, a, b, c, d, e, f, g, h, i, j, k)
-#define TRACE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l)
+#define TRACEPOINT(context, ...)
 
 #endif
 

src/validation.cpp

@@ -1304,7 +1304,7 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws)
                 tx.GetWitnessHash().ToString(),
                 entry->GetFee(),
                 entry->GetTxSize());
-        TRACE7(mempool, replaced,
+        TRACEPOINT(mempool, replaced,
                 it->GetTx().GetHash().data(),
                 it->GetTxSize(),
                 it->GetFee(),
@@ -1866,7 +1866,7 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra
 
         for (const COutPoint& hashTx : coins_to_uncache)
             active_chainstate.CoinsTip().Uncache(hashTx);
-        TRACE2(mempool, rejected,
+        TRACEPOINT(mempool, rejected,
                 tx->GetHash().data(),
                 result.m_state.GetRejectReason().c_str()
         );
@@ -2736,7 +2736,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
              Ticks<SecondsDouble>(m_chainman.time_index),
              Ticks<MillisecondsDouble>(m_chainman.time_index) / m_chainman.num_blocks_total);
 
-    TRACE6(validation, block_connected,
+    TRACEPOINT(validation, block_connected,
         block_hash.data(),
         pindex->nHeight,
         block.vtx.size(),
@@ -2912,7 +2912,7 @@ bool Chainstate::FlushStateToDisk(
             }
             m_last_flush = nNow;
             full_flush_completed = true;
-            TRACE5(utxocache, flush,
+            TRACEPOINT(utxocache, flush,
                    int64_t{Ticks<std::chrono::microseconds>(SteadyClock::now() - nNow)},
                    (uint32_t)mode,
                    (uint64_t)coins_count,