Repository: firecracker-microvm/firecracker
File: src/vmm/src/logger/metrics.rs (Rust)

Code before:
//! There are a number of advantages to this approach, including:
//! * We don't have to introduce an additional write (to reset the value) from the thread which does
//!   to actual writing, so less synchronization effort is required.
//! * We don't have to worry at all that much about losing some data if writing fails for a while
//!   (this could be a concern, I guess).
//! If if turns out this approach is not really what we want, it's pretty easy to resort to
//! something else, while working behind the same interface.

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 super::FcLineWriter;
use crate::devices::legacy;
use crate::devices::virtio::balloon::metrics as balloon_metrics;
use crate::devices::virtio::net::metrics as net_metrics;
use crate::devices::virtio::rng::metrics as entropy_metrics;
use crate::devices::virtio::vhost_user_metrics;
use crate::devices::virtio::virtio_block::metrics as block_metrics;
use crate::devices::virtio::vsock::metrics as vsock_metrics;

#[macro_export]
/// records aggregate (min/max/sum) for the given $metrics
/// This macro captures delta between start_time and current time
/// and updates min/max/sum metrics so it expects that start_time
/// is captured at the appropriate location.
macro_rules! record_latency_summary {
    ($metrics:expr, $start_time:ident) => {
        let delta_us = utils::time::get_time_us(utils::time::ClockType::Monotonic) - $start_time;
        $metrics.sum_us.add(delta_us);
        let min_us = $metrics.min_us.fetch();
        let max_us = $metrics.max_us.fetch();
        if (0 == min_us) || (min_us > delta_us) {
            $metrics.min_us.store(delta_us);
        }
        if (0 == max_us) || (max_us < delta_us) {
            $metrics.max_us.store(delta_us);
        }
    };
}

/// Static instance used for handling metrics.
pub static METRICS: Metrics<FirecrackerMetrics, FcLineWriter> =
    Metrics::<FirecrackerMetrics, FcLineWriter>::new(FirecrackerMetrics::new());

/// Metrics system.
// All member fields have types which are Sync, and exhibit interior mutability, so

Review comment (performance):
`#[macro_export]` exports macro for the whole crate. Instead it would be better to use `pub(crate) use record_latency_summary`. This way macro will be a part of the module and can be imported using normal `use logger::metrics::record_latency_summary` syntax.

Code after:
//! There are a number of advantages to this approach, including:
//! * We don't have to introduce an additional write (to reset the value) from the thread which does
//!   to actual writing, so less synchronization effort is required.
//! * We don't have to worry at all that much about losing some data if writing fails for a while
//!   (this could be a concern, I guess).
//! If if turns out this approach is not really what we want, it's pretty easy to resort to
//! something else, while working behind the same interface.

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 super::FcLineWriter;
use crate::devices::legacy;
use crate::devices::virtio::balloon::metrics as balloon_metrics;
use crate::devices::virtio::net::metrics as net_metrics;
use crate::devices::virtio::rng::metrics as entropy_metrics;
use crate::devices::virtio::vhost_user_metrics;
use crate::devices::virtio::virtio_block::metrics as block_metrics;
use crate::devices::virtio::vsock::metrics as vsock_metrics;

/// Static instance used for handling metrics.
pub static METRICS: Metrics<FirecrackerMetrics, FcLineWriter> =
    Metrics::<FirecrackerMetrics, FcLineWriter>::new(FirecrackerMetrics::new());

/// Metrics system.
// All member fields have types which are Sync, and exhibit interior mutability, so
// we can call operations on metrics using a non-mut static global variable.
#[derive(Debug)]
pub struct Metrics<T: Serialize, M: Write + Send> {
    // Metrics will get flushed here.
    metrics_buf: OnceLock<Mutex<M>>,
    pub app_metrics: T,
}

impl<T: Serialize + Debug, M: Write + Send + Debug> Metrics<T, M> {
    /// Creates a new instance of the current metrics.
    // TODO: We need a better name than app_metrics (something that says that these are the actual
    // values that we are writing to the metrics_buf).
    pub const fn new(app_metrics: T) -> Metrics<T, M> {
        Metrics {
            metrics_buf: OnceLock::new(),
            app_metrics,
        }
    }

    /// Initialize metrics system (once and only once).