Show metrics needing measurement in the UI.

In the UI, show a metric as needing a measurement because the source parameters were recently changed.

Implemented by:
- In the collector: adding a hash of the source parameters to measurements.
- In the API-server: compare the hash of the latest measurement with the hash of the current configuration when adding recent measurements to a report and mark the metric as outdated.
- TODO: In the UI: show metrics as needing a measurement if the last measurement is outdated.

Open issues:
- Seeing the source parameter hash is only useful in the most recent measurement, should the hash be removed from the latest measurement before inserting a new one?

Closes #3134.

components/collector/src/base_collectors/metric_collector.py

@@ -52,7 +52,7 @@ async def collect(self) -> MetricMeasurement | None:
                 source_measurement.source_uuid = source_uuid
         if issue_status_collectors:
             issue_statuses = await asyncio.gather(*issue_status_collectors)
-        return MetricMeasurement(measurements, issue_statuses)
+        return MetricMeasurement(self._metric, measurements, issue_statuses)
 
     def __source_collectors(self) -> list[Coroutine]:
         """Create the source collectors for the metric."""

components/collector/src/model/measurement.py

@@ -2,6 +2,9 @@
 
 from collections.abc import Sequence
 
+from shared.model.metric import Metric
+from shared.utils.functions import md5_hash
+
 from collector_utilities.type import URL, ErrorMessage, Value
 
 from .entity import Entities
@@ -52,20 +55,28 @@ def as_dict(self) -> dict[str, Value | Entities | ErrorMessage | URL | None]:
 class MetricMeasurement:
     """Class to hold measurements from one or more sources for one metric."""
 
-    def __init__(self, sources: Sequence[SourceMeasurement], issue_statuses: Sequence[IssueStatus]) -> None:
-        self.sources = sources
+    def __init__(
+        self,
+        metric: Metric,
+        source_measurements: Sequence[SourceMeasurement],
+        issue_statuses: Sequence[IssueStatus],
+    ) -> None:
+        self.metric = metric
+        self.sources = source_measurements
         self.issue_statuses = issue_statuses
-        self.has_error = any(source.has_error() for source in sources)
+        self.has_error = any(source_measurement.has_error() for source_measurement in source_measurements)
         self.metric_uuid: str | None = None
         self.report_uuid: str | None = None
 
     def as_dict(self) -> dict:
         """Return the metric measurement as dict."""
+        source_parameters = [source.get("parameters") for source in self.metric["sources"].values()]
         measurement = {
-            "sources": [source.as_dict() for source in self.sources],
+            "sources": [source_measurement.as_dict() for source_measurement in self.sources],
             "has_error": self.has_error,
             "metric_uuid": self.metric_uuid,
             "report_uuid": self.report_uuid,
+            "source_parameter_hash": md5_hash(str(source_parameters)),
         }
         if self.issue_statuses:
             measurement["issue_status"] = [issue_status.as_dict() for issue_status in self.issue_statuses]

components/collector/tests/base_collectors/test_collector.py

