Skip to content

Commit 953c177

Browse files
committed
Initial PR for performance test on integration test that running on CI
Signed-off-by: Senan Zedan <[email protected]>
1 parent ae6c726 commit 953c177

File tree

13 files changed

+164
-36
lines changed

13 files changed

+164
-36
lines changed

.github/workflows/performance-test.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ jobs:
8888
8989
- name: Run component benchmarks
9090
run: |
91+
mkdir -p reports
9192
export LD_LIBRARY_PATH=${PWD}/candle-binding/target/release
9293
make perf-bench-quick 2>&1 | tee reports/bench-output.txt
9394

Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ _run:
2121
-f tools/make/observability.mk \
2222
-f tools/make/openshift.mk \
2323
-f tools/make/e2e.mk \
24+
-f tools/make/performance.mk \
2425
$(MAKECMDGOALS)
2526

2627
.PHONY: _run

e2e/pkg/performance/load_generator.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,20 @@ func NewLoadGenerator(concurrency, rateLimit int, duration time.Duration) *LoadG
2828

2929
// LoadResult contains the results of a load test
3030
type LoadResult struct {
31-
TotalRequests int
32-
SuccessfulReqs int
33-
FailedReqs int
34-
Duration time.Duration
35-
AvgLatencyMs float64
36-
P50LatencyMs float64
37-
P90LatencyMs float64
38-
P95LatencyMs float64
39-
P99LatencyMs float64
40-
MaxLatencyMs float64
41-
MinLatencyMs float64
42-
ThroughputQPS float64
43-
Latencies []time.Duration
44-
Errors []error
31+
TotalRequests int
32+
SuccessfulReqs int
33+
FailedReqs int
34+
Duration time.Duration
35+
AvgLatencyMs float64
36+
P50LatencyMs float64
37+
P90LatencyMs float64
38+
P95LatencyMs float64
39+
P99LatencyMs float64
40+
MaxLatencyMs float64
41+
MinLatencyMs float64
42+
ThroughputQPS float64
43+
Latencies []time.Duration
44+
Errors []error
4545
}
4646

4747
// RequestFunc is a function that executes a single request

