test: add test to ensure we do not have unused metrics

Add a test that fails if there are metrics that are never incremented in
production code (e.g. outside of aggregation code and constructors).

Closes #5182

Co-authored-by: Milan Dhaduk <mdhaduk7@gmail.com>
Signed-off-by: Milan Dhaduk <mdhaduk7@gmail.com>
Co-authored-by: LDagnachew <leulmdagnachew@gmail.com>
Signed-off-by: LDagnachew <leulmdagnachew@gmail.com>
Signed-off-by: Patrick Roy <roypat@amazon.co.uk>

Author: 3 people

tests/host_tools/fcmetrics.py

@@ -18,7 +18,9 @@
 import jsonschema
 import pytest
 
+from framework import utils
 from framework.properties import global_props
+from framework.utils_repo import git_repo_files
 from host_tools.metrics import get_metrics_logger
 
 
@@ -563,3 +565,61 @@ def run(self):
                 time.sleep(1)
                 if self.running is False:
                     break
+
+
+def find_metrics_files():
+    """Gets a list of all Firecracker sources files ending with 'metrics.rs'"""
+    return list(git_repo_files(root="..", glob="*metrics.rs"))
+
+
+def extract_fields(file_path):
+    """Gets a list of all metrics defined in the given file, in the form tuples (name, type)"""
+    fields = utils.run_cmd(
+        rf'grep -Po "(?<=pub )(\w+): (Shared(?:Inc|Store)Metric|LatencyAggregateMetrics)" {file_path}'
+    ).stdout.strip()
+
+    return [field.split(": ", maxsplit=1) for field in fields.splitlines()]
+
+
+def is_file_production(filepath):
+    """Returns True iff accesses to metric fields in the given file should cause the metric be considered 'used in production code'. Excludes, for example, files in which the metrics are defined, where accesses happen as part of copy constructors, etc."""
+    path = filepath.lower()
+    return (
+        "/test/" in path
+        or "/tests/" in path
+        or path.endswith("_test.rs")
+        or "test_" in path
+        or "tests.rs" in path
+        or ("metrics.rs" in path and "vmm" in path)
+    )
+
+
+KNOWN_FALSE_POSITIVES = [
+    "min_us",
+    "max_us",
+    "sum_us",
+    "process_startup_time_us",
+    "process_startup_time_cpu_us",
+]
+
+
+def is_metric_used(field, field_type):
+    """Returns True iff the given metric has a production use in the firecracker codebase"""
+    if field in KNOWN_FALSE_POSITIVES:
+        return True
+
+    if field_type in ("SharedIncMetric", "SharedStoreMetric"):
+        pattern = rf"{field}\s*\.\s*store|{field}\s*\.\s*inc|{field}\s*\.\s*add|{field}\s*\.\s*fetch|METRICS.*{field}"
+    elif field_type == "LatencyAggregateMetrics":
+        pattern = rf"{field}\s*\.\s*record_latency_metrics"
+    else:
+        raise RuntimeError(f"Unknown metric type: {field_type}")
+
+    result = utils.run_cmd(f'grep -RPzo "{pattern}" ../src')
+
+    for line in result.stdout.strip().split("\0"):
+        if not line:
+            continue
+        if not is_file_production(line.split(":", maxsplit=1)[0]):
+            return True
+    return False

tests/integration_tests/style/test_rust.py

@@ -1,8 +1,10 @@
 # Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 # SPDX-License-Identifier: Apache-2.0
 """Tests ensuring codebase style compliance for Rust."""
+from collections import defaultdict
 
 from framework import utils
+from host_tools.fcmetrics import extract_fields, find_metrics_files, is_metric_used
 
 
 def test_rust_order():
@@ -24,3 +26,34 @@ def test_rust_style():
 
     # rustfmt prepends `"Diff in"` to the reported output.
     assert "Diff in" not in stdout
+
+
+def test_unused_metrics():
+    """Tests that all metrics defined in Firecracker's metrics.rs files actually have code
+    paths that increment them."""
+    metrics_files = find_metrics_files()
+    unused = defaultdict(list)
+
+    assert metrics_files
+
+    for file_path in metrics_files:
+        fields = extract_fields(file_path)
+        if not fields:
+            continue
+
+        for field, ty in fields:
+            if not is_metric_used(field, ty):
+                unused[file_path].append((field, ty))
+
+    # Grouped output
+    for file_path, fields in unused.items():
+        print(f"📄 Defined in: {file_path}")
+        print("Possibly Unused: \n")
+        for field, field_type in fields:
+            print(f"   ❌ {field} ({field_type})")
+
+        print()
+
+    assert (
+        not unused
+    ), "Unused metrics founds, see stdout. Please either hook them up, or remove them"