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

@@ -39,6 +39,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."""
+ monkeypatch.setattr(context_tracker, "_store_each", True)
+
+
+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 +290,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 +305,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:https://$(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
 

performance-metrics/tests/performance_metrics/conftest.py

@@ -0,0 +1,16 @@
+"""Shared fixtures and functions for performance metrics tests."""
+
+from pathlib import Path
+
+
+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 start_time.isdigit()
+ assert duration.isdigit()