-
Notifications
You must be signed in to change notification settings - Fork 38.7k
rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats' #29418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/29418. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. LLM Linter (✨ experimental)Possible typos and grammar issues:
drahtbot_id_5_m |
|
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the Possibly this is due to a silent merge conflict (the changes in this pull request being Leave a comment here, if you need help tracking down a confusing failure. |
bc156d8 to
91c90d7
Compare
|
Concept ACK 91c90d759503aa3fa15b330bba2148698cdc271d. |
|
Needs rebase if still relevant |
|
just fyi as this came up: You could count up message stats with the inbound and outbound message tracepoints. However, this does only gives you stats from the points on where you start hooking into the tracepoints, might not be as user friendly (RPC is easier to use than the tracepoints), and the tracepoints currently only work on Linux. While I personally don't need it, I see why some node operators need traffic statistics. E.g. How much of my (limited monthly) traffic is used up by inbound connections? |
|
Converted to draft due to failing CI and inactivity |
|
|
…e constant b3efb48 protocol: make message types constexpr (Vasil Dimov) 2fa9de0 net: make the list of known message types a compile time constant (Vasil Dimov) Pull request description: Turn the `std::vector` to `std::array` because it is cheaper and allows us to have the number of the messages as a compile time constant: `ALL_NET_MESSAGE_TYPES.size()` which can be used in future code to build other `std::array`s with that size. --- This change is part of #29418 but it makes sense on its own and would be good to have it, regardless of the fate of #29418. Also, if this is merged, that would reduce the size of #29418, thus the current standalone PR. ACKs for top commit: achow101: ACK b3efb48 jonatack: ACK b3efb48 maflcko: utACK b3efb48 🎊 willcl-ark: ACK b3efb48 Tree-SHA512: 6d3860c138c64514ebab13d97ea67893e2d346dfac30a48c3d9bc769a1970407375ea4170afdb522411ced306a14a9af4eede99e964d1fb1ea3efff5d5eb57af
|
|
a46cbdd to
db442f3
Compare
|
|
|
|
|
|
Before this change only per-peer stats were gathered. They are lost when the peer disconnects. So, collect the traffic stats globally in `CConnman`, broken down by * direction (sent or received, (2)) * network of the peer (IPv4, IPv6, Tor, I2P, CJDNS (5)) * connection type (inbound, full outbound, feeler, etc, (6)) * message type (verack, ping, etc, (36))
528b4d6 to
87d3d64
Compare
|
|
Introduce a new RPC `getnetmsgstats` that renders the global traffic stats from `CConnman`. Co-authored-by: Vasil Dimov <[email protected]>
Aggregation can be by either one of direction, network, connection type
or message type. For example if the following
```
{
"ipv4": { "ping": 3 },
"ipv6": { "ping": 4 }
}
```
is aggregated by network, then the result will be
```
{
"ping": 7
}
```
Co-authored-by: Vasil Dimov <[email protected]>
87d3d64 to
7d4e82c
Compare
|
|
|
Concept ACK Useful netstats for tracking. Normally, I'd sum and aggregate peer stats. This approach simplifies that. |
|
ACK 7d4e82c lgtm. Tested on master and with v30.1 in my node monitoring dashboard. |
Introduce a new RPC,
getnetmsgstatsto retrieve traffic bytes and count of messages possibly broken down by:Implements: #26337 Add per message stats to getnettotals rpc
Examples:
bitcoin-cli getnetmsgstats '["direction", "network", "connection_type", "message_type"]'
bitcoin-cli getnetmsgstats '["direction", "network", "connection_type"]'
bitcoin-cli getnetmsgstats '["direction", "connection_type", "message_type"]'
bitcoin-cli getnetmsgstats
Previous incarnations of this:
#27534 rpc: add 'getnetmsgstats', new rpc to view network message statistics (Thank you, @satsie!)
#28926 rpc: add 'getnetmsgstats' RPC (Thank you, @willcl-ark!)