diff --git a/src/vmm/src/arch/aarch64/vcpu.rs b/src/vmm/src/arch/aarch64/vcpu.rs index 39020b6fbb6..fe2e8953cd8 100644 --- a/src/vmm/src/arch/aarch64/vcpu.rs +++ b/src/vmm/src/arch/aarch64/vcpu.rs @@ -21,7 +21,8 @@ use crate::arch::aarch64::kvm::OptionalCapabilities; use crate::arch::aarch64::regs::{Aarch64RegisterVec, KVM_REG_ARM64_SVE_VLS}; use crate::cpu_config::aarch64::custom_cpu_template::VcpuFeatures; use crate::cpu_config::templates::CpuConfiguration; -use crate::logger::{IncMetric, METRICS, error}; +use crate::logger::{METRICS, error}; +use crate::metrics::IncMetric; use crate::vcpu::{VcpuConfig, VcpuError}; use crate::vstate::bus::Bus; use crate::vstate::memory::{Address, GuestMemoryMmap}; diff --git a/src/vmm/src/arch/x86_64/vcpu.rs b/src/vmm/src/arch/x86_64/vcpu.rs index c2c5352d5e8..2caa4098c7e 100644 --- a/src/vmm/src/arch/x86_64/vcpu.rs +++ b/src/vmm/src/arch/x86_64/vcpu.rs @@ -24,7 +24,8 @@ use crate::arch::x86_64::interrupts; use crate::arch::x86_64::msr::{MsrError, create_boot_msr_entries}; use crate::arch::x86_64::regs::{SetupFpuError, SetupRegistersError, SetupSpecialRegistersError}; use crate::cpu_config::x86_64::{CpuConfiguration, cpuid}; -use crate::logger::{IncMetric, METRICS}; +use crate::logger::METRICS; +use crate::metrics::IncMetric; use crate::vstate::bus::Bus; use crate::vstate::memory::GuestMemoryMmap; use crate::vstate::vcpu::{VcpuConfig, VcpuEmulation, VcpuError}; diff --git a/src/vmm/src/devices/legacy/i8042.rs b/src/vmm/src/devices/legacy/i8042.rs index 664002637e6..785e0027142 100644 --- a/src/vmm/src/devices/legacy/i8042.rs +++ b/src/vmm/src/devices/legacy/i8042.rs @@ -13,7 +13,8 @@ use log::warn; use serde::Serialize; use vmm_sys_util::eventfd::EventFd; -use crate::logger::{IncMetric, SharedIncMetric, error}; +use crate::logger::error; +use crate::metrics::{IncMetric, SharedIncMetric}; use crate::vstate::bus::BusDevice; /// Errors thrown by the i8042 device. diff --git a/src/vmm/src/devices/legacy/rtc_pl031.rs b/src/vmm/src/devices/legacy/rtc_pl031.rs index 96b1134eb1c..617727115da 100644 --- a/src/vmm/src/devices/legacy/rtc_pl031.rs +++ b/src/vmm/src/devices/legacy/rtc_pl031.rs @@ -7,7 +7,8 @@ use serde::Serialize; use vm_superio::Rtc; use vm_superio::rtc_pl031::RtcEvents; -use crate::logger::{IncMetric, SharedIncMetric, warn}; +use crate::logger::warn; +use crate::metrics::{IncMetric, SharedIncMetric}; /// Metrics specific to the RTC device. #[derive(Debug, Serialize, Default)] diff --git a/src/vmm/src/devices/legacy/serial.rs b/src/vmm/src/devices/legacy/serial.rs index 6b5c29aabf4..8693773c64e 100644 --- a/src/vmm/src/devices/legacy/serial.rs +++ b/src/vmm/src/devices/legacy/serial.rs @@ -22,7 +22,7 @@ use vmm_sys_util::epoll::EventSet; use vmm_sys_util::eventfd::EventFd; use crate::devices::legacy::EventFdTrigger; -use crate::logger::{IncMetric, SharedIncMetric}; +use crate::metrics::{IncMetric, SharedIncMetric}; use crate::vstate::bus::BusDevice; /// Received Data Available interrupt - for letting the driver know that diff --git a/src/vmm/src/devices/virtio/balloon/metrics.rs b/src/vmm/src/devices/virtio/balloon/metrics.rs index 0b438cae2d4..03a1e7eb8ae 100644 --- a/src/vmm/src/devices/virtio/balloon/metrics.rs +++ b/src/vmm/src/devices/virtio/balloon/metrics.rs @@ -36,7 +36,7 @@ use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; -use crate::logger::SharedIncMetric; +use crate::metrics::SharedIncMetric; /// Stores aggregated balloon metrics pub(super) static METRICS: BalloonDeviceMetrics = BalloonDeviceMetrics::new(); diff --git a/src/vmm/src/devices/virtio/block/vhost_user/device.rs b/src/vmm/src/devices/virtio/block/vhost_user/device.rs index dd08b8de7c8..ceac146a960 100644 --- a/src/vmm/src/devices/virtio/block/vhost_user/device.rs +++ b/src/vmm/src/devices/virtio/block/vhost_user/device.rs @@ -28,7 +28,8 @@ use crate::devices::virtio::vhost_user_metrics::{ VhostUserDeviceMetrics, VhostUserMetricsPerDevice, }; use crate::impl_device_type; -use crate::logger::{IncMetric, StoreMetric, log_dev_preview_warning}; +use crate::logger::log_dev_preview_warning; +use crate::metrics::{IncMetric, StoreMetric}; use crate::utils::u64_to_usize; use crate::vmm_config::drive::BlockDeviceConfig; use crate::vstate::memory::GuestMemoryMmap; diff --git a/src/vmm/src/devices/virtio/block/virtio/device.rs b/src/vmm/src/devices/virtio/block/virtio/device.rs index ecdd8ee4f6d..a04adffad6f 100644 --- a/src/vmm/src/devices/virtio/block/virtio/device.rs +++ b/src/vmm/src/devices/virtio/block/virtio/device.rs @@ -35,7 +35,8 @@ use crate::devices::virtio::generated::virtio_ring::VIRTIO_RING_F_EVENT_IDX; use crate::devices::virtio::queue::{InvalidAvailIdx, Queue}; use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType}; use crate::impl_device_type; -use crate::logger::{IncMetric, error, warn}; +use crate::logger::{error, warn}; +use crate::metrics::IncMetric; use crate::rate_limiter::{BucketUpdate, RateLimiter}; use crate::utils::u64_to_usize; use crate::vmm_config::RateLimiterConfig; diff --git a/src/vmm/src/devices/virtio/block/virtio/metrics.rs b/src/vmm/src/devices/virtio/block/virtio/metrics.rs index 0abe2a58963..17907b68890 100644 --- a/src/vmm/src/devices/virtio/block/virtio/metrics.rs +++ b/src/vmm/src/devices/virtio/block/virtio/metrics.rs @@ -84,7 +84,7 @@ use std::sync::{Arc, RwLock}; use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; -use crate::logger::{IncMetric, LatencyAggregateMetrics, SharedIncMetric}; +use crate::metrics::{IncMetric, LatencyAggregateMetrics, SharedIncMetric}; /// map of block drive id and metrics /// this should be protected by a lock before accessing. diff --git a/src/vmm/src/devices/virtio/block/virtio/request.rs b/src/vmm/src/devices/virtio/block/virtio/request.rs index 8fc83cf43da..6bf7a87b90f 100644 --- a/src/vmm/src/devices/virtio/block/virtio/request.rs +++ b/src/vmm/src/devices/virtio/block/virtio/request.rs @@ -17,7 +17,8 @@ pub use crate::devices::virtio::generated::virtio_blk::{ VIRTIO_BLK_T_FLUSH, VIRTIO_BLK_T_GET_ID, VIRTIO_BLK_T_IN, VIRTIO_BLK_T_OUT, }; use crate::devices::virtio::queue::DescriptorChain; -use crate::logger::{IncMetric, error}; +use crate::logger::error; +use crate::metrics::IncMetric; use crate::rate_limiter::{RateLimiter, TokenType}; use crate::vstate::memory::{ByteValued, Bytes, GuestAddress, GuestMemoryMmap}; diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs index 4ce89c342c7..4737195fb04 100755 --- a/src/vmm/src/devices/virtio/net/device.rs +++ b/src/vmm/src/devices/virtio/net/device.rs @@ -41,7 +41,8 @@ use crate::devices::{DeviceError, report_net_event_fail}; use crate::dumbo::pdu::arp::ETH_IPV4_FRAME_LEN; use crate::dumbo::pdu::ethernet::{EthernetFrame, PAYLOAD_OFFSET}; use crate::impl_device_type; -use crate::logger::{IncMetric, METRICS}; +use crate::logger::METRICS; +use crate::metrics::IncMetric; use crate::mmds::data_store::Mmds; use crate::mmds::ns::MmdsNetworkStack; use crate::rate_limiter::{BucketUpdate, RateLimiter, TokenType}; diff --git a/src/vmm/src/devices/virtio/net/event_handler.rs b/src/vmm/src/devices/virtio/net/event_handler.rs index 9d8c09a45f2..c286078146a 100644 --- a/src/vmm/src/devices/virtio/net/event_handler.rs +++ b/src/vmm/src/devices/virtio/net/event_handler.rs @@ -7,7 +7,8 @@ use vmm_sys_util::epoll::EventSet; use crate::devices::virtio::device::VirtioDevice; use crate::devices::virtio::net::device::Net; use crate::devices::virtio::net::{RX_INDEX, TX_INDEX}; -use crate::logger::{IncMetric, error, warn}; +use crate::logger::{error, warn}; +use crate::metrics::IncMetric; impl Net { const PROCESS_ACTIVATE: u32 = 0; diff --git a/src/vmm/src/devices/virtio/net/metrics.rs b/src/vmm/src/devices/virtio/net/metrics.rs index a2d18c8412b..52318159e7b 100644 --- a/src/vmm/src/devices/virtio/net/metrics.rs +++ b/src/vmm/src/devices/virtio/net/metrics.rs @@ -86,7 +86,7 @@ use std::sync::{Arc, RwLock}; use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; -use crate::logger::{IncMetric, LatencyAggregateMetrics, SharedIncMetric}; +use crate::metrics::{IncMetric, LatencyAggregateMetrics, SharedIncMetric}; /// map of network interface id and metrics /// this should be protected by a lock before accessing. diff --git a/src/vmm/src/devices/virtio/pmem/device.rs b/src/vmm/src/devices/virtio/pmem/device.rs index d128225aed8..ef2b793217c 100644 --- a/src/vmm/src/devices/virtio/pmem/device.rs +++ b/src/vmm/src/devices/virtio/pmem/device.rs @@ -22,7 +22,8 @@ use crate::devices::virtio::pmem::PMEM_QUEUE_SIZE; use crate::devices::virtio::pmem::metrics::{PmemMetrics, PmemMetricsPerDevice}; use crate::devices::virtio::queue::{DescriptorChain, InvalidAvailIdx, Queue, QueueError}; use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType}; -use crate::logger::{IncMetric, error}; +use crate::logger::error; +use crate::metrics::IncMetric; use crate::utils::{align_up, u64_to_usize}; use crate::vmm_config::pmem::PmemConfig; use crate::vstate::memory::{ByteValued, Bytes, GuestMemoryMmap, GuestMmapRegion}; diff --git a/src/vmm/src/devices/virtio/pmem/metrics.rs b/src/vmm/src/devices/virtio/pmem/metrics.rs index 02348b4ca51..e3b8e49e832 100644 --- a/src/vmm/src/devices/virtio/pmem/metrics.rs +++ b/src/vmm/src/devices/virtio/pmem/metrics.rs @@ -84,7 +84,7 @@ use std::sync::{Arc, RwLock}; use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; -use crate::logger::{IncMetric, LatencyAggregateMetrics, SharedIncMetric}; +use crate::metrics::{IncMetric, LatencyAggregateMetrics, SharedIncMetric}; /// map of pmem drive id and metrics /// this should be protected by a lock before accessing. diff --git a/src/vmm/src/devices/virtio/rng/device.rs b/src/vmm/src/devices/virtio/rng/device.rs index 1f2ce079aed..98854203048 100644 --- a/src/vmm/src/devices/virtio/rng/device.rs +++ b/src/vmm/src/devices/virtio/rng/device.rs @@ -22,7 +22,8 @@ use crate::devices::virtio::iovec::IoVecBufferMut; use crate::devices::virtio::queue::{FIRECRACKER_MAX_QUEUE_SIZE, InvalidAvailIdx, Queue}; use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType}; use crate::impl_device_type; -use crate::logger::{IncMetric, debug, error}; +use crate::logger::{debug, error}; +use crate::metrics::IncMetric; use crate::rate_limiter::{RateLimiter, TokenType}; use crate::vstate::memory::GuestMemoryMmap; diff --git a/src/vmm/src/devices/virtio/rng/metrics.rs b/src/vmm/src/devices/virtio/rng/metrics.rs index e02f5ce8cc4..76b181b8d5f 100644 --- a/src/vmm/src/devices/virtio/rng/metrics.rs +++ b/src/vmm/src/devices/virtio/rng/metrics.rs @@ -36,7 +36,7 @@ use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; -use crate::logger::SharedIncMetric; +use crate::metrics::SharedIncMetric; /// Stores aggregated entropy metrics pub(super) static METRICS: EntropyDeviceMetrics = EntropyDeviceMetrics::new(); diff --git a/src/vmm/src/devices/virtio/transport/mmio.rs b/src/vmm/src/devices/virtio/transport/mmio.rs index f5039281f16..4b86f76e761 100644 --- a/src/vmm/src/devices/virtio/transport/mmio.rs +++ b/src/vmm/src/devices/virtio/transport/mmio.rs @@ -15,7 +15,8 @@ use super::{VirtioInterrupt, VirtioInterruptType}; use crate::devices::virtio::device::VirtioDevice; use crate::devices::virtio::device_status; use crate::devices::virtio::queue::Queue; -use crate::logger::{IncMetric, METRICS, error, warn}; +use crate::logger::{METRICS, error, warn}; +use crate::metrics::IncMetric; use crate::utils::byte_order; use crate::vstate::bus::BusDevice; use crate::vstate::interrupts::InterruptError; diff --git a/src/vmm/src/devices/virtio/vhost_user_metrics.rs b/src/vmm/src/devices/virtio/vhost_user_metrics.rs index bb7f03b30a6..83d535e569e 100644 --- a/src/vmm/src/devices/virtio/vhost_user_metrics.rs +++ b/src/vmm/src/devices/virtio/vhost_user_metrics.rs @@ -79,7 +79,7 @@ use std::sync::{Arc, RwLock}; use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; -use crate::logger::{SharedIncMetric, SharedStoreMetric}; +use crate::metrics::{SharedIncMetric, SharedStoreMetric}; /// map of vhost_user drive id and metrics /// this should be protected by a lock before accessing. diff --git a/src/vmm/src/devices/virtio/vsock/metrics.rs b/src/vmm/src/devices/virtio/vsock/metrics.rs index d626d5dca34..b14fc2228be 100644 --- a/src/vmm/src/devices/virtio/vsock/metrics.rs +++ b/src/vmm/src/devices/virtio/vsock/metrics.rs @@ -39,7 +39,7 @@ use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; -use crate::logger::SharedIncMetric; +use crate::metrics::SharedIncMetric; /// Stores aggregate metrics of all Vsock connections/actions pub(super) static METRICS: VsockDeviceMetrics = VsockDeviceMetrics::new(); diff --git a/src/vmm/src/dumbo/tcp/endpoint.rs b/src/vmm/src/dumbo/tcp/endpoint.rs index 2fbb8466564..db4a958dddd 100644 --- a/src/vmm/src/dumbo/tcp/endpoint.rs +++ b/src/vmm/src/dumbo/tcp/endpoint.rs @@ -22,7 +22,8 @@ use crate::dumbo::pdu::bytes::NetworkBytes; use crate::dumbo::pdu::tcp::TcpSegment; use crate::dumbo::tcp::connection::{Connection, PassiveOpenError, RecvStatusFlags}; use crate::dumbo::tcp::{MAX_WINDOW_SIZE, NextSegmentStatus, seq_after}; -use crate::logger::{IncMetric, METRICS}; +use crate::logger::METRICS; +use crate::metrics::IncMetric; // TODO: These are currently expressed in cycles. Normally, they would be the equivalent of a // certain duration, depending on the frequency of the CPU, but we still have a bit to go until diff --git a/src/vmm/src/lib.rs b/src/vmm/src/lib.rs index 7186b904b8a..04b70efd4ad 100644 --- a/src/vmm/src/lib.rs +++ b/src/vmm/src/lib.rs @@ -88,6 +88,8 @@ pub mod dumbo; pub mod gdb; /// Logger pub mod logger; +/// Component-defined metrics traits and implementations +pub mod metrics; /// microVM Metadata Service MMDS pub mod mmds; /// PCI specific emulation code. diff --git a/src/vmm/src/logger/logging.rs b/src/vmm/src/logger/logging.rs index 8afdf976ffb..d0a173c5e8b 100644 --- a/src/vmm/src/logger/logging.rs +++ b/src/vmm/src/logger/logging.rs @@ -12,7 +12,8 @@ use log::{Log, Metadata, Record}; use serde::{Deserialize, Deserializer, Serialize}; use utils::time::LocalTime; -use super::metrics::{IncMetric, METRICS}; +use super::metrics::METRICS; +use crate::metrics::IncMetric; use crate::utils::open_file_write_nonblock; /// Default level filter for logger matching the swagger specification diff --git a/src/vmm/src/logger/metrics.rs b/src/vmm/src/logger/metrics.rs index 527ba911461..6733c8b969a 100644 --- a/src/vmm/src/logger/metrics.rs +++ b/src/vmm/src/logger/metrics.rs @@ -64,12 +64,13 @@ use std::fmt::Debug; use std::io::Write; use std::ops::Deref; -use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Mutex, OnceLock}; use serde::{Serialize, Serializer}; use utils::time::{ClockType, get_time_ns, get_time_us}; +// Import metric traits and implementations from the metrics module +use crate::metrics::{LatencyAggregateMetrics, SharedIncMetric, SharedStoreMetric, StoreMetric}; use super::FcLineWriter; use crate::devices::legacy; use crate::devices::virtio::balloon::metrics as balloon_metrics; @@ -176,110 +177,6 @@ pub enum MetricsError { Write(std::io::Error), } -/// Used for defining new types of metrics that act as a counter (i.e they are continuously updated -/// by incrementing their value). -pub trait IncMetric { - /// Adds `value` to the current counter. - fn add(&self, value: u64); - /// Increments by 1 unit the current counter. - fn inc(&self) { - self.add(1); - } - /// Returns current value of the counter. - fn count(&self) -> u64; - - /// Returns diff of current and old value of the counter. - /// Mostly used in process of aggregating per device metrics. - fn fetch_diff(&self) -> u64; -} - -/// Used for defining new types of metrics that do not need a counter and act as a persistent -/// indicator. -pub trait StoreMetric { - /// Returns current value of the counter. - fn fetch(&self) -> u64; - /// Stores `value` to the current counter. - fn store(&self, value: u64); -} - -/// Representation of a metric that is expected to be incremented from more than one thread, so more -/// synchronization is necessary. -// It's currently used for vCPU metrics. An alternative here would be -// to have one instance of every metric for each thread, and to -// aggregate them when writing. However this probably overkill unless we have a lot of vCPUs -// incrementing metrics very often. Still, it's there if we ever need it :-s -// We will be keeping two values for each metric for being able to reset -// counters on each metric. -// 1st member - current value being updated -// 2nd member - old value that gets the current value whenever metrics is flushed to disk -#[derive(Debug, Default)] -pub struct SharedIncMetric(AtomicU64, AtomicU64); -impl SharedIncMetric { - /// Const default construction. - pub const fn new() -> Self { - Self(AtomicU64::new(0), AtomicU64::new(0)) - } -} - -/// Representation of a metric that is expected to hold a value that can be accessed -/// from more than one thread, so more synchronization is necessary. -#[derive(Debug, Default)] -pub struct SharedStoreMetric(AtomicU64); -impl SharedStoreMetric { - /// Const default construction. - pub const fn new() -> Self { - Self(AtomicU64::new(0)) - } -} - -impl IncMetric for SharedIncMetric { - // While the order specified for this operation is still Relaxed, the actual instruction will - // be an asm "LOCK; something" and thus atomic across multiple threads, simply because of the - // fetch_and_add (as opposed to "store(load() + 1)") implementation for atomics. - // TODO: would a stronger ordering make a difference here? - fn add(&self, value: u64) { - self.0.fetch_add(value, Ordering::Relaxed); - } - - fn count(&self) -> u64 { - self.0.load(Ordering::Relaxed) - } - fn fetch_diff(&self) -> u64 { - self.0.load(Ordering::Relaxed) - self.1.load(Ordering::Relaxed) - } -} - -impl StoreMetric for SharedStoreMetric { - fn fetch(&self) -> u64 { - self.0.load(Ordering::Relaxed) - } - - fn store(&self, value: u64) { - self.0.store(value, Ordering::Relaxed); - } -} - -impl Serialize for SharedIncMetric { - /// Reset counters of each metrics. Here we suppose that Serialize's goal is to help with the - /// flushing of metrics. - /// !!! Any print of the metrics will also reset them. Use with caution !!! - fn serialize(&self, serializer: S) -> Result { - let snapshot = self.0.load(Ordering::Relaxed); - let res = serializer.serialize_u64(snapshot - self.1.load(Ordering::Relaxed)); - - if res.is_ok() { - self.1.store(snapshot, Ordering::Relaxed); - } - res - } -} - -impl Serialize for SharedStoreMetric { - fn serialize(&self, serializer: S) -> Result { - serializer.serialize_u64(self.0.load(Ordering::Relaxed)) - } -} - /// Reporter object which computes the process wall time and /// process CPU time and populates the metric with the results. #[derive(Debug)] @@ -680,72 +577,6 @@ impl SignalMetrics { } } -/// Provides efficient way to record LatencyAggregateMetrics -#[derive(Debug)] -pub struct LatencyMetricsRecorder<'a> { - start_time: u64, - metric: &'a LatencyAggregateMetrics, -} - -impl<'a> LatencyMetricsRecorder<'a> { - /// Const default construction. - fn new(metric: &'a LatencyAggregateMetrics) -> Self { - Self { - start_time: get_time_us(ClockType::Monotonic), - metric, - } - } -} -impl Drop for LatencyMetricsRecorder<'_> { - /// records aggregate (min/max/sum) for the given metric - /// This captures delta between self.start_time and current time - /// and updates min/max/sum metrics. - /// self.start_time is recorded in new() and metrics are updated in drop - fn drop(&mut self) { - let delta_us = get_time_us(ClockType::Monotonic) - self.start_time; - self.metric.sum_us.add(delta_us); - let min_us = self.metric.min_us.fetch(); - let max_us = self.metric.max_us.fetch(); - if (0 == min_us) || (min_us > delta_us) { - self.metric.min_us.store(delta_us); - } - if (0 == max_us) || (max_us < delta_us) { - self.metric.max_us.store(delta_us); - } - } -} - -/// Used to record Aggregate (min/max/sum) of latency metrics -#[derive(Debug, Default, Serialize)] -pub struct LatencyAggregateMetrics { - /// represents minimum value of the metrics in microseconds - pub min_us: SharedStoreMetric, - /// represents maximum value of the metrics in microseconds - pub max_us: SharedStoreMetric, - /// represents sum of the metrics in microseconds - pub sum_us: SharedIncMetric, -} -impl LatencyAggregateMetrics { - /// Const default construction. - pub const fn new() -> Self { - Self { - min_us: SharedStoreMetric::new(), - max_us: SharedStoreMetric::new(), - sum_us: SharedIncMetric::new(), - } - } - - /// returns a latency recorder which captures stores start_time - /// and updates the actual metrics at the end of recorders lifetime. - /// in short instead of below 2 lines : - /// 1st for start_time_us = get_time_us() - /// 2nd for delta_time_us = get_time_us() - start_time; and metrics.store(delta_time_us) - /// we have just `_m = metrics.record_latency_metrics()` - pub fn record_latency_metrics(&self) -> LatencyMetricsRecorder<'_> { - LatencyMetricsRecorder::new(self) - } -} - /// Structure provides Metrics specific to VCPUs' mode of functioning. /// Sample_count or number of kvm exits for IO and MMIO VM exits are covered by: /// `exit_io_in`, `exit_io_out`, `exit_mmio_read` and , `exit_mmio_write`. @@ -966,12 +797,13 @@ impl FirecrackerMetrics { mod tests { use std::io::{ErrorKind, LineWriter}; use std::sync::Arc; - use std::sync::atomic::fence; + use std::sync::atomic::{Ordering, fence}; use std::thread; use vmm_sys_util::tempfile::TempFile; use super::*; + use crate::metrics::IncMetric; #[test] fn test_init() { diff --git a/src/vmm/src/logger/mod.rs b/src/vmm/src/logger/mod.rs index 4bbbf9e19c8..77dc4f3a574 100644 --- a/src/vmm/src/logger/mod.rs +++ b/src/vmm/src/logger/mod.rs @@ -12,10 +12,11 @@ pub use logging::{ DEFAULT_INSTANCE_ID, DEFAULT_LEVEL, INSTANCE_ID, LOGGER, LevelFilter, LevelFilterFromStrError, LoggerConfig, LoggerInitError, LoggerUpdateError, }; -pub use metrics::{ - IncMetric, LatencyAggregateMetrics, METRICS, MetricsError, ProcessTimeReporter, - SharedIncMetric, SharedStoreMetric, StoreMetric, +// Re-export metric traits and implementations from the metrics module for backward compatibility +pub use crate::metrics::{ + IncMetric, LatencyAggregateMetrics, SharedIncMetric, SharedStoreMetric, StoreMetric, }; +pub use metrics::{METRICS, MetricsError, ProcessTimeReporter}; use utils::time::{ClockType, get_time_us}; /// Alias for `std::io::LineWriter`. diff --git a/src/vmm/src/metrics/mod.rs b/src/vmm/src/metrics/mod.rs new file mode 100644 index 00000000000..3b617d4d934 --- /dev/null +++ b/src/vmm/src/metrics/mod.rs @@ -0,0 +1,203 @@ +// Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Component-defined metrics traits and implementations. +//! +//! This module provides reusable metric traits and implementations that can be used +//! by any component without depending on Firecracker-specific logger implementation. +//! +//! # Design +//! The main design goals of this system are: +//! * Use lockless operations, preferably ones that don't require anything other than simple +//! reads/writes being atomic. +//! * Exploit interior mutability and atomics being Sync to allow all methods (including the ones +//! which are effectively mutable) to be callable on a global non-mut static. +//! * Rely on `serde` to provide the actual serialization for writing the metrics. +//! * Since all metrics start at 0, we implement the `Default` trait via derive for all of them, to +//! avoid having to initialize everything by hand. +//! +//! The system implements 2 types of metrics: +//! * Shared Incremental Metrics (SharedIncMetrics) - dedicated for the metrics which need a counter +//! (i.e the number of times an API request failed). These metrics are reset upon flush. +//! * Shared Store Metrics (SharedStoreMetrics) - are targeted at keeping a persistent value, it is +//! not intended to act as a counter (i.e for measure the process start up time for example). + +use std::sync::atomic::{AtomicU64, Ordering}; + +use serde::{Serialize, Serializer}; +use utils::time::{ClockType, get_time_us}; + +/// Used for defining new types of metrics that act as a counter (i.e they are continuously updated +/// by incrementing their value). +pub trait IncMetric { + /// Adds `value` to the current counter. + fn add(&self, value: u64); + /// Increments by 1 unit the current counter. + fn inc(&self) { + self.add(1); + } + /// Returns current value of the counter. + fn count(&self) -> u64; + + /// Returns diff of current and old value of the counter. + /// Mostly used in process of aggregating per device metrics. + fn fetch_diff(&self) -> u64; +} + +/// Used for defining new types of metrics that do not need a counter and act as a persistent +/// indicator. +pub trait StoreMetric { + /// Returns current value of the counter. + fn fetch(&self) -> u64; + /// Stores `value` to the current counter. + fn store(&self, value: u64); +} + +/// Representation of a metric that is expected to be incremented from more than one thread, so more +/// synchronization is necessary. +// It's currently used for vCPU metrics. An alternative here would be +// to have one instance of every metric for each thread, and to +// aggregate them when writing. However this probably overkill unless we have a lot of vCPUs +// incrementing metrics very often. Still, it's there if we ever need it :-s +// We will be keeping two values for each metric for being able to reset +// counters on each metric. +// 1st member - current value being updated +// 2nd member - old value that gets the current value whenever metrics is flushed to disk +#[derive(Debug, Default)] +pub struct SharedIncMetric(AtomicU64, AtomicU64); + +impl SharedIncMetric { + /// Const default construction. + pub const fn new() -> Self { + Self(AtomicU64::new(0), AtomicU64::new(0)) + } +} + +/// Representation of a metric that is expected to hold a value that can be accessed +/// from more than one thread, so more synchronization is necessary. +#[derive(Debug, Default)] +pub struct SharedStoreMetric(AtomicU64); + +impl SharedStoreMetric { + /// Const default construction. + pub const fn new() -> Self { + Self(AtomicU64::new(0)) + } +} + +impl IncMetric for SharedIncMetric { + // While the order specified for this operation is still Relaxed, the actual instruction will + // be an asm "LOCK; something" and thus atomic across multiple threads, simply because of the + // fetch_and_add (as opposed to "store(load() + 1)") implementation for atomics. + // TODO: would a stronger ordering make a difference here? + fn add(&self, value: u64) { + self.0.fetch_add(value, Ordering::Relaxed); + } + + fn count(&self) -> u64 { + self.0.load(Ordering::Relaxed) + } + + fn fetch_diff(&self) -> u64 { + self.0.load(Ordering::Relaxed) - self.1.load(Ordering::Relaxed) + } +} + +impl StoreMetric for SharedStoreMetric { + fn fetch(&self) -> u64 { + self.0.load(Ordering::Relaxed) + } + + fn store(&self, value: u64) { + self.0.store(value, Ordering::Relaxed); + } +} + +impl Serialize for SharedIncMetric { + /// Reset counters of each metrics. Here we suppose that Serialize's goal is to help with the + /// flushing of metrics. + /// !!! Any print of the metrics will also reset them. Use with caution !!! + fn serialize(&self, serializer: S) -> Result { + let snapshot = self.0.load(Ordering::Relaxed); + let res = serializer.serialize_u64(snapshot - self.1.load(Ordering::Relaxed)); + + if res.is_ok() { + self.1.store(snapshot, Ordering::Relaxed); + } + res + } +} + +impl Serialize for SharedStoreMetric { + fn serialize(&self, serializer: S) -> Result { + serializer.serialize_u64(self.0.load(Ordering::Relaxed)) + } +} + +/// Used to record Aggregate (min/max/sum) of latency metrics +#[derive(Debug, Default, Serialize)] +pub struct LatencyAggregateMetrics { + /// represents minimum value of the metrics in microseconds + pub min_us: SharedStoreMetric, + /// represents maximum value of the metrics in microseconds + pub max_us: SharedStoreMetric, + /// represents sum of the metrics in microseconds + pub sum_us: SharedIncMetric, +} + +impl LatencyAggregateMetrics { + /// Const default construction. + pub const fn new() -> Self { + Self { + min_us: SharedStoreMetric::new(), + max_us: SharedStoreMetric::new(), + sum_us: SharedIncMetric::new(), + } + } + + /// returns a latency recorder which captures stores start_time + /// and updates the actual metrics at the end of recorders lifetime. + /// in short instead of below 2 lines : + /// 1st for start_time_us = get_time_us() + /// 2nd for delta_time_us = get_time_us() - start_time; and metrics.store(delta_time_us) + /// we have just `_m = metrics.record_latency_metrics()` + pub fn record_latency_metrics(&self) -> LatencyMetricsRecorder<'_> { + LatencyMetricsRecorder::new(self) + } +} + +/// Provides efficient way to record LatencyAggregateMetrics +#[derive(Debug)] +pub struct LatencyMetricsRecorder<'a> { + start_time: u64, + metric: &'a LatencyAggregateMetrics, +} + +impl<'a> LatencyMetricsRecorder<'a> { + /// Const default construction. + fn new(metric: &'a LatencyAggregateMetrics) -> Self { + Self { + start_time: get_time_us(ClockType::Monotonic), + metric, + } + } +} + +impl Drop for LatencyMetricsRecorder<'_> { + /// records aggregate (min/max/sum) for the given metric + /// This captures delta between self.start_time and current time + /// and updates min/max/sum metrics. + /// self.start_time is recorded in new() and metrics are updated in drop + fn drop(&mut self) { + let delta_us = get_time_us(ClockType::Monotonic) - self.start_time; + self.metric.sum_us.add(delta_us); + let min_us = self.metric.min_us.fetch(); + let max_us = self.metric.max_us.fetch(); + if (0 == min_us) || (min_us > delta_us) { + self.metric.min_us.store(delta_us); + } + if (0 == max_us) || (max_us < delta_us) { + self.metric.max_us.store(delta_us); + } + } +} diff --git a/src/vmm/src/mmds/mod.rs b/src/vmm/src/mmds/mod.rs index 44e5a82b841..efd2c5ad44b 100644 --- a/src/vmm/src/mmds/mod.rs +++ b/src/vmm/src/mmds/mod.rs @@ -18,7 +18,8 @@ use micro_http::{ }; use serde_json::{Map, Value}; -use crate::logger::{IncMetric, METRICS}; +use crate::logger::METRICS; +use crate::metrics::IncMetric; use crate::mmds::data_store::{Mmds, MmdsDatastoreError as MmdsError, MmdsVersion, OutputFormat}; use crate::mmds::token::PATH_TO_TOKEN; use crate::mmds::token_headers::{ diff --git a/src/vmm/src/mmds/ns.rs b/src/vmm/src/mmds/ns.rs index fbb3870168f..2fc454d0bdf 100644 --- a/src/vmm/src/mmds/ns.rs +++ b/src/vmm/src/mmds/ns.rs @@ -25,7 +25,8 @@ use crate::dumbo::pdu::ipv4::{ use crate::dumbo::pdu::tcp::TcpError as TcpSegmentError; use crate::dumbo::tcp::NextSegmentStatus; use crate::dumbo::tcp::handler::{RecvEvent, TcpIPv4Handler, WriteEvent, WriteNextError}; -use crate::logger::{IncMetric, METRICS}; +use crate::logger::METRICS; +use crate::metrics::IncMetric; use crate::mmds::data_store::Mmds; use crate::utils::net::mac::MacAddr; diff --git a/src/vmm/src/signal_handler.rs b/src/vmm/src/signal_handler.rs index 0fdfa0d8b97..394cde09f61 100644 --- a/src/vmm/src/signal_handler.rs +++ b/src/vmm/src/signal_handler.rs @@ -7,7 +7,8 @@ use libc::{ use log::error; use crate::FcExitCode; -use crate::logger::{IncMetric, METRICS, StoreMetric}; +use crate::logger::METRICS; +use crate::metrics::{IncMetric, StoreMetric}; use crate::utils::signal::register_signal_handler; // The offset of `si_syscall` (offending syscall identifier) within the siginfo structure diff --git a/src/vmm/src/vstate/interrupts.rs b/src/vmm/src/vstate/interrupts.rs index 5246144d8f6..c939396f4b6 100644 --- a/src/vmm/src/vstate/interrupts.rs +++ b/src/vmm/src/vstate/interrupts.rs @@ -8,7 +8,8 @@ use kvm_ioctls::VmFd; use vmm_sys_util::eventfd::EventFd; use crate::Vm; -use crate::logger::{IncMetric, METRICS}; +use crate::logger::METRICS; +use crate::metrics::IncMetric; use crate::snapshot::Persist; #[derive(Debug, thiserror::Error, displaydoc::Display)] diff --git a/src/vmm/src/vstate/vcpu.rs b/src/vmm/src/vstate/vcpu.rs index 1efeee58a8d..dd66662984f 100644 --- a/src/vmm/src/vstate/vcpu.rs +++ b/src/vmm/src/vstate/vcpu.rs @@ -23,7 +23,8 @@ pub use crate::arch::{KvmVcpu, KvmVcpuConfigureError, KvmVcpuError, Peripherals, use crate::cpu_config::templates::{CpuConfiguration, GuestConfigError}; #[cfg(feature = "gdb")] use crate::gdb::target::{GdbTargetError, get_raw_tid}; -use crate::logger::{IncMetric, METRICS}; +use crate::logger::METRICS; +use crate::metrics::IncMetric; use crate::seccomp::{BpfProgram, BpfProgramRef}; use crate::utils::signal::{Killable, register_signal_handler, sigrtmin}; use crate::utils::sm::StateMachine;