-
Couldn't load subscription status.
- Fork 3
Enhance CI benchmarks with regression detection #531
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.
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
Merged
+109
−2
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
61122dd
Enhance CI benchmarks with regression detection
kamilkisiela 64d9b1c
Update ci.yaml
kamilkisiela 76e94ff
Update ci.yaml
kamilkisiela 381eb57
Update ci.yaml
kamilkisiela 112a83d
Update ci.yaml
kamilkisiela ce51469
Update k6.js
kamilkisiela 6d56899
Update ci-detect-regression.sh
kamilkisiela 4d9015d
Update ci.yaml
kamilkisiela 5cb74b3
Apply suggestion from @kamilkisiela
kamilkisiela 818c561
Apply suggestion from @kamilkisiela
kamilkisiela cb4be91
Apply suggestion from @kamilkisiela
kamilkisiela File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| ./results/main | ||
| ./results/pr |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| #!/bin/bash | ||
|
|
||
| # This script is meant to be run in CI (./github/workflows/ci.yaml#router-benchmark). | ||
| # | ||
| # It's a script to detect performance regression between two k6 benchmark summary files. | ||
| # It compares the 'http_reqs' rate metric from the PR summary against the main branch | ||
| # summary and determines if there is a regression of more than 2%. | ||
| # If a regression is detected, the script exits with a non-zero status code. | ||
|
|
||
| # Ensure jq and bc are installed | ||
| if ! command -v jq &> /dev/null | ||
| then | ||
| echo "jq could not be found. Please install jq to run this script." | ||
| exit 1 | ||
| fi | ||
| if ! command -v bc &> /dev/null | ||
| then | ||
| echo "bc could not be found. Please install bc to run this script." | ||
| exit 1 | ||
| fi | ||
|
|
||
| PR_SUMMARY="./bench/results/pr/k6_summary.json" | ||
| MAIN_SUMMARY="./bench/results/main/k6_summary.json" | ||
|
|
||
| # Check if the summary files exist | ||
| if [ ! -f "$PR_SUMMARY" ] || [ ! -f "$MAIN_SUMMARY" ]; then | ||
| echo "Benchmark summary files ($PR_SUMMARY and/or $MAIN_SUMMARY) not found." | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Extract the rate values | ||
| MAIN_RATE=$(jq '.metrics.http_reqs.values.rate' "$MAIN_SUMMARY") | ||
| PR_RATE=$(jq '.metrics.http_reqs.values.rate' "$PR_SUMMARY") | ||
|
|
||
| # Check if jq successfully extracted the rates | ||
| if [ -z "$MAIN_RATE" ] || [ -z "$PR_RATE" ] || [ "$MAIN_RATE" == "null" ] || [ "$PR_RATE" == "null" ]; then | ||
| echo "Could not extract rate from one or both summary files." | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Handle case where main rate is 0 to avoid division by zero | ||
| if (( $(echo "$MAIN_RATE == 0" | bc -l) )); then | ||
| echo "Main branch rate is zero, cannot calculate percentage change." | ||
| # If the main rate is 0, any positive PR rate is an improvement, not a regression. | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Calculate the percentage difference using bc | ||
| # scale determines the number of decimal places | ||
| diff=$(echo "scale=4; (($PR_RATE - $MAIN_RATE) / $MAIN_RATE) * 100" | bc) | ||
|
|
||
| # Print the results | ||
| echo "Main branch http_reqs rate: $MAIN_RATE" | ||
| echo "PR branch http_reqs rate: $PR_RATE" | ||
| printf "Difference: %.2f%%\n" "$diff" | ||
|
|
||
| # Check if the difference is a regression of more than 5% | ||
| # bc returns 1 for true, 0 for false. We compare with a negative number. | ||
| is_regression=$(echo "$diff < -2" | bc) | ||
|
|
||
| if [ "$is_regression" -eq 1 ]; then | ||
| echo "Performance regression detected! The PR is more than 2% slower than main." | ||
| exit 1 | ||
| else | ||
| echo "No significant performance regression detected." | ||
| exit 0 | ||
| fi |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Empty file.
Empty file.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do not chain PR a lot, but this should really be
${{ github.base_ref }}