refactor!: update to iroh 0.35 and non-global metrics tracking #41
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
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh-docs/pr/41/docs/iroh_docs/ Last updated: 2025-05-12T21:44:41Z |
9 tasks
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]>
Frando
commented
May 12, 2025
| @@ -750,6 +759,8 @@ impl Actor { | |||
| let author = get_author(&mut this.store, &author)?; | |||
| let mut replica = this.states.replica(namespace, &mut this.store)?; | |||
| replica.insert(&key, &author, hash, len)?; | |||
| this.metrics.new_entries_local.inc(); | |||
There was a problem hiding this comment.
These metric were previously tracked in sync::Replica::insert_entry
Frando
commented
May 12, 2025
| replica.insert_remote_entry(entry, from, content_status)?; | ||
| this.metrics.new_entries_remote.inc(); |
There was a problem hiding this comment.
These metric were previously tracked in sync::Replica::insert_entry
ramfox
approved these changes
May 12, 2025
Frando
changed the title
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
Updates metrics tracking to the new non-global tracking.
Metrics are tracked per sync actor.
We previously tracked a few metrics within the replica, this is removed and instead tracked from the sync actor now.
Depends on n0-computer/iroh#3262
Depends on n0-computer/iroh-blobs#85
Depends on n0-computer/iroh-gossip#58
Breaking Changes
metrics::Metricsnow implementsMetricsGroupfrom the recent ìroh-metrics` release (TODO: fill in version after release)static_corefromiroh-metrics, but instead are tracked perEngineand exposed viaEngine::metricsNotes & open questions
Change checklist