Add --metrics-path option

Signed-off-by: Pablo Barbáchano <[email protected]>

Author: pb8o

CHANGELOG.md

@@ -11,6 +11,9 @@
   making it safe to snapshot uVMs running on a newer host CPU (Cascade Lake)
   and restore on a host that has a Skylake CPU.
 
+- Added a new CLI option `--metrics-path PATH`. It accepts a file parameter
+  where metrics will be sent to.
+
 ### Fixed
 
 - Make the `T2` template more robust by explicitly disabling additional

docs/metrics.md

@@ -1,8 +1,11 @@
 # Firecracker Metrics Configuration
 
-For the metrics capability, Firecracker uses a single Metrics system.
-This system can be configured by sending a `PUT` API Request to the
-`/metrics` path.
+For the metrics capability, Firecracker uses a single Metrics system. This
+system can be configured either by: a) sending a `PUT` API Request to the
+`/metrics` path: or b) using the `--metrics-path` CLI option.
+
+Note the metrics configuration is **not** part of the guest configuration and is
+not restored from a snapshot.
 
 ## Prerequisites
 
@@ -17,7 +20,15 @@ mkfifo metrics.fifo
 touch metrics.file
 ```
 
-## Configuring the system
+## Configuring the system via CLI
+
+When launching Firecracker, use the CLI option to set the metrics file.
+
+```bash
+./firecracker --metrics-path metrics.fifo
+```
+
+## Configuring the system via API
 
 You can configure the Metrics system by sending the following API command:
 

src/api_server/swagger/firecracker.yaml

@@ -366,7 +366,7 @@ paths:
         204:
           description: Metrics system created.
         400:
-          description: Metrics system cannot be initialized due to bad input.
+          description: Metrics system cannot be initialized due to bad input request or metrics system already initialized.
           schema:
             $ref: "#/definitions/Error"
         default:

src/firecracker/src/main.rs

@@ -21,6 +21,7 @@ use vmm::signal_handler::register_signal_handlers;
 use vmm::version_map::{FC_VERSION_TO_SNAP_VERSION, VERSION_MAP};
 use vmm::vmm_config::instance_info::{InstanceInfo, VmState};
 use vmm::vmm_config::logger::{init_logger, LoggerConfig, LoggerLevel};
+use vmm::vmm_config::metrics::{init_metrics, MetricsConfig};
 use vmm::{EventManager, FcExitCode, HTTP_MAX_PAYLOAD_SIZE};
 
 // The reason we place default API socket under /run is that API socket is a
@@ -193,6 +194,11 @@ fn main_exitable() -> FcExitCode {
                     "Whether or not to include the file path and line number of the log's origin.",
                 ),
         )
+        .arg(
+            Argument::new("metrics-path")
+                .takes_value(true)
+                .help("Path to a fifo or a file used for configuring the metrics on startup."),
+        )
         .arg(Argument::new("boot-timer").takes_value(false).help(
             "Whether or not to load boot timer device for logging elapsed time since \
              InstanceStart command.",
@@ -289,7 +295,16 @@ fn main_exitable() -> FcExitCode {
             show_log_origin,
         );
         if let Err(err) = init_logger(logger_config, &instance_info) {
-            return generic_error_exit(&format!("Could not initialize logger:: {}", err));
+            return generic_error_exit(&format!("Could not initialize logger: {}", err));
+        };
+    }
+
+    if let Some(metrics_path) = arguments.single_value("metrics-path") {
+        let metrics_config = MetricsConfig {
+            metrics_path: PathBuf::from(metrics_path),
+        };
+        if let Err(err) = init_metrics(metrics_config) {
+            return generic_error_exit(&format!("Could not initialize metrics: {}", err));
         };
     }
 

src/logger/src/metrics.rs

@@ -9,7 +9,7 @@
 //! which we are capturing specific metrics.
 //!
 //! ## JSON example with metrics:
-//! ```bash
+//! ```json
 //! {
 //!  "utc_timestamp_ms": 1541591155180,
 //!  "api_server": {

tests/framework/microvm.py

@@ -18,6 +18,7 @@
 import shutil
 import time
 import weakref
+from pathlib import Path
 
 from threading import Lock
 from retry import retry
@@ -508,6 +509,7 @@ def spawn(
         log_file="log_fifo",
         log_level="Info",
         use_ramdisk=False,
+        metrics_path=None,
     ):
         """Start a microVM as a daemon or in a screen session."""
         # pylint: disable=subprocess-run-check
