From cbfe0d8eac5f6ae8344c79e1cf78615920afd38c Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sun, 6 Sep 2026 11:04:55 +0100 Subject: [PATCH] ci: retain exact benchmark collection provenance PR #1933 failed paired normalization benchmarks despite unchanged hot-path source. Preserve actual checkout identities, selected toolchains and sample order so investigation does not confuse PR head metadata with measured source. Keep thresholds and sample collection unchanged. Change-source: pulse-maintainer --- .github/workflows/build-and-test.yml | 1 + .../subsystems/deployment-installability.md | 14 ++++++++ scripts/BENCHMARK_EVIDENCE.md | 26 ++++++++++++++ .../installtests/build_release_assets_test.go | 24 +++++++++++++ scripts/run-ci-benchmarks.sh | 27 ++++++++++++++ scripts/tests/test-ci-benchmarks.sh | 35 +++++++++++++++++++ .../tests/test_benchmark_workflow_contract.py | 1 + 7 files changed, 128 insertions(+) create mode 100644 scripts/BENCHMARK_EVIDENCE.md diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 8c77c5472..01af79551 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -389,5 +389,6 @@ jobs: bench-results.txt bench-baseline.txt bench-comparison.txt + bench-metadata.txt if-no-files-found: warn retention-days: 7 diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index c8c8e4772..89e7110d7 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -15,6 +15,20 @@ ## Purpose +### Benchmark qualification evidence + +The Build and Test benchmark job retains `bench-metadata.txt` together with +baseline timings, candidate timings and their comparison even when the gate +fails. Metadata identifies each checkout's committed HEAD/tree, selected Go +version, platform, sample settings and paired execution order. A nested source +archive must not inherit its enclosing checkout's identity. These identities +are not clean-worktree or binary attestations; a PR checkout may be a synthetic +merge. Diagnostic provenance does not replace the existing regression gate, +frontend dependency audits or exact-candidate qualification. The workflow +wiring is checked by `TestBenchmarkQualificationRetainsProvenance` in +`scripts/installtests/build_release_assets_test.go`; executed collection cases +remain in `scripts/tests/test-ci-benchmarks.sh`. + ### Public Helm exact-package receipt The post-activation public Pages verification in `.github/workflows/helm-pages.yml` diff --git a/scripts/BENCHMARK_EVIDENCE.md b/scripts/BENCHMARK_EVIDENCE.md new file mode 100644 index 000000000..0a584eadb --- /dev/null +++ b/scripts/BENCHMARK_EVIDENCE.md @@ -0,0 +1,26 @@ +# CI benchmark evidence + +`run-ci-benchmarks.sh` writes `bench-metadata.txt` beside the timing files. +The workflow uploads all four files even when the regression gate fails. +Metadata records the candidate and baseline Git HEAD and committed tree IDs, +selected Go versions, platform, GOMAXPROCS setting, sample count, duration, +package list and paired sample start order/timestamps. It deliberately does +not dump environment variables, remote URLs or filesystem paths. + +HEAD/tree identify committed source, not a clean-worktree attestation or a +binary digest. The workflow adds a frontend embed stub. A PR checkout may be +a synthetic merge rather than the PR head; use the captured identity, not the +run API's head SHA alone, when reproducing it. Non-Git trees and nested archives +report identity unavailable rather than inheriting an enclosing checkout's SHA. + +Retain the original failed comparison. Compile both exact trees with the same +Go version before measuring, warm both, alternate their execution order and +collect at least ten samples. Record CPU/load observations and any departures +from CI (affinity, GOMAXPROCS, benchmark filtering or duration). The maintainer +heavy-work wrapper must still be used for builds; it does not prove an idle +host. A focused reproduction is diagnostic, not replacement qualification. + +Unchanged source in a benchmark's file does not establish unchanged generated +code, binary layout or runtime behaviour. Matching address-normalised +instructions alone do not prove equivalent timing. Do not waive the gate from +an unrelated diff, a single passing repeat or an unverified noise hypothesis. diff --git a/scripts/installtests/build_release_assets_test.go b/scripts/installtests/build_release_assets_test.go index 769d3b6c6..01f36cb52 100644 --- a/scripts/installtests/build_release_assets_test.go +++ b/scripts/installtests/build_release_assets_test.go @@ -3681,6 +3681,30 @@ func TestReleaseTrainCITriggersIncludeBuildAndE2E(t *testing.T) { } } +func TestBenchmarkQualificationRetainsProvenance(t *testing.T) { + content, err := os.ReadFile(repoFile(".github", "workflows", "build-and-test.yml")) + if err != nil { + t.Fatal(err) + } + job := workflowJobBlock(t, string(content), "benchmarks") + start := strings.Index(job, "- name: Upload benchmark evidence") + if start < 0 { + t.Fatal("missing benchmark evidence upload") + } + upload := job[start:] + for _, required := range []string{ + "if: always()", "bench-baseline.txt", "bench-results.txt", + "bench-comparison.txt", "bench-metadata.txt", + } { + if !strings.Contains(upload, required) { + t.Fatalf("benchmark upload must retain %q even on failure", required) + } + } + if !strings.Contains(job, "bash scripts/check-bench-regression.sh bench-comparison.txt") { + t.Fatal("provenance must not replace the benchmark regression gate") + } +} + func TestFrontendDependencySecurityAuditsAreRequired(t *testing.T) { workflowPath := repoFile(".github", "workflows", "build-and-test.yml") assertFileContainsAll(t, workflowPath, diff --git a/scripts/run-ci-benchmarks.sh b/scripts/run-ci-benchmarks.sh index 98bf296ce..8082c5c24 100755 --- a/scripts/run-ci-benchmarks.sh +++ b/scripts/run-ci-benchmarks.sh @@ -35,6 +35,32 @@ PACKAGES=( ./internal/hostmetrics/ ) +# Retain provenance without dumping credentials, remote URLs or working paths. +# PR checkouts may be synthetic merges; Go selects a toolchain for each tree. +metadata="${CURRENT_DIR}/bench-metadata.txt" +revision() { + # An archived tree nested inside another checkout must not inherit its SHA. + local prefix + prefix="$(git -C "$1" rev-parse --show-prefix 2>/dev/null)" || { echo unavailable; return; } + [[ -z "${prefix}" ]] || { echo unavailable; return; } + git -C "$1" rev-parse --verify "$2" 2>/dev/null || echo unavailable +} +{ + printf 'started_at=%s\nsamples=%s\nbenchtime=%s\n' \ + "$(date -u +%FT%TZ)" "${SAMPLE_COUNT}" "${BENCHTIME}" + printf 'platform=%s\n' "$(uname -sm)" + printf 'gomaxprocs=%s\n' "${GOMAXPROCS:-runtime-default}" + printf 'packages=%s\n' "${PACKAGES[*]}" + for label in candidate baseline; do + tree="${CURRENT_DIR}" + [[ "${label}" != baseline ]] || tree="${BASELINE_DIR}" + [[ -n "${tree}" ]] || continue + printf '%s.commit=%s\n' "${label}" "$(revision "${tree}" HEAD)" + printf '%s.tree=%s\n' "${label}" "$(revision "${tree}" 'HEAD^{tree}')" + printf '%s.go=%s\n' "${label}" "$(cd "${tree}" && go version)" + done +} > "${metadata}" + work_dir="$(mktemp -d)" trap 'rm -rf "${work_dir}"' EXIT @@ -46,6 +72,7 @@ run_sample() { local data_dir="${work_dir}/${label}-${round}" mkdir -p "${data_dir}" + printf 'sample=%s,%s,%s\n' "${label}" "${round}" "$(date -u +%FT%TZ)" >> "${metadata}" echo "=== ${label} benchmark sample ${round}/${SAMPLE_COUNT} ===" ( cd "${tree}" diff --git a/scripts/tests/test-ci-benchmarks.sh b/scripts/tests/test-ci-benchmarks.sh index 2375ef7a7..01dabe584 100755 --- a/scripts/tests/test-ci-benchmarks.sh +++ b/scripts/tests/test-ci-benchmarks.sh @@ -10,6 +10,10 @@ trap 'rm -rf "${WORK_DIR}"' EXIT mkdir -p "${WORK_DIR}/bin" "${WORK_DIR}/candidate" "${WORK_DIR}/baseline" cat > "${WORK_DIR}/bin/go" <<'EOF' #!/usr/bin/env bash +if [[ "$*" == version ]]; then + echo 'go version go1.26.7 linux/amd64' + exit 0 +fi printf '%s\t%s\n' "$PWD" "$*" >> "${FAKE_GO_LOG}" cat <<'RESULT' goos: linux @@ -45,6 +49,37 @@ mapfile -t calls < "${WORK_DIR}/go.log" [[ "${calls[4]}" == "${WORK_DIR}/candidate"$'\t'* ]] [[ "${calls[5]}" == "${WORK_DIR}/baseline"$'\t'* ]] +metadata="${WORK_DIR}/candidate/bench-metadata.txt" +grep -qFx 'samples=2' "${metadata}" +grep -qFx 'benchtime=100ms' "${metadata}" +grep -qFx 'candidate.commit=unavailable' "${metadata}" +grep -qFx 'baseline.go=go version go1.26.7 linux/amd64' "${metadata}" +[[ "$(grep -c '^sample=' "${metadata}")" == 4 ]] +! grep -qF "${WORK_DIR}" "${metadata}" + +# Real Git roots retain exact identities; a repeated run replaces old metadata. +for tree in candidate baseline; do + git -C "${WORK_DIR}/${tree}" init -q + git -C "${WORK_DIR}/${tree}" -c user.name=Test -c user.email=test@example.invalid \ + commit -qm fixture --allow-empty +done +PATH="${WORK_DIR}/bin:${PATH}" FAKE_GO_LOG="${WORK_DIR}/go.log" \ + PULSE_BENCH_CURRENT_DIR="${WORK_DIR}/candidate" \ + PULSE_BENCH_BASELINE_DIR="${WORK_DIR}/baseline" PULSE_BENCH_SAMPLE_COUNT=1 \ + bash "${ROOT_DIR}/scripts/run-ci-benchmarks.sh" >/dev/null +grep -qFx "candidate.commit=$(git -C "${WORK_DIR}/candidate" rev-parse HEAD)" "${metadata}" +grep -qFx "baseline.tree=$(git -C "${WORK_DIR}/baseline" rev-parse 'HEAD^{tree}')" "${metadata}" +[[ "$(grep -c '^sample=' "${metadata}")" == 2 ]] + +# A nested archive has no identity of its own; never attribute its parent's SHA. +mkdir -p "${WORK_DIR}/candidate/archive" +PATH="${WORK_DIR}/bin:${PATH}" FAKE_GO_LOG="${WORK_DIR}/go.log" \ + PULSE_BENCH_CURRENT_DIR="${WORK_DIR}/candidate/archive" \ + PULSE_BENCH_BASELINE_DIR='' PULSE_BENCH_SAMPLE_COUNT=1 \ + bash "${ROOT_DIR}/scripts/run-ci-benchmarks.sh" >/dev/null +grep -qFx 'candidate.commit=unavailable' "${WORK_DIR}/candidate/archive/bench-metadata.txt" +! grep -q '^baseline\.' "${WORK_DIR}/candidate/archive/bench-metadata.txt" + cat > "${WORK_DIR}/adequate.txt" <<'EOF' Example-4 100.0n ± 1% 111.0n ± 1% +11.00% (p=0.001 n=10) EOF diff --git a/scripts/tests/test_benchmark_workflow_contract.py b/scripts/tests/test_benchmark_workflow_contract.py index bc60ffab4..3dce0373a 100755 --- a/scripts/tests/test_benchmark_workflow_contract.py +++ b/scripts/tests/test_benchmark_workflow_contract.py @@ -22,6 +22,7 @@ class BenchmarkWorkflowContractTest(unittest.TestCase): self.assertNotIn("actions/cache/restore@", benchmark_job) self.assertIn("bench-baseline.txt", benchmark_job) self.assertIn("bench-comparison.txt", benchmark_job) + self.assertIn("bench-metadata.txt", benchmark_job) if __name__ == "__main__":