Enhance Benchmark Results with n8n #1162
Conversation
WalkthroughThis pull request updates the CI workflow by modifying the Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as CI Runner
participant Cache as Benchmark Cache
participant Webhook as n8n Endpoint
Runner->>Cache: Check for benchmark result & commit hash files
Cache-->>Runner: Return file existence status
Runner->>Runner: Set output variables based on cache
Runner->>Webhook: Trigger webhook with current & previous benchmark data, repo, and PR info
Webhook-->>Runner: Return acknowledgement/error
alt Branch is `main`
Runner->>Cache: Save benchmark result and commit hash files
end
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/ci.yml (4)
121-126: Caching Step Version Review
The "Set up cache" step usesactions/cache@v3, but a static analysis hint indicates that this runner version might be outdated. Please verify whether a newer version is available (or if this warning is a false positive) and update accordingly to ensure optimal compatibility with GitHub Actions.🧰 Tools
🪛 actionlint (1.7.4)
122-122: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
127-144: Ensure Consistent Outputs in Cache Read Step
In the "Read previous benchmark result" step, when./cache/bench_result.txtis missing, only thePREV_BENCH_RESULToutput is set to"null". This leaves thePREV_COMMIToutput undefined, which may cause issues in subsequent steps (for example, during webhook payload formation). Consider modifying the else block to also outputPREV_COMMIT=nullfor consistency. For example:- else - echo "PREV_BENCH_RESULT=null" >> $GITHUB_OUTPUT - fi + else + echo "PREV_BENCH_RESULT=null" >> $GITHUB_OUTPUT + echo "PREV_COMMIT=null" >> $GITHUB_OUTPUT + fi
145-165: Webhook Trigger Robustness and Error Handling
The "Trigger n8n webhook" step gathers the current benchmark result fromoutput.txtand posts the payload to the webhook URL. To enhance robustness, consider:
- Adding a check to verify that
output.txtexists before attempting to read it.- Logging the full response (or additional error details) from the
curlcommand to aid in troubleshooting when the webhook fails. For example:- curr_bench=$(cat output.txt | jq -R -s .) + if [ -f output.txt ]; then + curr_bench=$(cat output.txt | jq -R -s .) + else + echo "::error::output.txt not found" + exit 1 + fiThis addition will ensure that the step fails gracefully with clearer error messaging.
166-171: Cache Storage Step Commentary
The "Store benchmark result to cache" step correctly creates the cache directory and saves both the benchmark result and the commit hash when the branch ismain. Consider reviewing whether any stale cache files need to be purged to prevent data accumulation over time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci.yml
122-122: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1162 +/- ##
=======================================
Coverage 38.56% 38.56%
=======================================
Files 170 170
Lines 25441 25441
=======================================
Hits 9812 9812
Misses 14808 14808
Partials 821 821 ☔ View full report in Codecov by Sentry. |
chacha912
force-pushed
the
enhance-benchmark
branch
from
d7e451b to
2884059
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/ci.yml (4)
118-124: Benchmark Execution Step – Consider Enabling Strict Error Handling
The "Bench" step correctly runsmake benchand processes the output fromoutput.txt. To improve robustness, consider adding an explicitset -e(or error checking) at the start of the multi-line script so that any failure (for example, ifoutput.txtis missing) causes the step to exit immediately.Proposed diff:
-run: | - make bench - content=$(cat output.txt | jq -R -s .) - echo "BENCH_RESULT=$content" >> $GITHUB_OUTPUT +run: | + set -e + make bench + if [ ! -f output.txt ]; then + echo "Error: output.txt not found" && exit 1 + fi + content=$(cat output.txt | jq -R -s .) + echo "BENCH_RESULT=$content" >> $GITHUB_OUTPUT
125-130: Update Actions/Cache Version
The step "Set up cache" usesactions/cache@v3, but static analysis indicates that this version is outdated. Upgrading to a newer version (for example,actions/cache@v4if available) may offer improved performance and compatibility with the latest GitHub Actions features.Proposed diff:
- uses: actions/cache@v3 + uses: actions/cache@v4🧰 Tools
🪛 actionlint (1.7.4)
126-126: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
131-147: Cache Reading Step – Remove Trailing Whitespace
The "Read previous benchmark result" step correctly checks for cached benchmark files and sets the appropriate output variables. Note that static analysis detected trailing spaces on line 141. Please remove these to adhere to YAML lint standards.Proposed diff for line 141 (removing trailing spaces):
- +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 141-141: trailing spaces
(trailing-spaces)
148-168: Webhook Trigger Step – Enhance Debug Information
The "Trigger n8n webhook" step sends benchmark data to the webhook with basic error handling by checking for the "CURL_ERROR" string. While this approach is functionally acceptable, consider logging additional debug information (such as the HTTP status code or response body) when the webhook fails. This extra detail can be very helpful during troubleshooting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci.yml
126-126: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/ci.yml
[error] 141-141: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
169-175: Benchmark Cache Storage Step – Looks Good
This step correctly caches the benchmark results and commit hash on themainbranch. The implementation is straightforward and meets its intended purpose.
hackerwins
changed the title
Improves visibility of performance metrics by storing benchmark results from main branch and forwarding PR comparisons to n8n. Added automated unit conversion, categorized result tables, and performance difference highlighting to facilitate data-driven development decisions.
Improves visibility of performance metrics by storing benchmark results from main branch and forwarding PR comparisons to n8n. Added automated unit conversion, categorized result tables, and performance difference highlighting to facilitate data-driven development decisions.
What this PR does / why we need it:
This PR improves the readability of benchmark test results to help developers easily understand performance metrics and make more efficient development decisions.
The workflow now sends benchmark results from GitHub Actions to N8n, where the data is processed/analyzed before adding comments to the PR in GitHub.
n8n workflow
Key improvements:
nstoms,swhere appropriateBtoKB,MB,GBWhich issue(s) this PR fixes:
Fixes #1151
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist:
Summary by CodeRabbit
Summary by CodeRabbit