From 90dd02885ba7d1c6f0ee2a88a1f73e5e07c00d14 Mon Sep 17 00:00:00 2001 From: Zeeshan Lakhani Date: Mon, 12 Aug 2024 19:50:54 +0000 Subject: [PATCH] [network metrics] instance network interface schema This is related to the ongoing work in [plumbing instance/guest metrics](https://github.com/oxidecomputer/propolis/issues/742) through to an oximeter producer in propolis. Includes: - Rewriting the comment for `trait Target` in Oximeter to not being prepped for deprecation, as it does come in handy for custom needs - The main differentiating field among instance network information here is `interface_id`, which is exposed to propolis via the client's `NetworkInterfaceRequest`. We currently match the `nic.slot` to the `port.slot` to pass which of the requested nics `interface_id` uuids are part of the request. --- illumos-utils/src/opte/illumos.rs | 3 + oximeter/instruments/src/kstat/link.rs | 2 +- .../schema/instance-network-interface.toml | 70 +++++++++++++++++++ oximeter/types/src/traits.rs | 23 +++--- sled-agent/src/instance.rs | 25 +++++-- 5 files changed, 108 insertions(+), 15 deletions(-) create mode 100644 oximeter/oximeter/schema/instance-network-interface.toml diff --git a/illumos-utils/src/opte/illumos.rs b/illumos-utils/src/opte/illumos.rs index 90bf0bb16a..a0f278ade0 100644 --- a/illumos-utils/src/opte/illumos.rs +++ b/illumos-utils/src/opte/illumos.rs @@ -56,6 +56,9 @@ pub enum Error { #[error("Can't attach new ephemeral IP {0}, currently have {1}")] ImplicitEphemeralIpDetach(IpAddr, IpAddr), + + #[error("No matching NIC found for port {0} at slot {1}.")] + NoNicforPort(String, u32), } /// Delete all xde devices on the system. diff --git a/oximeter/instruments/src/kstat/link.rs b/oximeter/instruments/src/kstat/link.rs index 0317b43839..4d045131da 100644 --- a/oximeter/instruments/src/kstat/link.rs +++ b/oximeter/instruments/src/kstat/link.rs @@ -19,7 +19,7 @@ use oximeter::Sample; oximeter::use_timeseries!("sled-data-link.toml"); -// Helper function to extract the same kstat metrics from all link targets. +/// Helper function to extract the same kstat metrics from all link targets. fn extract_link_kstats( target: &T, named_data: &Named, diff --git a/oximeter/oximeter/schema/instance-network-interface.toml b/oximeter/oximeter/schema/instance-network-interface.toml new file mode 100644 index 0000000000..5e26d526c9 --- /dev/null +++ b/oximeter/oximeter/schema/instance-network-interface.toml @@ -0,0 +1,70 @@ +format_version = 1 + +[target] +name = "instance_network_interface" +description = "A network interface attached to a virtual machine instance" +authz_scope = "project" +versions = [ + { version = 1, fields = [ "interface_id", "instance_id", "project_id", "silo_id" ] }, +] + +[fields.interface_id] +type = "uuid" +description = "The ID of the network interface" + +[fields.instance_id] +type = "uuid" +description = "The ID of the virtual machine instance this network interface is attached to" + +[fields.project_id] +type = "uuid" +description = "The ID of the project containing the virtual machine instance this network interface is attached to" + +[fields.silo_id] +type = "uuid" +description = "The ID of the silo containing the virtual machine instance this network interface is attached to" + +[[metrics]] +name = "bytes_sent" +description = "Total number of bytes sent on the network interface" +units = "bytes" +datum_type = "cumulative_u64" +versions = [ + { added_in = 1, fields = [] } +] + +[[metrics]] +name = "bytes_received" +description = "Total number of bytes received on the network interface" +units = "bytes" +datum_type = "cumulative_u64" +versions = [ + { added_in = 1, fields = [] } +] + +[[metrics]] +name = "packets_sent" +description = "Total number of packets sent on the network interface" +units = "count" +datum_type = "cumulative_u64" +versions = [ + { added_in = 1, fields = [] } +] + +[[metrics]] +name = "packets_received" +description = "Total number of packets received on the network interface" +units = "count" +datum_type = "cumulative_u64" +versions = [ + { added_in = 1, fields = [] } +] + +[[metrics]] +name = "packets_dropped" +description = "Number of packets dropped on the RX queue of the network interface" +units = "count" +datum_type = "cumulative_u64" +versions = [ + { added_in = 1, fields = [] } +] diff --git a/oximeter/types/src/traits.rs b/oximeter/types/src/traits.rs index 91ecca817d..38088acdb3 100644 --- a/oximeter/types/src/traits.rs +++ b/oximeter/types/src/traits.rs @@ -23,12 +23,14 @@ use std::num::NonZeroU8; use std::ops::Add; use std::ops::AddAssign; -/// The `Target` trait identifies a source of metric data by a sequence of fields. -/// -/// A target is a single source of metric data, identified by a sequence of named and typed field -/// values. Users can write a single struct definition and derive this trait. The methods here -/// provide some introspection into the struct, listing its fields and their values. The struct -/// definition can be thought of as a schema, and an instance of that struct as identifying an +/// The `Target` trait identifies a source of metric data by a sequence of +/// fields. +/// +/// A target is a single source of metric data, identified by a sequence of +/// named and typed field values. Users can write a single struct definition and +/// derive this trait. The methods here provide some introspection into the +/// struct, listing its fields and their values. The struct definition can be +/// thought of as a schema, and an instance of that struct as identifying an /// individual target. /// /// Target fields may have one of a set of supported types: @@ -85,10 +87,11 @@ use std::ops::AddAssign; /// } /// ``` /// -/// **Important:** Deriving this trait is deprecated, and will be removed in the -/// future. Instead, define your timeseries schema through the TOML format -/// described in [the crate documentation](crate), and use the code -/// generated by the `use_timeseries` macro. +/// **Important:** Typically, you should define your timeseries schema through +/// the TOML format described in [the crate documentation](crate), and use the +/// code generated by the `use_timeseries` macro. However, if you need to handle +/// custom sampling logic and must define custom field definitions not published +/// as part of the target schema, you can implement this trait. pub trait Target { /// Return the name of the target, which is the snake_case form of the struct's name. fn name(&self) -> &'static str; diff --git a/sled-agent/src/instance.rs b/sled-agent/src/instance.rs index 0bcbc97fd2..f7ce2e83af 100644 --- a/sled-agent/src/instance.rs +++ b/sled-agent/src/instance.rs @@ -676,11 +676,28 @@ impl InstanceRunner { ) -> Result<(), Error> { let nics = running_zone .opte_ports() - .map(|port| propolis_client::types::NetworkInterfaceRequest { - name: port.name().to_string(), - slot: propolis_client::types::Slot(port.slot()), + .map(|port| { + self.requested_nics + .iter() + // We expect to match NIC slots to OPTE port slots. + // Error out if we can't find a NIC for a port. + .position(|nic| nic.slot == port.slot()) + .ok_or(Error::Opte( + illumos_utils::opte::Error::NoNicforPort( + port.name().into(), + port.slot().into(), + ), + )) + .map(|pos| { + let nic = &self.requested_nics[pos]; + propolis_client::types::NetworkInterfaceRequest { + interface_id: nic.id, + name: port.name().to_string(), + slot: propolis_client::types::Slot(port.slot()), + } + }) }) - .collect(); + .collect::, _>>()?; let migrate = match migrate { Some(params) => {