Skip to content

Commit 2d0c80b

Browse files
authored
refactor(bench): _framework decoupling — 5 phases + workflow/UX cleanup (Tracer-Cloud#2801)
* refactoring * fixing naming convention
1 parent 2d9b732 commit 2d0c80b

21 files changed

Lines changed: 902 additions & 72 deletions

.github/workflows/benchmark-image.yml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
name: Benchmark image — build + push to ECR
1+
name: Benchmark image — build + push to ECR (any adapter)
2+
3+
# Adapter-agnostic image build. The bench image carries the full
4+
# ``tests/benchmarks/`` tree, so every registered adapter ships in the
5+
# same image. ``benchmark-run.yml`` selects which adapter actually runs
6+
# via its ``config`` input. See ``tests/benchmarks/_framework/registry.py``
7+
# for the registration contract.
28

39
# Builds infra/bench/Dockerfile.bench and pushes the resulting image to the
410
# opensre-bench ECR repository. The bench container is what

.github/workflows/benchmark-promote-image.yml

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1-
name: Benchmark image — promote tag to task definition
1+
name: Benchmark image — promote tag to task definition (any adapter)
22

3+
# Adapter-agnostic image promotion. Rebinds the ECS task definition to
4+
# a specific image tag. The image carries every registered adapter; the
5+
# config supplied to ``benchmark-run.yml`` picks which one runs.
6+
#
37
# Manually-triggered workflow that runs `terraform apply -var=image_tag=<TAG>`
48
# in infra/bench/ to register a new ECS task definition revision pointing at
59
# the chosen ECR image. This is the privileged "deploy" step that comes
@@ -18,7 +22,7 @@ name: Benchmark image — promote tag to task definition
1822
# lock-table read/write, iam:PassRole) are granted by the github_actions_run_bench
1923
# inline policy in infra/bench/iam_oidc.tf — any apply of that module attaches them.
2024
# - Repo secrets seeded into AWS Secrets Manager
21-
# - Repo vars set (AWS_ACCOUNT_ID etc., see tests/benchmarks/cloudopsbench/AWS_BENCH_SETUP.md step 4)
25+
# - Repo vars set (AWS_ACCOUNT_ID etc., see infra/bench/AWS_BENCH_SETUP.md step 4)
2226

2327
on:
2428
workflow_dispatch:
@@ -51,7 +55,7 @@ jobs:
5155
AWS_ACCOUNT_ID: ${{ vars.AWS_ACCOUNT_ID }}
5256
run: |
5357
if [ -z "${AWS_ACCOUNT_ID:-}" ]; then
54-
echo "::error::Missing repo variable AWS_ACCOUNT_ID. See tests/benchmarks/cloudopsbench/AWS_BENCH_SETUP.md."
58+
echo "::error::Missing repo variable AWS_ACCOUNT_ID. See infra/bench/AWS_BENCH_SETUP.md."
5559
exit 1
5660
fi
5761

.github/workflows/benchmark-run.yml

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
1-
name: Benchmark — run on Fargate
1+
name: Benchmark — run on Fargate (any adapter)
22

33
# Manually-triggered benchmark run on AWS Fargate.
44
#
5-
# Why ECS RunTask instead of running on a GitHub-hosted ubuntu runner:
6-
# a full Cloud-OpsBench grid runs for hours and writes hundreds of MB of
7-
# artifacts — Fargate handles it cleanly, has the AWS Secrets Manager
8-
# entries already wired in via infra/bench/, and writes to the
9-
# per-run S3 bucket. The workflow's job is just to launch the task and
10-
# print where to watch.
5+
# Adapter-agnostic. This workflow does NOT know or care which benchmark
6+
# is running. The ``config`` input names a YAML file; the YAML names an
7+
# adapter (``benchmark: <name>``); the framework's registry resolves it
8+
# to a registered ``BenchmarkAdapter`` subclass. The same workflow runs
9+
# any adapter packaged into the bench image — CloudOpsBench today,
10+
# OpenRCA / ToolCallBench / future adapters tomorrow, with zero changes
11+
# here. See ``tests/benchmarks/_framework/registry.py``.
12+
#
13+
# Why ECS RunTask instead of a GitHub-hosted ubuntu runner: a full bench
14+
# grid runs for hours and writes hundreds of MB of artifacts — Fargate
15+
# handles it cleanly, has AWS Secrets Manager wired in via infra/bench/,
16+
# and writes to the per-run S3 bucket. The workflow's job is just to
17+
# launch the task and print where to watch.
1118
#
1219
# Trigger from the GitHub UI:
1320
# Actions → "Benchmark run (manual)" → Run workflow → fill inputs
@@ -24,9 +31,14 @@ on:
2431
workflow_dispatch:
2532
inputs:
2633
config:
27-
description: 'Path to YAML config inside the container'
34+
# No adapter-specific default — the operator must point at a
35+
# specific config. Leaving the field empty surfaces the choice
36+
# rather than silently dispatching the CloudOpsBench smoke. The
37+
# path is relative to the container's repo root. Examples:
38+
# tests/benchmarks/cloudopsbench/configs/cloudopsbench_smoke.yml
39+
# tests/benchmarks/<adapter>/configs/<config>.yml
40+
description: 'Path to YAML config inside the container (e.g. tests/benchmarks/<adapter>/configs/<config>.yml)'
2841
required: true
29-
default: 'tests/benchmarks/cloudopsbench/configs/cloudopsbench_smoke.yml'
3042
dev_mode:
3143
description: 'Dev mode (skip integrity gates, no pre-reg needed)'
3244
type: boolean
@@ -64,7 +76,7 @@ jobs:
6476
# Fail loudly BEFORE the AWS auth step if any required repo variable
6577
# is missing. Without this, an unset var would surface downstream as
6678
# an opaque error like "Task Definition can not be blank" from
67-
# describe-task-definition. See tests/benchmarks/cloudopsbench/AWS_BENCH_SETUP.md step 4 for how
79+
# describe-task-definition. See infra/bench/AWS_BENCH_SETUP.md step 4 for how
6880
# to set these.
6981
env:
7082
AWS_ACCOUNT_ID: ${{ vars.AWS_ACCOUNT_ID }}
@@ -83,7 +95,7 @@ jobs:
8395
if [ "${#missing[@]}" -gt 0 ]; then
8496
echo "::error::Missing repo variable(s): ${missing[*]}"
8597
echo "::error::Set them under Settings → Secrets and variables → Actions → Variables."
86-
echo "::error::Values come from \`cd infra/bench && terraform output\` — see tests/benchmarks/cloudopsbench/AWS_BENCH_SETUP.md step 4."
98+
echo "::error::Values come from \`cd infra/bench && terraform output\` — see infra/bench/AWS_BENCH_SETUP.md step 4."
8799
exit 1
88100
fi
89101
echo "All 5 required repo variables are set."
File renamed without changes.

tests/benchmarks/_framework/adapter_base.py

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,47 @@ class MyAdapter(BenchmarkAdapter):
9090
"""
9191

9292

93+
# --------------------------------------------------------------------------- #
94+
# Overfit-dimensions schema #
95+
# --------------------------------------------------------------------------- #
96+
97+
98+
class OverfitDimensions(BaseModel):
99+
"""Metadata keys that overfit guards use to extract case attributes.
100+
101+
The framework's overfit module (``_framework/overfit.py``) runs
102+
five guards that attribute lift across (a) the corpus's "system"
103+
dimension, (b) the corpus's "stratum" / fault-category dimension,
104+
and (c) per-case ground-truth objects. Different adapters emit
105+
cells with different metadata layouts — CloudOpsBench uses
106+
``metadata["system"]`` / ``metadata["fault_category"]`` /
107+
``metadata["ground_truth"]["fault_object"]``; another adapter may
108+
use different key names.
109+
110+
This model lets each adapter declare its key names without
111+
requiring the framework to know about them. The defaults match
112+
CloudOpsBench's schema so existing call sites continue to work;
113+
other adapters override ``BenchmarkAdapter.overfit_dimensions``
114+
to point at their own keys.
115+
116+
Phase 3 of the framework decoupling: the previous overfit guards
117+
hardcoded ``c["case"]["metadata"]["system"]`` etc. inline, which
118+
silently coupled the framework to CloudOpsBench's metadata shape.
119+
"""
120+
121+
model_config = ConfigDict(frozen=True, extra="forbid")
122+
123+
system_key: str = "system"
124+
"""``case.metadata[<key>]`` — the corpus's "system" attribute."""
125+
126+
stratum_key: str = "fault_category"
127+
"""``case.metadata[<key>]`` — the corpus's stratum / category attribute."""
128+
129+
gt_object_key: str = "fault_object"
130+
"""``case.metadata["ground_truth"][<key>]`` — the GT object name used
131+
by Guard C's cluster fingerprinting."""
132+
133+
93134
# --------------------------------------------------------------------------- #
94135
# The adapter interface #
95136
# --------------------------------------------------------------------------- #
@@ -137,6 +178,47 @@ def apply_config_overrides(self, config: Any) -> None: # noqa: ARG002 — defau
137178
"""
138179
return None
139180

181+
def overfit_dimensions(self) -> OverfitDimensions:
182+
"""Metadata keys the overfit guards should consult for this adapter.
183+
184+
Default returns ``OverfitDimensions()`` which matches the
185+
CloudOpsBench schema (``system``, ``fault_category``,
186+
``ground_truth.fault_object``). Adapters with a different
187+
metadata layout override this to point the guards at their own
188+
keys.
189+
190+
Phase 3 of the framework decoupling: previously
191+
``_framework/overfit.py`` indexed into these keys inline,
192+
coupling the framework to one adapter's metadata shape. This
193+
hook moves the schema declaration to the adapter that owns it.
194+
"""
195+
return OverfitDimensions()
196+
197+
def extend_provenance(self, provenance: dict[str, Any]) -> dict[str, Any]:
198+
"""Optional: inject adapter-specific fields into the provenance dict.
199+
200+
Called by ``_framework/provenance.py::capture_provenance`` after
201+
the framework finishes assembling its standard sections
202+
(``code``, ``config``, ``pre_registration``, ``models``,
203+
``environment``, ``dataset``, ``run_inputs``). Adapters can:
204+
- add a new top-level key (e.g. an adapter-specific run note)
205+
- extend an existing section (e.g. add ``min_tool_calls`` to
206+
``run_inputs``)
207+
- return the dict unchanged
208+
209+
Default is identity. The hook exists so the framework's
210+
provenance module stays adapter-agnostic — it does NOT import or
211+
reach into any adapter's internals to build the artifact. Before
212+
Phase 4 the framework imported ``cloudopsbench.bench_agent``
213+
directly to read ``min_tool_calls``; that import is gone and
214+
CloudOpsBench now overrides this hook instead.
215+
216+
Implementations should mutate-and-return for performance, or
217+
return a fresh dict if a non-destructive transform is needed —
218+
the framework respects the return value either way.
219+
"""
220+
return provenance
221+
140222
@abstractmethod
141223
def load_cases(self, filters: CaseFilters) -> Iterator[BenchmarkCase]:
142224
"""Stream cases matching the filter. Seeded random selection is the

tests/benchmarks/_framework/adapters.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,11 @@
1919

2020
from __future__ import annotations
2121

22-
from tests.benchmarks._framework.adapter_base import AdapterCapabilities, BenchmarkAdapter
22+
from tests.benchmarks._framework.adapter_base import (
23+
AdapterCapabilities,
24+
BenchmarkAdapter,
25+
OverfitDimensions,
26+
)
2327
from tests.benchmarks._framework.registry import (
2428
build_adapter,
2529
capabilities_for,
@@ -47,6 +51,7 @@
4751
"CaseScore",
4852
"MetricSchema",
4953
"Mode",
54+
"OverfitDimensions",
5055
"RunContext",
5156
"RunResult",
5257
"build_adapter",

tests/benchmarks/_framework/config.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ def _default_report_formats() -> list[Literal["json", "markdown", "html"]]:
3434
return ["json", "markdown"]
3535

3636

37-
def _capabilities_for_lint(benchmark_name: str) -> AdapterCapabilities:
38-
"""Look up an adapter's capabilities for use during config lint.
37+
def _resolve_capabilities_or_default(benchmark_name: str) -> AdapterCapabilities:
38+
"""Look up an adapter's capabilities; fall back to the all-False default.
3939
4040
Unknown adapters return the all-False default — every gated feature
4141
is refused. This is intentional: a typo in ``config.benchmark`` (e.g.
@@ -245,7 +245,7 @@ def lint(self) -> list[str]:
245245
# ``benchmark == "cloudopsbench"``) means a new adapter that opts
246246
# in to ``supports_agent_variant=True`` is automatically accepted
247247
# by the framework without changes here.
248-
adapter_caps = _capabilities_for_lint(self.benchmark)
248+
adapter_caps = _resolve_capabilities_or_default(self.benchmark)
249249
if self.agent_variant != "default" and not adapter_caps.supports_agent_variant:
250250
errors.append(
251251
f"agent_variant={self.agent_variant!r} requires the "

0 commit comments

Comments
 (0)