[network metrics] instance network interface schema

This is related to the ongoing work in [plumbing instance/guest metrics](oxidecomputer/propolis#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.
oxidecomputer · Aug 28, 2024 · f3251d7 · f3251d7
1 parent c3c5f84
commit f3251d7
Show file tree

Hide file tree

Showing 8 changed files with 205 additions and 265 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -323,10 +323,10 @@ cookie = "0.18"
 criterion = { version = "0.5.1", features = [ "async_tokio" ] }
 crossbeam = "0.8"
 crossterm = { version = "0.28.1", features = ["event-stream"] }
-crucible-agent-client = { git = "https://github.com/oxidecomputer/crucible", rev = "e58ca3693cb9ce0438947beba10e97ee38a0966b" }
-crucible-pantry-client = { git = "https://github.com/oxidecomputer/crucible", rev = "e58ca3693cb9ce0438947beba10e97ee38a0966b" }
-crucible-smf = { git = "https://github.com/oxidecomputer/crucible", rev = "e58ca3693cb9ce0438947beba10e97ee38a0966b" }
-crucible-common = { git = "https://github.com/oxidecomputer/crucible", rev = "e58ca3693cb9ce0438947beba10e97ee38a0966b" }
+crucible-agent-client = { git = "https://github.com/oxidecomputer/crucible", rev = "88c45eebf34027c3bc80e60a5f527738bcfaf16a" }
+crucible-pantry-client = { git = "https://github.com/oxidecomputer/crucible", rev = "88c45eebf34027c3bc80e60a5f527738bcfaf16a" }
+crucible-smf = { git = "https://github.com/oxidecomputer/crucible", rev = "88c45eebf34027c3bc80e60a5f527738bcfaf16a" }
+crucible-common = { git = "https://github.com/oxidecomputer/crucible", rev = "88c45eebf34027c3bc80e60a5f527738bcfaf16a" }
 csv = "1.3.0"
 curve25519-dalek = "4"
 datatest-stable = "0.2.9"
@@ -501,8 +501,8 @@ proc-macro2 = "1.0"
 progenitor = { git = "https://github.com/oxidecomputer/progenitor", branch = "main" }
 progenitor-client = { git = "https://github.com/oxidecomputer/progenitor", branch = "main" }
 bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "24a74d0c76b6a63961ecef76acb1516b6e66c5c9" }
-propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "24a74d0c76b6a63961ecef76acb1516b6e66c5c9" }
-propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "24a74d0c76b6a63961ecef76acb1516b6e66c5c9" }
+propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "d3d01a178f7c92dafe5d91375320853dbc196af0" }
+propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "d3d01a178f7c92dafe5d91375320853dbc196af0" }
 proptest = "1.5.0"
 qorb = { git = "https://github.com/oxidecomputer/qorb", branch = "master" }
 quote = "1.0"
@@ -786,17 +786,19 @@ opt-level = 3
 #dropshot = { path = "../dropshot/dropshot" }
 #[patch.crates-io]
 #steno = { path = "../steno" }
-#[patch."https://github.com/oxidecomputer/propolis"]
-#propolis-client = { path = "../propolis/lib/propolis-client" }
-#propolis-mock-server = { path = "../propolis/bin/mock-server" }
-#[patch."https://github.com/oxidecomputer/crucible"]
-#crucible-agent-client = { path = "../crucible/agent-client" }
-#crucible-pantry-client = { path = "../crucible/pantry-client" }
-#crucible-smf = { path = "../crucible/smf" }
+# [patch."https://github.com/oxidecomputer/propolis"]
+# propolis-client = { path = "../propolis/lib/propolis-client" }
+# propolis-mock-server = { path = "../propolis/bin/mock-server" }
+# [patch."https://github.com/oxidecomputer/crucible"]
+# crucible-agent-client = { path = "../crucible/agent-client" }
+# crucible-pantry-client = { path = "../crucible/pantry-client" }
+# crucible-smf = { path = "../crucible/smf" }
 #[patch.crates-io]
 #diesel = { path = "../../diesel/diesel" }
 #[patch."https://github.com/oxidecomputer/diesel-dtrace"]
 #diesel-dtrace = { path = "../diesel-dtrace" }
+# [patch."https://github.com/oxidecomputer/falcon"]
+# libfalcon = { path = "../falcon/lib" }
 
 #
 # Local client generation during development.

diff --git 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/illumos-utils/src/opte/non_illumos.rs b/illumos-utils/src/opte/non_illumos.rs
@@ -26,6 +26,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),
 }
 
 pub fn initialize_xde_driver(

diff --git 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<T>(
     target: &T,
     named_data: &Named,

diff --git a/oximeter/oximeter/schema/instance-network-interface.toml b/oximeter/oximeter/schema/instance-network-interface.toml
@@ -0,0 +1,88 @@
+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 = [] }
+]
+
+[[metrics]]
+name = "errors_sent"
+description = "Number of errors encountered when sending on the network interface"
+units = "count"
+datum_type = "cumulative_u64"
+versions = [
+    { added_in = 1, fields = [] }
+]
+
+[[metrics]]
+name = "errors_received"
+description = "Number of errors encountered when receiving on 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
@@ -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
@@ -671,11 +671,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::<Result<Vec<_>, _>>()?;
 
         let migrate = match migrate {
             Some(params) => {