Publish richer virtual disk statistics to oximeter

Cargo.toml

@@ -71,18 +71,18 @@ p9ds = { git = "https://github.com/oxidecomputer/p9fs" }
 softnpu = { git = "https://github.com/oxidecomputer/softnpu" }
 
 # Omicron-related
-internal-dns = { git = "https://github.com/oxidecomputer/omicron", branch = "main" }
-nexus-client = { git = "https://github.com/oxidecomputer/omicron", branch = "main" }
-omicron-common = { git = "https://github.com/oxidecomputer/omicron", branch = "main" }
+internal-dns = { git = "https://github.com/oxidecomputer/omicron", branch = "new-virtual-disk-timeseries-definition" }
+nexus-client = { git = "https://github.com/oxidecomputer/omicron", branch = "new-virtual-disk-timeseries-definition" }
+omicron-common = { git = "https://github.com/oxidecomputer/omicron", branch = "new-virtual-disk-timeseries-definition" }
 omicron-zone-package = "0.9.0"
-oximeter-instruments = { git = "https://github.com/oxidecomputer/omicron", branch = "main", default-features = false, features = ["kstat"] }
-oximeter-producer = { git = "https://github.com/oxidecomputer/omicron", branch = "main" }
-oximeter = { git = "https://github.com/oxidecomputer/omicron", branch = "main" }
-sled-agent-client = { git = "https://github.com/oxidecomputer/omicron", branch = "main" }
+oximeter-instruments = { git = "https://github.com/oxidecomputer/omicron", branch = "new-virtual-disk-timeseries-definition", default-features = false, features = ["kstat"] }
+oximeter-producer = { git = "https://github.com/oxidecomputer/omicron", branch = "new-virtual-disk-timeseries-definition" }
+oximeter = { git = "https://github.com/oxidecomputer/omicron", branch = "new-virtual-disk-timeseries-definition" }
+sled-agent-client = { git = "https://github.com/oxidecomputer/omicron", branch = "new-virtual-disk-timeseries-definition" }
 
 # Crucible
-crucible = { git = "https://github.com/oxidecomputer/crucible", rev = "e58ca3693cb9ce0438947beba10e97ee38a0966b" }
-crucible-client-types = { git = "https://github.com/oxidecomputer/crucible", rev = "e58ca3693cb9ce0438947beba10e97ee38a0966b" }
+crucible = { git = "https://github.com/oxidecomputer/crucible", rev = "db11baa532bf6fad73f22db0d6f2c914ab67b2fa" }
+crucible-client-types = { git = "https://github.com/oxidecomputer/crucible", rev = "db11baa532bf6fad73f22db0d6f2c914ab67b2fa" }
 
 # External dependencies
 anyhow = "1.0"
@@ -120,7 +120,7 @@ http = "0.2.9"
 hyper = "0.14"
 indicatif = "0.17.3"
 inventory = "0.3.0"
-kstat-rs = "0.2.3"
+kstat-rs = "0.2.4"
 lazy_static = "1.4"
 libc = "0.2"
 mockall = "0.12"

bin/propolis-server/src/lib/initializer.rs

@@ -11,13 +11,17 @@ use std::sync::Arc;
 use std::time::{SystemTime, UNIX_EPOCH};
 
 use crate::serial::Serial;
-use crate::stats::virtual_machine::VirtualMachine;
+use crate::stats::{
+    create_kstat_sampler, track_vcpu_kstats, virtual_disk::VirtualDiskProducer,
+    virtual_machine::VirtualMachine,
+};
 use crate::vm::{BlockBackendMap, CrucibleBackendMap, DeviceMap};
 use anyhow::{Context, Result};
 use crucible_client_types::VolumeConstructionRequest;
 pub use nexus_client::Client as NexusClient;
 use oximeter::types::ProducerRegistry;
 
+use oximeter_instruments::kstat::KstatSampler;
 use propolis::block;
 use propolis::chardev::{self, BlockingSource, Source};
 use propolis::common::{Lifecycle, GB, MB, PAGE_SIZE};
@@ -491,7 +495,8 @@ impl<'a> MachineInitializer<'a> {
             Nvme,
         }
 