e2e/pkg/performance/metrics_collector.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,13 @@ func (mc *MetricsCollector) MonitorPodMetrics(ctx context.Context, podName strin
110110

111111
// ResourceStats holds aggregated resource statistics
112112
type ResourceStats struct {
113-
AvgCPUCores float64
114-
MaxCPUCores float64
115-
MinCPUCores float64
116-
AvgMemoryMB float64
117-
MaxMemoryMB float64
118-
MinMemoryMB float64
119-
SampleCount int
113+
AvgCPUCores float64
114+
MaxCPUCores float64
115+
MinCPUCores float64
116+
AvgMemoryMB float64
117+
MaxMemoryMB float64
118+
MinMemoryMB float64
119+
SampleCount int
120120
}
121121

122122
// AggregateMetrics aggregates multiple pod metrics samples

perf/CI-STRATEGY.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,18 @@ Here are different approaches teams use, from most to least restrictive:
4444
### Strategy 1: Label-Based (CURRENT - RECOMMENDED) 🏷️
4545

4646
**When it runs:**
47+
4748
- Only when PR has `performance` label
4849
- Manual trigger via GitHub UI
4950

5051
**Pros:**
52+
5153
- ✅ Saves tons of CI time
5254
- ✅ Developers control when tests run
5355
- ✅ No noise on small PRs
5456

5557
**Cons:**
58+
5659
- ❌ Developers might forget to add label
5760
- ❌ Regressions could slip through
5861

@@ -63,6 +66,7 @@ Here are different approaches teams use, from most to least restrictive:
6366
### Strategy 2: Path-Based (Original Design) 📁
6467

6568
**When it runs:**
69+
6670
```yaml
6771
on:
6872
pull_request:
@@ -73,10 +77,12 @@ on:
7377
```
7478
7579
**Pros:**
80+
7681
- ✅ Automatic - no manual intervention
7782
- ✅ Catches regressions early
7883
7984
**Cons:**
85+
8086
- ❌ Runs too often (most PRs touch these paths)
8187
- ❌ High CI cost
8288
- ❌ Slows down development
@@ -88,6 +94,7 @@ on:
8894
### Strategy 3: Scheduled + Manual Only ⏰
8995
9096
**When it runs:**
97+
9198
```yaml
9299
on:
93100
schedule:
@@ -96,11 +103,13 @@ on:
96103
```
97104
98105
**Pros:**
106+
99107
- ✅ Minimal CI cost
100108
- ✅ No PR delays
101109
- ✅ Nightly baseline still updates
102110
103111
**Cons:**
112+
104113
- ❌ Regressions found after merge (too late!)
105114
- ❌ Developers must manually trigger
106115
@@ -111,6 +120,7 @@ on:
111120
### Strategy 4: Hybrid - Critical Paths Only 🎯
112121
113122
**When it runs:**
123+
114124
```yaml
115125
on:
116126
pull_request:
@@ -122,11 +132,13 @@ on:
122132
```
123133

124134
**Pros:**
135+
125136
- ✅ Automatic for critical code
126137
- ✅ Reduced CI usage vs path-based
127138
- ✅ Catches most important regressions
128139

129140
**Cons:**
141+
130142
- ❌ Still runs frequently
131143
- ❌ Can miss indirect performance impacts
132144

@@ -137,16 +149,19 @@ on:
137149
### Strategy 5: PR Size Based 📏
138150

139151
**When it runs:**
152+
140153
```yaml
141154
# Run only on large PRs (>500 lines changed)
142155
if: github.event.pull_request.additions + github.event.pull_request.deletions > 500
143156
```
144157
145158
**Pros:**
159+
146160
- ✅ Small PRs skip expensive tests
147161
- ✅ Large risky changes get tested
148162
149163
**Cons:**
164+
150165
- ❌ Single-line change can cause regression
151166
- ❌ Complex logic to maintain
152167
@@ -157,6 +172,7 @@ if: github.event.pull_request.additions + github.event.pull_request.deletions >
157172
### Strategy 6: Pre-merge Only (Protected Branch) 🔒
158173
159174
**When it runs:**
175+
160176
```yaml
161177
on:
162178
pull_request:
@@ -167,10 +183,12 @@ on:
167183
```
168184
169185
**Pros:**
186+
170187
- ✅ Tests final code before/after merge
171188
- ✅ Doesn't slow down draft PRs
172189
173190
**Cons:**
191+
174192
- ❌ Late feedback for developers
175193
- ❌ Might catch issues post-merge
176194
@@ -181,27 +199,31 @@ on:
181199
## Recommended Setup by Project Stage
182200
183201
### 🌱 Early Stage Project
202+
184203
```yaml
185204
Strategy: Scheduled + Manual
186205
Performance Tests: Nightly only
187206
Reason: Save CI budget, iterate fast
188207
```
189208
190209
### 🌿 Growing Project
210+
191211
```yaml
192212
Strategy: Label-Based (CURRENT)
193213
Performance Tests: On 'performance' label
194214
Reason: Balance cost vs safety
195215
```
196216
197217
### 🌳 Mature Project
218+
198219
```yaml
199220
Strategy: Hybrid Critical Paths
200221
Performance Tests: Auto on critical code
201222
Reason: High confidence, catch regressions
202223
```
203224
204225
### 🏢 Enterprise Project
226+
205227
```yaml
206228
Strategy: Every PR (Path-Based)
207229
Performance Tests: Always
@@ -251,6 +273,7 @@ No changes needed! Current setup is optimized.
251273
## Cost Analysis
252274

253275
Assuming:
276+
254277
- 10 PRs per day
255278
- 20 minutes per performance test
256279
- $0.008 per minute (GitHub Actions pricing)
@@ -271,6 +294,7 @@ Assuming:
271294
### For Developers
272295

273296
**When to add `performance` label:**
297+
274298
- ✅ Changing classification, cache, or decision engine
275299
- ✅ Modifying CGO bindings
276300
- ✅ Optimizing algorithms
@@ -282,6 +306,7 @@ Assuming:
282306
### For Reviewers
283307

284308
**Check for performance label:**
309+
285310
```markdown
286311
## Performance Checklist
287312
- [ ] Does this PR touch classification/cache/decision code?
@@ -292,6 +317,7 @@ Assuming:
292317
### For CI
293318

294319
**Monitor false negatives:**
320+
295321
- Track regressions found in nightly but missed in PRs
296322
- If >5% slip through, consider tightening strategy
297323

@@ -302,26 +328,30 @@ Assuming:
302328
### Q: What if a regression slips through?
303329

304330
**A:** Nightly workflow will catch it and create an issue. You can:
331+
305332
1. Revert the problematic PR
306333
2. Fix forward with a new PR
307334
3. Update baseline if intentional
308335

309336
### Q: Can I force performance tests on a PR without label?
310337

311338
**A:** Yes! Two ways:
339+
312340
1. Add `performance` label to PR
313341
2. Go to Actions tab → Performance Tests → Run workflow → Select your branch
314342

315343
### Q: What about main branch protection?
316344

317345
**A:** Performance tests are NOT required checks. They're:
346+
318347
- Advisory (warn but don't block)
319348
- Opt-in (run when needed)
320349
- Nightly will catch issues anyway
321350

322351
### Q: Should I run tests locally before PR?
323352

324353
**A:** Recommended for performance-critical changes:
354+
325355
```bash
326356
make perf-bench-quick # Takes 3-5 min
327357
make perf-compare # Compare vs baseline
@@ -339,11 +369,13 @@ make perf-compare # Compare vs baseline
339369
- Nightly workflow ensures baselines stay current
340370

341371
**To run performance tests on your PR:**
372+
342373
1. Add label: `performance`
343374
2. Wait for tests to complete (~15 min)
344375
3. Review results in PR comment
345376

346377
**Why nightly is still needed:**
378+
347379
- Updates baselines automatically
348380
- Catches anything that slipped through
349381
- Runs comprehensive 30s benchmarks

0 commit comments

Comments
 (0)