-
Notifications
You must be signed in to change notification settings - Fork 32
[benchmarker] Add benchmarker configuration and report definitions #1567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
966026a
5d785c1
a1ff4c5
f0ae25e
4a20965
645d179
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| from typing import Optional | ||
|
|
||
| from implicitdict import ImplicitDict | ||
|
|
||
|
|
||
| class BenchmarkActionName(str): | ||
| """Unique (within benchmark configuration) name for an action to perform, for instance at setup and/or teardown of one or more scenarios.""" | ||
|
|
||
| pass | ||
|
|
||
|
|
||
| class RunCommandActionSpecification(ImplicitDict): | ||
| """Shell command to run as a benchmark action.""" | ||
|
|
||
| command: str | ||
| """Shell command to run.""" | ||
|
|
||
| path: str | ||
| """Working folder in which to run the command. `$REPO_ROOT` will be replaced with the root folder of the repo.""" | ||
|
|
||
| env: dict[str, str] | ||
| """Override each environment variable key with the specified value before running the command.""" | ||
|
|
||
|
|
||
| class BenchmarkActionSpecification(ImplicitDict): | ||
| name: BenchmarkActionName | ||
|
|
||
| run_command: Optional[RunCommandActionSpecification] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| from typing import Optional | ||
|
|
||
| from implicitdict import ImplicitDict | ||
|
|
||
| from monitoring.benchmarker.configurations.artifacts.matplotlib_figure import ( | ||
| MatplotlibFigureSpecification, | ||
| ) | ||
| from monitoring.benchmarker.configurations.artifacts.raw_report import ( | ||
| RawReportSpecification, | ||
| ) | ||
|
|
||
|
|
||
| class ArtifactSpecification(ImplicitDict): | ||
| raw_report: Optional[RawReportSpecification] | ||
| matplotlib_figure: Optional[MatplotlibFigureSpecification] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| from enum import StrEnum | ||
| from typing import Optional | ||
|
|
||
| from implicitdict import ImplicitDict | ||
|
|
||
| from monitoring.monitorlib.expressions.types import ASTExpression, SymbolExpression | ||
|
|
||
|
|
||
| class AxisSpecification(ImplicitDict): | ||
| label: Optional[str] | ||
|
|
||
|
|
||
| class XYPlotType(StrEnum): | ||
| Scatter = "Scatter" | ||
|
|
||
|
|
||
| class XYPlotSpecification(ImplicitDict): | ||
| """Specification for a plot (artist) to show on a matplotlib Axes. | ||
|
|
||
| When evaluating expressions, the BenchmarkRunReport will be available as the `report` symbol.""" | ||
|
|
||
| type: XYPlotType | ||
|
|
||
| evaluation_context: Optional[list[SymbolExpression]] | ||
| """Symbols available to other expressions in this plot specification.""" | ||
|
|
||
| x_data_expr: Optional[ASTExpression] | ||
| """List of X data values for XY points. | ||
|
|
||
| Must have the same number of entries as y_data. | ||
| Defaults to 1, 2, 3, ..., N for N y_data values.""" | ||
|
|
||
| y_data_expr: ASTExpression | ||
| """List of Y data values for XY points.""" | ||
|
|
||
| render_expr: Optional[ASTExpression] | ||
| """If specified, whether this plot should be rendered (boolean). Default true.""" | ||
|
|
||
|
|
||
| class SubplotSpecification(ImplicitDict): | ||
| title: Optional[str] | ||
|
|
||
| evaluation_context: Optional[list[SymbolExpression]] | ||
| """Symbols available to other expressions in this subplot specification.""" | ||
|
|
||
| x_axis: Optional[AxisSpecification] | ||
| y_axis: Optional[AxisSpecification] | ||
| xy_plots: list[XYPlotSpecification] | ||
|
|
||
|
|
||
| class SubfigureSpecification(ImplicitDict): | ||
| title: Optional[str] | ||
|
|
||
| n_subplot_rows: int = 1 | ||
| n_subplot_cols: int = 1 | ||
|
|
||
| evaluation_context: Optional[list[SymbolExpression]] | ||
| """Symbols available to other expressions in this subfigure specification.""" | ||
|
|
||
| subplots: list[SubplotSpecification] | ||
|
|
||
|
|
||
| class MatplotlibFigureSpecification(ImplicitDict): | ||
| name: str | ||
| """Machine-level name for this figure. Used as the output file name.""" | ||
|
|
||
| n_subfigure_rows: int = 1 | ||
| n_subfigure_cols: int = 1 | ||
|
|
||
| evaluation_context: Optional[list[SymbolExpression]] | ||
| """Symbols available to other expressions in this figure specification.""" | ||
|
|
||
| subfigures: list[SubfigureSpecification] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| from implicitdict import ImplicitDict | ||
|
|
||
|
|
||
| class RawReportSpecification(ImplicitDict): | ||
| name: str | ||
| """Machine-level name for this report. Used as the output file name.""" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| from typing import Optional | ||
|
|
||
| from implicitdict import ImplicitDict | ||
|
|
||
| from monitoring.benchmarker.configurations.actions import ( | ||
| BenchmarkActionSpecification, | ||
| ) | ||
| from monitoring.benchmarker.configurations.artifacts.artifact import ( | ||
| ArtifactSpecification, | ||
| ) | ||
| from monitoring.benchmarker.configurations.loads import BenchmarkLoadSpecification | ||
| from monitoring.benchmarker.configurations.scenarios import ( | ||
| BenchmarkScenarioSpecification, | ||
| ) | ||
| from monitoring.benchmarker.configurations.users import BenchmarkUserSpecification | ||
| from monitoring.uss_qualifier.resources.definitions import ResourceCollection | ||
|
|
||
|
|
||
| class BenchmarkConfiguration(ImplicitDict): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Should we have a name there for identification of various config?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what we would do with that. Most other names are effectively handles/pointers to objects so they can be used multiple times within the configuration without duplication. I don't think we need that for the configuration itself. And, it will already have some kind of name by virtue of the path to the file it is defined in.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was more to display it e.g. in the graph / dumped data if data, to be able to track origin of a specific graph, like identifiers in a report (eventually with the hash/dirty flag to keep track of version, should included test evolve with time) |
||
| resources: Optional[ResourceCollection] | ||
| """Pool of uss_qualifier resources available for use by other resources in this benchmark configuration.""" | ||
|
|
||
| actions: Optional[list[BenchmarkActionSpecification]] | ||
| """Actions available to be performed during the benchmarker run.""" | ||
|
|
||
| user_types: list[BenchmarkUserSpecification] | ||
| """Types of users available to load the system under test during the benchmarker run.""" | ||
|
|
||
| loads: list[BenchmarkLoadSpecification] | ||
| """Loads that can be applied to the system under test during the benchmarker run.""" | ||
|
|
||
| scenarios: list[BenchmarkScenarioSpecification] | ||
| """The sequential list of scenarios to perform in the benchmarker run.""" | ||
|
|
||
| artifacts: Optional[list[ArtifactSpecification]] | ||
| """Artifacts to produce from the data collected during the benchmarker run.""" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BenjaminPelletier just a remark about security but I don't know how address it yet.
The fact that arbitrary command line could be run via configuration may be a risk for users.
We should think about sandboxing the execution. What do you think ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If any of our users were going to provide execution of benchmarker as a service using configurations they didn't control or approve, I agree this would need to be mitigated as a security concern. However, I expect benchmarker to generally be run by a user that already has full shell access, so automating shell commands doesn't seem like an escalation in privilege. Certainly all users should be careful about running code they're unfamiliar with and a benchmarker configuration is fairly close to code (without the restriction of requiring a benchmark designer to commit code into this repo before enabling others to run their benchmark). Sandboxing is difficult as one of the key initial use cases is automating container orchestration for a benchmark sweep against internode latency. But, I think we can certainly add features that could constrain actions (limiting capabilities) to fit within a certain security boundary if that were useful.
I agree this should be noted in the README when written.