Skip to content

Add stress benchmark CI workflow #3426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dheerajodha wants to merge 6 commits into
base: main
Choose a base branch
from
+117 −0
Open

Add stress benchmark CI workflow #3426

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3cc775d
feat(EC-1818): add stress benchmark CI workflow
dheerajodha Jul 21, 2026
26bde5c
chore(EC-1818): bump stress benchmark to 40 components, 10 workers
dheerajodha Jul 21, 2026
7b261b5
fix(EC-1818): address review feedback on benchmark workflow
dheerajodha Jul 21, 2026
4861700
chore(EC-1818): bump stress benchmark to 80 components
dheerajodha Jul 22, 2026
d350631
fix(EC-1818): capture stderr and use lowercase job ID
dheerajodha Jul 22, 2026
bb9a516
chore(EC-1818): revert stress components to default 10
dheerajodha Jul 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 117 additions & 0 deletions .github/workflows/benchmark.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
# Copyright The Conforma Contributors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
# SPDX-License-Identifier: Apache-2.0

---
name: Stress Benchmark

"on":
pull_request:
branches:

@fullsend-ai-review fullsend-ai-review Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] pattern-inconsistency

The workflow triggers only on pull_request without workflow_dispatch, unlike other CI workflows (lint.yaml, checks-codecov.yaml) that include it for manual re-runs. While omitting push is reasonable for a non-blocking benchmark, adding workflow_dispatch would allow ad-hoc performance investigations.

Suggested fix: Add workflow_dispatch to the on triggers.

- main
workflow_dispatch:

permissions:
contents: read

jobs:

stress:
runs-on: ubuntu-latest

@fullsend-ai-review fullsend-ai-review Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] naming-convention

The job ID Stress uses PascalCase, while the majority of workflows in the repo (11 of 15 job IDs) use lowercase. PascalCase is only used in checks-codecov.yaml.

Suggested fix: Consider renaming the job ID to stress for consistency with the dominant convention.

@fullsend-ai-review fullsend-ai-review Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] naming-convention

The job ID stress has no explicit name: field. All other workflows in the repo provide descriptive job names (e.g., Test, Lint, Analyze, Scorecard analysis). Without a name:, the GitHub Actions UI displays the raw job ID stress.

Suggested fix: Add name: Stress Benchmark to the job definition.

@fullsend-ai-review fullsend-ai-review Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] step-pattern-consistency

All other Go-based workflows in the repo include a Restore Cache step using actions/cache/restore after checkout. The benchmark workflow omits this step. Adding the cache would reduce CI wall time without affecting benchmark accuracy.

continue-on-error: true
Comment on lines +32 to +33

@coderabbitai coderabbitai Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Bound the report-only benchmark job.

A hung Quay pull or stress process can hold a runner for the platform’s multi-hour default. Add a timeout calibrated above the documented 6–8 minute runtime; continue-on-error will still keep it non-blocking.

Proposed fix 
   stress:
     runs-on: ubuntu-latest
+    timeout-minutes: 20
     continue-on-error: true
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
runs-on: ubuntu-latest
continue-on-error: true
runs-on: ubuntu-latest
timeout-minutes: 20
continue-on-error: true
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/benchmark.yaml around lines 32 - 33, Add a job-level
timeout to the benchmark job in the workflow containing runs-on and
continue-on-error, setting it above the documented 6–8 minute runtime while
still bounding hung pulls or stress processes. Preserve continue-on-error so
benchmark failures remain non-blocking.

env:
# Tuned for 4 vCPU / 16 GB CI runners to complete within 6-8 minutes.
# Code defaults are 10 components / 35 workers.
EC_STRESS_COMPONENTS: "10"
EC_STRESS_WORKERS: "10"

@fullsend-ai-review fullsend-ai-review Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] scope-alignment

The workflow sets EC_STRESS_COMPONENTS=40 and EC_STRESS_WORKERS=10, which differ from the code defaults in stress.go (defaultComponents=10, defaultWorkers=35). The rationale for these CI-specific values is not documented.

steps:
- name: Harden Runner
uses: step-security/harden-runner@58077d3c7e43986b6b15fba718e8ea69e387dfcc # v2.15.1
with:
egress-policy: audit
disable-telemetry: true

- name: Checkout repository
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

- name: Setup Go environment
uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6.3.0
with:
go-version-file: go.mod

@fullsend-ai-review fullsend-ai-review Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] missing established step pattern

Every existing workflow in this repo that sets up Go (checks-codecov, lint, codeql, release, update-go-containerregistry) includes a cache restore step using actions/cache/restore between checkout and setup-go. The benchmark workflow omits this step, which will result in slower CI runs as the Go module and build caches are not restored.

