From b2aa88da75c47302936e096b63950071dd4f7f84 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. --- Cargo.toml | 6 +- oximeter/instruments/src/kstat/link.rs | 2 +- oximeter/instruments/src/kstat/mod.rs | 1 - .../oximeter/schema/instance-data-link.toml | 90 +++++++++++++++++++ oximeter/types/src/traits.rs | 24 ++--- sled-agent/src/instance.rs | 19 +++- 6 files changed, 123 insertions(+), 19 deletions(-) create mode 100644 oximeter/oximeter/schema/instance-data-link.toml diff --git a/Cargo.toml b/Cargo.toml index cfb097ef3cf..55da57678d8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -778,9 +778,9 @@ 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/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" } diff --git a/oximeter/instruments/src/kstat/link.rs b/oximeter/instruments/src/kstat/link.rs index 0317b43839e..4d045131da1 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/instruments/src/kstat/mod.rs b/oximeter/instruments/src/kstat/mod.rs index a5020b9b615..b8e2e396ea0 100644 --- a/oximeter/instruments/src/kstat/mod.rs +++ b/oximeter/instruments/src/kstat/mod.rs @@ -201,7 +201,6 @@ pub trait KstatTarget: ) -> Result, Error>; } -/// Convert from a high-res timestamp into UTC, if possible. pub fn hrtime_to_utc(hrtime: i64) -> Result, Error> { let utc_now = Utc::now(); let hrtime_now = get_hires_time(); diff --git a/oximeter/oximeter/schema/instance-data-link.toml b/oximeter/oximeter/schema/instance-data-link.toml new file mode 100644 index 00000000000..c2a6e23e6f7 --- /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 91ecca817de..e3fbaf0a86e 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 0bcbc97fd28..9e4d58967ca 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) => {