Tracepoint Interface Tracking Issue #31274

1. 
   0xB10C commented at 5:42 pm on November 11, 2024: contributor

   With #26593 merged, a few ideas for the Bitcoin Core tracepoint interface that could be next steps. I posted these as a comment in #26593 (comment) but surfacing them here for more visibility.

   These ideas are all up for discussion and aren’t TODOs. The numbers don’t indicate order or priority but make it easier reference them.

   Dependencies

   1. • We could internalize the relevant macro parts of systemtap’s sys/sdt.h for the Linux tracepoints. This would allow us to drop the external dependency on systemtap, as we don’t use 99% of it. Some prior commentary on this can be found here: https://github.com/hebasto/bitcoin/pull/162#issuecomment-2074645621

   MacOS & FreeBSD support

   2. • In the past I’ve managed to use a simple macro build a bitcoind with tracepoints on macOS. While our ebpf based demo scripts aren’t compatible, @kouloumos DTrace scripts from #25541 are. This could look similar to https://github.com/0xB10C/bitcoin/blob/13b0ce221600fc7040502c834c51433ca96f91c3/src/util/trace.h#L35-L63. However, I currently don’t have access to a macOS system to further work on this - I’m looking to rent one.
   3. • The same could possible on FreeBSD with e.g. these macros https://github.com/0xB10C/bitcoin/blob/13b0ce221600fc7040502c834c51433ca96f91c3/src/util/trace.h#L104-L119. I haven’t tested this on FreeBSD yet. In #27458#pullrequestreview-1387810492, vasild mentiones he’d interested in FreeBSD tracepoints. My understanding is that the same macOS DTrace scripts from 25541 would work there too.
   4. • At least for macOS, we’d need an per-tracepoint interface definition similar to https://github.com/0xB10C/bitcoin/blob/13b0ce221600fc7040502c834c51433ca96f91c3/src/util/trace.h#L121-L236. With some more commentary, these could replace the list of tracepoints in https://github.com/bitcoin/bitcoin/blob/master/doc/tracing.md#tracepoint-documentation. This would solve something similar to #29877 (comment).

   Interface stabillity improvements

   5. • Even if we don’t do 4. (because we e.g. don’t want to do 2.), casting the tracepoint arguments to the type we expect to pass would be worthwhile to avoid problems like #29877. For some of our traceponts, we already do this: e.g. https://github.com/bitcoin/bitcoin/blob/900b17239fb25750fd30b4af6e6c38096374b71f/src/validation.cpp#L2902-L2907

   Example scripts

   6. • We could drop the example scripts from /contrib/tracing/* and maintain them, along with proper tests in a CI, Bitcoin Core version compatibility information, possibly libbpf-based C or Rust tools (https://github.com/bitcoin/bitcoin/issues/30298), … in an external repository like, for example, 0xb10c/tracing-scripts, bitcoin-core/tracing-scripts, or bitcoin-monitoring/tracing-scripts (what ever would works best).

   Maintain tracepoint interface, drop tracepoint implemenation

   7. • If we at some point decide that maintaining the tracepoints in Bitcoin Core adds too much maintenance burden compared to the actual usage they’re getting, we could drop the tracepoints but keep the tracepoint interface. We now have a unit test that includes a few nop tracepoints to check that the interface will still compile (https://github.com/0xB10C/bitcoin/blob/0de3e96e333090548a43e5e870c4cb8941d6baf1/src/test/util_trace_tests.cpp). This would allow us to drop the bcc python dependency in the CI and to remove the interface_usdt_* functional tests (which need to run in VM and can’t run in a container). Tracepoint users could maintain a patch on Bitcoin Core adding the tracepoints (and tests) they want back in. We’d however loose the tracepoints in release (or actually all) builds which currently allow e.g. everyone experiencing problems with their node to hook into them and extract data without needing to restart it.
2. 0xB10C added the label Feature on Nov 11, 2024
3. fanquake removed the label Feature on Nov 11, 2024
4. laanwj added the label interfaces on Nov 12, 2024
5. laanwj added the label Resource usage on Nov 12, 2024
6. 
   laanwj commented at 10:51 am on November 14, 2024: member

   We could internalize the relevant macro parts of systemtap’s sys/sdt.h for the Linux tracepoints. This would allow us to drop the external dependency on systemtap

   This still makes sense to me, it’s just a few macros which haven’t changed in years and are super unlikely to change significantly, because they insert ELF annotations (according to a well-known spec) and NOP instructions.

   Dropping a guix dependency would be good!

   But this is just Linux. i don’t know how it is for other platforms, if it’s very different then maintaining a “cross-platform tracepoints header” inside bitcoin core seems like scope creep.

   At least for macOS, we’d need an per-tracepoint interface definition similar to https://github.com/0xB10C/bitcoin/blob/13b0ce221600fc7040502c834c51433ca96f91c3/src/util/trace.h#L121-L236. With some more commentary, these could replace the list of tracepoints in https://github.com/bitcoin/bitcoin/blob/master/doc/tracing.md#tracepoint-documentation.

   If it helps unify things and formalize documentation, that’s of course nice!

   Even if we don’t do 4. (because we e.g. don’t want to do 2.), casting the tracepoint arguments to the type we expect to pass would be worthwhile to avoid problems like #29877. For some of our traceponts, we already do this

   Yes imo it makes sense to add explicit casts. After all the type is essential for the interface to not break, even when the types that are used internally change. Something to be wary for is if a (dumb, forced) cast results in nonsense on the interface.

   We could drop the example scripts from /contrib/tracing/* and maintain them, along with proper tests in a CI, Bitcoin Core version compatibility information, possibly libbpf-based C or Rust tools (https://github.com/bitcoin/bitcoin/issues/30298), … in an external repository like, for example, 0xb10c/tracing-scripts, bitcoin-core/tracing-scripts, or bitcoin-monitoring/tracing-scripts (what ever would works best).

   i think these are two separate points

   • Create a bitcoin core tracing library in (probably) rust, possibly under the bitcoin-core github org, to make it easier to hook into them from a “real” programming language instead scripting. Although i can also imagine people like Python for this if they want to quickly build one-off experiments (it’s still my preferable language for that).
   • Remove the python demo scripts from our repository.

   If we at some point decide that maintaining the tracepoints in Bitcoin Core adds too much maintenance burden compared to the actual usage they’re getting

   Yeah, one of the aims of the tracepoint interface was that it should be as hands-off as possible (which it is, compared to more intrusive tracing mechanisms that involve internal infrastructure). If it really starts taking significant maintenance time then i think the premise failed. But i don’t think that’s a likely scenario right now.

   FWIW, i think the kind of performance monitoring that you do is extremely important to health of the network. There’s a lot you can see by keeping tabs on specific events and statistics of real nodes, instead of say, just running some benchmarks and measuring full syncs once in a while.

   If anything, people aren’t doing enough of that, and it should be encouraged.

   Tracepoint users could maintain a patch on Bitcoin Core adding the tracepoints (and tests) they want back in.

   i don’t like this. Maintaining patches against a live codebase is horrible, and much more maintenance work than leaving them in.

   Also having them in the release is good, so if you want to investigate some older node version’s behavior you can just download a release, instead of finding how to build it with instrumentation first. Similar to that we now ship the debug symbols with the release downloads.

7. 
   0xB10C commented at 8:00 pm on November 14, 2024: contributor

   We could internalize the relevant macro parts of systemtap’s sys/sdt.h for the Linux tracepoints. This would allow us to drop the external dependency on systemtap

   This still makes sense to me, it’s just a few macros which haven’t changed in years and are super unlikely to change significantly, because they insert ELF annotations (according to a well-known spec) and NOP instructions.

   Dropping a guix dependency would be good!

   I just found out about https://github.com/libbpf/usdt which is a single usdt.h header file which could be an alternative for systemtaps sys/sdt.h.

8. 
   Maxzy000 commented at 2:45 am on November 16, 2024: none

   Let me guide you on this
9. 
   maflcko commented at 11:44 am on November 25, 2024: member

   There is also a TRACE_RAII idea from #28226, mostly for performance/timing tracking in that example.
10. bitcoin deleted a comment on Nov 25, 2024
11. 
    laanwj commented at 7:12 pm on November 25, 2024: member

    I just found out about https://github.com/libbpf/usdt which is a single usdt.h header file which could be an alternative for systemtaps sys/sdt.h.

    Neat, that seems preferable to maintaining our own! They even recommend copying the header file into your project.

12. 
    0xB10C commented at 2:39 pm on November 26, 2024: contributor

    There is also #27380 with discussion about a problem in the interface_usdt_mempool.py test.

    https://github.com/bitcoin/bitcoin/blob/f7144b24be09e7db2173a0b15d73f99a08b98118/test/functional/interface_usdt_mempool.py#L317-L320

    A possible fix might be #27380 (comment) - but even if this doesn’t fix #27380, it should probably be done.

Contributors
0xB10C laanwj Maxzy000 maflcko

Labels
Resource usage interfaces