-        for (name, device_spec) in &self.spec.devices.storage_devices {
+        'devloop: for (name, device_spec) in &self.spec.devices.storage_devices
+        {
             info!(
                 self.log,
                 "Creating storage device {} with properties {:?}",
@@ -542,14 +547,15 @@ impl<'a> MachineInitializer<'a> {
                 .await?;
 
             self.block_backends.insert(backend_name.clone(), backend.clone());
-            match device_interface {
+            let block_dev: Arc<dyn block::Device> = match device_interface {
                 DeviceInterface::Virtio => {
                     let vioblk = virtio::PciVirtioBlock::new(0x100);
 
                     self.devices
                         .insert(format!("pci-virtio-{}", bdf), vioblk.clone());
                     block::attach(vioblk.clone(), backend).unwrap();
-                    chipset.pci_attach(bdf, vioblk);
+                    chipset.pci_attach(bdf, vioblk.clone());
+                    vioblk
                 }
                 DeviceInterface::Nvme => {
                     // Limit data transfers to 1MiB (2^8 * 4k) in size
@@ -564,18 +570,59 @@ impl<'a> MachineInitializer<'a> {
                     self.devices
                         .insert(format!("pci-nvme-{bdf}"), nvme.clone());
                     block::attach(nvme.clone(), backend).unwrap();
-                    chipset.pci_attach(bdf, nvme);
+                    chipset.pci_attach(bdf, nvme.clone());
+                    nvme
                 }
             };
 
-            if let Some((id, backend)) = crucible {
-                let prev = self.crucible_backends.insert(id, backend);
+            if let Some((disk_id, backend)) = crucible {
+                let block_size = backend.block_size().await;
+                let prev = self.crucible_backends.insert(disk_id, backend);
                 if prev.is_some() {
                     return Err(Error::new(
                         ErrorKind::InvalidInput,
-                        format!("multiple disks with id {}", id),
+                        format!("multiple disks with id {}", disk_id),
                     ));
                 }
+
+                let Some(block_size) = block_size else {
+                    slog::error!(
+                        self.log,
+                        "Could not get Crucible backend block size, \
+                        virtual disk metrics can't be reported for it";
+                        "disk_id" => %disk_id,
+                    );
+                    continue 'devloop;
+                };
+
+                // Register the block device as a metric producer, if we've been
+                // setup to do so. Note we currently only do this for the Crucible
+                // backend, in which case we have the disk ID.
+                if let Some(registry) = &self.producer_registry {
+                    let stats = VirtualDiskProducer::new(
+                        block_size,
+                        self.properties.id,
+                        disk_id,
+                        &self.properties.metadata,
+                    );
+                    if let Err(e) = registry.register_producer(stats.clone()) {
+                        slog::error!(
+                            self.log,
+                            "Could not register virtual disk producer, \
+                            metrics will not be produced";
+                            "disk_id" => %disk_id,
+                            "error" => ?e,
+                        );
+                        continue 'devloop;
+                    };
+
+                    // Set the on-completion callback for the block device, to
+                    // update stats.
+                    let callback = move |op, result, duration| {
+                        stats.on_completion(op, result, duration);
+                    };
+                    block_dev.on_completion(Box::new(callback));
+                };
             }
         }
         Ok(())
@@ -1041,13 +1088,40 @@ impl<'a> MachineInitializer<'a> {
         Ok(ramfb)
     }
 
-    pub fn initialize_cpus(&mut self) -> Result<(), Error> {
+    pub async fn initialize_cpus(
+        &mut self,
+        kstat_sampler: &Option<KstatSampler>,
+    ) -> Result<(), Error> {
         for vcpu in self.machine.vcpus.iter() {
             vcpu.set_default_capabs().unwrap();
 
             // The vCPUs behave like devices, so add them to the list as well
             self.devices.insert(format!("vcpu-{}", vcpu.id), vcpu.clone());
         }
+        if let Some(sampler) = kstat_sampler.as_ref() {
+            track_vcpu_kstats(&self.log, sampler, self.properties).await;
+        }
         Ok(())
     }
+
+    /// Create an object used to sample kstats.
+    pub(crate) fn create_kstat_sampler(&self) -> Option<KstatSampler> {
+        let Some(registry) = &self.producer_registry else {
+            return None;
+        };
+        let sampler =
+            create_kstat_sampler(&self.log, u32::from(self.properties.vcpus))?;
+        match registry.register_producer(sampler.clone()) {
+            Ok(_) => Some(sampler),
+            Err(e) => {
+                slog::error!(
+                    self.log,
+                    "Failed to register kstat sampler in producer \
+                    registry, no kstat-based metrics will be produced";
+                    "error" => ?e,
+                );
+                None
+            }
+        }
+    }
 }