Skip to content

feat: congestion metrics #3491

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: congestion metrics #3491

wants to merge 1 commit into from

Conversation

Arqu
Copy link
Collaborator

@Arqu Arqu commented Oct 1, 2025 •
edited
Loading

Description

Needs n0-computer/iroh-metrics#35 and updating the Cargo.toml's again.

Breaking Changes

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.
    • List all breaking changes in the above "Breaking Changes" section.
    • Open an issue or PR on any number0 repos that are affected by this breaking change. Give guidance on how the updates should be handled or do the actual updates themselves. The major ones are:

@Arqu Arqu self-assigned this Oct 1, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 1, 2025 •
edited
Loading

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3491/docs/iroh/

Last updated: 2025-10-01T12:52:36Z

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 1, 2025 •
edited
Loading

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 4ffa7cf

@n0bot n0bot bot added this to iroh Oct 1, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Oct 1, 2025
@Arqu Arqu force-pushed the arqu/congestion_metrics branch from 9e23616 to a89e698 Compare October 1, 2025 12:51
@Arqu Arqu changed the title WIP: testing some congestion metrics and fixes feat: congestion metrics Oct 1, 2025
@Arqu Arqu moved this from 🏗 In progress to 👀 In review in iroh Oct 1, 2025
@Arqu Arqu marked this pull request as ready for review October 1, 2025 12:51
@dignifiedquire
Copy link
Contributor

conflicts

dignifiedquire
dignifiedquire reviewed Oct 6, 2025
View reviewed changes
iroh/src/magicsock/metrics.rs
path_failure_resets: Counter::default(),
path_packet_loss_rate: packet_loss_buckets(),
path_rtt_variance_ms: rtt_variance_buckets(),
path_quality_score: quality_score_buckets(),
Copy link
Contributor

Choose a reason for hiding this comment

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

(not blocking) it's a bit sad we have to do this manually when including histograms now :( I wonder if we could adjust our macro or sth

cc @Frando

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was looking for some magic syntax sugar with a default annotation or sth but couldn't work it out.
The problem is for each histogram you want to provide a specific set of buckets that make sense for that metric specifically, so you need to "adjust" it here. However if there was some magic #[default...] annotation I couldn't find where I pass in those funcs, it would be great.

@dignifiedquire dignifiedquire added this to the v0.93.0 milestone Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dignifiedquire dignifiedquire dignifiedquire left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@Arqu Arqu

Labels
None yet
Projects
iroh
Status: 👀 In review
Milestone
v0.93.0
Development

Successfully merging this pull request may close these issues.
2 participants
@Arqu @dignifiedquire