Skip to content

Commit

Permalink
[network metrics] Update how we track and initialize stats
Browse files Browse the repository at this point in the history
This currently brings in all non virtual_disk stats/initializer/kstat changes from
@bnaecker's #746 since that abstracts where
and how we do start tracking kstats.

Includes:
- Straighten out a bunch of the initialiazation logic for the pre-existing metrics, producer server, and registry. This grew pretty organically, and was getting
  unwieldy and confusing.
- Use virtual_machine:reset definition from TOML
- Move kstat sampler creation to machine initialization process, rather than buried in the producer server startup.
  This lets us use the sampler in-line when initializing the vCPUs, where it should be much easier to re-use when we want to add more kstat-based metrics,
  such as Viona link stats.
- Simplify and cleanup the ServerStats types, and remove the kstat sampler from it, which never should have been there at all.
- Bump kstat-rs dep, which includes the new version that is cross-platform. This gets rid of a bunch of cfg-soup in the stats modules.
  • Loading branch information
zeeshanlakhani committed Aug 26, 2024
1 parent 4b09a78 commit 1a57b7c
Show file tree
Hide file tree
Showing 8 changed files with 238 additions and 221 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
84 changes: 75 additions & 9 deletions bin/propolis-server/src/lib/initializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,19 @@ use std::os::unix::fs::FileTypeExt;
use std::sync::Arc;
use std::time::{SystemTime, UNIX_EPOCH};


use crate::serial::Serial;
use crate::stats::VirtualMachine;
use crate::vm::{BlockBackendMap, CrucibleBackendMap, DeviceMap, InterfaceIdentifiers};
use crate::stats::{
create_kstat_sampler, track_network_interface_kstats, track_vcpu_kstats,
VirtualMachine,
};
use crate::vm::{
BlockBackendMap, CrucibleBackendMap, DeviceMap, InterfaceIdentifiers,
};
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};
Expand Down Expand Up @@ -112,7 +117,6 @@ pub struct MachineInitializer<'a> {
pub(crate) log: slog::Logger,
pub(crate) machine: &'a Machine,
pub(crate) devices: DeviceMap,
pub(crate) interface_ids: InterfaceIdentifiers,
pub(crate) block_backends: BlockBackendMap,
pub(crate) crucible_backends: CrucibleBackendMap,
pub(crate) spec: &'a InstanceSpecV0,
Expand Down Expand Up @@ -583,10 +587,20 @@ impl<'a> MachineInitializer<'a> {
Ok(())
}

pub fn initialize_network_devices(
/// Initialize network devices, add them to the device map, and attach them
/// to the chipset.
///
/// If a KstatSampler is provided, this function will also track network
/// interface statistics.
pub async fn initialize_network_devices(
&mut self,
chipset: &RegisteredChipset,
kstat_sampler: &Option<KstatSampler>,
) -> Result<(), Error> {
// Only create the vector if the kstat_sampler is exists
let mut interface_ids: Option<InterfaceIdentifiers> =
kstat_sampler.as_ref().map(|_| Vec::new());

for (name, vnic_spec) in &self.spec.devices.network_devices {
info!(self.log, "Creating vNIC {}", name);
let instance_spec::v0::NetworkDeviceV0::VirtioNic(vnic_spec) =
Expand All @@ -606,7 +620,7 @@ impl<'a> MachineInitializer<'a> {
),
)
})?;
let bdf: pci::Bdf = vnic_spec.pci_path.try_into().map_err(|e| {
let bdf = vnic_spec.pci_path.try_into().map_err(|e| {
Error::new(
ErrorKind::InvalidInput,
format!("Couldn't get PCI BDF for vNIC {}: {}", name, e),
Expand Down Expand Up @@ -635,10 +649,26 @@ impl<'a> MachineInitializer<'a> {
)?;
self.devices
.insert(format!("pci-virtio-viona-{}", bdf), viona.clone());
self.interface_ids
.push((vnic_spec.interface_id, viona.instance_id()?));

// Only push to interface_ids if kstat_sampler is Some
if let Some(ref mut ids) = interface_ids {
ids.push((vnic_spec.interface_id, viona.instance_id()?));
}

chipset.pci_attach(bdf, viona);
}

kstat_sampler.as_ref().map(|sampler| {
interface_ids.map(|ids| {
track_network_interface_kstats(
&self.log,
sampler,
self.properties,
ids,
)
})
});

Ok(())
}

Expand Down Expand Up @@ -1045,13 +1075,49 @@ impl<'a> MachineInitializer<'a> {
Ok(ramfb)
}

pub fn initialize_cpus(&mut self) -> Result<(), Error> {
/// Initialize virtual CPUs by first setting their capabilities, inserting
/// them into the device map, and then, if a kstat sampler is provided,
/// tracking their kstats.
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.
///
/// This object is currently used to generate vCPU metrics, though guest NIC
/// metrics will be included soon.
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),
self.spec.devices.network_devices.len() as u32,
)?;
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
}
}
}
}
Loading

0 comments on commit 1a57b7c

Please sign in to comment.