From cf11a0b51bb7a57920b45e8827f1a4edad47ad09 Mon Sep 17 00:00:00 2001 From: Zeeshan Lakhani Date: Mon, 12 Aug 2024 19:50:54 +0000 Subject: [PATCH] [network metrics] instance data link 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 link 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. --- oximeter/instruments/src/kstat/link.rs | 2 +- .../oximeter/schema/instance-data-link.toml | 90 +++++++++++++++++++ oximeter/types/src/traits.rs | 24 ++--- sled-agent/src/instance.rs | 19 +++- 4 files changed, 120 insertions(+), 15 deletions(-) create mode 100644 oximeter/oximeter/schema/instance-data-link.toml 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-data-link.toml b/oximeter/oximeter/schema/instance-data-link.toml new file mode 100644 index 0000000000..c2a6e23e6f --- /dev/null +++ b/oximeter/oximeter/schema/instance-data-link.toml @@ -0,0 +1,90 @@ +format_version = 1 + +[target] +name = "instance_data_link" +description = "A network data link related to a guest/instance" +authz_scope = "fleet" +versions = [ + { version = 1, fields = [ "kind", "interface_id", "instance_id", "project_id", "silo_id", "sled_id", "sled_model", "sled_revision", "sled_serial" ] }, +] + +[[metrics]] +name = "bytes_sent" +description = "Total number of bytes sent on the data link" +units = "bytes" +datum_type = "cumulative_u64" +versions = [ + { added_in = 1, fields = [] } +] + +[[metrics]] +name = "bytes_received" +description = "Total number of bytes received on the data link" +units = "bytes" +datum_type = "cumulative_u64" +versions = [ + { added_in = 1, fields = [] } +] + +[[metrics]] +name = "packets_sent" +description = "Total number of packets sent on the data link" +units = "count" +datum_type = "cumulative_u64" +versions = [ + { added_in = 1, fields = [] } +] + +[[metrics]] +name = "packets_received" +description = "Total number of packets received on the data link" +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 data link" +units = "count" +datum_type = "cumulative_u64" +versions = [ + { added_in = 1, fields = [] } +] + +[fields.kind] +type = "string" +description = "The kind or class of the data link" + +[fields.interface_id] +type = "uuid" +description = "ID of the associated virtual network interface" + +[fields.instance_id] +type = "uuid" +description = "ID of the virtual machine instance" + +[fields.project_id] +type = "uuid" +description = "ID of the virtual machine instance's project" + +[fields.silo_id] +type = "uuid" +description = "ID of the virtual machine instance's silo" + +[fields.sled_id] +type = "uuid" +description = "ID of the sled managing the link's switch" + +[fields.sled_model] +type = "string" +description = "Model number of the sled managing the link's switch" + +[fields.sled_revision] +type = "u32" +description = "Revision number of the sled managing the link's switch" + +[fields.sled_serial] +type = "string" +description = "Serial number of the sled managing the link's switch" diff --git a/oximeter/types/src/traits.rs b/oximeter/types/src/traits.rs index 91ecca817d..e3fbaf0a86 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,12 @@ 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 as long as you +/// define the target as one of the field(s) in the struct. 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..9e4d58967c 100644 --- a/sled-agent/src/instance.rs +++ b/sled-agent/src/instance.rs @@ -676,11 +676,22 @@ 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()), + .filter_map(|port| { + let interface_id = self + .requested_nics + .iter() + // We expect to match NIC slots to OPTE port slots. + .find(|nic| nic.slot == port.slot()) + .map(|nic| nic.id); + interface_id.map(|id| { + propolis_client::types::NetworkInterfaceRequest { + interface_id: id, + name: port.name().to_string(), + slot: propolis_client::types::Slot(port.slot()), + } + }) }) - .collect(); + .collect::>(); let migrate = match migrate { Some(params) => {