refactor(portmapper)!: non-global metrics collection #20
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 tasks
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/net-tools/pr/20/docs/net_tools/ Last updated: 2025-05-05T09:14:43Z |
Frando
changed the title
Frando
changed the title
dignifiedquire
approved these changes
Apr 16, 2025
4 tasks
Arqu
approved these changes
Apr 30, 2025
portmapper/Cargo.toml
Outdated
| @@ -22,7 +22,7 @@ derive_more = { version = "1.0.0", features = ["debug", "display", "from", "try_ | |||
| futures-lite = "2.5" | |||
| futures-util = "0.3.25" | |||
| igd-next = { version = "0.15.1", features = ["aio_tokio"] } | |||
| iroh-metrics = { version = "0.32", default-features = false } | |||
| iroh-metrics = { git = "https://github.com/n0-computer/iroh-metrics", branch = "main" } | |||
There was a problem hiding this comment.
this is released now
dignifiedquire
force-pushed
the
Frando/metrics
branch
from
08a71b6 to
09df2aa
Compare
github-merge-queue bot
pushed a commit
to n0-computer/iroh
that referenced
this pull request
May 5, 2025
## Description Depends on n0-computer/net-tools#20 Until now, we were using a superglobal static `iroh_metrics::core::Core` struct to collect metrics into. This allowed us to use macros to track metrics from anywhere in the codebase. However, this also made it impossible to collect metrics *per endpoint*, which is what you want usually as soon as you have more than one endpoint in your app. This PR builds on n0-computer/iroh-metrics#15, n0-computer/iroh-metrics#22, and n0-computer/iroh-metrics#23. It removes the global metrics collection from all crates in the iroh repository. Instead, we now create and pass metrics collector structs to all places where we need to collect metrics. This PR disables the `static_core` feature from iroh-metrics, which means the macros for superglobal metrics collection are not available anymore. This is good, because otherwise we could easily miss metrics not tracked onto the proper metrics collector. This PR also updates iroh-dns-server and iroh-relay to use manual metrics collection. While this means that we have to pass our metrics structs to more places, it also makes metrics collection more visible, and we can now also split the metrics structs further easily if we want to separate concerns more. This PR should not change anything apart from metrics collection. Most places are straightforward conversions from the macros to methods on the metrics collectors. At a few places, logic was changed slightly to move metrics collection a layer up to save a few clones. ## Breaking Changes * All metrics structs (`iroh::metrics::{MagicsockMetrics, PortmapMetrics, NetReportMetrics}`) now implement `MetricsGroup` from the new version `0.34` of `iroh-metrics` and no longer implement traits from `[email protected]`. * Metrics are no longer registered onto the static superglobal `Core`. `iroh` does not use `static_core` feature of `iroh-metrics`. Metrics are now exposed from the subsystems that track them, see e.g. `Endpoint::metrics`. Several methods now take a `Metrics` argument. You can always pass `Default::default` if you don't want to unify metrics tracking with other sections. #### `iroh` * `iroh::metrics::{MagicsockMetrics, NetReportMetrics, PortmapMetrics}` all are now marked `non_exhaustive`, and implement `iroh_metrics::MetricsGroup` from `[email protected]` and no longer implement `iroh_metrics::Metric` from `[email protected]`. They also no longer implement `Clone` (put them into an `Arc` for cloning instead). * `iroh::net_report::Client::new` now takes `iroh::net_report::metrics::Metrics` as forth argument #### `iroh-dns-server` * `iroh_dns_server::server::Server::spawn` now takes `Metrics` as third argument * `iroh_dns_server::ZoneStore::persistent` now takes `Metrics` as third argument * `iroh_dns_server::ZoneStore::in_memory` now takes `Metrics` as third argument * `iroh_dns_server::ZoneStore::new` now takes `Metrics` as third argument * `iroh_dns_server::state::AppState` now has a public `metrics: Metrics` field * `iroh_dns_server::dns::DnsHandler::new` now takes `Metrics` as third argument * function `iroh_dns_server::metrics::init_metrics` is removed #### `iroh-relay` * `iroh_relay::metrics::{StunMetrics, Metrics}` all are now marked `non_exhaustive`, and implement `iroh_metrics::MetricsGroup` from `[email protected]` and no longer implement `iroh_metrics::Metric` from `[email protected]`. They also no longer implement `Clone` (put them into an `Arc` for cloning instead). ## Notes & open questions <!-- Any notes, remarks or open questions you have to make about the PR. --> ## Change checklist <!-- Remove any that are not relevant. --> - [x] Self-review. - [x] Documentation updates following the [style guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text), if relevant. - [x] Tests if relevant. - [x] All breaking changes documented. - [x] List all breaking changes in the above "Breaking Changes" section. - [x] 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. - [x] [`iroh-gossip`](https://github.com/n0-computer/iroh-gossip) - n0-computer/iroh-gossip#58 - [x] [`iroh-blobs`](https://github.com/n0-computer/iroh-blobs) - n0-computer/iroh-blobs#85 - [x] [`iroh-docs`](https://github.com/n0-computer/iroh-docs) - n0-computer/iroh-docs#41 --------- Co-authored-by: dignifiedquire <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Needed for n0-computer/iroh#3262
Updates
iroh-metricsto 0.34, and makes the metrics collection non-global. Metrics are now collected per portmapper service.Breaking Changes
portmapper::metrics::Metricsnow implementsMetricsGroupfrom[email protected], and no longer implementsMetricfrom[email protected].Notes & open questions
Change checklist