diff --git a/CHANGELOG.md b/CHANGELOG.md index bb56ed533a6..9b2d7b244d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ ### Added +- [#4145](https://github.com/firecracker-microvm/firecracker/pull/4145): + Added support for per net device metrics. In addition to aggregate metrics `net`, + each individual net device will emit metrics under the label `"net_{iface_id}"`. + E.g. the associated metrics for the endpoint `"/network-interfaces/eth0"` will + be available under `"net_eth0"` in the metrics json object. + ### Changed - Simplified and clarified the removal policy of deprecated API elements diff --git a/src/vmm/src/devices/mod.rs b/src/vmm/src/devices/mod.rs index 454e4330609..e2738677b81 100644 --- a/src/vmm/src/devices/mod.rs +++ b/src/vmm/src/devices/mod.rs @@ -17,14 +17,17 @@ pub mod virtio; pub use bus::{Bus, BusDevice, BusError}; use log::error; +use crate::devices::virtio::metrics::NetDeviceMetrics; use crate::devices::virtio::{QueueError, VsockError}; use crate::logger::{IncMetric, METRICS}; // Function used for reporting error in terms of logging -// but also in terms of METRICS net event fails. -pub(crate) fn report_net_event_fail(err: DeviceError) { +// but also in terms of metrics of net event fails. +// network metrics is reported per device so we need a handle to each net device's +// metrics `net_iface_metrics` to report metrics for that device. +pub(crate) fn report_net_event_fail(net_iface_metrics: &NetDeviceMetrics, err: DeviceError) { error!("{:?}", err); - METRICS.net.event_fails.inc(); + net_iface_metrics.event_fails.inc(); } pub(crate) fn report_balloon_event_fail(err: virtio::balloon::BalloonError) { diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs index d4d3f8448ec..9525e2f972d 100755 --- a/src/vmm/src/devices/virtio/net/device.rs +++ b/src/vmm/src/devices/virtio/net/device.rs @@ -37,6 +37,7 @@ use crate::vstate::memory::{ByteValued, Bytes, GuestMemoryMmap}; const FRAME_HEADER_MAX_LEN: usize = PAYLOAD_OFFSET + ETH_IPV4_FRAME_LEN; use crate::devices::virtio::iovec::IoVecBuffer; +use crate::devices::virtio::metrics::{NetDeviceMetrics, NetMetricsPerDevice}; use crate::devices::virtio::net::tap::Tap; use crate::devices::virtio::net::{ gen, NetError, NetQueue, MAX_BUFFER_SIZE, NET_QUEUE_SIZES, RX_INDEX, TX_INDEX, @@ -136,6 +137,7 @@ pub struct Net { /// The MMDS stack corresponding to this interface. /// Only if MMDS transport has been associated with it. pub mmds_ns: Option, + pub(crate) metrics: Arc, } impl Net { @@ -172,7 +174,7 @@ impl Net { } Ok(Net { - id, + id: id.clone(), tap, avail_features, acked_features: 0u64, @@ -190,6 +192,7 @@ impl Net { device_state: DeviceState::Inactive, activate_evt: EventFd::new(libc::EFD_NONBLOCK).map_err(NetError::EventFd)?, mmds_ns: None, + metrics: NetMetricsPerDevice::alloc(id), }) } @@ -272,7 +275,7 @@ impl Net { self.irq_trigger .trigger_irq(IrqType::Vring) .map_err(|err| { - METRICS.net.event_fails.inc(); + self.metrics.event_fails.inc(); DeviceError::FailedSignalingIrq(err) })?; } @@ -305,7 +308,7 @@ impl Net { // Returns true on successful frame delivery. fn rate_limited_rx_single_frame(&mut self) -> bool { if !Self::rate_limiter_consume_op(&mut self.rx_rate_limiter, self.rx_bytes_read as u64) { - METRICS.net.rx_rate_limiter_throttled.inc(); + self.metrics.rx_rate_limiter_throttled.inc(); return false; } @@ -331,6 +334,7 @@ impl Net { mem: &GuestMemoryMmap, data: &[u8], head: DescriptorChain, + net_metrics: &NetDeviceMetrics, ) -> Result<(), FrontendError> { let mut chunk = data; let mut next_descriptor = Some(head); @@ -343,13 +347,13 @@ impl Net { let len = std::cmp::min(chunk.len(), descriptor.len as usize); match mem.write_slice(&chunk[..len], descriptor.addr) { Ok(()) => { - METRICS.net.rx_count.inc(); + net_metrics.rx_count.inc(); chunk = &chunk[len..]; } Err(err) => { error!("Failed to write slice: {:?}", err); if let GuestMemoryError::PartialBuffer { .. } = err { - METRICS.net.rx_partial_writes.inc(); + net_metrics.rx_partial_writes.inc(); } return Err(FrontendError::GuestMemory(err)); } @@ -357,8 +361,9 @@ impl Net { // If chunk is empty we are done here. if chunk.is_empty() { - METRICS.net.rx_bytes_count.add(data.len() as u64); - METRICS.net.rx_packets_count.inc(); + let len = data.len() as u64; + net_metrics.rx_bytes_count.add(len); + net_metrics.rx_packets_count.inc(); return Ok(()); } @@ -376,7 +381,7 @@ impl Net { let queue = &mut self.queues[RX_INDEX]; let head_descriptor = queue.pop_or_enable_notification(mem).ok_or_else(|| { - METRICS.net.no_rx_avail_buffer.inc(); + self.metrics.no_rx_avail_buffer.inc(); FrontendError::EmptyQueue })?; let head_index = head_descriptor.index; @@ -385,10 +390,11 @@ impl Net { mem, &self.rx_frame_buf[..self.rx_bytes_read], head_descriptor, + &self.metrics, ); // Mark the descriptor chain as used. If an error occurred, skip the descriptor chain. let used_len = if result.is_err() { - METRICS.net.rx_fails.inc(); + self.metrics.rx_fails.inc(); 0 } else { // Safe to unwrap because a frame must be smaller than 2^16 bytes. @@ -432,18 +438,19 @@ impl Net { frame_iovec: &IoVecBuffer, tap: &mut Tap, guest_mac: Option, + net_metrics: &NetDeviceMetrics, ) -> Result { // Read the frame headers from the IoVecBuffer. This will return None // if the frame_iovec is empty. let header_len = frame_iovec.read_at(headers, 0).ok_or_else(|| { error!("Received empty TX buffer"); - METRICS.net.tx_malformed_frames.inc(); + net_metrics.tx_malformed_frames.inc(); NetError::VnetHeaderMissing })?; let headers = frame_bytes_from_buf(&headers[..header_len]).map_err(|e| { error!("VNET headers missing in TX frame"); - METRICS.net.tx_malformed_frames.inc(); + net_metrics.tx_malformed_frames.inc(); e })?; @@ -470,20 +477,21 @@ impl Net { if let Some(guest_mac) = guest_mac { let _ = EthernetFrame::from_bytes(headers).map(|eth_frame| { if guest_mac != eth_frame.src_mac() { - METRICS.net.tx_spoofed_mac_count.inc(); + net_metrics.tx_spoofed_mac_count.inc(); } }); } match Self::write_tap(tap, frame_iovec) { Ok(_) => { - METRICS.net.tx_bytes_count.add(frame_iovec.len() as u64); - METRICS.net.tx_packets_count.inc(); - METRICS.net.tx_count.inc(); + let len = frame_iovec.len() as u64; + net_metrics.tx_bytes_count.add(len); + net_metrics.tx_packets_count.inc(); + net_metrics.tx_count.inc(); } Err(err) => { error!("Failed to write to tap: {:?}", err); - METRICS.net.tap_write_fails.inc(); + net_metrics.tap_write_fails.inc(); } }; Ok(false) @@ -512,7 +520,7 @@ impl Net { match self.read_from_mmds_or_tap() { Ok(count) => { self.rx_bytes_read = count; - METRICS.net.rx_count.inc(); + self.metrics.rx_count.inc(); if !self.rate_limited_rx_single_frame() { self.rx_deferred_frame = true; break; @@ -525,7 +533,7 @@ impl Net { Some(err) if err == EAGAIN => (), _ => { error!("Failed to read tap: {:?}", err); - METRICS.net.tap_read_fails.inc(); + self.metrics.tap_read_fails.inc(); return Err(DeviceError::FailedReadTap); } }; @@ -580,7 +588,7 @@ impl Net { let buffer = match IoVecBuffer::from_descriptor_chain(mem, head) { Ok(buffer) => buffer, Err(_) => { - METRICS.net.tx_fails.inc(); + self.metrics.tx_fails.inc(); tx_queue .add_used(mem, head_index, 0) .map_err(DeviceError::QueueError)?; @@ -589,7 +597,7 @@ impl Net { }; if !Self::rate_limiter_consume_op(&mut self.tx_rate_limiter, buffer.len() as u64) { tx_queue.undo_pop(); - METRICS.net.tx_rate_limiter_throttled.inc(); + self.metrics.tx_rate_limiter_throttled.inc(); break; } @@ -600,6 +608,7 @@ impl Net { &buffer, &mut self.tap, self.guest_mac, + &self.metrics, ) .unwrap_or(false); if frame_consumed_by_mmds && !self.rx_deferred_frame { @@ -614,7 +623,7 @@ impl Net { } if !used_any { - METRICS.net.no_tx_avail_buffer.inc(); + self.metrics.no_tx_avail_buffer.inc(); } self.signal_used_queue(NetQueue::Tx)?; @@ -654,37 +663,38 @@ impl Net { /// This is called by the event manager responding to the guest adding a new /// buffer in the RX queue. pub fn process_rx_queue_event(&mut self) { - METRICS.net.rx_queue_event_count.inc(); + self.metrics.rx_queue_event_count.inc(); if let Err(err) = self.queue_evts[RX_INDEX].read() { // rate limiters present but with _very high_ allowed rate error!("Failed to get rx queue event: {:?}", err); - METRICS.net.event_fails.inc(); + self.metrics.event_fails.inc(); } else if self.rx_rate_limiter.is_blocked() { - METRICS.net.rx_rate_limiter_throttled.inc(); + self.metrics.rx_rate_limiter_throttled.inc(); } else { // If the limiter is not blocked, resume the receiving of bytes. - self.resume_rx().unwrap_or_else(report_net_event_fail); + self.resume_rx() + .unwrap_or_else(|err| report_net_event_fail(&self.metrics, err)); } } pub fn process_tap_rx_event(&mut self) { // This is safe since we checked in the event handler that the device is activated. let mem = self.device_state.mem().unwrap(); - METRICS.net.rx_tap_event_count.inc(); + self.metrics.rx_tap_event_count.inc(); // While there are no available RX queue buffers and there's a deferred_frame // don't process any more incoming. Otherwise start processing a frame. In the // process the deferred_frame flag will be set in order to avoid freezing the // RX queue. if self.queues[RX_INDEX].is_empty(mem) && self.rx_deferred_frame { - METRICS.net.no_rx_avail_buffer.inc(); + self.metrics.no_rx_avail_buffer.inc(); return; } // While limiter is blocked, don't process any more incoming. if self.rx_rate_limiter.is_blocked() { - METRICS.net.rx_rate_limiter_throttled.inc(); + self.metrics.rx_rate_limiter_throttled.inc(); return; } @@ -693,9 +703,10 @@ impl Net { // until we manage to receive this deferred frame. { self.handle_deferred_frame() - .unwrap_or_else(report_net_event_fail); + .unwrap_or_else(|err| report_net_event_fail(&self.metrics, err)); } else { - self.process_rx().unwrap_or_else(report_net_event_fail); + self.process_rx() + .unwrap_or_else(|err| report_net_event_fail(&self.metrics, err)); } } @@ -704,48 +715,51 @@ impl Net { /// This is called by the event manager responding to the guest adding a new /// buffer in the TX queue. pub fn process_tx_queue_event(&mut self) { - METRICS.net.tx_queue_event_count.inc(); + self.metrics.tx_queue_event_count.inc(); if let Err(err) = self.queue_evts[TX_INDEX].read() { error!("Failed to get tx queue event: {:?}", err); - METRICS.net.event_fails.inc(); + self.metrics.event_fails.inc(); } else if !self.tx_rate_limiter.is_blocked() // If the limiter is not blocked, continue transmitting bytes. { - self.process_tx().unwrap_or_else(report_net_event_fail); + self.process_tx() + .unwrap_or_else(|err| report_net_event_fail(&self.metrics, err)); } else { - METRICS.net.tx_rate_limiter_throttled.inc(); + self.metrics.tx_rate_limiter_throttled.inc(); } } pub fn process_rx_rate_limiter_event(&mut self) { - METRICS.net.rx_event_rate_limiter_count.inc(); + self.metrics.rx_event_rate_limiter_count.inc(); // Upon rate limiter event, call the rate limiter handler // and restart processing the queue. match self.rx_rate_limiter.event_handler() { Ok(_) => { // There might be enough budget now to receive the frame. - self.resume_rx().unwrap_or_else(report_net_event_fail); + self.resume_rx() + .unwrap_or_else(|err| report_net_event_fail(&self.metrics, err)); } Err(err) => { error!("Failed to get rx rate-limiter event: {:?}", err); - METRICS.net.event_fails.inc(); + self.metrics.event_fails.inc(); } } } pub fn process_tx_rate_limiter_event(&mut self) { - METRICS.net.tx_rate_limiter_event_count.inc(); + self.metrics.tx_rate_limiter_event_count.inc(); // Upon rate limiter event, call the rate limiter handler // and restart processing the queue. match self.tx_rate_limiter.event_handler() { Ok(_) => { // There might be enough budget now to send the frame. - self.process_tx().unwrap_or_else(report_net_event_fail); + self.process_tx() + .unwrap_or_else(|err| report_net_event_fail(&self.metrics, err)); } Err(err) => { error!("Failed to get tx rate-limiter event: {:?}", err); - METRICS.net.event_fails.inc(); + self.metrics.event_fails.inc(); } } } @@ -799,7 +813,7 @@ impl VirtioDevice for Net { let config_len = config_space_bytes.len() as u64; if offset >= config_len { error!("Failed to read config space"); - METRICS.net.cfg_fails.inc(); + self.metrics.cfg_fails.inc(); return; } if let Some(end) = offset.checked_add(data.len() as u64) { @@ -820,13 +834,13 @@ impl VirtioDevice for Net { .and_then(|(start, end)| config_space_bytes.get_mut(start..end)) else { error!("Failed to write config space"); - METRICS.net.cfg_fails.inc(); + self.metrics.cfg_fails.inc(); return; }; dst.copy_from_slice(data); self.guest_mac = Some(self.config_space.guest_mac); - METRICS.net.mac_address_updates.inc(); + self.metrics.mac_address_updates.inc(); } fn activate(&mut self, mem: GuestMemoryMmap) -> Result<(), ActivateError> { @@ -881,7 +895,7 @@ pub mod tests { use crate::dumbo::pdu::arp::{EthIPv4ArpFrame, ETH_IPV4_FRAME_LEN}; use crate::dumbo::pdu::ethernet::ETHERTYPE_ARP; use crate::dumbo::EthernetFrame; - use crate::logger::{IncMetric, METRICS}; + use crate::logger::IncMetric; use crate::rate_limiter::{BucketUpdate, RateLimiter, TokenBucket, TokenType}; use crate::vstate::memory::{Address, GuestMemory}; @@ -1008,7 +1022,7 @@ pub mod tests { // Check that the guest MAC was updated. let expected_guest_mac = MacAddr::from_bytes_unchecked(&new_config); assert_eq!(expected_guest_mac, net.guest_mac.unwrap()); - assert_eq!(METRICS.net.mac_address_updates.count(), 1); + assert_eq!(net.metrics.mac_address_updates.count(), 1); // Partial write (this is how the kernel sets a new mac address) - byte by byte. let new_config = [0x11, 0x22, 0x33, 0x44, 0x55, 0x66]; @@ -1041,7 +1055,7 @@ pub mod tests { th.add_desc_chain(NetQueue::Rx, 0, &[(0, 4096, VIRTQ_DESC_F_WRITE)]); th.net().queue_evts[RX_INDEX].read().unwrap(); check_metric_after_block!( - METRICS.net.event_fails, + th.net().metrics.event_fails, 1, th.simulate_event(NetEvent::RxQueue) ); @@ -1136,7 +1150,7 @@ pub mod tests { // Inject frame to tap and run epoll. let frame = inject_tap_tx_frame(&th.net(), 1000); check_metric_after_block!( - METRICS.net.rx_packets_count, + th.net().metrics.rx_packets_count, 1, th.event_manager.run_with_timeout(100).unwrap() ); @@ -1177,7 +1191,7 @@ pub mod tests { // Inject frame to tap and run epoll. let frame = inject_tap_tx_frame(&th.net(), 1000); check_metric_after_block!( - METRICS.net.rx_packets_count, + th.net().metrics.rx_packets_count, 1, th.event_manager.run_with_timeout(100).unwrap() ); @@ -1217,7 +1231,7 @@ pub mod tests { let frame_1 = inject_tap_tx_frame(&th.net(), 200); let frame_2 = inject_tap_tx_frame(&th.net(), 300); check_metric_after_block!( - METRICS.net.rx_packets_count, + th.net().metrics.rx_packets_count, 2, th.event_manager.run_with_timeout(100).unwrap() ); @@ -1248,7 +1262,7 @@ pub mod tests { th.add_desc_chain(NetQueue::Tx, 0, &[(0, 4096, 0)]); th.net().queue_evts[TX_INDEX].read().unwrap(); check_metric_after_block!( - METRICS.net.event_fails, + th.net().metrics.event_fails, 1, th.simulate_event(NetEvent::TxQueue) ); @@ -1287,7 +1301,7 @@ pub mod tests { // Send an invalid frame (too small, VNET header missing). th.add_desc_chain(NetQueue::Tx, 0, &[(0, 1, 0)]); check_metric_after_block!( - &METRICS.net.tx_malformed_frames, + th.net().metrics.tx_malformed_frames, 1, th.event_manager.run_with_timeout(100) ); @@ -1309,7 +1323,7 @@ pub mod tests { // Send an invalid frame (too small, VNET header missing). th.add_desc_chain(NetQueue::Tx, 0, &[(0, 0, 0)]); check_metric_after_block!( - &METRICS.net.tx_malformed_frames, + th.net().metrics.tx_malformed_frames, 1, th.event_manager.run_with_timeout(100) ); @@ -1347,7 +1361,7 @@ pub mod tests { // One frame is valid, one will not be handled because it includes write-only memory // so that leaves us with 2 malformed (no vnet header) frames. check_metric_after_block!( - &METRICS.net.tx_malformed_frames, + th.net().metrics.tx_malformed_frames, 2, th.event_manager.run_with_timeout(100) ); @@ -1377,7 +1391,7 @@ pub mod tests { let frame = th.write_tx_frame(&desc_list, 1000); check_metric_after_block!( - METRICS.net.tx_packets_count, + th.net().metrics.tx_packets_count, 1, th.event_manager.run_with_timeout(100).unwrap() ); @@ -1403,7 +1417,7 @@ pub mod tests { let _ = th.write_tx_frame(&desc_list, 1000); check_metric_after_block!( - METRICS.net.tap_write_fails, + th.net().metrics.tap_write_fails, 1, th.event_manager.run_with_timeout(100).unwrap() ); @@ -1430,7 +1444,7 @@ pub mod tests { let frame_2 = th.write_tx_frame(&desc_list, 600); check_metric_after_block!( - METRICS.net.tx_packets_count, + th.net().metrics.tx_packets_count, 2, th.event_manager.run_with_timeout(100).unwrap() ); @@ -1509,6 +1523,7 @@ pub mod tests { &buffer, &mut net.tap, Some(src_mac), + &net.metrics, ) .unwrap()) ); @@ -1537,7 +1552,7 @@ pub mod tests { // Check that a legit MAC doesn't affect the spoofed MAC metric. check_metric_after_block!( - &METRICS.net.tx_spoofed_mac_count, + net.metrics.tx_spoofed_mac_count, 0, Net::write_to_mmds_or_tap( net.mmds_ns.as_mut(), @@ -1546,12 +1561,13 @@ pub mod tests { &buffer, &mut net.tap, Some(guest_mac), + &net.metrics, ) ); // Check that a spoofed MAC increases our spoofed MAC metric. check_metric_after_block!( - &METRICS.net.tx_spoofed_mac_count, + net.metrics.tx_spoofed_mac_count, 1, Net::write_to_mmds_or_tap( net.mmds_ns.as_mut(), @@ -1560,6 +1576,7 @@ pub mod tests { &buffer, &mut net.tap, Some(not_guest_mac), + &net.metrics, ) ); } @@ -1572,7 +1589,7 @@ pub mod tests { // RX rate limiter events should error since the limiter is not blocked. // Validate that the event failed and failure was properly accounted for. check_metric_after_block!( - &METRICS.net.event_fails, + th.net().metrics.event_fails, 1, th.simulate_event(NetEvent::RxRateLimiter) ); @@ -1580,7 +1597,7 @@ pub mod tests { // TX rate limiter events should error since the limiter is not blocked. // Validate that the event failed and failure was properly accounted for. check_metric_after_block!( - &METRICS.net.event_fails, + th.net().metrics.event_fails, 1, th.simulate_event(NetEvent::TxRateLimiter) ); @@ -1598,7 +1615,7 @@ pub mod tests { // The RX queue is empty and rx_deffered_frame is set. th.net().rx_deferred_frame = true; check_metric_after_block!( - &METRICS.net.no_rx_avail_buffer, + th.net().metrics.no_rx_avail_buffer, 1, th.simulate_event(NetEvent::Tap) ); @@ -1611,7 +1628,7 @@ pub mod tests { // Fake an avail buffer; this time, tap reading should error out. th.rxq.avail.idx.set(1); check_metric_after_block!( - &METRICS.net.tap_read_fails, + th.net().metrics.tap_read_fails, 1, th.simulate_event(NetEvent::Tap) ); @@ -1623,19 +1640,19 @@ pub mod tests { th.activate_net(); th.net().tap.mocks.set_read_tap(ReadTapMock::TapFrame); - let rx_packets_count = METRICS.net.rx_packets_count.count(); + let rx_packets_count = th.net().metrics.rx_packets_count.count(); let _ = inject_tap_tx_frame(&th.net(), 1000); // Trigger a Tap event that. This should fail since there // are not any available descriptors in the queue check_metric_after_block!( - &METRICS.net.no_rx_avail_buffer, + th.net().metrics.no_rx_avail_buffer, 1, th.simulate_event(NetEvent::Tap) ); // The frame we read from the tap should be deferred now and // no frames should have been transmitted assert!(th.net().rx_deferred_frame); - assert_eq!(METRICS.net.rx_packets_count.count(), rx_packets_count); + assert_eq!(th.net().metrics.rx_packets_count.count(), rx_packets_count); // Let's add a second frame, which should really have the same // fate. @@ -1646,19 +1663,22 @@ pub mod tests { // since there's only one Descriptor Chain in the queue. th.add_desc_chain(NetQueue::Rx, 0, &[(0, 4096, VIRTQ_DESC_F_WRITE)]); check_metric_after_block!( - &METRICS.net.no_rx_avail_buffer, + th.net().metrics.no_rx_avail_buffer, 1, th.simulate_event(NetEvent::Tap) ); // We should still have a deferred frame assert!(th.net().rx_deferred_frame); // However, we should have delivered the first frame - assert_eq!(METRICS.net.rx_packets_count.count(), rx_packets_count + 1); + assert_eq!( + th.net().metrics.rx_packets_count.count(), + rx_packets_count + 1 + ); // Let's add one more descriptor and try to handle the last frame as well. th.add_desc_chain(NetQueue::Rx, 0, &[(0, 4096, VIRTQ_DESC_F_WRITE)]); check_metric_after_block!( - &METRICS.net.rx_packets_count, + th.net().metrics.rx_packets_count, 1, th.simulate_event(NetEvent::RxQueue) ); @@ -1675,7 +1695,7 @@ pub mod tests { th.net().rx_rate_limiter = RateLimiter::new(0, 0, 0, 0, 0, 0).unwrap(); // There is no actual event on the rate limiter's timerfd. check_metric_after_block!( - &METRICS.net.event_fails, + th.net().metrics.event_fails, 1, th.simulate_event(NetEvent::RxRateLimiter) ); @@ -1690,7 +1710,7 @@ pub mod tests { th.simulate_event(NetEvent::TxRateLimiter); // There is no actual event on the rate limiter's timerfd. check_metric_after_block!( - &METRICS.net.event_fails, + th.net().metrics.event_fails, 1, th.simulate_event(NetEvent::TxRateLimiter) ); @@ -1720,7 +1740,7 @@ pub mod tests { // assert that limiter is blocked assert!(th.net().tx_rate_limiter.is_blocked()); - assert_eq!(METRICS.net.tx_rate_limiter_throttled.count(), 1); + assert_eq!(th.net().metrics.tx_rate_limiter_throttled.count(), 1); // make sure the data is still queued for processing assert_eq!(th.txq.used.idx.get(), 0); } @@ -1731,7 +1751,7 @@ pub mod tests { // trigger the RX queue event handler th.simulate_event(NetEvent::TxQueue); - assert_eq!(METRICS.net.tx_rate_limiter_throttled.count(), 2); + assert_eq!(th.net().metrics.tx_rate_limiter_throttled.count(), 2); } // wait for 100ms to give the rate-limiter timer a chance to replenish @@ -1742,7 +1762,7 @@ pub mod tests { { // tx_count increments 1 from write_to_mmds_or_tap() check_metric_after_block!( - &METRICS.net.tx_count, + th.net().metrics.tx_count, 1, th.simulate_event(NetEvent::TxRateLimiter) ); @@ -1759,7 +1779,7 @@ pub mod tests { { // tx_count increments 1 from write_to_mmds_or_tap() check_metric_after_block!( - &METRICS.net.tx_count, + th.net().metrics.tx_count, 1, th.simulate_event(NetEvent::TxRateLimiter) ); @@ -1791,7 +1811,7 @@ pub mod tests { // assert that limiter is blocked assert!(th.net().rx_rate_limiter.is_blocked()); - assert_eq!(METRICS.net.rx_rate_limiter_throttled.count(), 1); + assert_eq!(th.net().metrics.rx_rate_limiter_throttled.count(), 1); assert!(th.net().rx_deferred_frame); // assert that no operation actually completed (limiter blocked it) assert!(&th.net().irq_trigger.has_pending_irq(IrqType::Vring)); @@ -1804,7 +1824,7 @@ pub mod tests { // trigger the RX queue event handler th.simulate_event(NetEvent::RxQueue); - assert_eq!(METRICS.net.rx_rate_limiter_throttled.count(), 2); + assert_eq!(th.net().metrics.rx_rate_limiter_throttled.count(), 2); } // wait for 100ms to give the rate-limiter timer a chance to replenish @@ -1816,7 +1836,7 @@ pub mod tests { let frame = &th.net().tap.mocks.read_tap.mock_frame(); // no longer throttled check_metric_after_block!( - &METRICS.net.rx_rate_limiter_throttled, + th.net().metrics.rx_rate_limiter_throttled, 0, th.simulate_event(NetEvent::RxRateLimiter) ); @@ -1854,7 +1874,7 @@ pub mod tests { // trigger the TX handler th.add_desc_chain(NetQueue::Tx, 0, &[(0, 4096, 0)]); check_metric_after_block!( - METRICS.net.tx_rate_limiter_throttled, + th.net().metrics.tx_rate_limiter_throttled, 1, th.simulate_event(NetEvent::TxQueue) ); @@ -1873,7 +1893,7 @@ pub mod tests { { // no longer throttled check_metric_after_block!( - &METRICS.net.tx_rate_limiter_throttled, + th.net().metrics.tx_rate_limiter_throttled, 0, th.simulate_event(NetEvent::TxRateLimiter) ); @@ -1902,14 +1922,14 @@ pub mod tests { { // trigger the RX handler check_metric_after_block!( - METRICS.net.rx_rate_limiter_throttled, + th.net().metrics.rx_rate_limiter_throttled, 1, th.simulate_event(NetEvent::Tap) ); // assert that limiter is blocked assert!(th.net().rx_rate_limiter.is_blocked()); - assert!(METRICS.net.rx_rate_limiter_throttled.count() >= 1); + assert!(th.net().metrics.rx_rate_limiter_throttled.count() >= 1); assert!(th.net().rx_deferred_frame); // assert that no operation actually completed (limiter blocked it) assert!(&th.net().irq_trigger.has_pending_irq(IrqType::Vring)); diff --git a/src/vmm/src/devices/virtio/net/event_handler.rs b/src/vmm/src/devices/virtio/net/event_handler.rs index 496a9b79dea..a3bd0499f18 100644 --- a/src/vmm/src/devices/virtio/net/event_handler.rs +++ b/src/vmm/src/devices/virtio/net/event_handler.rs @@ -8,7 +8,7 @@ use utils::epoll::EventSet; use crate::devices::virtio::net::device::Net; use crate::devices::virtio::{VirtioDevice, RX_INDEX, TX_INDEX}; -use crate::logger::{debug, error, warn, IncMetric, METRICS}; +use crate::logger::{debug, error, warn, IncMetric}; impl Net { fn register_runtime_events(&self, ops: &mut EventOps) { @@ -84,7 +84,7 @@ impl MutEventSubscriber for Net { _ if activate_fd == source => self.process_activate_event(ops), _ => { warn!("Net: Spurious event received: {:?}", source); - METRICS.net.event_fails.inc(); + self.metrics.event_fails.inc(); } } } else { diff --git a/src/vmm/src/devices/virtio/net/metrics.rs b/src/vmm/src/devices/virtio/net/metrics.rs new file mode 100644 index 00000000000..084b37bbbe3 --- /dev/null +++ b/src/vmm/src/devices/virtio/net/metrics.rs @@ -0,0 +1,435 @@ +// Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Defines the metrics system for Network devices. +//! +//! # Metrics format +//! The metrics are flushed in JSON when requested by vmm::logger::metrics::METRICS.write(). +//! +//! ## JSON example with metrics: +//! ```json +//! { +//! "net_eth0": { +//! "activate_fails": "SharedIncMetric", +//! "cfg_fails": "SharedIncMetric", +//! "mac_address_updates": "SharedIncMetric", +//! "no_rx_avail_buffer": "SharedIncMetric", +//! "no_tx_avail_buffer": "SharedIncMetric", +//! ... +//! } +//! "net_eth1": { +//! "activate_fails": "SharedIncMetric", +//! "cfg_fails": "SharedIncMetric", +//! "mac_address_updates": "SharedIncMetric", +//! "no_rx_avail_buffer": "SharedIncMetric", +//! "no_tx_avail_buffer": "SharedIncMetric", +//! ... +//! } +//! ... +//! "net_iface_id": { +//! "activate_fails": "SharedIncMetric", +//! "cfg_fails": "SharedIncMetric", +//! "mac_address_updates": "SharedIncMetric", +//! "no_rx_avail_buffer": "SharedIncMetric", +//! "no_tx_avail_buffer": "SharedIncMetric", +//! ... +//! } +//! "net": { +//! "activate_fails": "SharedIncMetric", +//! "cfg_fails": "SharedIncMetric", +//! "mac_address_updates": "SharedIncMetric", +//! "no_rx_avail_buffer": "SharedIncMetric", +//! "no_tx_avail_buffer": "SharedIncMetric", +//! ... +//! } +//! } +//! ``` +//! Each `net` field in the example above is a serializable `NetDeviceMetrics` structure +//! collecting metrics such as `activate_fails`, `cfg_fails`, etc. for the network device. +//! `net_eth0` represent metrics for the endpoint "/network-interfaces/eth0", +//! `net_eth1` represent metrics for the endpoint "/network-interfaces/eth1", and +//! `net_iface_id` represent metrics for the endpoint "/network-interfaces/{iface_id}" +//! network device respectively and `net` is the aggregate of all the per device metrics. +//! +//! # Limitations +//! Network device currently do not have `vmm::logger::metrics::StoreMetrics` so aggregate +//! doesn't consider them. +//! +//! # Design +//! The main design goals of this system are: +//! * To improve network device metrics by logging them at per device granularity. +//! * Continue to provide aggregate net metrics to maintain backward compatibility. +//! * Move NetDeviceMetrics out of from logger and decouple it. +//! * Use lockless operations, preferably ones that don't require anything other than simple +//! reads/writes being atomic. +//! * 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. +//! +//! * Devices could be created in any order i.e. the first device created could either be eth0 or +//! eth1 so if we use a vector for NetDeviceMetrics and call 1st device as net0, then net0 could +//! sometimes point to eth0 and sometimes to eth1 which doesn't help with analysing the metrics. +//! So, use Map instead of Vec to help understand which interface the metrics actually belongs to. +//! * We use "net_$iface_id" for the metrics name instead of "net_$tap_name" to be consistent with +//! the net endpoint "/network-interfaces/{iface_id}". +//! +//! The system implements 1 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. +//! We use NET_METRICS instead of adding an entry of NetDeviceMetrics +//! in Net so that metrics are accessible to be flushed even from signal handlers. + +use std::collections::BTreeMap; +use std::sync::{Arc, RwLock}; + +use serde::ser::SerializeMap; +use serde::{Serialize, Serializer}; + +use crate::logger::{IncMetric, SharedIncMetric}; + +/// map of network interface id and metrics +/// this should be protected by a lock before accessing. +#[derive(Debug)] +pub struct NetMetricsPerDevice { + /// used to access per net device metrics + pub metrics: BTreeMap>, +} + +impl NetMetricsPerDevice { + /// Allocate `NetDeviceMetrics` for net device having + /// id `iface_id`. Also, allocate only if it doesn't + /// exist to avoid overwriting previously allocated data. + /// lock is always initialized so it is safe the unwrap + /// the lock without a check. + pub fn alloc(iface_id: String) -> Arc { + if NET_METRICS.read().unwrap().metrics.get(&iface_id).is_none() { + NET_METRICS + .write() + .unwrap() + .metrics + .insert(iface_id.clone(), Arc::new(NetDeviceMetrics::default())); + } + NET_METRICS + .read() + .unwrap() + .metrics + .get(&iface_id) + .unwrap() + .clone() + } +} + +/// Pool of Network-related metrics per device behind a lock to +/// keep things thread safe. Since the lock is initialized here +/// it is safe to unwrap it without any check. +static NET_METRICS: RwLock = RwLock::new(NetMetricsPerDevice { + metrics: BTreeMap::new(), +}); + +/// This function facilitates aggregation and serialization of +/// per net device metrics. +pub fn flush_metrics(serializer: S) -> Result { + let net_metrics = NET_METRICS.read().unwrap(); + let metrics_len = net_metrics.metrics.len(); + // +1 to accomodate aggregate net metrics + let mut seq = serializer.serialize_map(Some(1 + metrics_len))?; + + let mut net_aggregated: NetDeviceMetrics = NetDeviceMetrics::default(); + + for (name, metrics) in net_metrics.metrics.iter() { + let devn = format!("net_{}", name); + // serialization will flush the metrics so aggregate before it. + let m: &NetDeviceMetrics = metrics; + net_aggregated.aggregate(m); + seq.serialize_entry(&devn, m)?; + } + seq.serialize_entry("net", &net_aggregated)?; + seq.end() +} + +/// Network-related metrics. +#[derive(Default, Debug, Serialize)] +pub struct NetDeviceMetrics { + /// Number of times when activate failed on a network device. + pub activate_fails: SharedIncMetric, + /// Number of times when interacting with the space config of a network device failed. + pub cfg_fails: SharedIncMetric, + /// Number of times the mac address was updated through the config space. + pub mac_address_updates: SharedIncMetric, + /// No available buffer for the net device rx queue. + pub no_rx_avail_buffer: SharedIncMetric, + /// No available buffer for the net device tx queue. + pub no_tx_avail_buffer: SharedIncMetric, + /// Number of times when handling events on a network device failed. + pub event_fails: SharedIncMetric, + /// Number of events associated with the receiving queue. + pub rx_queue_event_count: SharedIncMetric, + /// Number of events associated with the rate limiter installed on the receiving path. + pub rx_event_rate_limiter_count: SharedIncMetric, + /// Number of RX partial writes to guest. + pub rx_partial_writes: SharedIncMetric, + /// Number of RX rate limiter throttling events. + pub rx_rate_limiter_throttled: SharedIncMetric, + /// Number of events received on the associated tap. + pub rx_tap_event_count: SharedIncMetric, + /// Number of bytes received. + pub rx_bytes_count: SharedIncMetric, + /// Number of packets received. + pub rx_packets_count: SharedIncMetric, + /// Number of errors while receiving data. + pub rx_fails: SharedIncMetric, + /// Number of successful read operations while receiving data. + pub rx_count: SharedIncMetric, + /// Number of times reading from TAP failed. + pub tap_read_fails: SharedIncMetric, + /// Number of times writing to TAP failed. + pub tap_write_fails: SharedIncMetric, + /// Number of transmitted bytes. + pub tx_bytes_count: SharedIncMetric, + /// Number of malformed TX frames. + pub tx_malformed_frames: SharedIncMetric, + /// Number of errors while transmitting data. + pub tx_fails: SharedIncMetric, + /// Number of successful write operations while transmitting data. + pub tx_count: SharedIncMetric, + /// Number of transmitted packets. + pub tx_packets_count: SharedIncMetric, + /// Number of TX partial reads from guest. + pub tx_partial_reads: SharedIncMetric, + /// Number of events associated with the transmitting queue. + pub tx_queue_event_count: SharedIncMetric, + /// Number of events associated with the rate limiter installed on the transmitting path. + pub tx_rate_limiter_event_count: SharedIncMetric, + /// Number of RX rate limiter throttling events. + pub tx_rate_limiter_throttled: SharedIncMetric, + /// Number of packets with a spoofed mac, sent by the guest. + pub tx_spoofed_mac_count: SharedIncMetric, +} + +impl NetDeviceMetrics { + /// Net metrics are SharedIncMetric where the diff of current vs + /// old is serialized i.e. serialize_u64(current-old). + /// So to have the aggregate serialized in same way we need to + /// fetch the diff of current vs old metrics and add it to the + /// aggregate. + pub fn aggregate(&mut self, other: &Self) { + self.activate_fails.add(other.activate_fails.fetch_diff()); + self.cfg_fails.add(other.cfg_fails.fetch_diff()); + self.mac_address_updates + .add(other.mac_address_updates.fetch_diff()); + self.no_rx_avail_buffer + .add(other.no_rx_avail_buffer.fetch_diff()); + self.no_tx_avail_buffer + .add(other.no_tx_avail_buffer.fetch_diff()); + self.event_fails.add(other.event_fails.fetch_diff()); + self.rx_queue_event_count + .add(other.rx_queue_event_count.fetch_diff()); + self.rx_event_rate_limiter_count + .add(other.rx_event_rate_limiter_count.fetch_diff()); + self.rx_partial_writes + .add(other.rx_partial_writes.fetch_diff()); + self.rx_rate_limiter_throttled + .add(other.rx_rate_limiter_throttled.fetch_diff()); + self.rx_tap_event_count + .add(other.rx_tap_event_count.fetch_diff()); + self.rx_bytes_count.add(other.rx_bytes_count.fetch_diff()); + self.rx_packets_count + .add(other.rx_packets_count.fetch_diff()); + self.rx_fails.add(other.rx_fails.fetch_diff()); + self.rx_count.add(other.rx_count.fetch_diff()); + self.tap_read_fails.add(other.tap_read_fails.fetch_diff()); + self.tap_write_fails.add(other.tap_write_fails.fetch_diff()); + self.tx_bytes_count.add(other.tx_bytes_count.fetch_diff()); + self.tx_malformed_frames + .add(other.tx_malformed_frames.fetch_diff()); + self.tx_fails.add(other.tx_fails.fetch_diff()); + self.tx_count.add(other.tx_count.fetch_diff()); + self.tx_packets_count + .add(other.tx_packets_count.fetch_diff()); + self.tx_partial_reads + .add(other.tx_partial_reads.fetch_diff()); + self.tx_queue_event_count + .add(other.tx_queue_event_count.fetch_diff()); + self.tx_rate_limiter_event_count + .add(other.tx_rate_limiter_event_count.fetch_diff()); + self.tx_rate_limiter_throttled + .add(other.tx_rate_limiter_throttled.fetch_diff()); + self.tx_spoofed_mac_count + .add(other.tx_spoofed_mac_count.fetch_diff()); + } +} + +#[cfg(test)] +pub mod tests { + use super::*; + + #[test] + fn test_max_net_dev_metrics() { + // Note: this test has nothing to do with + // Net structure or IRQs, this is just to allocate + // metrics for max number of devices that system can have. + // we have 5-23 irq for net devices so max 19 net devices. + const MAX_NET_DEVICES: usize = 19; + + assert!(NET_METRICS.read().is_ok()); + assert!(NET_METRICS.write().is_ok()); + + for i in 0..MAX_NET_DEVICES { + let devn: String = format!("eth{}", i); + NetMetricsPerDevice::alloc(devn.clone()); + NET_METRICS + .read() + .unwrap() + .metrics + .get(&devn) + .unwrap() + .activate_fails + .inc(); + NET_METRICS + .read() + .unwrap() + .metrics + .get(&devn) + .unwrap() + .rx_bytes_count + .add(10); + NET_METRICS + .read() + .unwrap() + .metrics + .get(&devn) + .unwrap() + .tx_bytes_count + .add(5); + } + + for i in 0..MAX_NET_DEVICES { + let devn: String = format!("eth{}", i); + assert!( + NET_METRICS + .read() + .unwrap() + .metrics + .get(&devn) + .unwrap() + .activate_fails + .count() + >= 1 + ); + assert!( + NET_METRICS + .read() + .unwrap() + .metrics + .get(&devn) + .unwrap() + .rx_bytes_count + .count() + >= 10 + ); + assert_eq!( + NET_METRICS + .read() + .unwrap() + .metrics + .get(&devn) + .unwrap() + .tx_bytes_count + .count(), + 5 + ); + } + } + #[test] + fn test_signle_net_dev_metrics() { + // Use eth0 so that we can check thread safety with the + // `test_net_dev_metrics` which also uses the same name. + let devn = "eth0"; + + assert!(NET_METRICS.read().is_ok()); + assert!(NET_METRICS.write().is_ok()); + + NetMetricsPerDevice::alloc(String::from(devn)); + assert!(NET_METRICS.read().is_ok()); + assert!(NET_METRICS.read().unwrap().metrics.get(devn).is_some()); + + NET_METRICS + .read() + .unwrap() + .metrics + .get(devn) + .unwrap() + .activate_fails + .inc(); + assert!( + NET_METRICS + .read() + .unwrap() + .metrics + .get(devn) + .unwrap() + .activate_fails + .count() + > 0, + "{}", + NET_METRICS + .read() + .unwrap() + .metrics + .get(devn) + .unwrap() + .activate_fails + .count() + ); + // we expect only 2 tests (this and test_max_net_dev_metrics) + // to update activate_fails count for eth0. + assert!( + NET_METRICS + .read() + .unwrap() + .metrics + .get(devn) + .unwrap() + .activate_fails + .count() + <= 2, + "{}", + NET_METRICS + .read() + .unwrap() + .metrics + .get(devn) + .unwrap() + .activate_fails + .count() + ); + + NET_METRICS + .read() + .unwrap() + .metrics + .get(devn) + .unwrap() + .activate_fails + .inc(); + NET_METRICS + .read() + .unwrap() + .metrics + .get(devn) + .unwrap() + .rx_bytes_count + .add(5); + assert!( + NET_METRICS + .read() + .unwrap() + .metrics + .get(devn) + .unwrap() + .rx_bytes_count + .count() + >= 5 + ); + } +} diff --git a/src/vmm/src/devices/virtio/net/mod.rs b/src/vmm/src/devices/virtio/net/mod.rs index c7ea15f296a..ce96283e1a4 100644 --- a/src/vmm/src/devices/virtio/net/mod.rs +++ b/src/vmm/src/devices/virtio/net/mod.rs @@ -19,6 +19,7 @@ pub const TX_INDEX: usize = 1; pub mod device; mod event_handler; +pub mod metrics; pub mod persist; mod tap; pub mod test_utils; diff --git a/src/vmm/src/devices/virtio/net/test_utils.rs b/src/vmm/src/devices/virtio/net/test_utils.rs index 29ccaac9ac4..376920f56b6 100644 --- a/src/vmm/src/devices/virtio/net/test_utils.rs +++ b/src/vmm/src/devices/virtio/net/test_utils.rs @@ -363,7 +363,7 @@ pub mod test { IrqType, Net, VirtioDevice, MAX_BUFFER_SIZE, RX_INDEX, TX_INDEX, VIRTQ_DESC_F_NEXT, VIRTQ_DESC_F_WRITE, }; - use crate::logger::{IncMetric, METRICS}; + use crate::logger::IncMetric; use crate::vstate::memory::{ Address, Bytes, GuestAddress, GuestMemoryExtension, GuestMemoryMmap, }; @@ -497,7 +497,7 @@ pub mod test { // Inject frame to tap and run epoll. let frame = inject_tap_tx_frame(&self.net(), frame_len); check_metric_after_block!( - METRICS.net.rx_packets_count, + self.net().metrics.rx_packets_count, 0, self.event_manager.run_with_timeout(100).unwrap() ); @@ -525,7 +525,7 @@ pub mod test { )], ); check_metric_after_block!( - METRICS.net.rx_packets_count, + self.net().metrics.rx_packets_count, 1, self.event_manager.run_with_timeout(100).unwrap() ); diff --git a/src/vmm/src/logger/metrics.rs b/src/vmm/src/logger/metrics.rs index 17b1a81ae45..947c619cf40 100644 --- a/src/vmm/src/logger/metrics.rs +++ b/src/vmm/src/logger/metrics.rs @@ -72,6 +72,7 @@ use serde::{Serialize, Serializer}; use vm_superio::rtc_pl031::RtcEvents; use super::FcLineWriter; +use crate::devices::virtio::net::metrics as net_metrics; #[cfg(target_arch = "aarch64")] use crate::warn; @@ -201,6 +202,10 @@ pub trait IncMetric { } /// 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 @@ -254,6 +259,9 @@ impl IncMetric for SharedIncMetric { 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 { @@ -706,99 +714,6 @@ impl MmdsMetrics { } } -/// Network-related metrics. -#[derive(Debug, Default, Serialize)] -pub struct NetDeviceMetrics { - /// Number of times when activate failed on a network device. - pub activate_fails: SharedIncMetric, - /// Number of times when interacting with the space config of a network device failed. - pub cfg_fails: SharedIncMetric, - //// Number of times the mac address was updated through the config space. - pub mac_address_updates: SharedIncMetric, - /// No available buffer for the net device rx queue. - pub no_rx_avail_buffer: SharedIncMetric, - /// No available buffer for the net device tx queue. - pub no_tx_avail_buffer: SharedIncMetric, - /// Number of times when handling events on a network device failed. - pub event_fails: SharedIncMetric, - /// Number of events associated with the receiving queue. - pub rx_queue_event_count: SharedIncMetric, - /// Number of events associated with the rate limiter installed on the receiving path. - pub rx_event_rate_limiter_count: SharedIncMetric, - /// Number of RX partial writes to guest. - pub rx_partial_writes: SharedIncMetric, - /// Number of RX rate limiter throttling events. - pub rx_rate_limiter_throttled: SharedIncMetric, - /// Number of events received on the associated tap. - pub rx_tap_event_count: SharedIncMetric, - /// Number of bytes received. - pub rx_bytes_count: SharedIncMetric, - /// Number of packets received. - pub rx_packets_count: SharedIncMetric, - /// Number of errors while receiving data. - pub rx_fails: SharedIncMetric, - /// Number of successful read operations while receiving data. - pub rx_count: SharedIncMetric, - /// Number of times reading from TAP failed. - pub tap_read_fails: SharedIncMetric, - /// Number of times writing to TAP failed. - pub tap_write_fails: SharedIncMetric, - /// Number of transmitted bytes. - pub tx_bytes_count: SharedIncMetric, - /// Number of malformed TX frames. - pub tx_malformed_frames: SharedIncMetric, - /// Number of errors while transmitting data. - pub tx_fails: SharedIncMetric, - /// Number of successful write operations while transmitting data. - pub tx_count: SharedIncMetric, - /// Number of transmitted packets. - pub tx_packets_count: SharedIncMetric, - /// Number of TX partial reads from guest. - pub tx_partial_reads: SharedIncMetric, - /// Number of events associated with the transmitting queue. - pub tx_queue_event_count: SharedIncMetric, - /// Number of events associated with the rate limiter installed on the transmitting path. - pub tx_rate_limiter_event_count: SharedIncMetric, - /// Number of RX rate limiter throttling events. - pub tx_rate_limiter_throttled: SharedIncMetric, - /// Number of packets with a spoofed mac, sent by the guest. - pub tx_spoofed_mac_count: SharedIncMetric, -} -impl NetDeviceMetrics { - /// Const default construction. - pub const fn new() -> Self { - Self { - activate_fails: SharedIncMetric::new(), - cfg_fails: SharedIncMetric::new(), - mac_address_updates: SharedIncMetric::new(), - no_rx_avail_buffer: SharedIncMetric::new(), - no_tx_avail_buffer: SharedIncMetric::new(), - event_fails: SharedIncMetric::new(), - rx_queue_event_count: SharedIncMetric::new(), - rx_event_rate_limiter_count: SharedIncMetric::new(), - rx_partial_writes: SharedIncMetric::new(), - rx_rate_limiter_throttled: SharedIncMetric::new(), - rx_tap_event_count: SharedIncMetric::new(), - rx_bytes_count: SharedIncMetric::new(), - rx_packets_count: SharedIncMetric::new(), - rx_fails: SharedIncMetric::new(), - rx_count: SharedIncMetric::new(), - tap_read_fails: SharedIncMetric::new(), - tap_write_fails: SharedIncMetric::new(), - tx_bytes_count: SharedIncMetric::new(), - tx_malformed_frames: SharedIncMetric::new(), - tx_fails: SharedIncMetric::new(), - tx_count: SharedIncMetric::new(), - tx_packets_count: SharedIncMetric::new(), - tx_partial_reads: SharedIncMetric::new(), - tx_queue_event_count: SharedIncMetric::new(), - tx_rate_limiter_event_count: SharedIncMetric::new(), - tx_rate_limiter_throttled: SharedIncMetric::new(), - tx_spoofed_mac_count: SharedIncMetric::new(), - } - } -} - /// Performance metrics related for the moment only to snapshots. // These store the duration of creating/loading a snapshot and of // pausing/resuming the microVM. @@ -1146,6 +1061,22 @@ impl Serialize for SerializeToUtcTimestampMs { } } +#[derive(Default, Debug)] +// By using the below structure in FirecrackerMetrics it is easy +// to serialise Firecracker app_metrics as a signle json object which +// otherwise would have required extra string manipulation to pack +// net as part of the same json object as FirecrackerMetrics. +pub struct NetMetricsSerializeProxy; + +impl Serialize for NetMetricsSerializeProxy { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + net_metrics::flush_metrics(serializer) + } +} + /// Structure storing all metrics while enforcing serialization support on them. #[derive(Debug, Default, Serialize)] pub struct FirecrackerMetrics { @@ -1168,8 +1099,9 @@ pub struct FirecrackerMetrics { pub logger: LoggerSystemMetrics, /// Metrics specific to MMDS functionality. pub mmds: MmdsMetrics, + #[serde(flatten)] /// A network device's related metrics. - pub net: NetDeviceMetrics, + pub net_ser: NetMetricsSerializeProxy, /// Metrics related to API PATCH requests. pub patch_api_requests: PatchRequestsMetrics, /// Metrics related to API PUT requests. @@ -1206,7 +1138,7 @@ impl FirecrackerMetrics { latencies_us: PerformanceMetrics::new(), logger: LoggerSystemMetrics::new(), mmds: MmdsMetrics::new(), - net: NetDeviceMetrics::new(), + net_ser: NetMetricsSerializeProxy {}, patch_api_requests: PatchRequestsMetrics::new(), put_api_requests: PutRequestsMetrics::new(), #[cfg(target_arch = "aarch64")] diff --git a/tests/integration_tests/functional/test_metrics.py b/tests/integration_tests/functional/test_metrics.py index 036cc9fab54..27f2bae7c72 100644 --- a/tests/integration_tests/functional/test_metrics.py +++ b/tests/integration_tests/functional/test_metrics.py @@ -7,17 +7,15 @@ import platform -def test_flush_metrics(test_microvm_with_api): +def _validate_metrics(metrics): """ - Check the `FlushMetrics` vmm action. + This functions makes sure that all components + of FirecrackerMetrics struct are present. + In depth validation of metrics for each component + should be implemented in its own test. + e.g. validation of NetDeviceMetrics should implement + _validate_net_metrics() to check for breaking change etc. """ - microvm = test_microvm_with_api - microvm.spawn() - microvm.basic_config() - microvm.start() - - metrics = microvm.flush_metrics() - exp_keys = [ "utc_timestamp_ms", "api_server", @@ -44,7 +42,7 @@ def test_flush_metrics(test_microvm_with_api): if platform.machine() == "aarch64": exp_keys.append("rtc") - assert set(metrics.keys()) == set(exp_keys) + assert set(exp_keys).issubset(metrics.keys()) utc_time = datetime.datetime.now(datetime.timezone.utc) utc_timestamp_ms = math.floor(utc_time.timestamp() * 1000) @@ -54,3 +52,130 @@ def test_flush_metrics(test_microvm_with_api): # Epoch.Regression test for: # https://github.com/firecracker-microvm/firecracker/issues/2639 assert abs(utc_timestamp_ms - metrics["utc_timestamp_ms"]) < 1000 + + +class FcDeviceMetrics: + """ + Provides functions to validate breaking change and + aggregation of metrics + """ + + def __init__(self, name, validate_fn, num_dev): + self.dev_name = name + self.validate_dev_metrics = validate_fn + self.num_dev = num_dev + + def validate(self, microvm): + """ + validate breaking change of device metrics + """ + fc_metrics = microvm.flush_metrics() + + # make sure all items of FirecrackerMetrics are as expected + _validate_metrics(fc_metrics) + + # check for breaking change in device specific metrics + self.validate_dev_metrics(fc_metrics[self.dev_name]) + + # make sure "{self.name}" is aggregate of "{self.name}_*" + # and that there are only {num_dev} entries of "{self.name}_*" + self.validate_aggregation(fc_metrics) + print(f"\nsuccessfully validated aggregate of {self.dev_name} metrics") + + def validate_aggregation(self, fc_metrics): + """ + validate aggregation of device metrics + """ + metrics_aggregate = fc_metrics[self.dev_name] + metrics_calculated = {} + actual_num_devices = 0 + print(f"In aggregation of {self.dev_name} expected {self.num_dev=}") + for component_metric_names, component_metric_values in fc_metrics.items(): + if f"{self.dev_name}_" in component_metric_names: + print(f"found {component_metric_names} during aggr of {self.dev_name}") + actual_num_devices += 1 + for metrics_name, metric_value in component_metric_values.items(): + if metrics_name not in metrics_calculated: + metrics_calculated[metrics_name] = 0 + metrics_calculated[metrics_name] += metric_value + assert metrics_aggregate == metrics_calculated + assert self.num_dev == actual_num_devices + + +def test_flush_metrics(test_microvm_with_api): + """ + Check the `FlushMetrics` vmm action. + """ + microvm = test_microvm_with_api + microvm.spawn() + microvm.basic_config() + microvm.start() + + metrics = microvm.flush_metrics() + _validate_metrics(metrics) + + +def _validate_net_metrics(net_metrics): + exp_keys = [ + "activate_fails", + "cfg_fails", + "mac_address_updates", + "no_rx_avail_buffer", + "no_tx_avail_buffer", + "event_fails", + "rx_queue_event_count", + "rx_event_rate_limiter_count", + "rx_partial_writes", + "rx_rate_limiter_throttled", + "rx_tap_event_count", + "rx_bytes_count", + "rx_packets_count", + "rx_fails", + "rx_count", + "tap_read_fails", + "tap_write_fails", + "tx_bytes_count", + "tx_malformed_frames", + "tx_fails", + "tx_count", + "tx_packets_count", + "tx_partial_reads", + "tx_queue_event_count", + "tx_rate_limiter_event_count", + "tx_rate_limiter_throttled", + "tx_spoofed_mac_count", + ] + assert set(net_metrics.keys()) == set(exp_keys) + + +def test_net_metrics(test_microvm_with_api): + """ + Validate that NetDeviceMetrics doesn't have a breaking change + and "net" is aggregate of all "net_*" in the json object. + """ + test_microvm = test_microvm_with_api + test_microvm.spawn() + + # Set up a basic microVM. + test_microvm.basic_config() + + # randomly selected 10 as the number of net devices to test + num_net_devices = 10 + + net_metrics = FcDeviceMetrics("net", _validate_net_metrics, num_net_devices) + + # create more than 1 net devices to test aggregation + for _ in range(num_net_devices): + test_microvm.add_net_iface() + test_microvm.start() + + # check that the started microvm has "net" and "NUM_NET_DEVICES" number of "net_" metrics + net_metrics.validate(test_microvm) + + for i in range(num_net_devices): + # Test that network devices attached are operational. + # Verify if guest can run commands. + exit_code, _, _ = test_microvm.ssh_iface(i).run("sync") + # test that we get metrics while interacting with different interfaces + net_metrics.validate(test_microvm) + assert exit_code == 0