Suggested fix: Add a 'Restore Cache' step using actions/cache/restore between 'Checkout repository' and 'Setup Go environment', matching the pattern in other workflows.

cache: false

- name: Setup ORAS
uses: oras-project/setup-oras@1d808f7d7f6995cc68b7bf507bfe5c5446e1dc9d # v2.0.1

- name: Prepare benchmark data
run: |
cd benchmark/stress
./prepare_data.sh

- name: Build stress benchmark
run: go build -o benchmark/stress/stress ./benchmark/stress

@qodo-for-conforma qodo-for-conforma Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. go build missing cgo_enabled=0 📘 Rule violation ☼ Reliability

The workflow builds the stress benchmark binary using go build without setting CGO_ENABLED=0,
which can produce cgo-linked binaries and violate the build requirement. This can reduce
portability/reproducibility of the CI-built binary.
Agent Prompt 
## Issue description
The CI workflow builds a Go binary without disabling CGO (`CGO_ENABLED=0`), which violates the compliance requirement for Go builds.

## Issue Context
This workflow step builds `benchmark/stress/stress` via `go build` and should explicitly set `CGO_ENABLED=0` (or document a justified exception if cgo is required).

## Fix Focus Areas
- .github/workflows/benchmark.yaml[63-65]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


- name: Run stress benchmark
run: |
cd benchmark/stress
./stress 2>benchmark-stderr.txt | tee benchmark-output.txt

@fullsend-ai-review fullsend-ai-review Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] error handling

The benchmark step runs go run . 2>/dev/null | tee benchmark-output.txt, which discards all stderr output including Go compilation errors, runtime panics, and benchmark driver diagnostics. If the benchmark fails to compile or panics, the job summary silently reports 'No benchmark results found' with no indication of the root cause.

Suggested fix: Consider separating compilation from execution so build errors remain visible: go build -o stress . && ./stress 2>/dev/null | tee benchmark-output.txt.

- name: Write job summary

@fullsend-ai-review fullsend-ai-review Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] edge-case

go run . 2>/dev/null suppresses all stderr output, including Go compilation errors, runtime panics, and diagnostic messages from the benchmark driver. If the benchmark fails silently, the summary step reports 'No benchmark results found' with no indication of what went wrong.

Suggested fix: Redirect stderr to a log file instead: go run . 2>benchmark-stderr.txt | tee benchmark-output.txt

if: always()

@fullsend-ai-review fullsend-ai-review Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] error-handling

The Run stress benchmark step redirects stderr to /dev/null (./stress 2>/dev/null), silently discarding all error output including Go panics (the stress binary uses panic() in 6+ locations). Combined with continue-on-error: true, a failing benchmark produces no diagnostic output.

Suggested fix: Remove the 2>/dev/null redirect so errors appear in the CI step log, or redirect stderr to a file and include it in the job summary on failure.

run: |

@fullsend-ai-review fullsend-ai-review Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] error-handling

The pipeline ./stress 2>benchmark-stderr.txt | tee benchmark-output.txt does not use set -o pipefail. If the stress binary fails, tee still exits 0, so the step reports success. While continue-on-error: true and the summary step's guard clauses handle this gracefully, the step's green check could be misleading when investigating failures.

Suggested fix: Add set -o pipefail at the beginning of the run block, or check ${PIPESTATUS[0]} after the pipeline.

if [[ -s benchmark/stress/benchmark-stderr.txt ]]; then
{
echo "### Stderr"
echo '```'
tail -50 benchmark/stress/benchmark-stderr.txt
echo '```'
} >> "$GITHUB_STEP_SUMMARY"
fi

if [[ ! -f benchmark/stress/benchmark-output.txt ]]; then
echo "## Stress Benchmark" >> "$GITHUB_STEP_SUMMARY"
echo "Benchmark did not produce output." >> "$GITHUB_STEP_SUMMARY"
exit 0
fi

line=$(grep '^BenchmarkStress' benchmark/stress/benchmark-output.txt || true)

@qodo-for-conforma qodo-for-conforma Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Informational

3. Unbounded benchmarkstress grep 🐞 Bug ☼ Reliability

line is assigned from grep '^BenchmarkStress' ... without limiting to a single match, so
multiple result lines will create a multiline line value that can yield malformed metric
extraction and numeric formatting. The workflow should either enforce exactly one match or select a
deterministic single line (e.g., the last match) before parsing.
Agent Prompt 
### Issue description
The workflow assumes a single `BenchmarkStress...` result line, but `grep` can return multiple matches and the current code does not constrain or validate this. If multiple lines are present, downstream parsing can produce multiline numeric variables, which can break formatting or produce incorrect summary values.

