fix(client): forward run_id into ArgusGenericClient, uuid-harden replay log filenames

ArgusGenericClient (used by pytest-argus-reporter, and therefore dtest)
never accepted or forwarded run_id to ArgusClient, unlike ArgusSCTClient
and DriverMatrixTestsClient. Every replay log it created fell back to
the "unknown" run-id placeholder regardless of ARGUS_RUN_ID. The
ArgusReporter.argus_client cached_property also didn't pass run_id
through, so both call sites needed the fix.

Also replace the pid+process-counter uniqueness scheme in ReplayLog's
filename suffix with a uuid4 component, since pid-based uniqueness
doesn't hold when log_dir is a mount shared across hosts/containers
with independent pid namespaces. Clamp the sanitized run_id to 64
chars to bound filename length.

Author: CodeLieutenant

argus/client/generic/client.py

@@ -1,4 +1,6 @@
 import logging
+from uuid import UUID
+
 from argus.client.base import ArgusClient
 
 
@@ -14,10 +16,11 @@ class Routes(ArgusClient.Routes):
 
     def __init__(self, auth_token: str, base_url: str, log_dir, api_version="v1",
                  extra_headers: dict | None = None, timeout: int = 180, max_retries: int = 3,
-                 use_tunnel: bool | None = None, replay_log_only: bool = False) -> None:
+                 use_tunnel: bool | None = None, replay_log_only: bool = False,
+                 run_id: UUID | str | None = None) -> None:
         super().__init__(auth_token, base_url, log_dir=log_dir, api_version=api_version,
                          extra_headers=extra_headers, timeout=timeout, max_retries=max_retries,
-                         use_tunnel=use_tunnel, replay_log_only=replay_log_only)
+                         use_tunnel=use_tunnel, replay_log_only=replay_log_only, run_id=run_id)
 
     def submit_generic_run(self, build_id: str, run_id: str, started_by: str, build_url: str, sub_type: str = None, scylla_version: str | None = None):
         request_body = {

argus/client/replay_log.py

@@ -21,19 +21,17 @@
 
 import atexit
 import functools
-import itertools
 import json
 import logging
 import os
 import re
 import threading
 import time
+import uuid
 import weakref
 from pathlib import Path
 from typing import IO
 
-_instance_counter = itertools.count()
-
 LOGGER = logging.getLogger(__name__)
 
 # Allow only characters that are unambiguously safe inside a filename.
@@ -94,12 +92,13 @@ def __init__(
         run_id: str | None = None,
         test_type: str | None = None,
     ) -> None:
-        safe_run_id = _sanitize_for_filename(run_id or "unknown")
+        safe_run_id = _sanitize_for_filename(run_id or "unknown")[:64]
         log_dir_path = Path(log_dir)
-        # Nanosecond clock + pid + process-wide counter guarantees uniqueness
-        # across parallel processes and back-to-back instantiation, even when
-        # the system clock has coarser-than-nanosecond resolution.
-        suffix = f"{_now_ns()}_{os.getpid()}_{next(_instance_counter)}"
+        # A uuid4 guarantees uniqueness on its own, so collisions can't happen
+        # even when ``log_dir`` is a mount shared across hosts/containers where
+        # pids and clocks aren't comparable. The timestamp and pid are kept
+        # only to make the filename sortable/debuggable, not for uniqueness.
+        suffix = f"{_now_ns()}_{os.getpid()}_{uuid.uuid4().hex}"
         self._path: Path = log_dir_path / f"argus_replay_log_{safe_run_id}_{suffix}.jsonl"
         self._test_type: str = test_type or "unknown"
         self._lock = threading.Lock()

argus/client/tests/test_replay_log.py

@@ -522,3 +522,21 @@ def test_argus_client_replay_log_path_includes_run_id_when_set(tmp_path):
     assert str(run_id) in client.replay_log_path.name
     assert "scylla-cluster-tests" not in client.replay_log_path.name  # test_type isn't in filename
     client.close()
+
+
+def test_argus_generic_client_replay_log_path_includes_run_id_when_set(tmp_path):
+    # Regression test: ArgusGenericClient used to drop run_id entirely, so
+    # every replay log fell back to "unknown" regardless of what pytest-argus-
+    # reporter (or any other caller) passed in.
+    from argus.client.generic.client import ArgusGenericClient
+    run_id = uuid4()
+    client = ArgusGenericClient(
+        run_id=run_id,
+        auth_token="t",
+        base_url="https://test.example.com",
+        log_dir=tmp_path,
+        replay_log_only=True,
+    )
+    assert str(run_id) in client.replay_log_path.name
+    assert "unknown" not in client.replay_log_path.name
+    client.close()

pytest-argus-reporter/pytest_argus_reporter.py

@@ -215,6 +215,7 @@ def argus_client(self):
             log_dir=self.log_dir,
             use_tunnel=self.use_tunnel,
             extra_headers=self.extra_headers,
+            run_id=self.run_id,
         )
 
     def append_test_data(self, request, test_data):

pytest-argus-reporter/tests/test_argus_reporter.py

@@ -442,6 +442,31 @@ def test_1(argus_reporter):
     assert not requests_mock.called, "Requests are not made to Argus when post_reports is False"
 
 
+def test_replay_log_filename_carries_run_id(testdir, requests_mock):  # pylint: disable=redefined-outer-name
+    # Regression test: ArgusReporter.argus_client used to construct
+    # ArgusGenericClient without run_id, so the replay log filename always
+    # fell back to "unknown" instead of the actual ARGUS_RUN_ID (set to
+    # "1234" by the configure_needed_env_vars fixture).
+    testdir.makepyfile(
+        """
+        import pytest
+
+        def test_1(argus_reporter):
+            argus_reporter.argus_client  # force construction
+        """
+    )
+
+    result = testdir.runpytest("--argus-post-reports", "-s", "-v")
+
+    result.stdout.fnmatch_lines(["*::test_1 PASSED*"])
+    assert result.ret == 0
+
+    replay_logs = list(testdir.tmpdir.visit("argus_replay_log_*"))
+    assert replay_logs, "expected a replay log file to be created"
+    assert "1234" in replay_logs[0].basename
+    assert "unknown" not in replay_logs[0].basename
+
+
 def test_subtests(testdir, requests_mock):  # pylint: disable=redefined-outer-name
     """Make sure subtests are identified and reported."""