Skip to content

Commit

Permalink
fix: fix using wrong index in from_csv_row (#15802)
Browse files Browse the repository at this point in the history
# Overview

I accidentally specified the same index multiple times when hardcoding
from_csv_row.

Since hardcoding in this case is a no-no fix it by using built-in
dataclass methods to look up headers and converting to/from csv.

# Test Plan

- Added unit tests that fail before fix was implemented due to hard
coding
- Added unit tests that ensure ordering of converted data. Although this
tests the dataclass package more than it is testing our software, it is
still helpful to have this because the docs aren't super clear about if
the order is maintained

# Changelog

- Create a CSVStorageBase dataclass that contains shared methods
- Have RawActivityData and ProcessResourceUsageSnapshot inherit from
CSVStorageBase

# Review requests

Is it weird having a blank dataclass as a base? I was having trouble
when trying to use an ABC.

# Risk assessment

The lowest of lows
  • Loading branch information
DerekMaggio authored Jul 31, 2024
1 parent 39297bb commit a477f16
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 108 deletions.
2 changes: 1 addition & 1 deletion api/src/opentrons/util/performance_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def _track_a_function(
state_name: "RobotActivityState",
func: _UnderlyingFunction[_UnderlyingFunctionParameters, _UnderlyingFunctionReturn],
) -> typing.Callable[_UnderlyingFunctionParameters, _UnderlyingFunctionReturn]:
"""Track a function.
"""Wrap a passed function with RobotActivityTracker.track.
This function is a decorator that will track the given state for the
decorated function.
Expand Down
55 changes: 32 additions & 23 deletions performance-metrics/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,7 @@ It is assumed that you already have the other projects in the monorepo setup cor
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
### Pushing performance-metrics package to Flex

```bash
make -C performance-metrics push-no-restart-ot3 host=<flex-ip>
Expand Down Expand Up @@ -69,3 +47,34 @@ To disable it run:
```bash
make unset-performance-metrics-ff host=<ip>
```

## Available features

### Robot activity tracking

#### Description

Developers are able to track when the robot is in a block of code they choose to monitor. Looking at
`api/src/opentrons/util/performance_helpers.py` you will see a class called `TrackingFunctions`. This class
defines static methods which are decorators that can be used wrap arbitrary functions.

As of 2024-07-31, the following tracking functions are available:

- `track_analysis`
- `track_getting_cached_protocol_analysis`

Looking at `TrackingFunctions.track_analysis` we see that the underlying call to \_track_a_function specifies a string `"ANALYZING_PROTOCOL"`. Whenever a function that is wrapped with `TrackingFunctions.track_analysis` executes, the tracking function will label the underlying function as `"ANALYZING_PROTOCOL"`.

To see where tracking function is used look at `robot_server/robot-server/protocols/protocol_analyzer.py`. You will see that the `ProtocolAnalyzer.analyze` function is wrapped with `TrackingFunctions.track_analysis`. Whenever `ProtocolAnalyzer.analyze` is called, the tracking function will start a timer. When the `ProtocolAnalyzer.analyze` function completes, the tracking function will stop the timer. It will then store the function start time and duration to the csv file, /data/performance_metrics_data/robot_activity_data

#### Adding new tracking decorator

To add a new tracking decorator, go to `performance-metrics/src/performance_metrics/_types.py`, and look at RobotActivityState literal and add a new state.
Go to `api/src/opentrons/util/performance_helpers.py` and add a static method to the `TrackingFunctions` class that uses the new state.

You can now wrap your functions with your new tracking decorator.

### System resource tracking

performance-metrics also exposes a tracking application called `SystemResourceTracker`. The application is implemented as a systemd service on the robot and records system resource usage by process. See the `oe-core` repo for more details.
You can configure the system resource tracker by modifying the environment variables set for the service. The service file lives at `/lib/systemd/system/system-resource-tracker.service`. You can change the defined environment variables or remove them and define them in the robot's environment variables. See `performance-metrics/src/performance_metrics/system_resource_tracker/_config.py` to see what environment variables are available.
87 changes: 24 additions & 63 deletions performance-metrics/src/performance_metrics/_data_shapes.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,52 +4,48 @@
import typing
from pathlib import Path

from ._types import SupportsCSVStorage, StorableData, RobotActivityState
from ._types import StorableData, RobotActivityState
from ._util import get_timing_function

_timing_function = get_timing_function()


@dataclasses.dataclass(frozen=True)
class RawActivityData(SupportsCSVStorage):
class CSVStorageBase:
"""Base class for all data classes."""

@classmethod
def headers(cls) -> typing.Tuple[str, ...]:
"""Returns the headers for the BaseData class."""
return tuple([field.name for field in dataclasses.fields(cls)])

def csv_row(self) -> typing.Tuple[StorableData, ...]:
"""Returns the object as a CSV row."""
return dataclasses.astuple(self)

@classmethod
def from_csv_row(cls, row: typing.Sequence[StorableData]) -> "CSVStorageBase":
"""Returns an object from a CSV row."""
return cls(*row)


@dataclasses.dataclass(frozen=True)
class RawActivityData(CSVStorageBase):
"""Represents raw duration data with activity state information.
Attributes:
- function_start_time (int): The start time of the function.
- duration_measurement_start_time (int): The start time for duration measurement.
- duration_measurement_end_time (int): The end time for duration measurement.
- state (RobotActivityStates): The current state of the activity.
- func_start (int): The start time of the function.
- duration (int): The start time for duration measurement.
"""

state: RobotActivityState
func_start: int
duration: int

@classmethod
def headers(self) -> typing.Tuple[str, str, str]:
"""Returns the headers for the raw activity data."""
return ("state_name", "function_start_time", "duration")

def csv_row(self) -> typing.Tuple[str, int, int]:
"""Returns the raw activity data as a string."""
return (
self.state,
self.func_start,
self.duration,
)

@classmethod
def from_csv_row(cls, row: typing.Sequence[StorableData]) -> SupportsCSVStorage:
"""Returns a RawActivityData object from a CSV row."""
return cls(
state=typing.cast(RobotActivityState, row[0]),
func_start=int(row[1]),
duration=int(row[2]),
)


@dataclasses.dataclass(frozen=True)
class ProcessResourceUsageSnapshot(SupportsCSVStorage):
class ProcessResourceUsageSnapshot(CSVStorageBase):
"""Represents process resource usage data.
Attributes:
Expand All @@ -68,41 +64,6 @@ class ProcessResourceUsageSnapshot(SupportsCSVStorage):
system_cpu_time: float # seconds
memory_percent: float

@classmethod
def headers(self) -> typing.Tuple[str, str, str, str, str, str]:
"""Returns the headers for the process resource usage data."""
return (
"query_time",
"command",
"running_since",
"user_cpu_time",
"system_cpu_time",
"memory_percent",
)

def csv_row(self) -> typing.Tuple[int, str, float, float, float, float]:
"""Returns the process resource usage data as a string."""
return (
self.query_time,
self.command,
self.running_since,
self.user_cpu_time,
self.system_cpu_time,
self.memory_percent,
)

@classmethod
def from_csv_row(cls, row: typing.Sequence[StorableData]) -> SupportsCSVStorage:
"""Returns a ProcessResourceUsageData object from a CSV row."""
return cls(
query_time=int(row[0]),
command=str(row[1]),
running_since=float(row[2]),
user_cpu_time=float(row[3]),
system_cpu_time=float(row[4]),
memory_percent=float(row[4]),
)


@dataclasses.dataclass(frozen=True)
class MetricsMetadata:
Expand Down
5 changes: 2 additions & 3 deletions performance-metrics/src/performance_metrics/_metrics_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@
import csv
import typing
import logging
from ._data_shapes import MetricsMetadata
from ._types import SupportsCSVStorage
from ._data_shapes import MetricsMetadata, CSVStorageBase
from ._logging_config import LOGGER_NAME

logger = logging.getLogger(LOGGER_NAME)

T = typing.TypeVar("T", bound=SupportsCSVStorage)
T = typing.TypeVar("T", bound=CSVStorageBase)


class MetricsStore(typing.Generic[T]):
Expand Down
18 changes: 0 additions & 18 deletions performance-metrics/src/performance_metrics/_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,3 @@ def store(self) -> None:


StorableData = typing.Union[int, float, str]


class SupportsCSVStorage(typing.Protocol):
"""A protocol for classes that support CSV storage."""

@classmethod
def headers(self) -> typing.Tuple[str, ...]:
"""Returns the headers for the CSV data."""
...

def csv_row(self) -> typing.Tuple[StorableData, ...]:
"""Returns the object as a CSV row."""
...

@classmethod
def from_csv_row(cls, row: typing.Tuple[StorableData, ...]) -> "SupportsCSVStorage":
"""Returns an object from a CSV row."""
...
65 changes: 65 additions & 0 deletions performance-metrics/tests/performance_metrics/test_data_shapes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
"""Tests for the data shapes."""

from performance_metrics._data_shapes import ProcessResourceUsageSnapshot


def test_headers_ordering() -> None:
"""Tests that the headers are in the correct order."""
assert ProcessResourceUsageSnapshot.headers() == (
"query_time",
"command",
"running_since",
"user_cpu_time",
"system_cpu_time",
"memory_percent",
)


def test_csv_row_method_ordering() -> None:
"""Tests that the CSV row method returns the correct order."""
expected = (
1,
"test",
2,
3,
4,
5,
)

assert (
ProcessResourceUsageSnapshot(
query_time=1,
command="test",
running_since=2,
user_cpu_time=3,
system_cpu_time=4,
memory_percent=5,
).csv_row()
== expected
)

assert (
ProcessResourceUsageSnapshot(
command="test",
query_time=1,
user_cpu_time=3,
system_cpu_time=4,
running_since=2,
memory_percent=5,
).csv_row()
== expected
)

assert (
ProcessResourceUsageSnapshot.from_csv_row(
(
1,
"test",
2,
3,
4,
5,
)
).csv_row()
== expected
)

0 comments on commit a477f16

Please sign in to comment.