From ed9241202645d1314169e39bfe36766e151e553c Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 16 Aug 2026 08:13:46 +0200 Subject: [PATCH 1/2] :test_tube: test(collect): make the load test assert something it can fail (PERF-FIX-04) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test/load` was never a load test. It called `extractor.Extract` on ONE identical object in a single-threaded loop with no API server, cluster, sinks, controller or concurrency, and asserted nothing beyond "did not return an error" — so it could not fail for being slow, allocating more, or regressing. It duplicated `BenchmarkExtract` while carrying the name `load-test:10k` and a nightly `ubuntu-latest-8-cores` job, which read as cluster-scale evidence nobody had measured. `test/load/load_test.go` was worse: it asserted env-var parsing and logged. Option (b) from the story — make it honest — plus assertions that can fail: - Delete `test/load/` and the `load` build tag (untagged code was invisible to golangci-lint, so this also widens lint coverage). - Add `TestExtractHotPathBudget` in `internal/collect`, driving the SINGLE shared `extractWorkload` that `BenchmarkExtract` also drives — one workload definition, no duplication. Test-package functions are not importable across packages, so co-location is the only way to genuinely share it. - Vary the object: a deterministic pool of 128 distinct Deployments, built outside the timed region, ~1 in 7 omitting `status.readyReplicas` so the CEL `has()` expression takes both branches. - Assert three ceilings against a recorded baseline +25%, with a vacuous-pass guard (N > 0, non-zero alloc counters) so a broken benchmark cannot report zeroes and sail through every check. Calibration, because a test that false-reds is worse than one that cannot fail. B/op and allocs/op are hardware-independent for a fixed toolchain, so they sit at baseline +25% and carry the regression signal. ns/op is not, and it is measured under two different conditions: - In the required PR gate this runs inside `go test $(go list ./...) -coverprofile`, i.e. instrumented (+9% measured) while ~40 sibling packages, several spinning envtest apiserver/etcd, contend for the runner. Two concurrent commands alone cost 2.8x on an 18-core laptop. The in-code default is therefore a deliberately absurd 50x net for order-of-magnitude regressions. - `task extract-budget` runs the test alone and uninstrumented, so it pins KOLECT_EXTRACT_MAX_NS_PER_OP=204408 (8x baseline) — ~6x tighter, and that is where the real latency gate lives. Calibrated hardware should set it to its own measured ns/op x 1.25 for the literal >25% floor. Baseline (S-LOCAL, darwin/arm64 Apple M5 Max, go1.26.5, worst of -count=5): 25551 ns/op · 32218 B/op · 452 allocs/op. Varying the pool did not move the allocation profile from the single-object measurement it replaces. Naming and CI: - `task load-test` / `task load-test:10k` -> `task extract-budget`, whose description states what is and is not exercised so `task --list` cannot mislead. - Workflow job `load-test-10k` (8-core, disabled behind `run_scale_jobs`) -> `extract-budget` on plain `ubuntu-latest`, no envtest cache, running every nightly instead of never. `scale-envtest-10k` remains the only 10k scale job. - `KOLECT_LOAD_TEST` / `KOLECT_LOAD_TEST_MAX` are gone; `hack/perf-report.sh` no longer derives a "load" scale tier from a dead env var. - LAB-DOC-03 now fails closed if a `load-test-10k` job reappears. - ADR-0603/0706/0707 (incl. the L5 tier row, mermaid node and ownership note), REQUIREMENTS NFR-PERF-1, performance.md, the load-test runbook tiers, testing.md, guidelines.md and setup.md restated: this is a micro-benchmark budget, not a scale tier. --- .github/workflows/e2e-nightly.yaml | 23 +-- Taskfile.yml | 18 +-- docs/REQUIREMENTS.md | 2 +- docs/adr/0603-performance-scalability.md | 4 +- .../0706-testing-merge-gate-architecture.md | 7 +- docs/adr/0707-lab-harness.md | 8 +- docs/development/guidelines.md | 8 +- docs/development/setup.md | 24 ++- docs/development/testing.md | 11 +- docs/operator-manual/load-test-runbook.md | 15 +- docs/operator-manual/performance.md | 22 ++- hack/perf-report.sh | 6 +- hack/task/Taskfile.test.yml | 33 ++-- .../test/docs_lab_doc_03_scale_claims_test.sh | 14 +- internal/collect/extractor_bench_test.go | 101 ++++++++---- internal/collect/extractor_budget_test.go | 152 ++++++++++++++++++ test/load/collect_test.go | 73 --------- test/load/load_test.go | 34 ---- 18 files changed, 341 insertions(+), 214 deletions(-) create mode 100644 internal/collect/extractor_budget_test.go delete mode 100644 test/load/collect_test.go delete mode 100644 test/load/load_test.go diff --git a/.github/workflows/e2e-nightly.yaml b/.github/workflows/e2e-nightly.yaml index 1d0d0e40..f0e58ec1 100644 --- a/.github/workflows/e2e-nightly.yaml +++ b/.github/workflows/e2e-nightly.yaml @@ -187,28 +187,29 @@ jobs: retention-days: 14 if-no-files-found: warn - load-test-10k: - name: load-test-10k - # Skipped by default: ubuntu-latest-8-cores runners unavailable (jobs never leave queue). - # Re-enable via workflow_dispatch with run_scale_jobs=true once org runners are provisioned. - if: github.event_name == 'workflow_dispatch' && inputs.run_scale_jobs == true - runs-on: ubuntu-latest-8-cores - timeout-minutes: 45 + # PERF-FIX-04: this used to be `load-test-10k` on ubuntu-latest-8-cores, which read + # as cluster-scale evidence. It never was: it is a single-threaded, in-process + # extractor micro-benchmark with no API server, cluster, sinks or export path. + # Renamed, moved to a plain runner, and made to actually run every nightly. + # In-cluster scale evidence is the `scale-envtest-10k` job below. + extract-budget: + name: extract-budget + runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + # No envtest cache on purpose: this job never starts an API server. - uses: ./.github/actions/go-cache - with: - envtest: true - uses: go-task/setup-task@01a4adf9db2d14c1de7a560f09170b6e0df736aa # v2.1.0 with: version: 3.51.1 - - name: Opt-in 10k load tier - run: task load-test:10k + - name: Extractor hot-path budget (micro-benchmark, not a scale tier) + run: task extract-budget env: CI: "true" diff --git a/Taskfile.yml b/Taskfile.yml index b0a4720c..3f2a617e 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -224,15 +224,15 @@ tasks: cmds: - task: test:bench - load-test: - desc: Opt-in bounded load tests (requires KOLECT_LOAD_TEST=1, max 10000) - cmds: - - task: test:load-test - - load-test:10k: - desc: Opt-in 10k load tier (KOLECT_LOAD_TEST_MAX=10000) - cmds: - - task: test:load-test:10k + extract-budget: + desc: >- + Extractor hot-path budget — fails if ns/op, B/op or allocs/op regress >25% + vs the recorded baseline. EXERCISES: Extract over 128 varied objects, + single-threaded, in-process. NOT exercised: API server, cluster, informers, + sinks, controller, concurrency, export. Not cluster-scale evidence — see + docs/operator-manual/load-test-runbook.md for the scale tiers. + cmds: + - task: test:extract-budget perf-report: desc: Local perf snapshot via hack/perf-report.sh (CI writes artifacts/perf-snapshot.md) diff --git a/docs/REQUIREMENTS.md b/docs/REQUIREMENTS.md index 052d37ce..ba504213 100644 --- a/docs/REQUIREMENTS.md +++ b/docs/REQUIREMENTS.md @@ -122,7 +122,7 @@ IDs are stable handles for discussion (`FR--`). | ID | Target | | --- | --- | -| NFR-PERF-1 | **Design target:** **100,000** collected rows/cluster (sharded exports; **unexecuted AR-02**). Default CI proves ≤500 synthetic; opt-in ≤2k synthetic ≠ in-cluster soak; Nightly **10k** CI **disabled / unverified** (`ubuntu-latest-8-cores` opt-in). Store **≤512 MiB @ 10k**, operator RSS **2–4 GiB @ 100k** — see [load-test runbook](operator-manual/load-test-runbook.md) / [performance](operator-manual/performance.md) | +| NFR-PERF-1 | **Design target:** **100,000** collected rows/cluster (sharded exports; **unexecuted AR-02**). Default CI proves ≤500 synthetic; the in-process `task extract-budget` micro-benchmark is **not** a scale tier; Nightly **10k** envtest CI (`scale-envtest-10k`) **disabled / unverified** (`ubuntu-latest-8-cores` opt-in). Store **≤512 MiB @ 10k**, operator RSS **2–4 GiB @ 100k** — see [load-test runbook](operator-manual/load-test-runbook.md) / [performance](operator-manual/performance.md) | | NFR-PERF-2 | Giant cluster: 1000+ nodes — namespace-scoped informers + paginated list mandatory | | NFR-PERF-3 | Fleet: **100–500+** clusters via **shared sink** ([ADR-0501](adr/0501-multi-cluster-fleet.md)); no hub merge tier | | NFR-PERF-4 | One shared informer per GVK; memory scales with objects × GVKs, not with target count | diff --git a/docs/adr/0603-performance-scalability.md b/docs/adr/0603-performance-scalability.md index 16fc6d71..8dd01569 100644 --- a/docs/adr/0603-performance-scalability.md +++ b/docs/adr/0603-performance-scalability.md @@ -23,8 +23,8 @@ operator**. | Tier | Scope | Collected rows | Clusters | Test tier / evidence | | --- | --- | --- | --- | --- | | **CI / dev** | Synthetic envtest | ≤500 | 1 | `task test` — Active | -| **Opt-in load** | Synthetic | ≤2,000 | 1 | `KOLECT_LOAD_TEST=1 task load-test` — synthetic ≠ in-cluster soak | -| **Nightly load** | Synthetic | **10,000** | 1 | `task load-test:10k` on `ubuntu-latest-8-cores` — **disabled / opt-in** until runners exist ([load-test runbook](../operator-manual/load-test-runbook.md)) | +| **Extractor budget** | In-process, no cluster | n/a (micro-benchmark) | 1 | `task extract-budget` — hot-path ns/op + B/op budget; **not** a scale tier, ≠ in-cluster soak | +| **Nightly load** | Synthetic envtest | **10,000** | 1 | `scale-envtest-10k` on `ubuntu-latest-8-cores` — **disabled / opt-in** until runners exist ([load-test runbook](../operator-manual/load-test-runbook.md)) | | **Baseline production** | Single cluster | **10,000+** | 1 | Metrics + pprof — **unverified** until a named SHA / hardware evidence is published | | **Design target** | Single cluster | **100,000** | 1–2 cloud | Manual cloud soak — **unexecuted (AR-02)** | | **Fleet** | Shared Postgres/Git sink | 10k–100k × N operators | **many** | One ServiceMonitor per cluster release | diff --git a/docs/adr/0706-testing-merge-gate-architecture.md b/docs/adr/0706-testing-merge-gate-architecture.md index 4c2e5123..1bbd2da7 100644 --- a/docs/adr/0706-testing-merge-gate-architecture.md +++ b/docs/adr/0706-testing-merge-gate-architecture.md @@ -25,7 +25,7 @@ gates (e.g. Q16 RBAC audit, Q15 supply-chain attestations) without debate. | **L2 — Golden / contract** | OpenAPI fragments, sample YAML decode, extractor goldens | checked-in `test/` + `config/samples/` | `task test`; samples per [ADR-0301](0301-event-driven-informers.md) | | **L3 — Integration** | Real Postgres, Kafka, Git, S3, GCS, Redis, NATS via **testcontainers** | `-tags=integration` | `task test-integration` | | **L4 — E2E** | Kind cluster: Helm install, smoke, export asserts | `hack/kind/e2e/`, `hack/e2e/` | `task test:e2e`; nightly workflow | -| **L5 — Load / perf (opt-in)** | Bounded synthetic scale (≤2000 objects) | `-tags=load`, `KOLECT_LOAD_TEST=1` | `task load-test`; `task bench`; `task perf-report` | +| **L5 — Perf budget / benchmarks** | In-process extractor hot path (no API server, cluster, sinks or concurrency) | Default gate + nightly | `task extract-budget`; `task bench`; `task perf-report` | **Direction:** most tests live at L0–L2; every new sink backend must reach **L3** before merge ([NFR-EXT-3](../REQUIREMENTS.md)); L4 catches wiring regressions webhooks/RBAC/informers miss @@ -80,7 +80,10 @@ matrix remains nightly + manual dispatch. ### Scale and load bounds - Default **`task test` / `task coverage`**: synthetic object caps **≤500** ([ADR-0603](0603-performance-scalability.md)). -- **`task load-test`**: requires `KOLECT_LOAD_TEST=1`; hard cap **2000** objects — never in default CI. +- **`task extract-budget`**: extractor hot-path budget — fails when ns/op, B/op or allocs/op + regress **>25%** against a recorded baseline. In-process only: no API server, cluster, sinks, + controller or concurrency, so it is **not** cluster-scale evidence (that is the opt-in + `scale-envtest-10k` job). Cheap enough to run in the default gate. - **`task bench`**: micro-benchmarks on hot paths (CEL/JSONPath extract); safe in dev/CI excerpt via `task perf-report`. diff --git a/docs/adr/0707-lab-harness.md b/docs/adr/0707-lab-harness.md index 4fa1fa2a..2515efc8 100644 --- a/docs/adr/0707-lab-harness.md +++ b/docs/adr/0707-lab-harness.md @@ -30,7 +30,7 @@ Merge-gate architecture ([ADR-0706](0706-testing-merge-gate-architecture.md)) al | Tier | Role | | --- | --- | | **L4 Kind e2e** | Wiring smoke — `hack/kind/e2e/`, `hack/e2e/`; Tier 0 blocks merge | -| **L5 load** | Opt-in `task load-test` (≤2000), `task perf-report`; not merge-blocking | +| **L5 perf** | `task extract-budget` (in-process extractor budget), `task perf-report`; not merge-blocking | | **100k cloud** | Separate runbook / `hack/loadtest/` — unexecuted claim gate | The lab harness must **not** become a Kind wrapper, a second CI pyramid, or a public claim that @@ -72,7 +72,7 @@ The lab harness must **not** become a Kind wrapper, a second CI pyramid, or a pu | **C2 DR-\* only** | Matches proven protocols | Ubuntu A–G catalogue orphans | | **C3 Registry: schedule → primary IDs + aliases** | DR-\* primary for multi-node; LAB-\* aliases for Ubuntu mapping | Small registry file to maintain | -### (d) Relationship to Kind L4 / loadtest L5 +### (d) Relationship to Kind L4 / perf-budget L5 | Option | Pros | Cons | | --- | --- | --- | @@ -138,7 +138,7 @@ flowchart TB subgraph ci ["CI / merge (ADR-0706)"] l0l3["L0–L3 unit / envtest / integration"] l4["L4 Kind e2e
hack/kind + hack/e2e"] - l5["L5 load / perf-report
opt-in"] + l5["L5 extract-budget / perf-report
in-process, not scale"] end subgraph lab ["L4.5 Lab harness (this ADR)"] @@ -209,7 +209,7 @@ flowchart TB ## Cross-links - [ADR-0706: Testing and merge-gate architecture](0706-testing-merge-gate-architecture.md) — L4 Kind / - L5 load ownership; this ADR sits beside them as maintainer **L4.5**. + L5 perf-budget ownership; this ADR sits beside them as maintainer **L4.5**. - [Local lab runbook](../operator-manual/local-lab-runbook.md) — LAB-DOC-01 adaptive schedules, `tier=auto`, isolation, and real `hack/lab/` flags. - [Lab evidence bundle](../operator-manual/lab-evidence-bundle.md) — LAB-DOC-02 publishable schema and diff --git a/docs/development/guidelines.md b/docs/development/guidelines.md index d6c382e3..d0ccec43 100644 --- a/docs/development/guidelines.md +++ b/docs/development/guidelines.md @@ -65,7 +65,9 @@ Operator test expectations. Pyramid tiers, coverage floors, and CI gates: - **Mocks** — mockery on small interfaces only. - **Metrics** — assert Prometheus counters/histograms in controller tests where behavior changes. - **Scale tests bounded** — default `task test` caps synthetic objects (500); load tests require - `KOLECT_LOAD_TEST=1` and `-tags=load` (max 2000). Never run 10k-object suites in default CI. + the opt-in envtest scale test (`TestEngine_ScaleEnvtestOptIn`, `KOLECT_SCALE_TEST_MAX`). Never + run 10k-object suites in default CI. `task extract-budget` is an in-process extractor + micro-benchmark budget — it is **not** cluster-scale evidence and must never be named as such. ## 5. Performance and scalability @@ -79,7 +81,9 @@ Operator test expectations. Pyramid tiers, coverage floors, and CI gates: - **Rate limits and circuit breakers** — per-sink `gobreaker`; transient sink/API errors requeue with jitter; terminal config errors stop requeue ([ADR-0602](../adr/0602-error-taxonomy.md)). - **Profiling** — pprof on `:6060` behind feature gate (default off); document in [operator-manual/performance.md](../operator-manual/performance.md). -- **Benchmarks** — `task bench` (`-short`, `-benchmem`); `BenchmarkExtract` for CEL/JSONPath hot path. +- **Benchmarks** — `task bench` (`-short`, `-benchmem`); `BenchmarkExtract` for the CEL/JSONPath hot + path. `TestExtractHotPathBudget` drives the same workload and **fails** on a >25% regression; + re-record the baseline consts in `internal/collect/extractor_budget_test.go` with evidence. ## 6. Definition of done (per change) diff --git a/docs/development/setup.md b/docs/development/setup.md index ea69bd34..f3eb59ff 100644 --- a/docs/development/setup.md +++ b/docs/development/setup.md @@ -252,17 +252,29 @@ go test -short -bench=. -benchmem ./internal/collect/... Uses `-short` so long sub-benchmarks are skipped on laptops. Suitable for CI and quick regression checks. See [operator-manual/performance.md](../operator-manual/performance.md) and [ADR-0603](../adr/0603-performance-scalability.md). -### Load tests (opt-in, bounded) - -**Not** part of default `task test`. Requires explicit opt-in and caps at **2000** synthetic objects: +### Extractor hot-path budget (`task extract-budget`) ```sh -KOLECT_LOAD_TEST=1 task load-test +task extract-budget # equivalent: -KOLECT_LOAD_TEST=1 go test -tags=load -count=1 -timeout=15m ./test/load/... +go test -run TestExtractHotPathBudget -count=1 -v ./internal/collect/... ``` -Never run 10k-object load tests locally unless you have dedicated hardware and understand API-server +`TestExtractHotPathBudget` drives the same workload as `BenchmarkExtract` — 128 varied +Deployment-shaped objects through `collect.Extractor.Extract` — and **fails** when ns/op, B/op or +allocs/op exceed the recorded baseline by more than 25%. It also runs as part of `task test`. + +**What it exercises:** the extractor hot path, single-threaded, in-process. +**What it does not:** API server, cluster, informers, sinks, controller, concurrency, export. It is +a micro-benchmark budget, **not** cluster-scale evidence — the scale tiers live in the +[load test runbook](../operator-manual/load-test-runbook.md). + +B/op and allocs/op are hardware-independent and carry the regression signal; the ns/op ceiling is +deliberately coarse so it cannot false-red on a shared CI runner. Tighten per machine with +`KOLECT_EXTRACT_MAX_NS_PER_OP` (also `_BYTES_` / `_ALLOCS_`). + +Cluster scale is the opt-in envtest test `TestEngine_ScaleEnvtestOptIn` (`KOLECT_SCALE_TEST_MAX`). +Never run 10k-object suites locally unless you have dedicated hardware and understand API-server load. Default envtest suites cap synthetic objects at **500**. ### Performance report (`task perf-report`) diff --git a/docs/development/testing.md b/docs/development/testing.md index 26eec0a0..857358d7 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -17,11 +17,12 @@ Kollect is **TDD-first**. Quality gates follow a six-tier test pyramid (L0–L5) | **L2 — Golden / contract** | OpenAPI fragments, sample YAML, extractor goldens | Yes | `task test` | | **L3 — Integration** | Real Postgres, Kafka, Git, S3, GCS, Redis, NATS (testcontainers) | Yes | `task test-integration` | | **L4 — E2E** | Kind cluster: Helm install, smoke, export asserts | **PR smoke (required)** + nightly / extended | `task test:e2e` | -| **L5 — Load / perf** | Bounded synthetic scale (≤2000 objects), micro-benchmarks | Opt-in | `task load-test` · `task perf-report` | +| **L5 — Perf budget / benchmarks** | In-process extractor hot path (no cluster, sinks or concurrency) | Default + nightly | `task extract-budget` · `task bench` · `task perf-report` | **Direction:** Most tests live at L0–L2. Every new sink backend must reach **L3** before merge -([NFR-EXT-3](../REQUIREMENTS.md)). L4 catches wiring regressions that unit tests miss. L5 stays -opt-in so default CI stays fast. +([NFR-EXT-3](../REQUIREMENTS.md)). L4 catches wiring regressions that unit tests miss. L5 is an +in-process budget, not a scale tier: it asserts the extractor hot path has not regressed >25% and +says nothing about cluster scale. Cluster scale is the opt-in `scale-envtest-10k` job. ## Coverage target @@ -124,7 +125,7 @@ For **local** runs the variable is optional: export `GIT_EXPORT_TEST_REPO` to a Kind L4 proves single-node wiring. Separately, published **v0.16.0** was exercised on a Talos lab with **1 control plane + 2 workers** (`quick+sinks`, **ready with conditions**). Maintainer -multi-node / existing-cluster evidence sits as **L4.5** beside Kind L4 and load L5 — +multi-node / existing-cluster evidence sits as **L4.5** beside Kind L4 and perf-budget L5 — [ADR-0707: Lab harness architecture](../adr/0707-lab-harness.md). Publishable shape, redaction, and an example matrix live in the [lab evidence bundle contract](../operator-manual/lab-evidence-bundle.md). @@ -160,7 +161,7 @@ or the 100k cloud gate. Raw protocols stay local-only — see the | `task test-integration` | L3 sink/transport integration (Docker) | | `task test:e2e` | L4 kind smoke (setup → smoke → teardown) | | `task bench` | Micro-benchmarks on hot paths | -| `KOLECT_LOAD_TEST=1 task load-test` | L5 bounded load (≤2000 objects, opt-in) | +| `task extract-budget` | L5 extractor hot-path budget (in-process; not cluster scale) | | `task perf-report` | Benchmark + unit pass summary (local only, gitignored output) | Full local setup: [development/setup.md](../development/setup.md). diff --git a/docs/operator-manual/load-test-runbook.md b/docs/operator-manual/load-test-runbook.md index 59df8860..19d35248 100644 --- a/docs/operator-manual/load-test-runbook.md +++ b/docs/operator-manual/load-test-runbook.md @@ -13,12 +13,14 @@ plainly planned / unverified / disabled). Re-enable Active wording for Nightly 1 | Tier | Workload shape | Execution layer | Last evidence / status | | --- | --- | --- | --- | | CI default | ≤500 synthetic objects | envtest (`task test`) | ✅ Active — every PR / local `task test` | -| CI extended | ≤2,000 synthetic objects | envtest opt-in (`KOLECT_LOAD_TEST=1 task load-test`) | ✅ Opt-in — **synthetic extraction**; ≠ in-cluster 10k collection/export/soak proof | -| Nightly 10k | 10,000 synthetic objects | `task load-test:10k` / scale envtest on `ubuntu-latest-8-cores` | ⬜ **Disabled / opt-in / unverified** — `load-test-10k` and `scale-envtest-10k` in `.github/workflows/e2e-nightly.yaml` run only on `workflow_dispatch` with `run_scale_jobs=true` because 8-core runners are unavailable; **no current green SHA** | +| Extractor budget | 128 varied objects, in-process, single-threaded | `task extract-budget` — **no** API server, cluster, informers, sinks, controller or concurrency | ✅ Active — every PR / nightly `extract-budget` job. **Not a scale tier**: it is a micro-benchmark budget (ns/op, B/op, allocs/op vs baseline +25%), **not** collection/export/soak proof | +| Nightly 10k | 10,000 synthetic objects | scale envtest (`TestEngine_ScaleEnvtestOptIn`) on `ubuntu-latest-8-cores` | ⬜ **Disabled / opt-in / unverified** — `scale-envtest-10k` in `.github/workflows/e2e-nightly.yaml` runs only on `workflow_dispatch` with `run_scale_jobs=true` because 8-core runners are unavailable; **no current green SHA** | | Laptop / L4.5 lab | Bounded schedule (e.g. `quick+sinks`) | Single-host existing cluster (Kind / K3s / Talos) | Named pin only — see [local lab runbook](local-lab-runbook.md) and [lab evidence bundle](lab-evidence-bundle.md); **does not** satisfy the 100k / two-cluster gate | | **Design proof** | **100,000** collected rows | **2× public cloud clusters** (GKE target) | ⬜ **Planned / unexecuted (AR-02)** — no SHA / date / hardware yet | -**Bounded `task load-test` (≤2k) is not in-cluster 10k proof.** Nightly 10k CI is **not** Active +**`task extract-budget` synthetic extraction is not in-cluster 10k proof.** It has no API server, +cluster, sinks or concurrency — a green run says the extractor hot path did not regress, nothing +about scale. Nightly 10k CI is **not** Active while 8-core jobs stay disabled. **100k = manual cloud gate only** — laptop / lab READY WITH CONDITIONS evidence does not close AR-02. @@ -27,8 +29,8 @@ CONDITIONS evidence does not close AR-02. | Claim | Required artifact | Current state | | --- | --- | --- | | CI ≤500 | PR / `task test` logs | Active | -| Opt-in ≤2k | Local / CI with `KOLECT_LOAD_TEST=1` | Opt-in synthetic | -| Nightly 10k | Green `load-test-10k` / `scale-envtest-10k` run (SHA + date + runner) | **Unverified** — jobs disabled | +| Extractor budget | `task extract-budget` (default gate + nightly `extract-budget` job) | Active — micro-benchmark only, not a scale claim | +| Nightly 10k | Green `scale-envtest-10k` run (SHA + date + runner) | **Unverified** — job disabled | | Laptop L4.5 | Redacted [lab evidence bundle](lab-evidence-bundle.md) for a named pin | Bounded single-host only | | 100k / two-cluster | Completed soak per this runbook (metrics + SHA + hardware) | **Unexecuted** | @@ -147,7 +149,8 @@ dispatch queue saturation (no dedicated log line — metric only). - **No** 100k job in `.github/workflows/` on `ubuntu-latest` - **No** GKE execution in CI — maintainer runs manually when ready - **No** treating laptop / Talos L4.5 evidence as the 100k / two-cluster claim -- **No** equating bounded `task load-test` synthetic extraction with Nightly 10k or in-cluster soak +- **No** equating `task extract-budget` synthetic extraction with Nightly 10k or in-cluster soak +- **No** CI job name that implies cluster scale from an in-process micro-benchmark (PERF-FIX-04) ## Related diff --git a/docs/operator-manual/performance.md b/docs/operator-manual/performance.md index 0a3c327f..26306d97 100644 --- a/docs/operator-manual/performance.md +++ b/docs/operator-manual/performance.md @@ -12,8 +12,8 @@ must match [load-test-runbook.md](load-test-runbook.md) evidence status. | Tier | Workload shape | Clusters | Execution layer | Evidence status | | --- | --- | --- | --- | --- | | Dev / CI default | ≤500 synthetic | 1 | envtest (`task test`) | ✅ Active — every PR | -| Opt-in load | ≤2,000 synthetic | 1 | `KOLECT_LOAD_TEST=1 task load-test` | ✅ Opt-in — synthetic extraction; ≠ in-cluster 10k soak | -| Nightly 10k | **10,000** synthetic | 1 | `task load-test:10k` on `ubuntu-latest-8-cores` | ⬜ **Disabled / unverified** — jobs opt-in only (`run_scale_jobs`); no green SHA while runners unavailable | +| Extractor budget | 128 varied objects, in-process | 0 (no cluster) | `task extract-budget` | ✅ Active — micro-benchmark budget; **not a scale tier**, ≠ in-cluster 10k soak | +| Nightly 10k | **10,000** synthetic | 1 | `scale-envtest-10k` on `ubuntu-latest-8-cores` | ⬜ **Disabled / unverified** — job opt-in only (`run_scale_jobs`); no green SHA while runners unavailable | | Baseline production | **10,000+** in-cluster | 1 | Metrics + pprof; manual load | ⬜ **Unverified** — no published SHA / date / hardware evidence yet | | Laptop / L4.5 lab | Bounded schedule | 1 (single-host) | Existing cluster (Kind / K3s / Talos) | Named pin only — [local lab](local-lab-runbook.md) / [evidence bundle](lab-evidence-bundle.md); **does not** satisfy 100k / two-cluster gate | | Design target | **100,000** | 1–2 cloud | Manual cloud soak ([load-test runbook](load-test-runbook.md)) | ⬜ **Planned / unexecuted (AR-02)** — needs export sharding + Postgres bulk upsert | @@ -106,16 +106,24 @@ and example PromQL for alerting. separate from Prometheus metrics (`:8080` / `:8443`). Helm sets `pprof.enabled: false` by default; enable in dev overlays only. -## Benchmarks and load tests +## Benchmarks and the extractor budget ```bash task bench # writes artifacts/bench/*.txt -KOLECT_LOAD_TEST=1 task load-test +task extract-budget # fails on a >25% hot-path regression ``` -For local perf summaries (`task perf-report`), see [contributor setup](../development/setup.md). +`task extract-budget` runs `TestExtractHotPathBudget`, which drives the same workload as +`BenchmarkExtract` over 128 varied objects and asserts ns/op, B/op and allocs/op against a +recorded baseline +25%. **What it exercises:** `collect.Extractor.Extract`, single-threaded, +in-process. **What it does not:** API server, cluster, informers, sinks, controller, concurrency, +export. It is not cluster-scale evidence — see the tier table above. + +The B/op and allocs/op ceilings are hardware-independent and carry the regression signal; the +ns/op ceiling is deliberately coarse so it cannot false-red on a shared CI runner. Tighten it per +machine with `KOLECT_EXTRACT_MAX_NS_PER_OP` (also `_BYTES_` / `_ALLOCS_`). -Default `go test ./...` excludes `load`-tagged tests. +For local perf summaries (`task perf-report`), see [contributor setup](../development/setup.md). ## Early bottleneck checklist @@ -124,7 +132,7 @@ Default `go test ./...` excludes `load`-tagged tests. | High `kollect_informer_objects`, high RSS | Cluster-wide informer for multi-namespace targets | Namespace-scope targets; split profiles | | High `kollect_workqueue_depth` on `inventory` | Export or aggregation on hot path | Raise inventory workers; increase `spec.exportMinInterval` | | High export bytes rate, low object churn | Missing payload dedupe | Verify debounce + content-hash skip | -| Bench regression in `BenchmarkExtract` | CEL/JSONPath hot path | Profile extractor; check attribute count | +| `TestExtractHotPathBudget` red / `BenchmarkExtract` regression | CEL/JSONPath hot path | Profile extractor; check attribute count and CEL complexity | | High RSS on large clusters | Full in-memory collect store | Namespace-scoped targets; raise export interval ([ADR-0603](../adr/0603-performance-scalability.md)) | ## Fleet operations diff --git a/hack/perf-report.sh b/hack/perf-report.sh index 073363eb..d5cbdfc5 100755 --- a/hack/perf-report.sh +++ b/hack/perf-report.sh @@ -97,10 +97,10 @@ fi METRICS_LIST="$(grep -E '^\s+Name:\s+"kollect_' "${ROOT}/internal/metrics/metrics.go" 2>/dev/null \ | sed 's/.*"\(kollect_[^"]*\)".*/\1/' | sort -u | tr '\n' ', ' | sed 's/, $//')" +# PERF-FIX-04: there is no "load" tier here. This snapshot is in-process +# micro-benchmark + unit-test evidence only; cluster scale tiers live in +# docs/operator-manual/load-test-runbook.md. SCALE_TIER="dev" -if [[ "${KOLECT_LOAD_TEST:-}" == "1" ]]; then - SCALE_TIER="load" -fi if [[ "${CI:-}" == "true" ]]; then SCALE_TIER="ci" fi diff --git a/hack/task/Taskfile.test.yml b/hack/task/Taskfile.test.yml index 29611173..bed00cc2 100644 --- a/hack/task/Taskfile.test.yml +++ b/hack/task/Taskfile.test.yml @@ -78,23 +78,22 @@ tasks: cp "$out" artifacts/bench/latest.txt echo "wrote $out" - load-test: - desc: Opt-in bounded load tests (KOLECT_LOAD_TEST=1, max 10000 objects); not in default test + extract-budget: + desc: >- + Assert the extractor hot-path budget (ns/op, B/op, allocs/op vs a recorded + baseline +25%). EXERCISES: collect.Extractor.Extract over 128 varied + objects, single-threaded, in-process. Does NOT exercise an API server, + cluster, informers, sinks, controller, concurrency or export — it is a + micro-benchmark budget, not cluster-scale evidence. Also runs in task + test:run. Override ceilings with KOLECT_EXTRACT_MAX_{NS,BYTES,ALLOCS}_PER_OP. env: CGO_ENABLED: "1" + # This task runs the budget test ALONE and uninstrumented, so it can pin a + # far tighter latency gate than the in-code default (which has to survive + # the parallel, coverage-instrumented PR gate). 8x the S-LOCAL baseline of + # 25551 ns/op: enough headroom for an unpinned GitHub runner, ~6x tighter + # than the default. On calibrated hardware, set this to your own measured + # ns/op x 1.25 for the real >25% regression gate. + KOLECT_EXTRACT_MAX_NS_PER_OP: "204408" cmds: - - | - if [ "${KOLECT_LOAD_TEST:-}" != "1" ]; then - echo "Refusing load test: set KOLECT_LOAD_TEST=1 to opt in (max 10000 synthetic objects)." >&2 - exit 1 - fi - - go test -tags=load -count=1 -timeout=30m ./test/load/... - - load-test:10k: - desc: Opt-in 10k load tier (nightly ubuntu-latest-8-cores) - env: - CGO_ENABLED: "1" - KOLECT_LOAD_TEST: "1" - KOLECT_LOAD_TEST_MAX: "10000" - cmds: - - go test -tags=load -count=1 -timeout=30m ./test/load/... + - go test -run TestExtractHotPathBudget -count=1 -v -timeout=10m ./internal/collect/... diff --git a/hack/test/docs_lab_doc_03_scale_claims_test.sh b/hack/test/docs_lab_doc_03_scale_claims_test.sh index 694e91fc..32c973fc 100755 --- a/hack/test/docs_lab_doc_03_scale_claims_test.sh +++ b/hack/test/docs_lab_doc_03_scale_claims_test.sh @@ -32,8 +32,16 @@ pass() { [[ -f "${EVIDENCE}" ]] || fail "${EVIDENCE} is missing" # --- Workflow truth: 10k jobs disabled unless workflow_dispatch + run_scale_jobs --- -grep -qF -- 'load-test-10k:' "${WORKFLOW}" || fail "workflow missing load-test-10k job" grep -qF -- 'scale-envtest-10k:' "${WORKFLOW}" || fail "workflow missing scale-envtest-10k job" + +# PERF-FIX-04: the extractor micro-benchmark must never be named as a scale tier +# again. It has no API server, cluster, sinks or concurrency; a "load-test-10k" +# job name on an 8-core runner read as cluster-scale evidence nobody measured. +if grep -qE '^[[:space:]]*load-test-10k:' "${WORKFLOW}"; then + fail "workflow must not reintroduce a load-test-10k job (PERF-FIX-04: micro-benchmark != scale tier)" +fi +grep -qE '^[[:space:]]*extract-budget:' "${WORKFLOW}" || + fail "workflow missing extract-budget job (extractor hot-path budget)" grep -qF -- 'ubuntu-latest-8-cores' "${WORKFLOW}" || fail "workflow must name ubuntu-latest-8-cores runner" @@ -106,9 +114,9 @@ grep -Eqi 'Evidence|unverified|disabled|planned|opt-in' "${PERF}" || fail "performance.md scale tiers must state evidence status (or planned/unverified/disabled)" pass "scale tier tables expose shape, layer, and evidence status" -# Bounded task load-test ≠ in-cluster 10k proof. +# In-process extractor budget != in-cluster 10k proof. if ! grep -Eqi 'not (equivalent|equal|a substitute)|≠|does not (prove|satisfy|equal)|synthetic extraction' "${RUNBOOK}" "${PERF}"; then - fail "docs must distinguish bounded task load-test / synthetic extraction from in-cluster 10k proof" + fail "docs must distinguish in-process synthetic extraction from in-cluster 10k proof" fi # 100k remains unexecuted / AR-02 gate. diff --git a/internal/collect/extractor_bench_test.go b/internal/collect/extractor_bench_test.go index 0c8f4a80..38efa7c4 100644 --- a/internal/collect/extractor_bench_test.go +++ b/internal/collect/extractor_bench_test.go @@ -4,6 +4,8 @@ package collect import ( + "fmt" + "strings" "testing" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -11,51 +13,92 @@ import ( kollectdevv1alpha1 "github.com/platformrelay/kollect/api/v1alpha1" ) -func deploymentFixture() *unstructured.Unstructured { - return &unstructured.Unstructured{Object: map[string]any{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]any{ - "name": "demo", - "namespace": "default", - "uid": "bench-uid", - "labels": map[string]any{ - "app": "kollect", +// extractPoolSize is the number of *distinct* objects cycled through the hot +// path. A single reused object measures perfect cache locality and inflates the +// result; a bounded pool keeps the measurement reproducible while still walking +// different map layouts, string lengths and CEL branches on every iteration. +const extractPoolSize = 128 + +// deploymentPool builds a deterministic pool of Deployment-shaped objects. +// Deterministic (index-derived, no RNG) so the recorded budget is reproducible. +// Every field that the measured attribute set touches varies across the pool, +// and roughly one object in seven omits status.readyReplicas so the CEL has() +// expression exercises both branches. +func deploymentPool(n int) []*unstructured.Unstructured { + pool := make([]*unstructured.Unstructured, n) + + for i := range pool { + obj := map[string]any{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]any{ + "name": fmt.Sprintf("demo-%03d-%s", i, strings.Repeat("x", i%17)), + "namespace": fmt.Sprintf("ns-%02d", i%13), + "uid": fmt.Sprintf("00000000-0000-0000-0000-%012d", i), + "labels": map[string]any{ + "app": "kollect", + "app.kubernetes.io/part-of": fmt.Sprintf("tier-%d", i%5), + }, }, - }, - "spec": map[string]any{ - "replicas": int64(3), - "template": map[string]any{ - "metadata": map[string]any{ - "labels": map[string]any{"app": "kollect"}, + "spec": map[string]any{ + "replicas": int64(i%9 + 1), + "template": map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"app": "kollect"}, + }, }, }, - }, - "status": map[string]any{ - "readyReplicas": int64(3), - }, - }} -} + } -func BenchmarkExtract(b *testing.B) { - extractor, err := NewExtractor() - if err != nil { - b.Fatalf("NewExtractor: %v", err) + if i%7 != 0 { + obj["status"] = map[string]any{"readyReplicas": int64(i%9 + 1)} + } else { + obj["status"] = map[string]any{"observedGeneration": int64(i)} + } + + pool[i] = &unstructured.Unstructured{Object: obj} } - obj := deploymentFixture() - attrs := []kollectdevv1alpha1.AttributeSpec{ + return pool +} + +// extractAttrs is the measured attribute set: two JSONPath attributes and one +// CEL attribute, matching the shape of a realistic KollectInventory. +func extractAttrs() []kollectdevv1alpha1.AttributeSpec { + return []kollectdevv1alpha1.AttributeSpec{ {Name: "name", Path: "{.metadata.name}"}, {Name: "replicas", Path: "{.spec.replicas}"}, {Name: "ready", Path: "cel:has(object.status.readyReplicas) ? object.status.readyReplicas : 0"}, } +} + +// extractWorkload is the SINGLE definition of the extractor hot path. Both +// BenchmarkExtract (human-readable numbers via `task bench`) and +// TestExtractHotPathBudget (assertions that can fail) drive it, so there is +// exactly one workload to keep honest. +func extractWorkload(b *testing.B) { + b.Helper() + + extractor, err := NewExtractor() + if err != nil { + b.Fatalf("NewExtractor: %v", err) + } + + // Built outside the timed region: this measures Extract, not the fixture + // generator. + pool := deploymentPool(extractPoolSize) + attrs := extractAttrs() b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { - if _, err := extractor.Extract(obj, attrs); err != nil { + if _, err := extractor.Extract(pool[i%len(pool)], attrs); err != nil { b.Fatal(err) } } } + +func BenchmarkExtract(b *testing.B) { + extractWorkload(b) +} diff --git a/internal/collect/extractor_budget_test.go b/internal/collect/extractor_budget_test.go new file mode 100644 index 00000000..16bba3fc --- /dev/null +++ b/internal/collect/extractor_budget_test.go @@ -0,0 +1,152 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 Konrad Heimel + +package collect + +import ( + "os" + "strconv" + "testing" +) + +// --------------------------------------------------------------------------- +// Extractor hot-path budget. +// +// WHAT THIS EXERCISES: collect.Extractor.Extract over a bounded pool of varied +// Deployment-shaped objects with two JSONPath attributes and one CEL +// attribute, single-threaded, in-process. +// +// WHAT THIS DOES *NOT* EXERCISE: no API server, no informers, no cluster, no +// sinks, no controller, no concurrency, no export path. It is a micro-benchmark +// budget, NOT a load test and NOT cluster-scale evidence. In-cluster scale +// lives in the opt-in envtest scale test (TestEngine_ScaleEnvtestOptIn) and in +// docs/operator-manual/load-test-runbook.md. +// +// CALIBRATION (why the two dimensions have different strictness): +// +// - Bytes/op and allocs/op are hardware-INDEPENDENT for a fixed Go toolchain +// and word size — an M-series laptop and ubuntu-latest agree. These carry +// the regression-detection weight and are set at the recorded baseline +// +25%. The 25% headroom is also what absorbs a Go patch-release bump. +// +// - Ns/op is hardware-DEPENDENT. A ceiling calibrated on the M-series +// baseline would false-red on slower CI runners, and the default unit gate +// runs packages in parallel, which adds scheduling noise. So the default +// ns/op ceiling is a deliberately coarse net for catastrophic regressions +// only. Tighten it per-machine with KOLECT_EXTRACT_MAX_NS_PER_OP once you +// have a baseline for that hardware (recommended: measured × 1.25). +// +// RECORDED BASELINE — regenerate with `task bench` after changing the workload +// (worst of `-count=5`), then update the const block below in the same commit: +// +// S-LOCAL, darwin/arm64 Apple M5 Max, go1.26.5, extractPoolSize=128: +// 25551 ns/op · 32218 B/op · 452 allocs/op (~39 000 ops/s) +// +// For reference, the pre-variation single-object baseline recorded in +// agent-context/PERFTEST-RESULTS-2026-08-16.md was 27679 ns/op · 32219 B/op · +// 452 allocs/op — varying the object pool did not move the allocation profile. +// +// All three ceilings are env-overridable so a calibrated runner can assert the +// real >25% regression floor without the unit gate false-redding. +// --------------------------------------------------------------------------- + +const ( + // Recorded S-LOCAL baseline (darwin/arm64 Apple M5 Max, go1.26.5), varied pool. + baselineNsPerOp = 25551 + baselineBytesPerOp = 32218 + baselineAllocsPerOp = 452 + + // Regression headroom: the acceptance criterion is "fails if throughput + // regresses by more than 25%". + budgetHeadroomNumerator = 125 + budgetHeadroomDenominator = 100 + + // Hardware slack applied to the ns/op ceiling ONLY (see CALIBRATION above). + // This default has to survive the WORST case: the required PR gate runs + // `go test $(go list ./...) -coverprofile`, so this test executes with + // coverage instrumentation (measured +9% locally) while ~40 sibling + // packages — several of them spinning envtest kube-apiserver/etcd — compete + // for a 4-core runner. Two concurrent commands alone cost 2.8x on an + // 18-core laptop. 50x is deliberately absurd: it catches only an + // order-of-magnitude regression, and a ceiling that false-reds on every PR + // is worse than one that never fires. + // + // `task extract-budget` runs this test ALONE and pins a much tighter + // KOLECT_EXTRACT_MAX_NS_PER_OP, which is where the real latency gate lives. + nsPerOpHardwareSlack = 50 +) + +// envBudget reads an int64 budget override, failing loudly on garbage rather +// than silently falling back to the default. +func envBudget(t *testing.T, key string, def int64) int64 { + t.Helper() + + raw := os.Getenv(key) + if raw == "" { + return def + } + + v, err := strconv.ParseInt(raw, 10, 64) + if err != nil || v <= 0 { + t.Fatalf("%s must be a positive integer, got %q", key, raw) + } + + return v +} + +func withHeadroom(base int64) int64 { + return base * budgetHeadroomNumerator / budgetHeadroomDenominator +} + +// TestExtractHotPathBudget asserts the extractor hot path stays inside its +// recorded allocation and latency budget. Unlike a bare benchmark, this FAILS. +func TestExtractHotPathBudget(t *testing.T) { + res := testing.Benchmark(extractWorkload) + + // Guard against a vacuous pass: a failed or zero-iteration benchmark + // reports zeroes, which would satisfy every ceiling below. + if res.N <= 0 { + t.Fatalf("benchmark did not run (N=%d) — extractWorkload failed", res.N) + } + if res.MemAllocs == 0 || res.MemBytes == 0 { + t.Fatalf("benchmark reported no allocation data (allocs=%d bytes=%d); "+ + "the memory ceiling would pass vacuously", res.MemAllocs, res.MemBytes) + } + + nsPerOp := res.NsPerOp() + bytesPerOp := res.AllocedBytesPerOp() + allocsPerOp := res.AllocsPerOp() + + maxNsPerOp := envBudget(t, "KOLECT_EXTRACT_MAX_NS_PER_OP", + withHeadroom(baselineNsPerOp)*nsPerOpHardwareSlack) + maxBytesPerOp := envBudget(t, "KOLECT_EXTRACT_MAX_BYTES_PER_OP", + withHeadroom(baselineBytesPerOp)) + maxAllocsPerOp := envBudget(t, "KOLECT_EXTRACT_MAX_ALLOCS_PER_OP", + withHeadroom(baselineAllocsPerOp)) + + t.Logf("extract hot path: N=%d %d ns/op (%.0f ops/s) %d B/op %d allocs/op "+ + "[ceilings: %d ns/op, %d B/op, %d allocs/op]", + res.N, nsPerOp, 1e9/float64(nsPerOp), bytesPerOp, allocsPerOp, + maxNsPerOp, maxBytesPerOp, maxAllocsPerOp) + + // Memory ceiling — hardware-independent, this is the real regression gate. + if bytesPerOp > maxBytesPerOp { + t.Errorf("memory ceiling exceeded: %d B/op > %d B/op (baseline %d B/op +25%%); "+ + "re-profile the extractor or re-record the baseline with evidence", + bytesPerOp, maxBytesPerOp, baselineBytesPerOp) + } + if allocsPerOp > maxAllocsPerOp { + t.Errorf("allocation ceiling exceeded: %d allocs/op > %d allocs/op "+ + "(baseline %d allocs/op +25%%)", + allocsPerOp, maxAllocsPerOp, baselineAllocsPerOp) + } + + // Throughput floor, expressed as a latency ceiling. Coarse by default; set + // KOLECT_EXTRACT_MAX_NS_PER_OP on calibrated hardware for the real +25% gate. + if nsPerOp > maxNsPerOp { + t.Errorf("throughput floor breached: %d ns/op > %d ns/op ceiling "+ + "(%.0f ops/s < %.0f ops/s); S-LOCAL baseline is %d ns/op", + nsPerOp, maxNsPerOp, 1e9/float64(nsPerOp), 1e9/float64(maxNsPerOp), + baselineNsPerOp) + } +} diff --git a/test/load/collect_test.go b/test/load/collect_test.go deleted file mode 100644 index 43ec4ecc..00000000 --- a/test/load/collect_test.go +++ /dev/null @@ -1,73 +0,0 @@ -// SPDX-License-Identifier: MIT -// Copyright (c) 2026 Konrad Heimel - -//go:build load - -package load_test - -import ( - "os" - "strconv" - "testing" - "time" - - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - - kollectdevv1alpha1 "github.com/platformrelay/kollect/api/v1alpha1" - "github.com/platformrelay/kollect/internal/collect" -) - -const maxLoadObjects = 2000 - -func loadTestObjectCount(t *testing.T) int { - t.Helper() - - max := maxLoadObjects - if v := os.Getenv("KOLECT_LOAD_TEST_MAX"); v != "" { - n, err := strconv.Atoi(v) - if err != nil || n <= 0 || n > 10000 { - t.Fatalf("KOLECT_LOAD_TEST_MAX must be 1..10000: %q", v) - } - max = n - } - - return max -} - -func TestLoadExtract(t *testing.T) { - if os.Getenv("KOLECT_LOAD_TEST") != "1" { - t.Skip("set KOLECT_LOAD_TEST=1 to run load tests") - } - - extractor, err := collect.NewExtractor() - if err != nil { - t.Fatalf("NewExtractor: %v", err) - } - - attrs := []kollectdevv1alpha1.AttributeSpec{ - {Name: "name", Path: "{.metadata.name}"}, - {Name: "replicas", Path: "{.spec.replicas}"}, - } - - obj := &unstructured.Unstructured{Object: map[string]any{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]any{ - "name": "load-demo", "namespace": "default", - }, - "spec": map[string]any{"replicas": int64(1)}, - }} - - max := loadTestObjectCount(t) - - start := time.Now() - for i := 0; i < max; i++ { - if _, err := extractor.Extract(obj, attrs); err != nil { - t.Fatalf("extract %d: %v", i, err) - } - } - - elapsed := time.Since(start) - t.Logf("extracted %d objects in %s (%.0f ops/s)", - max, elapsed, float64(max)/elapsed.Seconds()) -} diff --git a/test/load/load_test.go b/test/load/load_test.go deleted file mode 100644 index c2f0ffe9..00000000 --- a/test/load/load_test.go +++ /dev/null @@ -1,34 +0,0 @@ -// SPDX-License-Identifier: MIT -// Copyright (c) 2026 Konrad Heimel - -//go:build load - -package load_test - -import ( - "os" - "strconv" - "testing" -) - -const defaultLoadTestMaxObjects = 2000 -const maxLoadTestMaxObjects = 10000 - -// TestSyntheticObjectCap verifies the load-test harness respects the ADR-0603 object cap. -// Full collection/export load scenarios are added here as the reconcile path matures. -func TestSyntheticObjectCap(t *testing.T) { - if os.Getenv("KOLECT_LOAD_TEST") != "1" { - t.Skip("set KOLECT_LOAD_TEST=1 to run load tests") - } - - max := defaultLoadTestMaxObjects - if v := os.Getenv("KOLECT_LOAD_TEST_MAX"); v != "" { - n, err := strconv.Atoi(v) - if err != nil || n <= 0 || n > maxLoadTestMaxObjects { - t.Fatalf("KOLECT_LOAD_TEST_MAX must be 1..%d: %q", maxLoadTestMaxObjects, v) - } - max = n - } - - t.Logf("load test harness ready (cap=%d objects)", max) -} From 6dcd9418746e56bf563db2b235c6dc1ec210222e Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 16 Aug 2026 08:51:40 +0200 Subject: [PATCH 2/2] :memo: docs(collect): stop claiming a >25% wall-clock gate the budget does not enforce MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent review, F1 (P1): seven surfaces said `task extract-budget` fails a >25% ns/op regression. It does not. The reviewer injected a CPU-only slowdown into `Extract` and measured throughput fall 39144 -> 11078 ops/s (-72%, 3.5x) at UNCHANGED B/op and allocs/op. It passed both the task-pinned ceiling (204408) and the in-code default (1596900). The claim was false on every surface that made it. Coordinator decision: fix the overclaiming, not the ceiling. Do NOT tighten ns/op. The asymmetry argument stands and the reviewer independently confirmed it — B/op is hardware-independent (arm64 32217 vs amd64 32212, unchanged under -cover, 32980/459 under -race, and unchanged across go1.26.5 -> go1.26.6) while ns/op had only a 4.65x margin under -race on the fastest hardware available, so tightening would red the nightly. A flaky wall-clock gate is worse than none. ON THE RECORD, so a later reader does not think it was dropped silently: PERF-FIX-04's acceptance criterion "throughput regresses >25% => test FAILS" is AMENDED to be met for the ALLOCATION dimensions only. B/op and allocs/op are a real >25% gate that fails on any runner; the ns/op ceiling is a catastrophic-regression net and is now described as exactly that everywhere. This amendment is recorded in the test's own header comment, not just here. Corrected surfaces: Taskfile.yml, hack/task/Taskfile.test.yml, ADR-0706, docs/development/{guidelines,testing,setup}.md, docs/operator-manual/ performance.md, docs/operator-manual/load-test-runbook.md, and the CALIBRATION / headroom / assertion comments in extractor_budget_test.go. performance.md and setup.md had additionally contradicted themselves inside a single section. The ns/op failure message now says what breaching it means. Also from review: - F5 (P3): dropped `CGO_ENABLED: "1"` from test:extract-budget — carried over from the deleted load-test task; nothing here needs cgo. Verified green. - F6 (P3): dropped the "+9% coverage instrumentation" claim. The reviewer measured -cover at 24994 ns/op, marginally FASTER than uninstrumented. The 50x default absorbs runner contention, not -cover; the comment now says so. - F7 (P3): `task bench` and hack/perf-report.sh ran the whole package, so a red TestExtractHotPathBudget could fail a benchmark task. Both now pass `-run='^$'`, making them pure benchmark passes as their names imply. Proved: KOLECT_EXTRACT_MAX_BYTES_PER_OP=1 task bench is still green. - MD046: the new performance.md admonition tripped the same indented-code-block trap as 4ccba7305; promoted to a subsection, per that fix. F2 (nightly -race margin) and F3 (never measured on native ubuntu x86) are deliberately NOT addressed here — the coordinator is filing them as a follow-up to re-pin against the first real nightly `extract-budget` ns/op from CI. Rebased onto origin/main (PERF-FIX-06 Go 1.26.6 + CHANGELOG sync), 0 conflicts. Baseline re-measured under go1.26.6: 32218 B/op / 452 allocs/op unchanged, ns/op 25077-26311 across -count=5, so the recorded 25551 still holds. --- Taskfile.yml | 13 ++-- .../0706-testing-merge-gate-architecture.md | 12 ++-- docs/development/guidelines.md | 6 +- docs/development/setup.md | 18 +++-- docs/development/testing.md | 7 +- docs/operator-manual/load-test-runbook.md | 2 +- docs/operator-manual/performance.md | 25 ++++--- hack/perf-report.sh | 4 +- hack/task/Taskfile.test.yml | 36 ++++++---- internal/collect/extractor_budget_test.go | 70 +++++++++++-------- 10 files changed, 120 insertions(+), 73 deletions(-) diff --git a/Taskfile.yml b/Taskfile.yml index 3f2a617e..19d20664 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -226,10 +226,15 @@ tasks: extract-budget: desc: >- - Extractor hot-path budget — fails if ns/op, B/op or allocs/op regress >25% - vs the recorded baseline. EXERCISES: Extract over 128 varied objects, - single-threaded, in-process. NOT exercised: API server, cluster, informers, - sinks, controller, concurrency, export. Not cluster-scale evidence — see + Extractor hot-path budget. ENFORCED: B/op and allocs/op at the recorded + baseline +25% — hardware-independent, so a >25% allocation regression + fails anywhere. NOT enforced: a >25% wall-clock gate; the ns/op ceiling is + a coarse catastrophic-regression net only (a CPU-only 3.5x slowdown at + unchanged allocations passes it) — pin KOLECT_EXTRACT_MAX_NS_PER_OP to + your own measured ns/op x 1.25 for a real latency floor. EXERCISES: + Extract over 128 varied objects, single-threaded, in-process. NOT + exercised: API server, cluster, informers, sinks, controller, concurrency, + export. Not cluster-scale evidence — see docs/operator-manual/load-test-runbook.md for the scale tiers. cmds: - task: test:extract-budget diff --git a/docs/adr/0706-testing-merge-gate-architecture.md b/docs/adr/0706-testing-merge-gate-architecture.md index 1bbd2da7..25e8eeec 100644 --- a/docs/adr/0706-testing-merge-gate-architecture.md +++ b/docs/adr/0706-testing-merge-gate-architecture.md @@ -80,10 +80,14 @@ matrix remains nightly + manual dispatch. ### Scale and load bounds - Default **`task test` / `task coverage`**: synthetic object caps **≤500** ([ADR-0603](0603-performance-scalability.md)). -- **`task extract-budget`**: extractor hot-path budget — fails when ns/op, B/op or allocs/op - regress **>25%** against a recorded baseline. In-process only: no API server, cluster, sinks, - controller or concurrency, so it is **not** cluster-scale evidence (that is the opt-in - `scale-envtest-10k` job). Cheap enough to run in the default gate. +- **`task extract-budget`**: extractor hot-path budget. It **enforces >25%** on `B/op` and + `allocs/op` only — those are hardware-independent for a fixed toolchain, so an allocation + regression fails on any runner. It does **not** enforce a >25% wall-clock gate: the `ns/op` + ceiling is a coarse catastrophic-regression net, because a ceiling loose enough to be safe on a + shared CI runner is far too loose to catch 25%. A CPU-only regression at unchanged allocations + can pass it; pin `KOLECT_EXTRACT_MAX_NS_PER_OP` per machine for a real latency floor. In-process + only: no API server, cluster, sinks, controller or concurrency, so it is **not** cluster-scale + evidence (that is the opt-in `scale-envtest-10k` job). Cheap enough to run in the default gate. - **`task bench`**: micro-benchmarks on hot paths (CEL/JSONPath extract); safe in dev/CI excerpt via `task perf-report`. diff --git a/docs/development/guidelines.md b/docs/development/guidelines.md index d0ccec43..0916a9d2 100644 --- a/docs/development/guidelines.md +++ b/docs/development/guidelines.md @@ -82,8 +82,10 @@ Operator test expectations. Pyramid tiers, coverage floors, and CI gates: jitter; terminal config errors stop requeue ([ADR-0602](../adr/0602-error-taxonomy.md)). - **Profiling** — pprof on `:6060` behind feature gate (default off); document in [operator-manual/performance.md](../operator-manual/performance.md). - **Benchmarks** — `task bench` (`-short`, `-benchmem`); `BenchmarkExtract` for the CEL/JSONPath hot - path. `TestExtractHotPathBudget` drives the same workload and **fails** on a >25% regression; - re-record the baseline consts in `internal/collect/extractor_budget_test.go` with evidence. + path. `TestExtractHotPathBudget` drives the same workload and **fails on a >25% `B/op` or + `allocs/op` regression**; its `ns/op` ceiling is only a catastrophic-regression net, not a >25% + wall-clock gate. Re-record the baseline consts in + `internal/collect/extractor_budget_test.go` with evidence. ## 6. Definition of done (per change) diff --git a/docs/development/setup.md b/docs/development/setup.md index f3eb59ff..103c298b 100644 --- a/docs/development/setup.md +++ b/docs/development/setup.md @@ -261,18 +261,24 @@ go test -run TestExtractHotPathBudget -count=1 -v ./internal/collect/... ``` `TestExtractHotPathBudget` drives the same workload as `BenchmarkExtract` — 128 varied -Deployment-shaped objects through `collect.Extractor.Extract` — and **fails** when ns/op, B/op or -allocs/op exceed the recorded baseline by more than 25%. It also runs as part of `task test`. +Deployment-shaped objects through `collect.Extractor.Extract` — and checks ns/op, B/op and +allocs/op against a recorded baseline. It also runs as part of `task test`. + +**Enforced at baseline +25%:** `B/op` and `allocs/op`. Hardware-independent for a fixed Go +toolchain (verified across arm64/amd64, `-cover`, `-race`, and the go1.26.5 -> 1.26.6 bump), so an +allocation regression fails on any runner. + +**Not enforced:** a >25% wall-clock gate. The `ns/op` ceiling is a coarse catastrophic-regression +net only — any ceiling loose enough to be safe on a shared CI runner is far too loose to catch +25%, and a CPU-only regression at unchanged allocations will pass it. For a real latency floor, +measure your own hardware and pin `KOLECT_EXTRACT_MAX_NS_PER_OP` to that x 1.25 (also `_BYTES_` / +`_ALLOCS_`). **What it exercises:** the extractor hot path, single-threaded, in-process. **What it does not:** API server, cluster, informers, sinks, controller, concurrency, export. It is a micro-benchmark budget, **not** cluster-scale evidence — the scale tiers live in the [load test runbook](../operator-manual/load-test-runbook.md). -B/op and allocs/op are hardware-independent and carry the regression signal; the ns/op ceiling is -deliberately coarse so it cannot false-red on a shared CI runner. Tighten per machine with -`KOLECT_EXTRACT_MAX_NS_PER_OP` (also `_BYTES_` / `_ALLOCS_`). - Cluster scale is the opt-in envtest test `TestEngine_ScaleEnvtestOptIn` (`KOLECT_SCALE_TEST_MAX`). Never run 10k-object suites locally unless you have dedicated hardware and understand API-server load. Default envtest suites cap synthetic objects at **500**. diff --git a/docs/development/testing.md b/docs/development/testing.md index 857358d7..e04827a5 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -21,8 +21,9 @@ Kollect is **TDD-first**. Quality gates follow a six-tier test pyramid (L0–L5) **Direction:** Most tests live at L0–L2. Every new sink backend must reach **L3** before merge ([NFR-EXT-3](../REQUIREMENTS.md)). L4 catches wiring regressions that unit tests miss. L5 is an -in-process budget, not a scale tier: it asserts the extractor hot path has not regressed >25% and -says nothing about cluster scale. Cluster scale is the opt-in `scale-envtest-10k` job. +in-process budget, not a scale tier: it fails a >25% regression in the extractor's `B/op` or +`allocs/op` (its `ns/op` ceiling is only a catastrophic-regression net), and says nothing about +cluster scale. Cluster scale is the opt-in `scale-envtest-10k` job. ## Coverage target @@ -161,7 +162,7 @@ or the 100k cloud gate. Raw protocols stay local-only — see the | `task test-integration` | L3 sink/transport integration (Docker) | | `task test:e2e` | L4 kind smoke (setup → smoke → teardown) | | `task bench` | Micro-benchmarks on hot paths | -| `task extract-budget` | L5 extractor hot-path budget (in-process; not cluster scale) | +| `task extract-budget` | L5 extractor hot-path budget — >25% gate on B/op + allocs/op only (in-process; not cluster scale) | | `task perf-report` | Benchmark + unit pass summary (local only, gitignored output) | Full local setup: [development/setup.md](../development/setup.md). diff --git a/docs/operator-manual/load-test-runbook.md b/docs/operator-manual/load-test-runbook.md index 19d35248..6077f62c 100644 --- a/docs/operator-manual/load-test-runbook.md +++ b/docs/operator-manual/load-test-runbook.md @@ -13,7 +13,7 @@ plainly planned / unverified / disabled). Re-enable Active wording for Nightly 1 | Tier | Workload shape | Execution layer | Last evidence / status | | --- | --- | --- | --- | | CI default | ≤500 synthetic objects | envtest (`task test`) | ✅ Active — every PR / local `task test` | -| Extractor budget | 128 varied objects, in-process, single-threaded | `task extract-budget` — **no** API server, cluster, informers, sinks, controller or concurrency | ✅ Active — every PR / nightly `extract-budget` job. **Not a scale tier**: it is a micro-benchmark budget (ns/op, B/op, allocs/op vs baseline +25%), **not** collection/export/soak proof | +| Extractor budget | 128 varied objects, in-process, single-threaded | `task extract-budget` — **no** API server, cluster, informers, sinks, controller or concurrency | ✅ Active — every PR / nightly `extract-budget` job. **Not a scale tier**: it is a micro-benchmark budget that enforces >25% on B/op and allocs/op only (the ns/op ceiling is a catastrophic-regression net, not a wall-clock gate), **not** collection/export/soak proof | | Nightly 10k | 10,000 synthetic objects | scale envtest (`TestEngine_ScaleEnvtestOptIn`) on `ubuntu-latest-8-cores` | ⬜ **Disabled / opt-in / unverified** — `scale-envtest-10k` in `.github/workflows/e2e-nightly.yaml` runs only on `workflow_dispatch` with `run_scale_jobs=true` because 8-core runners are unavailable; **no current green SHA** | | Laptop / L4.5 lab | Bounded schedule (e.g. `quick+sinks`) | Single-host existing cluster (Kind / K3s / Talos) | Named pin only — see [local lab runbook](local-lab-runbook.md) and [lab evidence bundle](lab-evidence-bundle.md); **does not** satisfy the 100k / two-cluster gate | | **Design proof** | **100,000** collected rows | **2× public cloud clusters** (GKE target) | ⬜ **Planned / unexecuted (AR-02)** — no SHA / date / hardware yet | diff --git a/docs/operator-manual/performance.md b/docs/operator-manual/performance.md index 26306d97..0cbfb455 100644 --- a/docs/operator-manual/performance.md +++ b/docs/operator-manual/performance.md @@ -110,18 +110,25 @@ enable in dev overlays only. ```bash task bench # writes artifacts/bench/*.txt -task extract-budget # fails on a >25% hot-path regression +task extract-budget # fails a >25% B/op or allocs/op regression ``` `task extract-budget` runs `TestExtractHotPathBudget`, which drives the same workload as -`BenchmarkExtract` over 128 varied objects and asserts ns/op, B/op and allocs/op against a -recorded baseline +25%. **What it exercises:** `collect.Extractor.Extract`, single-threaded, -in-process. **What it does not:** API server, cluster, informers, sinks, controller, concurrency, -export. It is not cluster-scale evidence — see the tier table above. - -The B/op and allocs/op ceilings are hardware-independent and carry the regression signal; the -ns/op ceiling is deliberately coarse so it cannot false-red on a shared CI runner. Tighten it per -machine with `KOLECT_EXTRACT_MAX_NS_PER_OP` (also `_BYTES_` / `_ALLOCS_`). +`BenchmarkExtract` over 128 varied objects and checks ns/op, B/op and allocs/op against a recorded +baseline. **What it exercises:** `collect.Extractor.Extract`, single-threaded, in-process. **What +it does not:** API server, cluster, informers, sinks, controller, concurrency, export. It is not +cluster-scale evidence — see the tier table above. + +### What the budget does and does not enforce + +**Enforced at baseline +25%:** `B/op` and `allocs/op`. These are hardware-independent for a fixed +Go toolchain, so an allocation regression fails on any runner. + +**Not enforced:** a >25% wall-clock gate. The `ns/op` ceiling is a coarse catastrophic-regression +net only — a ceiling loose enough to be safe on a shared CI runner is far too loose to catch 25%, +and a CPU-only regression at unchanged allocations will pass it. For a real latency floor, measure +your own hardware and pin `KOLECT_EXTRACT_MAX_NS_PER_OP` to that value x 1.25 (also `_BYTES_` / +`_ALLOCS_`). For local perf summaries (`task perf-report`), see [contributor setup](../development/setup.md). diff --git a/hack/perf-report.sh b/hack/perf-report.sh index d5cbdfc5..884b5380 100755 --- a/hack/perf-report.sh +++ b/hack/perf-report.sh @@ -36,7 +36,9 @@ GIT_SHA="$(git rev-parse --short HEAD 2>/dev/null || echo unknown)" BENCH_FILE="${BENCH_DIR}/latest.txt" ( - go test -short -bench=. -benchmem ./internal/collect/... 2>&1 | tee "$BENCH_FILE" + # -run='^$' keeps this a pure benchmark pass: the unit tests (including + # TestExtractHotPathBudget) are measured separately below. + go test -short -bench=. -benchmem -run='^$' ./internal/collect/... 2>&1 | tee "$BENCH_FILE" ) || true UNIT_RC=0 diff --git a/hack/task/Taskfile.test.yml b/hack/task/Taskfile.test.yml index bed00cc2..3d10fbd2 100644 --- a/hack/task/Taskfile.test.yml +++ b/hack/task/Taskfile.test.yml @@ -66,7 +66,10 @@ tasks: - defer: bash hack/kind/e2e/teardown.sh bench: - desc: Run Go micro-benchmarks (-short, -benchmem); safe for dev/CI + desc: >- + Run Go micro-benchmarks (-short, -benchmem); safe for dev/CI. Filters + tests out with -run='^$' so a red TestExtractHotPathBudget cannot fail + this task — use task extract-budget for the budget assertions. env: CGO_ENABLED: "1" cmds: @@ -74,26 +77,29 @@ tasks: - | stamp="$(date -u +%Y%m%dT%H%M%SZ)" out="artifacts/bench/${stamp}.txt" - go test -short -bench=. -benchmem ./internal/collect/... 2>&1 | tee "$out" + go test -short -bench=. -benchmem -run='^$' ./internal/collect/... 2>&1 | tee "$out" cp "$out" artifacts/bench/latest.txt echo "wrote $out" extract-budget: desc: >- - Assert the extractor hot-path budget (ns/op, B/op, allocs/op vs a recorded - baseline +25%). EXERCISES: collect.Extractor.Extract over 128 varied - objects, single-threaded, in-process. Does NOT exercise an API server, - cluster, informers, sinks, controller, concurrency or export — it is a - micro-benchmark budget, not cluster-scale evidence. Also runs in task - test:run. Override ceilings with KOLECT_EXTRACT_MAX_{NS,BYTES,ALLOCS}_PER_OP. + Assert the extractor hot-path budget. ENFORCED as a >25% regression gate: + B/op and allocs/op vs the recorded baseline (hardware-independent for a + fixed toolchain). NOT enforced: a >25% wall-clock gate — the ns/op ceiling + is only a coarse catastrophic-regression net, and a CPU-only slowdown at + unchanged allocations can pass it. EXERCISES: collect.Extractor.Extract + over 128 varied objects, single-threaded, in-process. Does NOT exercise an + API server, cluster, informers, sinks, controller, concurrency or export — + it is a micro-benchmark budget, not cluster-scale evidence. Also runs in + task test:run. Override ceilings with + KOLECT_EXTRACT_MAX_{NS,BYTES,ALLOCS}_PER_OP. env: - CGO_ENABLED: "1" - # This task runs the budget test ALONE and uninstrumented, so it can pin a - # far tighter latency gate than the in-code default (which has to survive - # the parallel, coverage-instrumented PR gate). 8x the S-LOCAL baseline of - # 25551 ns/op: enough headroom for an unpinned GitHub runner, ~6x tighter - # than the default. On calibrated hardware, set this to your own measured - # ns/op x 1.25 for the real >25% regression gate. + # This task runs the budget test ALONE, so it can pin a tighter ns/op + # ceiling than the in-code default (which must survive the parallel PR + # gate). 8x the S-LOCAL baseline of 25551 ns/op: enough headroom for an + # unpinned GitHub runner, ~6x tighter than the default — but still a + # catastrophic-regression net, NOT a 25% gate. On calibrated hardware set + # this to your own measured ns/op x 1.25 for a real latency floor. KOLECT_EXTRACT_MAX_NS_PER_OP: "204408" cmds: - go test -run TestExtractHotPathBudget -count=1 -v -timeout=10m ./internal/collect/... diff --git a/internal/collect/extractor_budget_test.go b/internal/collect/extractor_budget_test.go index 16bba3fc..cc84988b 100644 --- a/internal/collect/extractor_budget_test.go +++ b/internal/collect/extractor_budget_test.go @@ -22,32 +22,42 @@ import ( // lives in the opt-in envtest scale test (TestEngine_ScaleEnvtestOptIn) and in // docs/operator-manual/load-test-runbook.md. // -// CALIBRATION (why the two dimensions have different strictness): +// WHAT IS ACTUALLY ENFORCED — read this before quoting a number from it: // -// - Bytes/op and allocs/op are hardware-INDEPENDENT for a fixed Go toolchain -// and word size — an M-series laptop and ubuntu-latest agree. These carry -// the regression-detection weight and are set at the recorded baseline -// +25%. The 25% headroom is also what absorbs a Go patch-release bump. +// - B/op and allocs/op ARE a >25% regression gate. They are +// hardware-INDEPENDENT for a fixed Go toolchain and word size: measured +// identical on darwin/arm64 and amd64 (32217 vs 32212 B/op), unchanged +// under -cover, and only marginally higher under -race (32980 B/op / 459 +// allocs). They also held unchanged across the go1.26.5 -> go1.26.6 bump. +// A regression in either fails anywhere this test runs. // -// - Ns/op is hardware-DEPENDENT. A ceiling calibrated on the M-series -// baseline would false-red on slower CI runners, and the default unit gate -// runs packages in parallel, which adds scheduling noise. So the default -// ns/op ceiling is a deliberately coarse net for catastrophic regressions -// only. Tighten it per-machine with KOLECT_EXTRACT_MAX_NS_PER_OP once you -// have a baseline for that hardware (recommended: measured × 1.25). +// - ns/op is NOT a >25% gate. It is a deliberately coarse +// catastrophic-regression net and nothing more. Wall-clock is not portable +// across CI hardware, so any ceiling loose enough to be safe on a shared +// runner is far too loose to catch 25%. Concretely: an injected CPU-only +// regression that made Extract 3.5x slower (39144 -> 11078 ops/s) at +// UNCHANGED B/op and allocs/op passed both the task-pinned ceiling and the +// in-code default. If you need the real latency gate, measure your own +// hardware and set KOLECT_EXTRACT_MAX_NS_PER_OP to that × 1.25. +// +// This asymmetry is deliberate and is on the record: PERF-FIX-04's acceptance +// criterion ("throughput regresses >25% => FAILS") is met for the ALLOCATION +// dimensions only. It was not silently dropped — a flaky wall-clock gate on +// shared CI runners was judged worse than no wall-clock gate. // // RECORDED BASELINE — regenerate with `task bench` after changing the workload // (worst of `-count=5`), then update the const block below in the same commit: // -// S-LOCAL, darwin/arm64 Apple M5 Max, go1.26.5, extractPoolSize=128: +// S-LOCAL, darwin/arm64 Apple M5 Max, go1.26.6, extractPoolSize=128: // 25551 ns/op · 32218 B/op · 452 allocs/op (~39 000 ops/s) // // For reference, the pre-variation single-object baseline recorded in // agent-context/PERFTEST-RESULTS-2026-08-16.md was 27679 ns/op · 32219 B/op · // 452 allocs/op — varying the object pool did not move the allocation profile. // -// All three ceilings are env-overridable so a calibrated runner can assert the -// real >25% regression floor without the unit gate false-redding. +// All three ceilings are env-overridable via +// KOLECT_EXTRACT_MAX_{NS,BYTES,ALLOCS}_PER_OP, so a calibrated machine can pin +// a real latency floor without the shared unit gate false-redding. // --------------------------------------------------------------------------- const ( @@ -56,23 +66,25 @@ const ( baselineBytesPerOp = 32218 baselineAllocsPerOp = 452 - // Regression headroom: the acceptance criterion is "fails if throughput - // regresses by more than 25%". + // Regression headroom. This is a genuine >25% gate for B/op and allocs/op. + // It is ALSO applied to ns/op, but there it is only the starting point for + // nsPerOpHardwareSlack below — the shipped ns/op ceiling is not a 25% gate. budgetHeadroomNumerator = 125 budgetHeadroomDenominator = 100 // Hardware slack applied to the ns/op ceiling ONLY (see CALIBRATION above). // This default has to survive the WORST case: the required PR gate runs - // `go test $(go list ./...) -coverprofile`, so this test executes with - // coverage instrumentation (measured +9% locally) while ~40 sibling - // packages — several of them spinning envtest kube-apiserver/etcd — compete - // for a 4-core runner. Two concurrent commands alone cost 2.8x on an - // 18-core laptop. 50x is deliberately absurd: it catches only an + // `go test $(go list ./...) -coverprofile` while ~40 sibling packages — + // several of them spinning envtest kube-apiserver/etcd — compete for a + // 4-core runner. Coverage instrumentation itself is in the noise here + // (measured at or slightly below the uninstrumented number), so contention, + // not -cover, is what this absorbs: two concurrent commands alone cost 2.8x + // on an 18-core laptop. 50x is deliberately absurd. It catches only an // order-of-magnitude regression, and a ceiling that false-reds on every PR // is worse than one that never fires. // - // `task extract-budget` runs this test ALONE and pins a much tighter - // KOLECT_EXTRACT_MAX_NS_PER_OP, which is where the real latency gate lives. + // `task extract-budget` runs this test ALONE and pins a tighter + // KOLECT_EXTRACT_MAX_NS_PER_OP — still a coarse net, not a 25% gate. nsPerOpHardwareSlack = 50 ) @@ -129,7 +141,7 @@ func TestExtractHotPathBudget(t *testing.T) { res.N, nsPerOp, 1e9/float64(nsPerOp), bytesPerOp, allocsPerOp, maxNsPerOp, maxBytesPerOp, maxAllocsPerOp) - // Memory ceiling — hardware-independent, this is the real regression gate. + // Memory ceiling — hardware-independent, so this IS the >25% regression gate. if bytesPerOp > maxBytesPerOp { t.Errorf("memory ceiling exceeded: %d B/op > %d B/op (baseline %d B/op +25%%); "+ "re-profile the extractor or re-record the baseline with evidence", @@ -141,11 +153,13 @@ func TestExtractHotPathBudget(t *testing.T) { allocsPerOp, maxAllocsPerOp, baselineAllocsPerOp) } - // Throughput floor, expressed as a latency ceiling. Coarse by default; set - // KOLECT_EXTRACT_MAX_NS_PER_OP on calibrated hardware for the real +25% gate. + // Latency ceiling — a catastrophic-regression net, NOT a >25% gate. Set + // KOLECT_EXTRACT_MAX_NS_PER_OP on calibrated hardware for a real floor. if nsPerOp > maxNsPerOp { - t.Errorf("throughput floor breached: %d ns/op > %d ns/op ceiling "+ - "(%.0f ops/s < %.0f ops/s); S-LOCAL baseline is %d ns/op", + t.Errorf("latency ceiling exceeded: %d ns/op > %d ns/op "+ + "(%.0f ops/s < %.0f ops/s); S-LOCAL baseline is %d ns/op. This is a "+ + "catastrophic-regression net, so breaching it means something is very "+ + "wrong (or the runner is heavily loaded) — not merely a 25%% regression", nsPerOp, maxNsPerOp, 1e9/float64(nsPerOp), 1e9/float64(maxNsPerOp), baselineNsPerOp) }