### Issue Context
The job summary parser treats `line` as a single record for metric extraction. This should be made explicit by either:
- rejecting ambiguous output (multiple matches), or
- choosing one authoritative line.

### Fix Focus Areas
- .github/workflows/benchmark.yaml[89-99]

### Suggested fix approach
- Use a deterministic single-line selection, e.g.:
  - `line=$(grep '^BenchmarkStress' benchmark/stress/benchmark-output.txt | tail -n 1 || true)`
- Optionally validate count:
  - `count=$(grep -c '^BenchmarkStress' ... || true)` and if `count != 1`, write a note to `$GITHUB_STEP_SUMMARY` and skip metric parsing.
- Consider including the selected raw line in the summary for traceability.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

if [[ -z "$line" ]]; then
echo "## Stress Benchmark" >> "$GITHUB_STEP_SUMMARY"
echo "No benchmark results found in output." >> "$GITHUB_STEP_SUMMARY"
exit 0
fi

ns_op=$(echo "$line" | grep -oP '[\d.]+ ns/op' | awk '{print $1}')

@qodo-for-conforma qodo-for-conforma Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Zeroed benchmark metrics 🐞 Bug ≡ Correctness

The job summary silently reports 0 for execution time/memory when parsing fails because empty
extracted values are coerced to 0 via ${var:-0}. This can produce a plausible-looking benchmark
table that masks parsing breakage or missing metrics in the benchmark output.
Agent Prompt 
### Issue description
The workflow extracts `ns/op`, `peak-RSS-bytes`, `allocated-bytes/op`, and `heap-bytes-from-system` from the benchmark output, but if any extraction yields an empty string the summary falls back to `0` using `${var:-0}`. This can silently publish incorrect results (e.g., `0.0s`, `0 MB`).

### Issue Context
This is a report-only workflow; its primary value is accurate visibility. If parsing fails, the summary should clearly indicate missing/unknown values and ideally include the raw result line(s) used.

### Fix Focus Areas
- .github/workflows/benchmark.yaml[89-117]

### Suggested fix approach
- Detect missing extractions and render `N/A` (or similar) instead of defaulting to 0.
- Optionally append the matched `BenchmarkStress...` result line to the summary for debugging.
- Example pattern:
  - after extraction, check `[[ -z "$ns_op" ]]` etc.
  - if missing, set `secs="N/A"` / `rss_mb="N/A"` rather than computing numeric conversions.
  - keep the summary step non-failing and informative.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

peak_rss=$(echo "$line" | grep -oP '[\d.]+ peak-RSS-bytes' | awk '{print $1}')

@fullsend-ai-review fullsend-ai-review Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] injection

Shell variables (ns_op, peak_rss, alloc, heap) are interpolated into double-quoted awk command strings. The upstream grep -oP constrains values to digits and dots, making awk injection practically infeasible, but the pattern is fragile. Using awk -v to pass variables is a safer idiom at zero cost.

Suggested fix: Use awk -v to pass variables: secs=$(awk -v val="${ns_op:-0}" 'BEGIN {printf "%.1f", val / 1000000000}')

alloc=$(echo "$line" | grep -oP '[\d.]+ allocated-bytes/op' | awk '{print $1}')
heap=$(echo "$line" | grep -oP '[\d.]+ heap-bytes-from-system' | awk '{print $1}')

secs=$(awk "BEGIN {printf \"%.1f\", ${ns_op:-0} / 1000000000}")
rss_mb=$(awk "BEGIN {printf \"%.0f\", ${peak_rss:-0} / 1048576}")
alloc_mb=$(awk "BEGIN {printf \"%.0f\", ${alloc:-0} / 1048576}")
heap_mb=$(awk "BEGIN {printf \"%.0f\", ${heap:-0} / 1048576}")

{
echo "## Stress Benchmark"
echo ""
echo "| Metric | Value |"
echo "|--------|-------|"
echo "| Components | ${EC_STRESS_COMPONENTS} |"
echo "| Workers | ${EC_STRESS_WORKERS} |"
echo "| Execution time | ${secs}s |"
echo "| Peak RSS | ${rss_mb} MB |"
echo "| Allocated memory | ${alloc_mb} MB |"
echo "| Heap from system | ${heap_mb} MB |"
} >> "$GITHUB_STEP_SUMMARY"
Loading