-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[system test] [perf] Performance perf report into PR #12124
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12124 +/- ##
============================================
- Coverage 74.81% 74.78% -0.04%
- Complexity 6619 6624 +5
============================================
Files 377 377
Lines 25329 25349 +20
Branches 3394 3398 +4
============================================
+ Hits 18951 18957 +6
- Misses 4991 5007 +16
+ Partials 1387 1385 -2 🚀 New features to boost your workflow:
|
6ef694a to
aaf5e72
Compare
...test/src/test/java/io/strimzi/systemtest/performance/UserOperatorScalabilityPerformance.java
Outdated
Show resolved
Hide resolved
...est/src/test/java/io/strimzi/systemtest/performance/TopicOperatorScalabilityPerformance.java
Outdated
Show resolved
Hide resolved
...est/src/test/java/io/strimzi/systemtest/performance/TopicOperatorScalabilityPerformance.java
Outdated
Show resolved
Hide resolved
...test/src/test/java/io/strimzi/systemtest/performance/UserOperatorScalabilityPerformance.java
Outdated
Show resolved
Hide resolved
| ${{ steps.generate_report.outputs.summary }} | ||
| - name: Add performance report to job summary |
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.
This is cool, maybe there could be some usage for common sts execution as well?
7fa2d0c to
cea51a6
Compare
|
/gha run pipeline=performance |
|
⏳ System test verification started: link The following 2 job(s) will be executed:
Tests will start after successful build completion. |
|
❌ System test verification failed: link |
|
/gha run pipeline=performance |
|
⏳ System test verification started: link The following 2 job(s) will be executed:
Tests will start after successful build completion. |
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
|
🎉 System test verification passed: link |
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
|
/gha run pipeline=performance |
|
⏳ System test verification started: link The following 2 job(s) will be executed:
Tests will start after successful build completion. |
|
🎉 System test verification passed: link |
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
| /** | ||
| * Find the latest timestamped results directory | ||
| */ | ||
| function findLatestResultsDir(baseDir) { |
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.
there are multiple results from one run that you need to find the latest?
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.
Also, this was mainly for local testing where I had multiple directories and runs:
├── 2025-11-20-15-57-41
│ └── user-operator
│ └── latencyUseCase
│ ├── users-1000-tp--cache--bq--bb-100-bt-100-utp-
│ ├── users-2000-tp--cache--bq--bb-100-bt-100-utp-
│ └── users-3000-tp--cache--bq--bb-100-bt-100-utp-
├── 2025-11-20-16-10-15
│ └── user-operator
│ └── latencyUseCase
│ ├── users-1000-tp--cache--bq--bb-100-bt-100-utp-
│ ├── users-2000-tp--cache--bq--bb-100-bt-100-utp-
│ └── users-3000-tp--cache--bq--bb-100-bt-100-utp-
├── 2025-11-20-16-35-19
│ └── user-operator
│ └── latencyUseCase
│ ├── users-1000-tp--cache--bq--bb-100-bt-100-utp-
│ ├── users-2000-tp--cache--bq--bb-100-bt-100-utp-
│ └── users-3000-tp--cache--bq--bb-100-bt-100-utp-
├── 2025-11-20-17-02-58
│ └── user-operator
│ └── latencyUseCase
│ ├── users-1000-tp--cache--bq--bb-100-bt-100-utp-
│ ├── users-2000-tp--cache--bq--bb-100-bt-100-utp-
│ └── users-3000-tp--cache--bq--bb-100-bt-100-utp-
├── 2025-11-20-17-15-26
│ └── user-operator
│ └── latencyUseCase
│ ├── users-1000-tp--cache--bq--bb-100-bt-100-utp-
│ ├── users-2000-tp--cache--bq--bb-100-bt-100-utp-
│ └── users-3000-tp--cache--bq--bb-100-bt-100-utp-
├── 2025-11-21-09-50-34
│ ├── topic-operator
│ │ └── scalabilityUseCase
│ │ ├── max-batch-size-100-max-linger-time-100-with-clients-false-number-of-topics-2
│ │ └── max-batch-size-100-max-linger-time-100-with-clients-false-number-of-topics-3
│ └── user-operator
│ ├── latencyUseCase
│ │ ├── users-10-tp--cache--bq--bb-100-bt-100-utp-
│ │ ├── users-20-tp--cache--bq--bb-100-bt-100-utp-
│ │ └── users-30-tp--cache--bq--bb-100-bt-100-utp-
│ └── scalabilityUseCase
│ ├── users-10-tp--cache--bq--bb-100-bt-100-utp-
│ ├── users-12-tp--cache--bq--bb-100-bt-100-utp-
│ ├── users-14-tp--cache--bq--bb-100-bt-100-utp-
│ └── users-16-tp--cache--bq--bb-100-bt-100-utp-
└── 2025-11-24-12-16-22
└── user-operator
└── latencyUseCase
├── users-1000-tp--cache--bq--bb-100-bt-100-utp-
├── users-2000-tp--cache--bq--bb-100-bt-100-utp-
└── users-3000-tp--cache--bq--bb-100-bt-100-utp-so it will always pick latest. I think I can re-name that method (to something like that findTimestampedResultsDir``)? But in general per one architecture there should be just only ONE timestamp`. If more architectures are run then we handle it differently... (we don't care about that here).
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
|
/gha run pipeline=performance |
|
⏳ System test verification started: link The following 2 job(s) will be executed:
Tests will start after successful build completion. |
|
🎉 System test verification passed: link |
Frawless
left a comment
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.
The changes are fine form my POV. I am not sure with 450 lines of js to generate the report especially when we have something similar already in the tests. AFAIU it is not trivial to re-use it. I wonder what others think about it. Otherwise I am fine with it as long as it will be used :)
|
Basically, there are two approaches... the first one is (where we would have +500 LOC to merge two or more results from architectures): Performance Test ResultsTest Run: Topic OperatorUse Case: scalabilityUseCase Configuration:
Results:
User OperatorUse Case: scalabilityUseCase Configuration:
Results:
Use Case: latencyUseCase Configuration:
Results:
|
|
or second one have (but with no need + 450LOC of javascript) => but with price with a lot of redudancy from my POV: Performance Test ResultsAMD64Test Run: Topic OperatorUse Case: latencyUseCase Configuration:
Results:
Use Case: scalabilityUseCase Configuration:
Results:
User OperatorUse Case: latencyUseCase Configuration:
Results:
Use Case: scalabilityUseCase Configuration:
Results:
ARCH (another arch)Test Run: Topic OperatorUse Case: latencyUseCase Configuration:
Results:
Use Case: scalabilityUseCase Configuration:
Results:
User OperatorUse Case: latencyUseCase Configuration:
Results:
Use Case: scalabilityUseCase Configuration:
Results:
|
|
@see-quick I approved it, but I agree that we should have something for the regular STs as well - it would be nice to have. |
|
For the JS generator - my idea is following: What about creating two reports in Java rather than just one? The first one would be the full one, second one would be shorter - basically what you are doing in the JS generator. It would be named like WDYT? |
Hmmm, interesting and how do you imagine this short/compact report? If, for instance, this is a maybe something like this? Performance Test ResultsTest Run: Topic Operator
User Operator
This is what you mean by shorter or any different? |
TBH, now I'm a bit confused. |
That report is per-architecture, because each architecture generates it.
I am just merging architectures to reduce redundancy over those both architectures so instead of having and with that JavaScript generator, I just merged those two arches, which eventually gives you this: |
|
After offline conversation with @see-quick (and finally understanding what is going on) I think that there is not much we can actually do about it - in case that we really want to have the reports as the comments. I had another idea to just leave comment saying that the jobs were (or weren't) successful and then links to the reports (generated by the Java generator) for each of the architecture, so anyone can have a look at them without much post-processing. But if we want to keep it this way, I guess we will have to live with the 450 LoC on the JS generator. |
Type of change
Description
This PR adds a perf report, which might look like this [1] (ignore those numbers; important is the format). This report would always be appended to the PR as a message when performance tests are triggered.
Currently, I am using only one agent (i.e.,
ubuntu-latest) for testing purposes on my fork, but the plan is to use both x64 and arm-based agents.[1] - see-quick#15 (comment)
Checklist