Add stress benchmark CI workflow#3426
Conversation
Add a report-only GitHub Actions workflow that runs the stress benchmark on PRs to main, surfacing peak memory and execution time in the job summary without blocking merges. Uses oras to pull pre-built benchmark data from Quay with upstream regeneration as fallback. Ref: EC-1818 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 12:24 PM UTC · Completed 12:41 PM UTC |
📝 WalkthroughWalkthroughAdds a GitHub Actions “Stress Benchmark” workflow triggered for pull requests targeting ChangesStress benchmark workflow
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant BenchmarkRunner
participant StressBenchmark
participant JobSummary
GitHubActions->>BenchmarkRunner: Trigger stress benchmark workflow
BenchmarkRunner->>StressBenchmark: Build and execute benchmark
StressBenchmark-->>BenchmarkRunner: Produce stdout and stderr logs
BenchmarkRunner->>JobSummary: Parse metrics and publish summary
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8).github/workflows/benchmark.yamlTraceback (most recent call last): Comment |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
ReviewFindingsLow
Note: This PR modifies files under Previous runReviewFindingsMedium
Low
Labels: PR already has appropriate labels (github_actions, ci, size: M, requires-manual-review). Adding Review effort label for reviewer guidance. Previous run (2)ReviewFindingsMedium
Low
Previous run (3)ReviewFindingsHigh
Low
Previous run (4)ReviewFindingsLow
Note: This PR modifies files under Previous run (5)ReviewFindingsHigh
Low
Labels: PR adds a GitHub Actions CI workflow for benchmark automation |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
ci
github_actions
Initial CI run with 10/5 completed in under a minute. Increase to 40/10 to target 5-6 minute total job time on CI runners. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 1:12 PM UTC · Completed 1:26 PM UTC |
Superseded by updated review
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
requires-manual-review
Rename workflow to "Stress Benchmark", add workflow_dispatch trigger, separate build from execution to surface compilation errors, and document CI-specific env var overrides. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 2:08 PM UTC · Completed 2:24 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
removed
the
requires-manual-review
Doubles component count from 40 to 80 to increase peak RSS (~4 GB), making memory regressions more visible in CI job summaries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 Review · |
Redirect stderr to a file instead of /dev/null so panics and errors are surfaced in the job summary. Rename job ID from Stress to stress to match the dominant lowercase convention in the repo. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 12:02 PM UTC · Completed 12:16 PM UTC |
Superseded by updated review
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
requires-manual-review
All components use the same image digest, so internal caches deduplicate after the first — higher counts don't add memory pressure. Use the code default to avoid misleading numbers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 12:21 PM UTC · Completed 12:36 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Review · ❌ Terminated · Started 1:08 PM UTC · Ended 1:23 PM UTC |
PR Summary by QodoAdd Stress Benchmark GitHub Actions workflow
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/workflows/benchmark.yaml:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 9e1158a9-cb54-49bb-999b-074a054f1b55
📒 Files selected for processing (1)
.github/workflows/benchmark.yaml
| runs-on: ubuntu-latest | ||
| continue-on-error: true |
There was a problem hiding this comment.
🩺 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.
| 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.
Code Review by Qodo
Context used✅ Compliance rules (platform):
27 rules 1. go build missing CGO_ENABLED=0
|
![qodo-for-conforma[bot]](https://avatars.githubusercontent.com/in/3609154?s=60&v=4){kind=small}
| ./prepare_data.sh | ||
|
|
||
| - name: Build stress benchmark | ||
| run: go build -o benchmark/stress/stress ./benchmark/stress |
There was a problem hiding this comment.
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
| exit 0 | ||
| fi | ||
|
|
||
| ns_op=$(echo "$line" | grep -oP '[\d.]+ ns/op' | awk '{print $1}') |
There was a problem hiding this comment.
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
| exit 0 | ||
| fi | ||
|
|
||
| line=$(grep '^BenchmarkStress' benchmark/stress/benchmark-output.txt || true) |
There was a problem hiding this comment.
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
|
🤖 Finished Review · ✅ Success · Started 1:08 PM UTC · Completed 1:23 PM UTC |
1 participant
What:
Add a report-only GitHub Actions workflow that runs the stress benchmark on PRs to main, surfacing peak memory and execution time in the job summary without blocking merges. Uses oras to pull pre-built benchmark data from Quay with upstream regeneration as fallback.
Why:
Tickets:
Ref: EC-1818