@@ -65,8 +65,8 @@ async def _fetch_measurements(self, mock_async_get_request, number=1, side_effec
                     await self.collector.collect_metrics(session)
                     await asyncio.gather(*self.collector.running_tasks)  # Wait for the running tasks to finish
 
-    def _source(self, **kwargs: str) -> dict[str, str | None | list[dict[str, str]]]:
-        """Create a source."""
+    def expected_source(self, **kwargs: str | None) -> dict[str, str | None | list[dict[str, str]]]:
+        """Create an expected source."""
         connection_error = kwargs.get("connection_error")
         entities = kwargs.get(
             "entities",
@@ -91,16 +91,29 @@ def _source(self, **kwargs: str) -> dict[str, str | None | list[dict[str, str]]]
             "source_uuid": SOURCE_ID,
         }
 
+    def expected_measurement(
+        self,
+        *,
+        source_parameter_hash: str = "0f6df164677525409f6269ec97e4118d",
+        metric_uuid: str = "metric_uuid",
+        **expected_source_kwargs,
+    ) -> dict[str, bool | list | str]:
+        """Create an expected inserted measurement."""
+        return {
+            "has_error": "connection_error" in expected_source_kwargs,
+            "sources": [self.expected_source(**expected_source_kwargs)],
+            "metric_uuid": metric_uuid,
+            "report_uuid": "report1",
+            "source_parameter_hash": source_parameter_hash,
+        }
+
     async def test_fetch_successful(self):
         """Test fetching a test metric."""
         mock_async_get_request = AsyncMock()
         mock_async_get_request.json.side_effect = [self.pip_json]
         with patch(self.create_measurement) as post:
             await self._fetch_measurements(mock_async_get_request)
-        post.assert_called_once_with(
-            self.database,
-            {"has_error": False, "sources": [self._source()], "metric_uuid": "metric_uuid", "report_uuid": "report1"},
-        )
+        post.assert_called_once_with(self.database, self.expected_measurement())
 
     async def test_fetch_without_sources(self):
         """Test fetching measurement for a metric without sources."""
@@ -142,15 +155,7 @@ async def test_fetch_with_client_error(self):
         """Test fetching measurement when getting measurements fails."""
         with patch(self.create_measurement) as post:
             await self._fetch_measurements(None, side_effect=[aiohttp.ClientConnectionError("error")])
-        post.assert_called_once_with(
-            self.database,
-            {
-                "has_error": True,
-                "sources": [self._source(connection_error="error")],
-                "metric_uuid": "metric_uuid",
-                "report_uuid": "report1",
-            },
-        )
+        post.assert_called_once_with(self.database, self.expected_measurement(connection_error="error"))
 
     async def test_fetch_with_empty_client_error(self):
         """Test fetching measurement when getting measurements fails with an 'empty' exception.
@@ -159,26 +164,15 @@ async def test_fetch_with_empty_client_error(self):
         """
         with patch(self.create_measurement) as post:
             await self._fetch_measurements(None, side_effect=[aiohttp.ClientPayloadError()])
-        post.assert_called_once_with(
-            self.database,
-            {
-                "has_error": True,
-                "sources": [self._source(connection_error="ClientPayloadError")],
-                "metric_uuid": "metric_uuid",
-                "report_uuid": "report1",
-            },
-        )
+        post.assert_called_once_with(self.database, self.expected_measurement(connection_error="ClientPayloadError"))
 
     async def test_fetch_with_post_error(self):
         """Test fetching measurement when posting fails."""
         mock_async_get_request = AsyncMock()
         mock_async_get_request.json.side_effect = [self.pip_json]
         with patch(self.create_measurement) as post:
             await self._fetch_measurements(mock_async_get_request)
-        post.assert_called_once_with(
-            self.database,
-            {"has_error": False, "sources": [self._source()], "metric_uuid": "metric_uuid", "report_uuid": "report1"},
-        )
+        post.assert_called_once_with(self.database, self.expected_measurement())
 
     @patch("asyncio.sleep", AsyncMock(side_effect=[RuntimeError]))
     async def test_collect(self):
@@ -202,10 +196,7 @@ async def test_fetch_twice(self):
         mock_async_get_request.json.side_effect = [self.pip_json, self.pip_json]
         with patch(self.create_measurement) as post:
             await self._fetch_measurements(mock_async_get_request, number=2)
-        post.assert_called_once_with(
-            self.database,
-            {"has_error": False, "sources": [self._source()], "metric_uuid": "metric_uuid", "report_uuid": "report1"},
-        )
+        post.assert_called_once_with(self.database, self.expected_measurement())
 
     @patch.object(config, "MEASUREMENT_LIMIT", 1)
     async def test_fetch_in_batches(self):
@@ -216,14 +207,8 @@ async def test_fetch_in_batches(self):
         with patch(self.create_measurement) as post:
             self.client["quality_time_db"]["reports"].insert_one(create_report(metric_id=METRIC_ID2))
             await self._fetch_measurements(mock_async_get_request, number=2)
-        expected_call1 = call(
-            self.database,
-            {"has_error": False, "sources": [self._source()], "metric_uuid": "metric_uuid", "report_uuid": "report1"},
-        )
-        expected_call2 = call(
-            self.database,
-            {"has_error": False, "sources": [self._source()], "metric_uuid": "metric_uuid2", "report_uuid": "report1"},
-        )
+        expected_call1 = call(self.database, self.expected_measurement())
+        expected_call2 = call(self.database, self.expected_measurement(metric_uuid="metric_uuid2"))
         post.assert_has_calls(calls=[expected_call1, expected_call2])
 
     @patch.object(config, "MEASUREMENT_LIMIT", 1)
@@ -241,18 +226,15 @@ async def test_prioritize_edited_metrics(self):
             self.client["quality_time_db"]["reports"].insert_one(report2)
             await self._fetch_measurements(mock_async_get_request, number=2)
 
-        expected_call1 = call(
-            self.database,
-            {"has_error": False, "sources": [self._source()], "metric_uuid": "metric_uuid", "report_uuid": "report1"},
-        )
+        expected_call1 = call(self.database, self.expected_measurement())
         expected_call2 = call(
             self.database,
-            {
-                "has_error": False,
-                "sources": [self._source(api_url=edited_url, landing_url=edited_url)],
-                "metric_uuid": "metric_uuid2",
-                "report_uuid": "report1",
-            },
+            self.expected_measurement(
+                source_parameter_hash="ebfcffc74dc017799cb4da33090a9867",
+                metric_uuid="metric_uuid2",
+                api_url=edited_url,
+                landing_url=edited_url,
+            ),
         )
         post.assert_has_calls(calls=[expected_call1, expected_call2])
 
@@ -281,12 +263,13 @@ async def test_missing_mandatory_parameter_with_default_value(self):
             await self._fetch_measurements(mock_async_get_request)
         post.assert_called_once_with(
             self.database,
-            {
-                "has_error": False,
-                "sources": [self._source(value="0", entities=[], api_url="", landing_url="")],
-                "metric_uuid": "metric_uuid",
-                "report_uuid": "report1",
-            },
+            self.expected_measurement(
+                source_parameter_hash="8c3b464958e9ad0f20fb2e3b74c80519",
+                value="0",
+                entities=[],
+                api_url="",
+                landing_url="",
+            ),
         )
 
     @patch("pathlib.Path.open", new_callable=mock_open)

components/shared_code/src/shared/model/measurement.py

@@ -294,3 +294,7 @@ def summarize(self) -> dict[str, str | dict[str, str | None]]:
             "start": self["start"],
             "end": self["end"],
         }
+
+    def source_parameter_hash(self) -> str:
+        """Return the source parameter hash."""
+        return cast(str, self.get("source_parameter_hash", ""))

components/shared_code/src/shared/model/metric.py

@@ -6,6 +6,7 @@
 from datetime import date
 from typing import TYPE_CHECKING, cast
 
+from shared.utils.functions import md5_hash
 from shared.utils.type import (
     Direction,
     MetricId,
@@ -154,7 +155,11 @@ def _get_measured_attribute_type(entity: dict[str, list[dict[str, str]]], attrib
 
     def summarize(self, measurements: list[Measurement], **kwargs) -> dict:
         """Add a summary of the metric to the report."""
-        latest_measurement = measurements[-1] if measurements else None
+        if measurements:
+            latest_measurement = measurements[-1]
+            latest_measurement["outdated"] = latest_measurement.source_parameter_hash() != self.source_parameter_hash()
+        else:
+            latest_measurement = None
         summary = dict(self)
         summary["scale"] = self.scale()
         summary["status"] = self.status(latest_measurement)
@@ -165,3 +170,8 @@ def summarize(self, measurements: list[Measurement], **kwargs) -> dict:
             summary["issue_status"] = self.issue_statuses(latest_measurement)
         summary.update(kwargs)
         return summary
+
+    def source_parameter_hash(self) -> str:
+        """Return a hash of the source parameters that can be used to check for changed parameters."""
+        source_parameters = [source.get("parameters") for source in self.sources]
+        return md5_hash(str(source_parameters))

components/shared_code/tests/shared/model/test_metric.py

@@ -62,6 +62,7 @@ def test_summarize_measurements(self):
                 "latest_measurement": {
                     "count": {"value": 1, "start": measurement_timestamp},
                     "end": measurement_timestamp,
+                    "outdated": True,
                     "sources": [],
                     "start": measurement_timestamp,
                     "status": "target_met",
@@ -151,3 +152,7 @@ def test_not_equal(self):
     def test_not_equal_to_string(self):
         """Test that a metric is not equal to a string."""
         self.assertNotEqual(self.create_metric(), "foo")
+
+    def test_source_parameter_hash(self):
+        """Test the source parameter hash."""
+        self.assertEqual("d751713988987e9331980363e24189ce", self.create_metric().source_parameter_hash())