Skip to content

Commit

Permalink
feat: configure performance-metrics to work on robot (#15316)
Browse files Browse the repository at this point in the history
# Overview

Adds packaging scripts and configuration to enable pushing and running
performance metrics on a robot.

Currently running analysis and getting a cached analysis are the only
things that are tracked on the robot

Closes https://opentrons.atlassian.net/browse/EXEC-497

# Test Plan

- Follow the steps in
[README](https://github.com/Opentrons/opentrons/blob/b7274f123ee5cbdd93430a52668d07b0c90b6ae8/performance-metrics/README.md)
to setup the env
- Run robot analysis for 2-3 different protocols by going to the setup
screen for the protocols
- Repeat running analysis for each protocol
- SSH into the robot and go to /data/performance_metrics_data/
- There should be 2 files 
  - robot_context_data
  - robot_context_data_headers
- Check their content
  - robot_context_data should have lines that look like this, 
    ```
    ANALYZING_PROTOCOL,1718739269730290312,14028043893
    GETTING_CACHED_ANALYSIS,1718739323578647179,47381526
    
    ...
    ```
  - robot_context_data_headers should have a single line 
    ```
    state_name,function_start_time,duration
    ```

- After you are finished run `make unset-performance-metrics-ff`
- Run a protocol and make sure performance metrics are no longer being
recorded. Also, make sure you can still run the analysis and get a
cached analysis


- [x] Verify analysis and cached analysis on the OT-2 dev server
- [x] Verify analysis and cached analysis on the Flex dev server
- [x] Verify analysis and cached analysis on OT-2
- [x] Verify analysis and cached analysis on Flex Dev Kit
- [x] Verify analysis and cached analysis on Flex @skowalski08 

# Changelog

## `performance_helpers.py`

- Add TrackingFunctions class which contains functions intended to be
used as decorators
- Generalize wrapper creation function
- Make all other entities in the file private

## `performance-metrics/Makefile`

- Add push targets for OT-2 and Flex
- Add helper target `update-robot-version` to hack the robot into
thinking it is on the same version as the app
- Add `set-performance-metrics-ff` and `unset-performance-metrics-ff`
- Add a get-logs target 

## `performance-metrics/setup.py`

Remove shared-data as a dep for performance-metrics as it does not use
it

## `robot-server/Pipfile`

Remove performance-metrics as a dependency as robot_server is not using
it directly

## `robot-server/robot_server/protocols/protocol_analyzer.py`

Wrap `analyze` and label it as `ANALYZING_PROTOCOL`

## `robot-server/robot_server/protocols/router.py`

Wrap logic that gets cached_analysis and label it as
`GETTING_CACHED_PROTOCOL_ANALYSIS`

# Review requests

- Take a look at my test plan, is there anything else that should be
added to reduce the risk?
- Am I installing the performance-metrics package correctly on the flex
by just unpacking it in `/opt/opentrons-robot-server`?
- 

# Risk assessment

Medium? `performance-metrics` is now wrapping production code. But the
package is not being built and installed on robots, it has to be pushed
by a dev. Furthermore, nothing will be run without setting the feature
flag
  • Loading branch information
DerekMaggio authored Jun 25, 2024
1 parent cdbc6c1 commit 4247d54
Show file tree
Hide file tree
Showing 15 changed files with 534 additions and 228 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ update-server/LICENSE
shared-data/python/LICENSE
shared-data/LICENSE
robot-server/LICENSE
performance-metrics/LICENSE

# typescript incremental files
*.tsbuildinfo
Expand Down
62 changes: 49 additions & 13 deletions api/src/opentrons/util/performance_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
)


class StubbedTracker:
class _StubbedTracker:
"""A stubbed tracker that does nothing."""

def __init__(self, storage_location: Path, should_track: bool) -> None:
Expand Down Expand Up @@ -60,10 +60,10 @@ def store(self) -> None:
pass


# Ensure that StubbedTracker implements SupportsTracking
# Ensure that _StubbedTracker implements SupportsTracking
# but do not create a runtime dependency on performance_metrics
if typing.TYPE_CHECKING:
_: typing.Type["SupportsTracking"] = StubbedTracker
_: typing.Type["SupportsTracking"] = _StubbedTracker


def _handle_package_import() -> typing.Type["SupportsTracking"]:
Expand All @@ -76,13 +76,12 @@ def _handle_package_import() -> typing.Type["SupportsTracking"]:

return RobotContextTracker
except ImportError:
return StubbedTracker
return _StubbedTracker


package_to_use = _handle_package_import()
_package_to_use = _handle_package_import()
_robot_context_tracker: typing.Optional["SupportsTracking"] = None


# TODO: derek maggio (06-03-2024): investigate if _should_track should be
# reevaluated each time _get_robot_context_tracker is called. I think this
# might get stuck in a state where after the first call, _should_track is
Expand All @@ -94,19 +93,30 @@ def _get_robot_context_tracker() -> "SupportsTracking":
"""Singleton for the robot context tracker."""
global _robot_context_tracker
if _robot_context_tracker is None:
_robot_context_tracker = package_to_use(
_robot_context_tracker = _package_to_use(
get_performance_metrics_data_dir(), _should_track
)
return _robot_context_tracker


def track_analysis(
func: _UnderlyingFunction[_UnderlyingFunctionParameters, _UnderlyingFunctionReturn]
) -> _UnderlyingFunction[_UnderlyingFunctionParameters, _UnderlyingFunctionReturn]:
"""Track the analysis of a protocol and store each run."""
# TODO: derek maggio (06-03-2024): generalize creating wrapper functions for tracking different states
def _track_a_function(
state_name: "RobotContextState",
func: _UnderlyingFunction[_UnderlyingFunctionParameters, _UnderlyingFunctionReturn],
) -> typing.Callable[_UnderlyingFunctionParameters, _UnderlyingFunctionReturn]:
"""Track a function.
This function is a decorator that will track the given state for the
decorated function.
Args:
state_name: The state to annotate the tracked function with.
func: The function to decorate.
Returns:
The decorated function.
"""
tracker: SupportsTracking = _get_robot_context_tracker()
wrapped = tracker.track(state="ANALYZING_PROTOCOL")(func)
wrapped = tracker.track(state=state_name)(func)

@functools.wraps(func)
def wrapper(
Expand All @@ -116,6 +126,32 @@ def wrapper(
try:
return wrapped(*args, **kwargs)
finally:
# TODO: derek maggio (06-18-2024): After investigation, it appears on startup
# that the first call to tracker.store() will not actually store the data.
# The second call stores both rows of data.

tracker.store()

return wrapper


class TrackingFunctions:
"""A class for tracking functions."""

@staticmethod
def track_analysis(
func: _UnderlyingFunction[
_UnderlyingFunctionParameters, _UnderlyingFunctionReturn
]
) -> typing.Callable[_UnderlyingFunctionParameters, _UnderlyingFunctionReturn]:
"""Track a function that runs an analysis."""
return _track_a_function("ANALYZING_PROTOCOL", func)

@staticmethod
def track_getting_cached_protocol_analysis(
func: _UnderlyingFunction[
_UnderlyingFunctionParameters, _UnderlyingFunctionReturn
]
) -> typing.Callable[_UnderlyingFunctionParameters, _UnderlyingFunctionReturn]:
"""Track a function that gets cached analysis."""
return _track_a_function("GETTING_CACHED_ANALYSIS", func)
6 changes: 3 additions & 3 deletions api/tests/opentrons/util/test_performance_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

from pathlib import Path
from opentrons.util.performance_helpers import (
StubbedTracker,
_StubbedTracker,
_get_robot_context_tracker,
)


def test_return_function_unchanged() -> None:
"""Test that the function is returned unchanged when using StubbedTracker."""
tracker = StubbedTracker(Path("/path/to/storage"), True)
"""Test that the function is returned unchanged when using _StubbedTracker."""
tracker = _StubbedTracker(Path("/path/to/storage"), True)

def func_to_track() -> None:
pass
Expand Down
1 change: 0 additions & 1 deletion performance-metrics/.gitignore

This file was deleted.

87 changes: 85 additions & 2 deletions performance-metrics/Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,38 @@
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)

# Find the branch, sha, version that will be used to update the VERSION.json file
version_file = $(call python_get_git_version,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:
Expand All @@ -20,12 +54,61 @@ 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,src,,$(version_file))

.PHONY: push-ot3
push-ot3: push-no-restart-ot3
$(call restart-service,$(host),$(ssh_key),$(ssh_opts),"opentrons-robot-server")

.PHONY: override-robot-version
override-robot-version:
$(eval update_dict := '{"opentrons_api_version": "$(version)", "update_server_version": "$(version)", "robot_server_version": "$(version)", "server_utils_version": "$(version)", "opentrons_hardware_version": "$(version)"}')
$(call sync-version-file,$(host),$(ssh_key),$(ssh_opts),'$(update_dict)')
$(call restart-service,$(host),$(ssh_key),$(ssh_opts),"opentrons-robot-server")


.PHONY: set-performance-metrics-ff
set-performance-metrics-ff:
@curl \
-H "opentrons-version: *" \
-X POST $(host):31950/settings \
-H "content-type: application/json" \
-d '{"id": "enablePerformanceMetrics", "value": true}'

.PHONY: unset-performance-metrics-ff
unset-performance-metrics-ff:
@curl \
-H "opentrons-version: *" \
-X POST $(host):31950/settings \
-H "content-type: application/json" \
-d '{"id": "enablePerformanceMetrics", "value": false}'

.PHONY: test
test:
Expand Down
70 changes: 69 additions & 1 deletion performance-metrics/README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,71 @@
# Performance Metrics

Project to gather various performance metrics for the Opentrons Flex.
Library to gather various performance metrics for the Opentrons Flex.

Currently being imported inside of `opentrons.util.performance_helpers` which defines
helper function used inside other projects

## Setup

It is assumed that you already have the other projects in the monorepo setup correctly.

```bash
make -C performance-metrics setup
```

### Testing against OT-2 Dev Server

```bash
make -C robot-server dev-ot2
```

### Testing against real OT-2

To push development packages to OT-2 run the following commands from the root directory of this repo:

```bash
make -C performance-metrics push-no-restart host=<ot2-ip>
make -C api push-no-restart host=<ot2-ip>
make -C robot-server push host=<ot2-ip>
```

### Testing against Flex Dev Server

```bash
make -C robot-server dev-flex
```

### Testing against real Flex

```bash
make -C performance-metrics push-no-restart-ot3 host=<flex-ip>
make -C api push-no-restart-ot3 host=<flex-ip>
make -C robot-server push-ot3 host=<flex-ip>
```

### Extra step when running against real robots

Once this is done you might need to hack getting your robot and app to think they are on the same version.
Go to your app -> Settings -> General and find your app version

Then run

```bash
make -C performance-metrics override-robot-version version=<app-version> host=<robot-ip>
```

this will make the app think that the robot is on the same version

### Enabling performance-metrics feature flag

Performance metrics usage is hidden behind a feature flag. To enable it run the following command:

```bash
make set-performance-metrics-ff host=<ip>
```

To disable it run:

```bash
make unset-performance-metrics-ff host=<ip>
```
11 changes: 11 additions & 0 deletions performance-metrics/setup.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[bdist_wheel]
universal = 1

[metadata]
license_files = ../LICENSE

[pep8]
ignore = E221,E222

[aliases]
test=pytest
4 changes: 1 addition & 3 deletions performance-metrics/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ def get_version():
KEYWORDS = ["robots", "protocols", "synbio", "pcr", "automation", "lab"]
DESCRIPTION = "Library for working with performance metrics on the Opentrons robots"
PACKAGES = find_packages(where="src", exclude=["tests.*", "tests"])
INSTALL_REQUIRES = [
f"opentrons-shared-data=={VERSION}",
]
INSTALL_REQUIRES = []


def read(*parts):
Expand Down
2 changes: 1 addition & 1 deletion performance-metrics/src/performance_metrics/data_shapes.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class RawContextData(SupportsCSVStorage):
@classmethod
def headers(self) -> typing.Tuple[str, str, str]:
"""Returns the headers for the raw context data."""
return ("state_id", "function_start_time", "duration")
return ("state_name", "function_start_time", "duration")

def csv_row(self) -> typing.Tuple[str, int, int]:
"""Returns the raw context data as a string."""
Expand Down
1 change: 1 addition & 0 deletions performance-metrics/src/performance_metrics/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

RobotContextState = typing.Literal[
"ANALYZING_PROTOCOL",
"GETTING_CACHED_ANALYSIS",
"RUNNING_PROTOCOL",
"CALIBRATING",
"ROBOT_STARTING_UP",
Expand Down
2 changes: 1 addition & 1 deletion robot-server/Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ sqlalchemy2-stubs = "==0.0.2a21"
# limited by tavern
python-box = "==6.1.0"
types-paho-mqtt = "==1.6.0.20240106"
performance-metrics = {file = "../performance-metrics", editable = true}
pyusb = "==1.2.1"
performance-metrics = {file = "../performance-metrics", editable = true}

[packages]
anyio = "==3.7.1"
Expand Down
Loading

0 comments on commit 4247d54

Please sign in to comment.