From e9f000d68082657def986becaed448f80c63f4cc Mon Sep 17 00:00:00 2001 From: Giles Hutton Date: Wed, 15 Apr 2026 13:58:35 +0100 Subject: [PATCH 1/3] Adds metrics helper test fixture --- tests/conftest.py | 8 +++ tests/metrics.py | 112 +++++++++++++++++++++++++++++++++++++++ tests/test_rate_limit.py | 37 +++---------- 3 files changed, 127 insertions(+), 30 deletions(-) create mode 100644 tests/metrics.py diff --git a/tests/conftest.py b/tests/conftest.py index cce6a03f..f211f70d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,6 +8,7 @@ import requests import yaml +from metrics import MetricsClient from server import FileActivityService @@ -132,6 +133,13 @@ def fact_config(request, monitored_dir, logs_dir): config_file.close() +@pytest.fixture +def metrics(fact_config): + """Client for taking metrics snapshots from the FACT endpoint.""" + config, _ = fact_config + return MetricsClient(config['endpoint']['address']) + + @pytest.fixture def test_container(request, docker_client, monitored_dir, ignored_dir): """ diff --git a/tests/metrics.py b/tests/metrics.py new file mode 100644 index 00000000..1e9807b2 --- /dev/null +++ b/tests/metrics.py @@ -0,0 +1,112 @@ +import re + +import requests + + +class MetricsSnapshot: + """ + A parsed snapshot of Prometheus/OpenMetrics metrics. + + Supports querying by metric name and labels: + + ss = metrics.snapshot() + assert ss.get("rate_limiter_events", label="Dropped") == 5 + assert ss.get("bpf_events", label="Added") > 0 + + Metric names are matched without the "stackrox_fact_" prefix and + "_total" counter suffix, so "rate_limiter_events" matches + "stackrox_fact_rate_limiter_events_total". + """ + + _PREFIX = "stackrox_fact_" + _TOTAL_SUFFIX = "_total" + _LINE_RE = re.compile( + r'^(?P\S+?)(?:\{(?P[^}]*)\})?\s+(?P\S+)$' + ) + _LABEL_RE = re.compile(r'(\w+)="([^"]*)"') + + def __init__(self, text): + self._entries = [] + for line in text.splitlines(): + if line.startswith('#') or not line.strip(): + continue + + m = self._LINE_RE.match(line) + if not m: + continue + + name, raw, labels = m.group('name', 'value', 'labels') + + value = float(raw) if '.' in raw else int(raw) + labels = dict(self._LABEL_RE.findall(labels or '')) + + self._entries.append((name, labels, value)) + + @classmethod + def _normalize(cls, name): + if name.startswith(cls._PREFIX): + name = name[len(cls._PREFIX):] + if name.endswith(cls._TOTAL_SUFFIX): + name = name[:-len(cls._TOTAL_SUFFIX)] + return name + + def get(self, metric, **labels): + """ + Get the value of a metric, optionally filtered by labels. + + Args: + metric: Metric name, with or without the "stackrox_fact_" + prefix and "_total" suffix. + **labels: Label key=value pairs to match. + + Returns: + The metric value as int or float. + + Raises: + KeyError: If no matching metric is found. + ValueError: If multiple metrics match. + """ + target = self._normalize(metric) + matches = [] + for name, entry_labels, value in self._entries: + if self._normalize(name) != target: + continue + if all(entry_labels.get(k) == v for k, v in labels.items()): + matches.append(value) + + if not matches: + label_desc = ', '.join(f'{k}="{v}"' for k, v in labels.items()) + key = f'{metric}{{{label_desc}}}' if label_desc else metric + available = '\n '.join( + f'{n} {ls} = {v}' for n, ls, v in self._entries + ) + raise KeyError( + f'metric {key!r} not found. Available:\n {available}' + ) + if len(matches) > 1: + raise ValueError( + f'{metric} matched {len(matches)} entries; use labels to ' + f'narrow the result' + ) + return matches[0] + + def get_all(self, metric, **labels): + """Like get(), but returns a list of all matching values.""" + target = self._normalize(metric) + return [ + value for name, entry_labels, value in self._entries + if self._normalize(name) == target + and all(entry_labels.get(k) == v for k, v in labels.items()) + ] + + +class MetricsClient: + """Fetches metrics snapshots from a FACT endpoint.""" + + def __init__(self, address): + self._url = f'http://{address}/metrics' + + def snapshot(self, timeout=30): + resp = requests.get(self._url, timeout=timeout) + resp.raise_for_status() + return MetricsSnapshot(resp.text) diff --git a/tests/test_rate_limit.py b/tests/test_rate_limit.py index 080606f7..f566fb31 100644 --- a/tests/test_rate_limit.py +++ b/tests/test_rate_limit.py @@ -3,11 +3,11 @@ from time import sleep import pytest -import requests import yaml from event import Event, EventType, Process + @pytest.fixture def rate_limited_config(fact, fact_config, monitored_dir): """ @@ -23,11 +23,10 @@ def rate_limited_config(fact, fact_config, monitored_dir): sleep(0.1) return config, config_file -def test_rate_limit_drops_events(rate_limited_config, monitored_dir, server): +def test_rate_limit_drops_events(rate_limited_config, monitored_dir, server, metrics): """ Test that the rate limiter drops events when the rate limit is exceeded. """ - config, _ = rate_limited_config num_files = 100 start_time = time.time() @@ -51,19 +50,8 @@ def test_rate_limit_drops_events(rate_limited_config, monitored_dir, server): assert received_count < num_files, \ f'Expected rate limiting to drop some events, but received all {received_count}' - metrics_response = requests.get(f'http://{config["endpoint"]["address"]}/metrics') - assert metrics_response.status_code == 200 - - metrics_text = metrics_response.text - assert 'rate_limiter_events' in metrics_text, 'rate_limiter_events metric not found' - - dropped_count = 0 - for line in metrics_text.split('\n'): - if 'rate_limiter_events' in line and 'label="Dropped"' in line: - parts = line.split() - if len(parts) >= 2: - dropped_count = int(parts[1]) - break + ss = metrics.snapshot() + dropped_count = ss.get("rate_limiter_events", label="Dropped") assert dropped_count > 0, 'Expected rate limiter to report dropped events in metrics' @@ -71,11 +59,10 @@ def test_rate_limit_drops_events(rate_limited_config, monitored_dir, server): assert total_accounted == num_files, 'Expected rate limiter to see all events' -def test_rate_limit_unlimited(monitored_dir, server, fact_config): +def test_rate_limit_unlimited(monitored_dir, server, metrics): """ Test that the default config (rate_limit=0) allows all events through. """ - config, _ = fact_config num_files = 20 events = [] process = Process.from_proc() @@ -90,18 +77,8 @@ def test_rate_limit_unlimited(monitored_dir, server, fact_config): server.wait_events(events) - metrics_response = requests.get(f'http://{config["endpoint"]["address"]}/metrics') - assert metrics_response.status_code == 200 - - metrics_text = metrics_response.text - - dropped_count = 0 - for line in metrics_text.split('\n'): - if 'rate_limiter_events' in line and 'label="Dropped"' in line: - parts = line.split() - if len(parts) >= 2: - dropped_count = int(parts[1]) - break + ss = metrics.snapshot() + dropped_count = ss.get("rate_limiter_events", label="Dropped") assert dropped_count == 0, \ f'Expected no dropped events with unlimited rate limiting, but got {dropped_count}' From a408b2a6da7eb9a0450ae936ec293f85a1aa10fc Mon Sep 17 00:00:00 2001 From: Giles Hutton Date: Thu, 30 Apr 2026 15:51:59 +0100 Subject: [PATCH 2/3] Use prometheus_client parser and address review feedback Replace custom regex parser with prometheus_client.parser, apply removeprefix/removesuffix cleanup, and add request timeout. --- tests/metrics.py | 30 +++++------------------------- tests/requirements.txt | 1 + 2 files changed, 6 insertions(+), 25 deletions(-) diff --git a/tests/metrics.py b/tests/metrics.py index 1e9807b2..114a870e 100644 --- a/tests/metrics.py +++ b/tests/metrics.py @@ -1,6 +1,5 @@ -import re - import requests +from prometheus_client.parser import text_string_to_metric_families class MetricsSnapshot: @@ -20,35 +19,16 @@ class MetricsSnapshot: _PREFIX = "stackrox_fact_" _TOTAL_SUFFIX = "_total" - _LINE_RE = re.compile( - r'^(?P\S+?)(?:\{(?P[^}]*)\})?\s+(?P\S+)$' - ) - _LABEL_RE = re.compile(r'(\w+)="([^"]*)"') def __init__(self, text): self._entries = [] - for line in text.splitlines(): - if line.startswith('#') or not line.strip(): - continue - - m = self._LINE_RE.match(line) - if not m: - continue - - name, raw, labels = m.group('name', 'value', 'labels') - - value = float(raw) if '.' in raw else int(raw) - labels = dict(self._LABEL_RE.findall(labels or '')) - - self._entries.append((name, labels, value)) + for family in text_string_to_metric_families(text): + for sample in family.samples: + self._entries.append((sample.name, sample.labels, sample.value)) @classmethod def _normalize(cls, name): - if name.startswith(cls._PREFIX): - name = name[len(cls._PREFIX):] - if name.endswith(cls._TOTAL_SUFFIX): - name = name[:-len(cls._TOTAL_SUFFIX)] - return name + return name.removeprefix(cls._PREFIX).removesuffix(cls._TOTAL_SUFFIX) def get(self, metric, **labels): """ diff --git a/tests/requirements.txt b/tests/requirements.txt index 61679796..6300a48c 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,6 +1,7 @@ docker==7.1.0 grpcio==1.76.0 grpcio-tools==1.76.0 +prometheus-client==0.22.1 pytest==8.4.1 requests==2.32.4 pyyaml==6.0.3 From 0cfa9328474e0222c54f46c26563be027aec90d8 Mon Sep 17 00:00:00 2001 From: Giles Hutton Date: Thu, 30 Apr 2026 15:55:59 +0100 Subject: [PATCH 3/3] remove get_metric_value in favor of metrics fixture --- tests/test_path_rmdir.py | 93 +++++++++++++++------------------------- tests/utils.py | 34 --------------- 2 files changed, 35 insertions(+), 92 deletions(-) diff --git a/tests/test_path_rmdir.py b/tests/test_path_rmdir.py index b834285c..860a2b0a 100644 --- a/tests/test_path_rmdir.py +++ b/tests/test_path_rmdir.py @@ -4,39 +4,16 @@ import pytest from event import Event, EventType, Process -from utils import get_metric_value -def get_inode_removed_count(fact_config): - """ - Query Prometheus metrics to get the count of removed inodes. - - Args: - fact_config: The fact configuration tuple (config dict, config file path). - - Returns: - The current value of host_scanner_scan{label="InodeRemoved"} metric. - """ - value = get_metric_value(fact_config, "host_scanner_scan", {"label": "InodeRemoved"}) - return int(value) if value is not None else 0 - - -def get_kernel_rmdir_processed(fact_config): - """ - Query Prometheus metrics to get the count of processed (non-ignored) rmdir events. - - Args: - fact_config: The fact configuration tuple (config dict, config file path). - - Returns: - The difference between Total and Ignored kernel_path_rmdir_events. - """ - total_str = get_metric_value(fact_config, "kernel_path_rmdir_events", {"label": "Total"}) - ignored_str = get_metric_value(fact_config, "kernel_path_rmdir_events", {"label": "Ignored"}) +def get_inode_removed_count(metrics): + return metrics.snapshot().get("host_scanner_scan", label="InodeRemoved") - total = int(total_str) if total_str is not None else 0 - ignored = int(ignored_str) if ignored_str is not None else 0 +def get_kernel_rmdir_processed(metrics): + ss = metrics.snapshot() + total = ss.get("kernel_path_rmdir_events", label="Total") + ignored = ss.get("kernel_path_rmdir_events", label="Ignored") return total - ignored @@ -46,7 +23,7 @@ def get_kernel_rmdir_processed(fact_config): pytest.param('файл', id='Cyrillic'), pytest.param('日本語', id='Japanese'), ]) -def test_rmdir_empty(monitored_dir, server, fact_config, dirname): +def test_rmdir_empty(monitored_dir, server, metrics, dirname): """ Tests that removing an empty directory properly cleans up inode tracking. @@ -60,14 +37,14 @@ def test_rmdir_empty(monitored_dir, server, fact_config, dirname): Args: monitored_dir: Temporary directory path for creating the test directory. server: The server instance to communicate with. - fact_config: The fact configuration. + metrics: The metrics client fixture. dirname: Directory name to test (including UTF-8 variants). """ process = Process.from_proc() # Get baseline metric counts - initial_inode_removed = get_inode_removed_count(fact_config) - initial_kernel_rmdir = get_kernel_rmdir_processed(fact_config) + initial_inode_removed = get_inode_removed_count(metrics) + initial_kernel_rmdir = get_kernel_rmdir_processed(metrics) # Create a directory test_dir = os.path.join(monitored_dir, dirname) @@ -94,7 +71,7 @@ def test_rmdir_empty(monitored_dir, server, fact_config, dirname): server.wait_events([e2]) # Check that file deletion incremented the metric by exactly 1 - count_after_file = get_inode_removed_count(fact_config) + count_after_file = get_inode_removed_count(metrics) file_delta = count_after_file - initial_inode_removed assert file_delta == 1, \ f"Expected exactly 1 inode removed for file deletion, got {file_delta}" @@ -103,8 +80,8 @@ def test_rmdir_empty(monitored_dir, server, fact_config, dirname): os.rmdir(test_dir) # Check metrics after directory deletion - final_inode_removed = get_inode_removed_count(fact_config) - final_kernel_rmdir = get_kernel_rmdir_processed(fact_config) + final_inode_removed = get_inode_removed_count(metrics) + final_kernel_rmdir = get_kernel_rmdir_processed(metrics) inode_delta = final_inode_removed - initial_inode_removed kernel_delta = final_kernel_rmdir - initial_kernel_rmdir @@ -115,7 +92,7 @@ def test_rmdir_empty(monitored_dir, server, fact_config, dirname): f"Expected exactly 1 kernel rmdir event processed, got {kernel_delta}" -def test_rmdir_recursive(monitored_dir, server, fact_config): +def test_rmdir_recursive(monitored_dir, server, metrics): """ Tests that removing a directory tree recursively cleans up all inode tracking. @@ -130,13 +107,13 @@ def test_rmdir_recursive(monitored_dir, server, fact_config): Args: monitored_dir: Temporary directory path for creating test directories. server: The server instance to communicate with. - fact_config: The fact configuration. + metrics: The metrics client fixture. """ process = Process.from_proc() # Get baseline metric counts - initial_inode_removed = get_inode_removed_count(fact_config) - initial_kernel_rmdir = get_kernel_rmdir_processed(fact_config) + initial_inode_removed = get_inode_removed_count(metrics) + initial_kernel_rmdir = get_kernel_rmdir_processed(metrics) # Create nested directories level1 = os.path.join(monitored_dir, 'level1') @@ -186,8 +163,8 @@ def test_rmdir_recursive(monitored_dir, server, fact_config): server.wait_events(unlink_events) # Check that all inodes and kernel events were tracked - final_inode_removed = get_inode_removed_count(fact_config) - final_kernel_rmdir = get_kernel_rmdir_processed(fact_config) + final_inode_removed = get_inode_removed_count(metrics) + final_kernel_rmdir = get_kernel_rmdir_processed(metrics) inode_delta = final_inode_removed - initial_inode_removed kernel_delta = final_kernel_rmdir - initial_kernel_rmdir @@ -198,7 +175,7 @@ def test_rmdir_recursive(monitored_dir, server, fact_config): f"Expected exactly 3 kernel rmdir events processed, got {kernel_delta}" -def test_rmdir_ignored(monitored_dir, ignored_dir, server, fact_config): +def test_rmdir_ignored(monitored_dir, ignored_dir, server, metrics): """ Tests that directories removed outside monitored paths don't affect tracking. @@ -208,13 +185,13 @@ def test_rmdir_ignored(monitored_dir, ignored_dir, server, fact_config): monitored_dir: Temporary directory path that is monitored. ignored_dir: Temporary directory path that is not monitored. server: The server instance to communicate with. - fact_config: The fact configuration. + metrics: The metrics client fixture. """ process = Process.from_proc() # Get baseline metric counts - initial_inode_removed = get_inode_removed_count(fact_config) - initial_kernel_rmdir = get_kernel_rmdir_processed(fact_config) + initial_inode_removed = get_inode_removed_count(metrics) + initial_kernel_rmdir = get_kernel_rmdir_processed(metrics) # Create directory in ignored path ignored_subdir = os.path.join(ignored_dir, 'ignored_subdir') @@ -228,8 +205,8 @@ def test_rmdir_ignored(monitored_dir, ignored_dir, server, fact_config): os.rmdir(ignored_subdir) # Metrics should not have changed - inode_after_ignored = get_inode_removed_count(fact_config) - kernel_after_ignored = get_kernel_rmdir_processed(fact_config) + inode_after_ignored = get_inode_removed_count(metrics) + kernel_after_ignored = get_kernel_rmdir_processed(metrics) assert inode_after_ignored == initial_inode_removed, \ f"Ignored path operations should not increment inode_removed metric" assert kernel_after_ignored == initial_kernel_rmdir, \ @@ -260,8 +237,8 @@ def test_rmdir_ignored(monitored_dir, ignored_dir, server, fact_config): server.wait_events(deletion_events) # Metrics should have incremented by exactly 2 inodes and 1 kernel rmdir - final_inode_removed = get_inode_removed_count(fact_config) - final_kernel_rmdir = get_kernel_rmdir_processed(fact_config) + final_inode_removed = get_inode_removed_count(metrics) + final_kernel_rmdir = get_kernel_rmdir_processed(metrics) inode_delta = final_inode_removed - initial_inode_removed kernel_delta = final_kernel_rmdir - initial_kernel_rmdir @@ -272,7 +249,7 @@ def test_rmdir_ignored(monitored_dir, ignored_dir, server, fact_config): f"Expected exactly 1 kernel rmdir event processed, got {kernel_delta}" -def test_rmdir_with_parent_inode(monitored_dir, server, fact_config): +def test_rmdir_with_parent_inode(monitored_dir, server, metrics): """ Tests that directory deletion properly handles parent inode relationships. @@ -282,13 +259,13 @@ def test_rmdir_with_parent_inode(monitored_dir, server, fact_config): Args: monitored_dir: Temporary directory path for creating test directories. server: The server instance to communicate with. - fact_config: The fact configuration. + metrics: The metrics client fixture. """ process = Process.from_proc() # Get baseline metric counts - initial_inode_removed = get_inode_removed_count(fact_config) - initial_kernel_rmdir = get_kernel_rmdir_processed(fact_config) + initial_inode_removed = get_inode_removed_count(metrics) + initial_kernel_rmdir = get_kernel_rmdir_processed(metrics) # Create a subdirectory subdir = os.path.join(monitored_dir, 'subdir') @@ -325,8 +302,8 @@ def test_rmdir_with_parent_inode(monitored_dir, server, fact_config): server.wait_events(deletion_events) # Check metrics incremented (file + subdir) - inode_after_subdir = get_inode_removed_count(fact_config) - kernel_after_subdir = get_kernel_rmdir_processed(fact_config) + inode_after_subdir = get_inode_removed_count(metrics) + kernel_after_subdir = get_kernel_rmdir_processed(metrics) inode_delta_subdir = inode_after_subdir - initial_inode_removed kernel_delta_subdir = kernel_after_subdir - initial_kernel_rmdir @@ -356,8 +333,8 @@ def test_rmdir_with_parent_inode(monitored_dir, server, fact_config): # Final metric check: should be 3 total inodes (test_file, subdir, new_file) # and 1 total kernel rmdir (subdir) - final_inode_removed = get_inode_removed_count(fact_config) - final_kernel_rmdir = get_kernel_rmdir_processed(fact_config) + final_inode_removed = get_inode_removed_count(metrics) + final_kernel_rmdir = get_kernel_rmdir_processed(metrics) inode_total_delta = final_inode_removed - initial_inode_removed kernel_total_delta = final_kernel_rmdir - initial_kernel_rmdir diff --git a/tests/utils.py b/tests/utils.py index 684632fc..a2d58538 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,8 +1,6 @@ import os import re -import requests - def join_path_with_filename(directory, filename): """ @@ -73,35 +71,3 @@ def rust_style_join(args): args: The string to quote """ return ' '.join(rust_style_quote(arg) for arg in args) - - -def get_metric_value(fact_config, metric_name, labels=None): - """ - Query Prometheus metrics endpoint to get the value of a metric. - - Args: - fact_config: The fact configuration tuple (config dict, config file path). - metric_name: Name of the metric to query (e.g., "host_scanner_scan"). - labels: Optional dict of label filters (e.g., {"label": "InodeRemoved"}). - - Returns: - The metric value as a string if found, None otherwise. - """ - config, _ = fact_config - response = requests.get(f'http://{config["endpoint"]["address"]}/metrics') - assert response.status_code == 200 - - labels = labels or {} - - for line in response.text.split('\n'): - if metric_name not in line: - continue - - # Check if all label filters match - if all(f'{k}="{v}"' in line for k, v in labels.items()): - # Format: metric_name{label="value"} 42 - parts = line.split() - if len(parts) >= 2: - return parts[-1] - - return None