diff --git a/Cargo.lock b/Cargo.lock index 1959dd96b92..3c7f69a5149 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2386,9 +2386,9 @@ dependencies = [ [[package]] name = "iroh-metrics" -version = "1.0.1" +version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "291065721ad7c477b972e581bbc528df031dc8eb5e39fe1ff3300ae5dfb157ef" +checksum = "8ede55536349842337f7cdc63012ec33dc637945f3259c98a0763aac364cdf09" dependencies = [ "http-body-util", "hyper", diff --git a/iroh-dns-server/Cargo.toml b/iroh-dns-server/Cargo.toml index c2065154cd9..a3679507ab1 100644 --- a/iroh-dns-server/Cargo.toml +++ b/iroh-dns-server/Cargo.toml @@ -32,7 +32,7 @@ http = "1.0.0" humantime = "2.2.0" humantime-serde = "1.1.1" iroh-base = { version = "1.2.0", path = "../iroh-base", features = ["key"] } -iroh-metrics = { version = "1.0.1", features = ["service"] } +iroh-metrics = { version = "1.0.2", features = ["service"] } iroh-dns = { version = "1.3.0", path = "../iroh-dns" } lru = "0.18.0" n0-mainline = "0.6" diff --git a/iroh-relay/Cargo.toml b/iroh-relay/Cargo.toml index 6f2a6533b11..9627a9592c0 100644 --- a/iroh-relay/Cargo.toml +++ b/iroh-relay/Cargo.toml @@ -35,7 +35,7 @@ hyper = { version = "1", features = ["server", "client", "http1"] } hyper-util = "0.1.1" iroh-base = { version = "1.2.0", path = "../iroh-base", default-features = false, features = ["key", "relay"] } iroh-dns = { version = "1.3.0", path = "../iroh-dns" } -iroh-metrics = { version = "1.0.1", default-features = false } +iroh-metrics = { version = "1.0.2", default-features = false } lru = "0.18.0" n0-error = "1.0.0" n0-future = "0.3" diff --git a/iroh/Cargo.toml b/iroh/Cargo.toml index 2ad7cac17a5..bf96b03f292 100644 --- a/iroh/Cargo.toml +++ b/iroh/Cargo.toml @@ -65,7 +65,7 @@ url = { version = "2.5", features = ["serde"] } webpki_types = { package = "rustls-pki-types", version = "1.12" } # metrics -iroh-metrics = { version = "1.0.1", default-features = false } +iroh-metrics = { version = "1.0.2", default-features = false } futures-util = "0.3" diff --git a/iroh/bench/Cargo.toml b/iroh/bench/Cargo.toml index 93b2e9c9362..dc991b2878c 100644 --- a/iroh/bench/Cargo.toml +++ b/iroh/bench/Cargo.toml @@ -9,7 +9,7 @@ publish = false bytes = "1.11" hdrhistogram = { version = "7.2", default-features = false } iroh = { version = "1.2.0", path = "..", default-features = false } -iroh-metrics = { version = "1.0.1", optional = true } +iroh-metrics = { version = "1.0.2", optional = true } n0-future = "0.3" n0-error = "1.0.0" noq = "1.3.0" diff --git a/iroh/src/address_lookup.rs b/iroh/src/address_lookup.rs index 394e919e2c0..e3ddb7102e3 100644 --- a/iroh/src/address_lookup.rs +++ b/iroh/src/address_lookup.rs @@ -120,11 +120,13 @@ use crate::{Endpoint, endpoint::EndpointError}; #[cfg(not(wasm_browser))] pub mod dns; pub mod memory; +mod metrics; pub mod pkarr; #[cfg(not(wasm_browser))] pub use dns::*; pub use memory::*; +pub use metrics::{Metrics, ServiceLabels}; pub use pkarr::*; /// Trait for structs that can be converted into [`AddressLookup`]s. @@ -465,9 +467,19 @@ pub struct AddressLookupServices { last_data: Arc>>, /// Optional filter applied to all data before publishing to any service. addr_filter: Arc>>, + /// Metrics for lookup outcomes. + metrics: Arc, } impl AddressLookupServices { + /// Creates a registry recording lookup outcomes in `metrics`. + pub(crate) fn with_metrics(metrics: Arc) -> Self { + Self { + metrics, + ..Default::default() + } + } + /// Sets the address filter applied before publishing to any service. /// /// When set, all address data is filtered once before being distributed @@ -554,14 +566,15 @@ impl AddressLookupServices { &self, endpoint_id: EndpointId, ) -> impl Stream, AddressLookupFailed>> + use<> { + self.metrics.lookups.inc(); let services = self.services.read().expect("poisoned"); if services.is_empty() { - AddressLookupStream::empty() + AddressLookupStream::empty(self.metrics.clone()) } else { let streams = services .iter() .filter_map(|service| service.resolve(endpoint_id)); - AddressLookupStream::new(streams) + AddressLookupStream::new(streams, self.metrics.clone()) } } } @@ -584,24 +597,30 @@ struct AddressLookupStream { errors: Vec, did_emit: bool, closed: bool, + metrics: Arc, } impl AddressLookupStream { - fn empty() -> Self { + fn empty(metrics: Arc) -> Self { Self { streams: None, errors: Vec::new(), did_emit: false, closed: false, + metrics, } } - fn new(streams: impl Iterator>>) -> Self { + fn new( + streams: impl Iterator>>, + metrics: Arc, + ) -> Self { Self { streams: Some(MergeBounded::from_iter(streams)), errors: Vec::new(), did_emit: false, closed: false, + metrics, } } } @@ -621,22 +640,32 @@ impl Stream for AddressLookupStream { Some(inner) => inner, None => { this.closed = true; + this.metrics.lookups_failed.inc(); return Poll::Ready(Some(Err(e!(AddressLookupFailed::NoServiceConfigured)))); } }; let item = match ready!(Pin::new(&mut inner).poll_next(cx)) { Some(Ok(item)) => { this.did_emit = true; + this.metrics + .service_results + .get_or_create(&ServiceLabels::new(item.provenance())) + .inc(); Some(Ok(Ok(item))) } Some(Err(error)) => { debug!("address lookup error: {error:#}"); + this.metrics + .service_errors + .get_or_create(&ServiceLabels::new(error.provenance)) + .inc(); this.errors.push(error.clone()); Some(Ok(Err(error))) } None => { this.closed = true; if !this.did_emit { + this.metrics.lookups_failed.inc(); let errors = std::mem::take(&mut this.errors); Some(Err(e!(AddressLookupFailed::NoResults { errors }))) } else { @@ -1140,6 +1169,59 @@ mod tests { Ok(()) } + /// Lookup outcomes are counted per service. + #[tokio::test] + #[traced_test] + async fn address_lookup_metrics() -> Result { + let mut rng = rand_chacha::ChaCha8Rng::seed_from_u64(0u64); + let endpoint_id = SecretKey::from_bytes(&rng.random()).public(); + + // One succeeding and one failing service. + let succeeding = MemoryLookup::with_provenance("static-test"); + let data = EndpointData::from_iter([TransportAddr::Ip("127.0.0.1:1".parse().unwrap())]); + succeeding.add_endpoint_info(EndpointInfo::from_parts(endpoint_id, data)); + let services = AddressLookupServices::default(); + services.add(succeeding); + services.add(FailingAddressLookup { + delay: Duration::from_millis(10), + }); + let _results: Vec<_> = services.resolve(endpoint_id).collect().await; + + let metrics = &services.metrics; + assert_eq!(metrics.lookups.get(), 1); + assert_eq!(metrics.lookups_failed.get(), 0); + assert_eq!( + metrics + .service_results + .get(&ServiceLabels::new("static-test")) + .map(|counter| counter.get()), + Some(1) + ); + assert_eq!( + metrics + .service_errors + .get(&ServiceLabels::new("failing-test")) + .map(|counter| counter.get()), + Some(1) + ); + + // Only failing services: the lookup itself is counted as failed. + let services = AddressLookupServices::default(); + services.add(FailingAddressLookup { + delay: Duration::from_millis(10), + }); + let _results: Vec<_> = services.resolve(endpoint_id).collect().await; + assert_eq!(services.metrics.lookups_failed.get(), 1); + + // No services configured: also counted as failed. + let services = AddressLookupServices::default(); + let _results: Vec<_> = services.resolve(endpoint_id).collect().await; + assert_eq!(services.metrics.lookups.get(), 1); + assert_eq!(services.metrics.lookups_failed.get(), 1); + + Ok(()) + } + #[test] fn concurrent_address_lookup_addr_filter() { use iroh_base::RelayUrl; diff --git a/iroh/src/address_lookup/metrics.rs b/iroh/src/address_lookup/metrics.rs new file mode 100644 index 00000000000..ea199a8a00e --- /dev/null +++ b/iroh/src/address_lookup/metrics.rs @@ -0,0 +1,48 @@ +//! Metrics for address lookup. + +use iroh_metrics::{Counter, EncodeLabelSet, Family, MetricsGroup}; +use serde::{Deserialize, Serialize}; + +/// Labels identifying an address lookup service by its provenance string, +/// see [`crate::address_lookup::Item::provenance`]. +#[derive( + Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, EncodeLabelSet, +)] +pub struct ServiceLabels { + /// Provenance string of the service. + pub service: String, +} + +impl ServiceLabels { + /// Creates labels for the given service provenance. + pub fn new(service: impl Into) -> Self { + Self { + service: service.into(), + } + } +} + +/// Metrics collected by address lookup. +/// +/// A lookup is one call to [`AddressLookupServices::resolve`] and queries all +/// configured services at once. Each service can yield several results and +/// errors per lookup; those are counted in the `service_*` counters, labeled +/// by the service's provenance (e.g. `dns`, `pkarr`). +/// +/// [`AddressLookupServices::resolve`]: crate::address_lookup::AddressLookupServices::resolve +#[derive(Debug, Serialize, Deserialize, MetricsGroup)] +#[non_exhaustive] +#[metrics(name = "address_lookup", default)] +pub struct Metrics { + /// Lookups started. + pub lookups: Counter, + /// Lookups that ended without a single result. + /// + /// Includes lookups with no services configured. Lookups abandoned early + /// (e.g. once a connection is established) are not counted. + pub lookups_failed: Counter, + /// Results yielded per service. + pub service_results: Family, + /// Errors yielded per service. + pub service_errors: Family, +} diff --git a/iroh/src/metrics.rs b/iroh/src/metrics.rs index 3ed88648379..2b3872da93a 100644 --- a/iroh/src/metrics.rs +++ b/iroh/src/metrics.rs @@ -6,7 +6,10 @@ use iroh_metrics::MetricsGroupSet; pub use iroh_relay::server::Metrics as RelayMetrics; use serde::{Deserialize, Serialize}; -pub use crate::{net_report::Metrics as NetReportMetrics, socket::Metrics as SocketMetrics}; +pub use crate::{ + address_lookup::Metrics as AddressLookupMetrics, net_report::Metrics as NetReportMetrics, + socket::Metrics as SocketMetrics, +}; /// Metrics collected by an [`crate::endpoint::Endpoint`]. /// @@ -19,6 +22,8 @@ pub struct EndpointMetrics { pub socket: Arc, /// Metrics collected by net reports. pub net_report: Arc, + /// Metrics collected by address lookup. + pub address_lookup: Arc, } #[cfg(test)] @@ -26,12 +31,27 @@ mod tests { use super::EndpointMetrics; #[test] fn test_serde() { + use crate::address_lookup::ServiceLabels; + let metrics = EndpointMetrics::default(); metrics.socket.actor_link_change.inc(); metrics.net_report.reports.inc_by(10); + metrics + .address_lookup + .service_results + .get_or_create(&ServiceLabels::new("dns")) + .inc_by(3); let encoded = postcard::to_stdvec(&metrics).unwrap(); let decoded: EndpointMetrics = postcard::from_bytes(&encoded).unwrap(); assert_eq!(decoded.socket.actor_link_change.get(), 1); assert_eq!(decoded.net_report.reports.get(), 10); + assert_eq!( + decoded + .address_lookup + .service_results + .get(&ServiceLabels::new("dns")) + .map(|counter| counter.get()), + Some(3) + ); } } diff --git a/iroh/src/socket.rs b/iroh/src/socket.rs index 5c2f6ca8a3d..2b5711f7bda 100644 --- a/iroh/src/socket.rs +++ b/iroh/src/socket.rs @@ -889,7 +889,8 @@ impl EndpointInner { configured_addrs, } = opts; - let address_lookup = address_lookup::AddressLookupServices::default(); + let address_lookup = + address_lookup::AddressLookupServices::with_metrics(metrics.address_lookup.clone()); let port_mapper = portmapper::create_client(&portmapper_config); let relay_transport_configs: Vec<_> = transport_configs