Skip to content
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

feat(libp2p): track bandwidth per transport protocol stack #4727

Merged
merged 29 commits into from
Nov 10, 2023

Conversation

mxinden
Copy link
Member

@mxinden mxinden commented Oct 25, 2023

Description

Previously one could use the with_bandwidth_logging to measure the overall bandwidth usage, only differentiated by inbound and outbound traffic.

With this commit bandwidth can be tracked per transport protocol stack (e.g. /ip4/tcp) exposed as Prometheus metrics through libp2p_metrics::BandwidthMetricTransport.

# HELP libp2p_bandwidth_bytes Bandwidth usage by direction and transport protocols.
# TYPE libp2p_bandwidth_bytes counter
# UNIT libp2p_bandwidth_bytes bytes
libp2p_bandwidth_bytes_total{protocols="/ip4/tcp",direction="Outbound"} 1352
libp2p_bandwidth_bytes_total{protocols="/ip4/tcp",direction="Inbound"} 1484

Notes & open questions

@jxs this is my suggestion on how to expose bandwidth metrics. What do you think?

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

Previously one could use the `with_bandwidth_logging` to measure the overall bandwidth usage, only
differentiated by inbound and outbound traffic.

With this commit bandwidth can be tracked per transport protocol stack (e.g. `/ip4/tcp`). In
addition this commit adds the ability to redirect these metrics to a `prometheus_client::Registry`.

```
bandwidth_total{protocols="/ip4/tcp",direction="inbound"} 363
bandwidth_total{protocols="/ip4/tcp",direction="outbound"} 348
```
libp2p/src/bandwidth.rs Outdated Show resolved Hide resolved
libp2p/src/builder.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments but overall this is pretty exciting :)

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks really nice Max! Thanks for going ahead and implement it, and thanks Thomas also reviewing

misc/metrics/src/lib.rs Outdated Show resolved Hide resolved
libp2p/src/bandwidth.rs Outdated Show resolved Hide resolved
misc/metrics/src/bandwidth.rs Outdated Show resolved Hide resolved
misc/metrics/src/bandwidth.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Looking forward to land this :)

examples/metrics/src/main.rs Outdated Show resolved Hide resolved
examples/metrics/src/main.rs Outdated Show resolved Hide resolved
misc/metrics/src/bandwidth.rs Outdated Show resolved Hide resolved
misc/metrics/src/bandwidth.rs Outdated Show resolved Hide resolved
misc/metrics/src/bandwidth.rs Outdated Show resolved Hide resolved
misc/metrics/src/bandwidth.rs Outdated Show resolved Hide resolved
misc/metrics/src/bandwidth.rs Outdated Show resolved Hide resolved
misc/server/src/main.rs Show resolved Hide resolved
@mxinden
Copy link
Member Author

mxinden commented Nov 1, 2023

@thomaseizinger @jxs this is ready for a full review. Thank you for the comments thus far.

@mxinden mxinden marked this pull request as ready for review November 1, 2023 19:11
Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thank you Max!

Cargo.toml Show resolved Hide resolved
libp2p/src/bandwidth.rs Outdated Show resolved Hide resolved
misc/metrics/src/bandwidth.rs Show resolved Hide resolved
misc/metrics/src/bandwidth.rs Show resolved Hide resolved
misc/metrics/src/bandwidth.rs Show resolved Hide resolved
misc/metrics/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great Max, thanks for this!

@mxinden mxinden added the send-it label Nov 4, 2023
@mxinden
Copy link
Member Author

mxinden commented Nov 4, 2023

Thank you for the help!

@thomaseizinger
Copy link
Contributor

@mxinden Needs rustfmt.

@mxinden
Copy link
Member Author

mxinden commented Nov 10, 2023

@thomaseizinger CI catching my mistakes 🎉

Run git fetch origin master:master
From https://github.com/libp2p/rust-libp2p
 * [new branch]        master     -> master
++ jq -e -r '.packages[] | select(.name == "libp2p-metrics") | .manifest_path'
++ cargo metadata --format-version=1 --no-deps
+ MANIFEST_PATH=/home/runner/work/rust-libp2p/rust-libp2p/misc/metrics/Cargo.toml
++ dirname /home/runner/work/rust-libp2p/rust-libp2p/misc/metrics/Cargo.toml
+ DIR_TO_CRATE=/home/runner/work/rust-libp2p/rust-libp2p/misc/metrics
++ git merge-base d9b6d7dca4c87ad37d4d569d852a360b5d408abd master
+ MERGE_BASE=9ab0e6f0fea3382607c72fc1c9086786ff541d98
++ git diff d9b6d7dca4c87ad37d4d569d852a360b5d408abd..9ab0e6f0fea3382607c72fc1c9086786ff541d98 --name-status -- /home/runner/work/rust-libp2p/rust-libp2p/misc/metrics/src /home/runner/work/rust-libp2p/rust-libp2p/misc/metrics/Cargo.toml
+ SRC_DIFF_TO_BASE='M	misc/metrics/Cargo.toml
D	misc/metrics/src/bandwidth.rs
M	misc/metrics/src/lib.rs'
++ git diff d9b6d7dca4c87ad37d4d569d852a360b5d408abd..9ab0e6f0fea3382607c72fc1c9086786ff541d98 --name-only -- /home/runner/work/rust-libp2p/rust-libp2p/misc/metrics/CHANGELOG.md
+ CHANGELOG_DIFF=misc/metrics/CHANGELOG.md
++ awk '-F ' '/^## [0-9]+\.[0-9]+\.[0-9]+/{print $2; exit}' /home/runner/work/rust-libp2p/rust-libp2p/misc/metrics/CHANGELOG.md
+ VERSION_IN_CHANGELOG=0.14.0
++ cargo metadata --format-version=1 --no-deps
++ jq -e -r '.packages[] | select(.name == "libp2p-metrics") | .version'
+ VERSION_IN_MANIFEST=0.14.0
+ [[ 0.14.0 != \0\.\1\4\.\0 ]]
+ '[' -z 'M	misc/metrics/Cargo.toml
D	misc/metrics/src/bandwidth.rs
M	misc/metrics/src/lib.rs' ']'
+ '[' -z misc/metrics/CHANGELOG.md ']'
+ git tag
+ grep -q '^libp2p-metrics-v0.14.0$'
+ echo 'v0.14.0 of '\''libp2p-metrics'\'' has already been released, please bump the version.'
v0.14.0 of 'libp2p-metrics' has already been released, please bump the version.
+ exit 1

@mergify mergify bot merged commit 441c242 into master Nov 10, 2023
71 checks passed
@mergify mergify bot deleted the bandwidth-metrics-per-protocol branch November 10, 2023 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants