feat(robot-server, api, performance-metrics): get performance metrics to work on robot

api/src/opentrons/__init__.py

@@ -145,7 +145,7 @@ async def initialize() -> ThreadManagedHardware:
     """
     robot_conf = robot_configs.load()
     logging_config.log_init(robot_conf.log_level)
-
+    log.info(f"Logging Level: {robot_conf.log_level}")
     log.info(f"API server version: {version}")
     log.info(f"Robot Name: {name()}")
 

api/src/opentrons/util/performance_helpers.py

@@ -1,5 +1,6 @@
 """Performance helpers for tracking robot context."""
 
+import functools
 from pathlib import Path
 from opentrons_shared_data.performance.dev_types import (
     SupportsTracking,
@@ -9,21 +10,24 @@
 from opentrons_shared_data.robot.dev_types import RobotTypeEnum
 from typing import Callable, Type
 from opentrons.config import (
-    feature_flags as ff,
     get_performance_metrics_data_dir,
     robot_configs,
+    feature_flags as ff,
 )
 
 
 _should_track = ff.enable_performance_metrics(
     RobotTypeEnum.robot_literal_to_enum(robot_configs.load().model)
 )
+STORE_EACH = _should_track
 
 
 class StubbedTracker(SupportsTracking):
     """A stubbed tracker that does nothing."""
 
-    def __init__(self, storage_location: Path, should_track: bool) -> None:
+    def __init__(
+        self, storage_location: Path, should_track: bool, store_each: bool
+    ) -> None:
         """Initialize the stubbed tracker."""
         pass
 
@@ -62,15 +66,25 @@ def _get_robot_context_tracker() -> SupportsTracking:
     """Singleton for the robot context tracker."""
     global _robot_context_tracker
     if _robot_context_tracker is None:
-        # TODO: replace with path lookup and should_store lookup
         _robot_context_tracker = package_to_use(
-            get_performance_metrics_data_dir(), _should_track
+            get_performance_metrics_data_dir(), _should_track, STORE_EACH
         )
     return _robot_context_tracker
 
 
 def track_analysis(func: F) -> F:
-    """Track the analysis of a protocol."""
-    return _get_robot_context_tracker().track(RobotContextState.ANALYZING_PROTOCOL)(
-        func
-    )
+    """Track the analysis of a protocol and optionally store each run."""
+    # This will probably not stick around very long but it gives me
+    # the ability to test this on a robot
+
+    # Typing a decorator that wraps a decorator with args, nope
+    @functools.wraps(func)
+    def wrapper(*args, **kwargs):  # type: ignore # noqa: ANN002, ANN003, ANN201
+        tracker = _get_robot_context_tracker()
+        tracked_func = tracker.track(RobotContextState.ANALYZING_PROTOCOL)(func)
+
+        result = tracked_func(*args, **kwargs)
+
+        return result
+
+    return wrapper  # type: ignore

api/tests/opentrons/cli/test_cli.py

@@ -20,6 +20,16 @@
 # Enable tracking for the RobotContextTracker
 # This must come before the import of the analyze CLI
 context_tracker = _get_robot_context_tracker()
+@pytest.mark.usefixtures("override_data_store")
+def test_track_analysis(tmp_path: Path) -> None:
+    """Test that the RobotContextTracker tracks analysis."""
+…        requirements = {"apiLevel": "2.15"}
+
+        def run(protocol):
+            pass
+        """
+    )
+    protocol_source_file = tmp_path / "protocol.py"
 
 # Ignore the type error for the next line, as we're setting a private attribute for testing purposes
 context_tracker._should_track = True  # type: ignore[attr-defined]
@@ -39,6 +49,26 @@ def override_data_store(tmp_path: Path) -> Iterator[None]:
     context_tracker._store = old_store  # type: ignore[attr-defined]
 
 
+@pytest.fixture
+def monkeypatch_set_store_each_to_true(monkeypatch: pytest.MonkeyPatch) -> None:
+    """Override the STORE_EACH flag for the RobotContextTracker."""
+    context_tracker._store_each = True  # type: ignore[attr-defined]
+
+
+def verify_metrics_store_file(file_path: Path, expected_length: int) -> None:
+    """Verify that the metrics store file contains the expected number of lines."""
+    with open(file_path, "r") as f:
+        stored_data = f.readlines()
+        stored_data = [line.strip() for line in stored_data if line.strip()]
+        assert len(stored_data) == expected_length
+        for line in stored_data:
+            state_id, start_time, duration = line.strip().split(",")
+            assert state_id.isdigit()
+            assert state_id == str(RobotContextState.ANALYZING_PROTOCOL.state_id)
+            assert start_time.isdigit()
+            assert duration.isdigit()
+
+
 def _list_fixtures(version: int) -> Iterator[Path]:
     return Path(__file__).parent.glob(
         f"../../../../shared-data/protocol/fixtures/{version}/*.json"
@@ -270,9 +300,9 @@ def test_python_error_line_numbers(
     assert error["detail"] == expected_detail
 
 
-@pytest.mark.usefixtures("override_data_store")
-def test_track_analysis(tmp_path: Path) -> None:
-    """Test that the RobotContextTracker tracks analysis."""
+@pytest.mark.usefixtures("override_data_store", "monkeypatch_set_store_each_to_true")
+def test_track_analysis_with_store_each_run(tmp_path: Path) -> None:
+    """Test that the RobotContextTracker tracks analysis and stores each run."""
     protocol_source = textwrap.dedent(
         """
         requirements = {"apiLevel": "2.15"}
@@ -285,23 +315,12 @@ def run(protocol):
     protocol_source_file.write_text(protocol_source, encoding="utf-8")
     store = context_tracker._store  # type: ignore[attr-defined]
 
-    num_storage_entities_before_analysis = len(store._data)
+    verify_metrics_store_file(store.metadata.data_file_location, 0)
 
     _get_analysis_result([protocol_source_file])
 
-    assert len(store._data) == num_storage_entities_before_analysis + 1
-
-    with open(store.metadata.data_file_location, "r") as f:
-        stored_data = f.readlines()
-        assert len(stored_data) == 0
+    verify_metrics_store_file(store.metadata.data_file_location, 1)
 
-    context_tracker.store()
+    _get_analysis_result([protocol_source_file])
 
-    with open(store.metadata.data_file_location, "r") as f:
-        stored_data = f.readlines()
-        stored_data = [line.strip() for line in stored_data if line.strip()]
-        assert len(stored_data) == 1
-        state_id, start_time, duration = stored_data[0].strip().split(",")
-        assert state_id == str(RobotContextState.ANALYZING_PROTOCOL.state_id)
-        assert start_time.isdigit()
-        assert duration.isdigit()
+    verify_metrics_store_file(store.metadata.data_file_location, 2)

api/tests/opentrons/util/test_performance_helpers.py

@@ -10,7 +10,7 @@
 
 def test_return_function_unchanged() -> None:
     """Test that the function is returned unchanged when using StubbedTracker."""
-    tracker = StubbedTracker(Path("/path/to/storage"), True)
+    tracker = StubbedTracker(Path("/path/to/storage"), True, store_each=True)
 
     def func_to_track() -> None:
         pass

performance-metrics/.gitignore

@@ -1 +1,2 @@
-.ruff_cache/
+.ruff_cache/
+.logs/

performance-metrics/Makefile

@@ -1,4 +1,35 @@
 include ../scripts/python.mk
+include ../scripts/push.mk
+
+ot_project := $(OPENTRONS_PROJECT)
+project_rs_default = $(if $(ot_project),$(ot_project),robot-stack)
+project_ir_default = $(if $(ot_project),$(ot_project),ot3)
+
+SHX := npx shx
+
+# Host key location for robot
+ssh_key ?= $(default_ssh_key)
+# Other SSH args for robot
+ssh_opts ?= $(default_ssh_opts)
+# Helper to safely bundle ssh options
+ssh_helper = $(if $(ssh_key),-i $(ssh_key)) $(ssh_opts)
+
+# Defined separately than the clean target so the wheel file doesn’t have to
+# depend on a PHONY target
+
+# Find the version of the wheel from git using a helper script. We
+# use python here so we can use the same version normalization that will be
+# used to create the wheel.
+wheel_file = dist/$(call python_get_wheelname,performance-metrics,$(project_rs_default),performance_metrics,$(BUILD_NUMBER))
+
+# Find the version of the sdist file from git using a helper script.
+sdist_file = dist/$(call python_get_sdistname,performance_metrics,$(project_rs_default),performance_metrics)
+
+
+clean_cmd = $(SHX) rm -rf 'build' '**/*.egg-info' '**/__pycache__' **/*.pyc '.mypy_cache' '.pytest_cache'
+clean_wheel_cmd = $(clean_cmd) dist/*.whl
+clean_sdist_cmd = $(clean_cmd) dist/*.tar.gz
+clean_all_cmd = $(clean_cmd) dist
 
 .PHONY: lint
 lint:
@@ -20,12 +51,72 @@ teardown:
 
 .PHONY: clean
 clean:
-	rm -rf build dist *.egg-info .mypy_cache .pytest_cache src/performance_metrics.egg-info
+	$(clean_all_cmd)
 
 .PHONY: wheel
 wheel:
+	$(clean_wheel_cmd)
 	$(python) setup.py $(wheel_opts) bdist_wheel
-	rm -rf build
+	$(SHX) rm -rf build
+	$(SHX) ls dist
+
+.PHONY: sdist
+sdist: export OPENTRONS_PROJECT=$(project_rs_default)
+sdist:
+	$(clean_sdist_cmd)
+	$(python) setup.py sdist
+	$(SHX) rm -rf build
+	$(SHX) ls dist
+
+.PHONY: push-no-restart
+push-no-restart: wheel
+	$(call push-python-package,$(host),$(ssh_key),$(ssh_opts),$(wheel_file))
+
+.PHONY: push
+push: push-no-restart
+	$(call restart-service,$(host),$(ssh_key),$(ssh_opts),"opentrons-robot-server")
+
+.PHONY: push-no-restart-ot3
+push-no-restart-ot3: sdist
+	$(call push-python-sdist,$(host),$(ssh_key),$(ssh_opts),$(sdist_file),"/opt/opentrons-robot-server","performance_metrics",,,$(version_file))
+
+.PHONY: push-ot3
+push-ot3: push-no-restart-ot3
+	$(call restart-server,$(host),$(ssh_key),$(ssh_opts),"opentrons-robot-server")
+
+.PHONY: set-debug-log-level
+set-debug-log-level:
+	@echo "Setting log level to debug on robot"
+	curl \
+	  -X POST\
+	  -H "opentrons-version: *" \
+	  -H "Content-Type: application/json" \
+	  -d '{"log_level": "debug"}' \
+	  http://$(host):31950/settings/log_level/local | jq
+
+.PHONY: add-performance-metrics-ff
+add-performance-metrics-ff:
+	@echo "Adding performance metrics feature flag to robot"
+	ssh -i $(ssh_key) $(ssh_opts) root@$(host) "touch /data/robot.env && \
+		grep -q 'OT_API_FF_enablePerformanceMetrics' /data/robot.env && \
+		sed -i 's/OT_API_FF_enablePerformanceMetrics=false/OT_API_FF_enablePerformanceMetrics=true/' /data/robot.env || \
+		echo 'OT_API_FF_enablePerformanceMetrics=true' \
+		>> /data/robot.env"
+
+
+.PHONY: get-logs
+get-logs:
+	@echo "Creating logs directory"
+	mkdir -p .logs
+
+	@echo "Getting logs from robot"
+	ssh -i $(ssh_key) $(ssh_opts) root@$(host) "journalctl \
+		--no-pager \
+		--boot \
+		--catalog \
+		--unit opentrons-robot-server.service" \
+		> .logs/robot-server.log
+
 
 .PHONY: test
 test:

performance-metrics/pyproject.toml

@@ -0,0 +1,3 @@
+[build-system]
+requires = ["setuptools", "wheel"]
+build-backend = "setuptools.build_meta:__legacy__"

performance-metrics/setup.cfg

@@ -0,0 +1,11 @@
+[bdist_wheel]
+universal = 1
+
+[metadata]
+license_files = ../LICENSE
+
+[pep8]
+ignore = E221,E222
+
+[aliases]
+test=pytest

performance-metrics/src/performance_metrics/robot_context_tracker.py

@@ -1,5 +1,6 @@
 """Module for tracking robot context and execution duration for different operations."""
 
+import inspect
 from pathlib import Path
 import platform
 
@@ -48,7 +49,9 @@ class RobotContextTracker(SupportsTracking):
 
     METADATA_NAME: Final[Literal["robot_context_data"]] = "robot_context_data"
 
-    def __init__(self, storage_location: Path, should_track: bool = False) -> None:
+    def __init__(
+        self, storage_location: Path, should_track: bool, store_each: bool
+    ) -> None:
         """Initializes the RobotContextTracker with an empty storage list."""
         self._store = MetricsStore[RawContextData](
             MetricsMetadata(
@@ -58,6 +61,7 @@ def __init__(self, storage_location: Path, should_track: bool = False) -> None:
             )
         )
         self._should_track = should_track
+        self._store_each = store_each
 
         if self._should_track:
             self._store.setup()
@@ -76,25 +80,54 @@ def inner_decorator(func: Callable[P, R]) -> Callable[P, R]:
             if not self._should_track:
                 return func
 
-            @wraps(func)
-            def wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
-                function_start_time = timing_function()
-                duration_start_time = perf_counter_ns()
-                try:
-                    result = func(*args, **kwargs)
-                finally:
-                    duration_end_time = perf_counter_ns()
-                    self._store.add(
-                        RawContextData(
-                            func_start=function_start_time,
-                            duration=duration_end_time - duration_start_time,
-                            state=state,
+            if inspect.iscoroutinefunction(func):
+
+                @wraps(func)
+                async def wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
+                    function_start_time = timing_function()
+                    duration_start_time = perf_counter_ns()
+                    try:
+                        result = await func(*args, **kwargs)
+                    finally:
+                        duration_end_time = perf_counter_ns()
+
+                        self._store.add(
+                            RawContextData(
+                                func_start=function_start_time,
+                                duration=duration_end_time - duration_start_time,
+                                state=state,
+                            )
                         )
-                    )
 
-                return result
+                        if self._store_each:
+                            self.store()
+
+                    return result  # type: ignore
+
+            else:
+
+                @wraps(func)
+                def wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
+                    function_start_time = timing_function()
+                    duration_start_time = perf_counter_ns()
+                    try:
+                        result = func(*args, **kwargs)
+                    finally:
+                        duration_end_time = perf_counter_ns()
+                        self._store.add(
+                            RawContextData(
+                                func_start=function_start_time,
+                                duration=duration_end_time - duration_start_time,
+                                state=state,
+                            )
+                        )
+
+                        if self._store_each:
+                            self.store()
+
+                    return result
 
-            return wrapper
+            return wrapper  # type: ignore
 
         return inner_decorator