fix: unit tests and simplify telemetry manager

Author: ilana-n

aiperf/gpu_telemetry/telemetry_manager.py

@@ -2,7 +2,6 @@
 # SPDX-License-Identifier: Apache-2.0
 
 import asyncio
-from urllib.parse import urlparse
 
 from aiperf.common.base_component_service import BaseComponentService
 from aiperf.common.config import ServiceConfig, UserConfig
@@ -80,24 +79,11 @@ def __init__(
             user_config.gpu_telemetry is not None
         )
 
-        # Normalize to list
-        user_endpoints = (
-            []
-            if user_config.gpu_telemetry is None
-            else [user_config.gpu_telemetry]
-            if isinstance(user_config.gpu_telemetry, str)
-            else list(user_config.gpu_telemetry)
-        )
+        user_endpoints = user_config.gpu_telemetry_urls or []
+        if isinstance(user_endpoints, str):
+            user_endpoints = [user_endpoints]
 
-        # Validate and normalize URLs
-        valid_endpoints = []
-        for endpoint in user_endpoints:
-            try:
-                parsed = urlparse(endpoint)
-                if parsed.scheme in ("http", "https") and parsed.netloc:
-                    valid_endpoints.append(self._normalize_dcgm_url(endpoint))
-            except Exception:
-                continue
+        valid_endpoints = [self._normalize_dcgm_url(url) for url in user_endpoints]
 
         # Store user-provided endpoints separately for display filtering (excluding auto-inserted defaults)
         self._user_provided_endpoints = [

tests/gpu_telemetry/test_telemetry_manager.py

@@ -47,6 +47,7 @@ def _create_manager_with_mocked_base(self, user_config):
     def test_initialization_default_endpoint(self):
         """Test initialization with no user-provided endpoints uses defaults."""
         mock_user_config = MagicMock(spec=UserConfig)
+        mock_user_config.gpu_telemetry = None
         mock_user_config.gpu_telemetry_urls = None
 
         manager = self._create_manager_with_mocked_base(mock_user_config)
@@ -56,6 +57,7 @@ def test_initialization_custom_endpoints(self):
         """Test initialization with custom user-provided endpoints."""
         mock_user_config = MagicMock(spec=UserConfig)
         custom_endpoint = "http://gpu-node-01:9401/metrics"
+        mock_user_config.gpu_telemetry = ["dashboard"]  # User configured telemetry
         mock_user_config.gpu_telemetry_urls = [custom_endpoint]
 
         manager = self._create_manager_with_mocked_base(mock_user_config)
@@ -69,6 +71,7 @@ def test_initialization_custom_endpoints(self):
     def test_initialization_string_endpoint(self):
         """Test initialization converts single string endpoint to list and prepends defaults."""
         mock_user_config = MagicMock(spec=UserConfig)
+        mock_user_config.gpu_telemetry = ["dashboard"]  # User configured telemetry
         mock_user_config.gpu_telemetry_urls = "http://single-node:9401/metrics"
 
         manager = self._create_manager_with_mocked_base(mock_user_config)
@@ -80,15 +83,16 @@ def test_initialization_string_endpoint(self):
         assert len(manager._dcgm_endpoints) == 3
 
     def test_initialization_filters_invalid_urls(self):
-        """Test initialization filters out invalid URLs."""
+        """Test initialization with only valid URLs (invalid ones filtered by user_config validator)."""
         mock_user_config = MagicMock(spec=UserConfig)
-        mock_user_config.gpu_telemetry_urls = [
-            "http://valid:9401/metrics",  # Valid
-            "not-a-url",  # Invalid - no scheme
-            "ftp://wrong-scheme:9401",  # Invalid - wrong scheme
-            "http://another-valid:9401",  # Valid
-            "",  # Invalid - empty
+        # user_config validator would have already filtered out invalid URLs
+        # so telemetry_manager only receives valid ones
+        valid_urls = [
+            "http://valid:9401/metrics",
+            "http://another-valid:9401/metrics",
         ]
+        mock_user_config.gpu_telemetry = ["dashboard"]  # User configured telemetry
+        mock_user_config.gpu_telemetry_urls = valid_urls
 
         manager = self._create_manager_with_mocked_base(mock_user_config)
 
@@ -102,11 +106,13 @@ def test_initialization_filters_invalid_urls(self):
     def test_initialization_deduplicates_endpoints(self):
         """Test initialization removes duplicate endpoints while preserving order."""
         mock_user_config = MagicMock(spec=UserConfig)
-        mock_user_config.gpu_telemetry_urls = [
+        urls_with_duplicates = [
             "http://node1:9401/metrics",
             "http://node2:9401/metrics",
             "http://node1:9401/metrics",  # Duplicate
         ]
+        mock_user_config.gpu_telemetry = ["dashboard"]  # User configured telemetry
+        mock_user_config.gpu_telemetry_urls = urls_with_duplicates
 
         manager = self._create_manager_with_mocked_base(mock_user_config)
 
@@ -120,11 +126,13 @@ def test_initialization_deduplicates_endpoints(self):
     def test_user_provides_default_endpoint(self):
         """Test that explicitly providing a default endpoint doesn't duplicate it."""
         mock_user_config = MagicMock(spec=UserConfig)
-        mock_user_config.gpu_telemetry = [
+        urls = [
             "http://localhost:9400/metrics",  # This is a default
             "http://node1:9401/metrics",
             "http://localhost:9401/metrics",  # This is also a default
         ]
+        mock_user_config.gpu_telemetry = ["dashboard"]  # User configured telemetry
+        mock_user_config.gpu_telemetry_urls = urls
 
         manager = self._create_manager_with_mocked_base(mock_user_config)
 
@@ -520,13 +528,12 @@ def _create_manager_with_mocked_base(self, user_config):
         return manager
 
     def test_invalid_endpoints_filtered_during_init(self):
-        """Test that empty string and invalid URLs are filtered during initialization."""
+        """Test that only valid URLs reach telemetry_manager (invalid ones filtered by user_config validator)."""
         mock_user_config = MagicMock(spec=UserConfig)
-        mock_user_config.gpu_telemetry_urls = [
-            "",  # Empty string - filtered
-            "/metrics",  # No scheme - filtered
-            "http://valid:9401/metrics",  # Valid
-        ]
+        # user_config validator would have already filtered out invalid URLs
+        # so telemetry_manager only receives valid ones
+        mock_user_config.gpu_telemetry = ["dashboard"]  # User configured telemetry
+        mock_user_config.gpu_telemetry_urls = ["http://valid:9401/metrics"]
 
         manager = self._create_manager_with_mocked_base(mock_user_config)
 
@@ -572,6 +579,7 @@ def test_both_defaults_included_when_no_user_config(self):
         """Test that both default endpoints (9400 and 9401) are included with no user config."""
         mock_user_config = MagicMock(spec=UserConfig)
         mock_user_config.gpu_telemetry = None
+        mock_user_config.gpu_telemetry_urls = None
 
         manager = self._create_manager_with_mocked_base(mock_user_config)
 
@@ -585,16 +593,19 @@ def test_user_explicitly_configured_telemetry_flag(self):
         # Test with None (not configured)
         mock_user_config = MagicMock(spec=UserConfig)
         mock_user_config.gpu_telemetry = None
+        mock_user_config.gpu_telemetry_urls = None
         manager = self._create_manager_with_mocked_base(mock_user_config)
         assert manager._user_explicitly_configured_telemetry is False
 
         # Test with value (configured)
-        mock_user_config.gpu_telemetry = "http://custom:9401"
+        mock_user_config.gpu_telemetry = ["dashboard"]  # User configured telemetry
+        mock_user_config.gpu_telemetry_urls = ["http://custom:9401/metrics"]
         manager = self._create_manager_with_mocked_base(mock_user_config)
         assert manager._user_explicitly_configured_telemetry is True
 
         # Test with empty list (configured)
-        mock_user_config.gpu_telemetry = []
+        mock_user_config.gpu_telemetry = []  # User explicitly passed --gpu-telemetry with no args
+        mock_user_config.gpu_telemetry_urls = []
         manager = self._create_manager_with_mocked_base(mock_user_config)
         assert manager._user_explicitly_configured_telemetry is True
 
@@ -607,6 +618,7 @@ def _create_test_manager(self):
         manager = TelemetryManager.__new__(TelemetryManager)
         manager.service_id = "test_manager"
         manager._collectors = {}
+        manager._collector_id_to_url = {}
         manager._dcgm_endpoints = list(DEFAULT_DCGM_ENDPOINTS)
         manager._user_provided_endpoints = []
         manager._user_explicitly_configured_telemetry = False
@@ -750,6 +762,7 @@ def _create_test_manager(self, user_requested, user_endpoints):
         manager = TelemetryManager.__new__(TelemetryManager)
         manager.service_id = "test_manager"
         manager._collectors = {}
+        manager._collector_id_to_url = {}
         manager._dcgm_endpoints = list(DEFAULT_DCGM_ENDPOINTS) + user_endpoints
         manager._user_provided_endpoints = user_endpoints
         manager._user_explicitly_configured_telemetry = user_requested