@@ -547,6 +549,11 @@ def spawn(
             self.jailer.extra_args.update({"log-path": log_file, "level": log_level})
             self.start_console_logger(log_fifo)
 
+        if metrics_path is not None:
+            self.create_jailed_resource(metrics_path, create_jail=True)
+            metrics_path = Path(metrics_path)
+            self.jailer.extra_args.update({"metrics-path": metrics_path.name})
+
         if self.metadata_file:
             if os.path.exists(self.metadata_file):
                 LOG.debug("metadata file exists, adding as a jailed resource")

tests/integration_tests/build/test_coverage.py

@@ -29,9 +29,9 @@
 # Checkout the cpuid crate. In the future other
 # differences may appear.
 if utils.is_io_uring_supported():
-    COVERAGE_DICT = {"Intel": 84.77, "AMD": 84.11, "ARM": 83.86}
+    COVERAGE_DICT = {"Intel": 84.77, "AMD": 84.11, "ARM": 83.78}
 else:
-    COVERAGE_DICT = {"Intel": 81.89, "AMD": 81.25, "ARM": 80.95}
+    COVERAGE_DICT = {"Intel": 81.89, "AMD": 81.25, "ARM": 80.86}
 
 PROC_MODEL = proc.proc_type()
 

tests/integration_tests/functional/test_cmd_line_parameters.py

@@ -4,7 +4,9 @@
 
 import logging
 import platform
+from pathlib import Path
 
+import host_tools.logging as log_tools
 from host_tools.cargo_build import get_firecracker_binaries
 from conftest import _test_images_s3_bucket
 from framework.artifacts import ArtifactCollection
@@ -80,3 +82,111 @@ def test_describe_snapshot_all_versions(bin_cloner_path):
         assert code == 0
         assert stderr == ""
         assert target_version in stdout
+
+
+def test_cli_metrics_path(test_microvm_with_api):
+    """
+    Test --metrics-path parameter
+
+    @type: functional
+    """
+    microvm = test_microvm_with_api
+    metrics_fifo_path = Path(microvm.path) / "metrics_ndjson.fifo"
+    metrics_fifo = log_tools.Fifo(metrics_fifo_path)
+    microvm.spawn(metrics_path=metrics_fifo_path)
+    microvm.basic_config()
+    microvm.start()
+
+    metrics = microvm.flush_metrics(metrics_fifo)
+
+    exp_keys = [
+        "utc_timestamp_ms",
+        "api_server",
+        "balloon",
+        "block",
+        "deprecated_api",
+        "get_api_requests",
+        "i8042",
+        "latencies_us",
+        "logger",
+        "mmds",
+        "net",
+        "patch_api_requests",
+        "put_api_requests",
+        "seccomp",
+        "vcpu",
+        "vmm",
+        "uart",
+        "signals",
+        "vsock",
+    ]
+
+    if platform.machine() == "aarch64":
+        exp_keys.append("rtc")
+
+    assert set(metrics.keys()) == set(exp_keys)
+
+
+def test_cli_metrics_path_if_metrics_initialized_twice_fail(test_microvm_with_api):
+    """
+    Given: a running firecracker with metrics configured with the CLI option
+    When: Configure metrics via API
+    Then: API returns an error
+
+    @type: functional
+    """
+    microvm = test_microvm_with_api
+
+    # First configure the µvm metrics with --metrics-path
+    metrics_path = Path(microvm.path) / "metrics.ndjson"
+    metrics_path.touch()
+    microvm.spawn(metrics_path=metrics_path)
+
+    # Then try to configure it with PUT /metrics
+    metrics2_path = Path(microvm.path) / "metrics2.ndjson"
+    metrics2_path.touch()
+    response = microvm.metrics.put(
+        metrics_path=microvm.create_jailed_resource(metrics2_path)
+    )
+
+    # It should fail with HTTP 400 because it's already configured
+    assert response.status_code == 400
+    assert response.json() == {
+        "fault_message": "Reinitialization of metrics not allowed."
+    }
+
+
+def test_cli_metrics_if_resume_no_metrics(test_microvm_with_api, microvm_factory):
+    """
+    Check that metrics configuration is not part of the snapshot
+
+    @type: functional
+    """
+    # Given: a snapshot of a FC with metrics configured with the CLI option
+    uvm1 = test_microvm_with_api
+    metrics_path = Path(uvm1.path) / "metrics.ndjson"
+    metrics_path.touch()
+    uvm1.spawn(metrics_path=metrics_path)
+    uvm1.basic_config()
+    uvm1.start()
+
+    mem_path = Path(uvm1.jailer.chroot_path()) / "test.mem"
+    snapshot_path = Path(uvm1.jailer.chroot_path()) / "test.snap"
+    uvm1.pause_to_snapshot(
+        mem_file_path=mem_path.name,
+        snapshot_path=snapshot_path.name,
+    )
+    assert mem_path.exists()
+
+    # When: restoring from the snapshot
+    uvm2 = microvm_factory.build()
+    uvm2.spawn()
+    uvm2.restore_from_snapshot(
+        snapshot_vmstate=snapshot_path,
+        snapshot_mem=mem_path,
+        snapshot_disks=[uvm1.rootfs_file],
+    )
+
+    # Then: the old metrics configuration does not exist
+    metrics2 = Path(uvm2.jailer.chroot_path()) / metrics_path.name
+    assert not metrics2.exists()