Skip to content

Commit 2ce6916

Browse files
thelastbackspaceShubh
authored andcommitted
feat: GitHub Action for the baseline check, and run --markdown
action.yml runs traceperf run with --baseline, fails the job on more calls per request, writes the job summary and keeps one PR comment up to date. CI runs the action on the batched app (must pass) and its N+1 version (must be caught).
1 parent 5140f2d commit 2ce6916

7 files changed

Lines changed: 212 additions & 2 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,40 @@ jobs:
188188
- run: node compat.js
189189
working-directory: test/compat
190190

191+
# The GitHub Action in action.yml, run against a local build: the batched
192+
# app must pass its baseline, the N+1 version of it must fail.
193+
action:
194+
needs: build
195+
runs-on: ubuntu-latest
196+
steps:
197+
- uses: actions/checkout@v4
198+
- uses: actions/setup-node@v4
199+
with:
200+
node-version: 22.x
201+
- uses: actions/download-artifact@v4
202+
with:
203+
name: dist
204+
path: dist
205+
- id: same
206+
uses: ./
207+
with:
208+
run: examples/slow-apps/clean-batched/server.js --path /orders -n 30
209+
baseline: examples/slow-apps/clean-batched/perf-baseline.json
210+
version: ./
211+
comment: 'false'
212+
- id: nplus1
213+
uses: ./
214+
with:
215+
run: examples/slow-apps/n-plus-one/server.js --path /orders -n 30
216+
baseline: examples/slow-apps/clean-batched/perf-baseline.json
217+
version: ./
218+
comment: 'false'
219+
fail: 'false'
220+
- name: The batched app passed and the N+1 one was caught
221+
run: |
222+
test "${{ steps.same.outputs.regressed }}" = false
223+
test "${{ steps.nplus1.outputs.regressed }}" = true
224+
191225
# Bun and Deno run the ESM build (nesting, stats, exporters, browser entry).
192226
other-runtimes:
193227
needs: build
@@ -215,7 +249,7 @@ jobs:
215249
run: test/runtimes/workerd.sh
216250

217251
publish-npm:
218-
needs: [build, test, runtime, browser, driver-compat, other-runtimes]
252+
needs: [build, test, runtime, browser, driver-compat, action, other-runtimes]
219253
if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/v')
220254
runs-on: ubuntu-latest
221255
permissions:

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313
- `traceperf/auto` wrote each request tree with its own `appendFileSync`, and under load that showed up as the app's "CPU cost" (`writeFileUtf8`). Writes are batched every 20 ms.
1414

1515
### Added
16+
- **GitHub Action** (`uses: thelastbackspace/traceperf@<tag>`): runs the baseline check, fails the job on more calls per request, and posts one pull-request comment it updates on every push. `traceperf run --markdown file.md` writes the same summary.
1617
- `traceperf run` reads `traceperf.config.json` (or `--config file`); command-line options win. A `login` step logs in before the traffic and sends the returned token or cookies with every request; `${NAME}` in the file is read from the environment.
1718
- `npm run eval` scores `traceperf run` on 20 apps in `examples/slow-apps`: 12 with a planted problem, 8 clean ones where the right answer is "no issues". CI fails on any missed problem or false alarm. Before these fixes: 17/17 found, 15 false alarms; after: 17/17, 0.
1819

‎README.md‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ Checked on real apps, not only on the planted ones: Prisma's REST example, the R
101101
- `-c 10` concurrency, `-d 5s` duration or `-n 500` requests, `--method POST`, `--port 3000` (detected by default).
102102
- `-H "Authorization: Bearer $TOKEN"` (repeatable): request headers, for APIs behind a login.
103103
- `--probe 10`: requests per path sent one at a time before the load (0 for none).
104-
- `--html report.html`, `--output calls.jsonl` (keep the trees), `--json`.
104+
- `--html report.html`, `--output calls.jsonl` (keep the trees), `--json`, `--markdown summary.md` (for a PR comment or job summary).
105105
- `--warmup 3`: untimed requests per path first, so cold-start work doesn't look like a problem.
106106
- `--no-profile`: don't sample the CPU.
107107
- Any start command works: `npx traceperf run -- npm start`.
@@ -149,6 +149,27 @@ It fails when a route makes more calls of any kind per request (database queries
149149
- run: npx traceperf run dist/server.js --path /orders --path /users/1 --baseline perf-baseline.json
150150
```
151151
152+
Or use the action, which also comments on the pull request with what changed, and updates that one comment on every push:
153+
154+
<!-- docs:skip -->
155+
```yaml
156+
permissions:
157+
pull-requests: write
158+
steps:
159+
- uses: actions/checkout@v4
160+
- run: npm ci && npm run build && npm run db:seed
161+
- uses: thelastbackspace/traceperf@v1.9.0
162+
with:
163+
run: dist/server.js --path /orders --path /users/1 # or leave out and use traceperf.config.json
164+
baseline: perf-baseline.json
165+
```
166+
167+
> ### traceperf: ✖ more calls per request than the baseline
168+
> - ✖ GET /orders: SELECT * FROM items WHERE order_id = ? 0 → 12 per request (an N+1?)
169+
> - ✖ new problem: N+1 in GET /orders
170+
171+
Inputs: `run`, `baseline`, `comment` (default `true`), `fail` (default `true`), `version` (traceperf version, default `1`). Output: `regressed`. The same report goes to the job summary; `--markdown file.md` writes it without the action.
172+
152173
`examples/slow-apps/` has 20 small apps: 12 with one planted problem each (blocking in your code and in built-ins, sequential awaits, N+1 over SQL and over HTTP, a slow or flaky dependency, failing requests…) and 8 clean ones where the right answer is "no issues". `npm run eval` scores `run` on them, and CI fails on a missed problem or a false alarm.
153174

154175
The same naming works without `run`: `TRACEPERF_PROFILE_BLOCKING=1 node --require traceperf/auto server.js` adds the function names to the exit summary (sampling costs a few percent CPU, so use it while investigating). In code: `profileSlowCalls({ select: root => blockingRanges(root), mergeInlined: true, onProfile })`, then `explain(roots, { hotspots })`.

‎action.yml‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
name: traceperf
2+
description: Fail a pull request that adds database queries or HTTP calls per request (an N+1), and comment what changed
3+
branding:
4+
icon: activity
5+
color: orange
6+
inputs:
7+
run:
8+
description: 'Arguments for `traceperf run`: the app and its paths, e.g. "dist/server.js --path /orders --path /users/1". Leave empty to use traceperf.config.json.'
9+
required: false
10+
default: ''
11+
baseline:
12+
description: Committed baseline file (create it with `npx traceperf run ... --save-baseline perf-baseline.json`)
13+
required: false
14+
default: perf-baseline.json
15+
comment:
16+
description: Post (and update) one comment on the pull request with the result
17+
required: false
18+
default: 'true'
19+
fail:
20+
description: Fail the job when a route makes more calls per request than the baseline
21+
required: false
22+
default: 'true'
23+
version:
24+
description: traceperf version to run (npm version or range, or the path of a local build)
25+
required: false
26+
default: '1'
27+
github-token:
28+
description: Token for the comment (needs pull-requests write permission)
29+
required: false
30+
default: ${{ github.token }}
31+
outputs:
32+
regressed:
33+
description: '"true" when a route makes more calls per request than the baseline'
34+
value: ${{ steps.run.outputs.regressed }}
35+
runs:
36+
using: composite
37+
steps:
38+
- id: run
39+
shell: bash
40+
env:
41+
TP_ARGS: ${{ inputs.run }}
42+
TP_BASELINE: ${{ inputs.baseline }}
43+
TP_VERSION: ${{ inputs.version }}
44+
run: |
45+
report="$RUNNER_TEMP/traceperf.md"
46+
set +e
47+
# shellcheck disable=SC2086 # the arguments are a word list on purpose
48+
case "$TP_VERSION" in
49+
# A local build (this repository's own CI tests the action with it)
50+
./*|/*) cli=(node "$TP_VERSION/dist/cli.js") ;;
51+
*) cli=(npx --yes "traceperf@$TP_VERSION") ;;
52+
esac
53+
"${cli[@]}" run $TP_ARGS --baseline "$TP_BASELINE" --markdown "$report"
54+
code=$?
55+
set -e
56+
if [ ! -s "$report" ]; then
57+
printf '### traceperf: the run failed before it produced a report (exit %s)\n\nSee the job log.\n' "$code" > "$report"
58+
fi
59+
cat "$report" >> "$GITHUB_STEP_SUMMARY"
60+
echo "report=$report" >> "$GITHUB_OUTPUT"
61+
echo "code=$code" >> "$GITHUB_OUTPUT"
62+
if [ "$code" -ne 0 ]; then echo "regressed=true" >> "$GITHUB_OUTPUT"; else echo "regressed=false" >> "$GITHUB_OUTPUT"; fi
63+
- if: inputs.comment == 'true' && github.event_name == 'pull_request'
64+
shell: bash
65+
env:
66+
GH_TOKEN: ${{ inputs.github-token }}
67+
REPORT: ${{ steps.run.outputs.report }}
68+
PR: ${{ github.event.pull_request.number }}
69+
REPO: ${{ github.repository }}
70+
run: |
71+
# One comment per PR, updated on every push (found by its marker)
72+
body="$(printf '<!-- traceperf-action -->\n%s' "$(cat "$REPORT")")"
73+
id="$(gh api "repos/$REPO/issues/$PR/comments" --paginate --jq '.[] | select(.body | startswith("<!-- traceperf-action -->")) | .id' | head -n 1)"
74+
if [ -n "$id" ]; then
75+
gh api --method PATCH "repos/$REPO/issues/comments/$id" -f body="$body" > /dev/null
76+
else
77+
gh api --method POST "repos/$REPO/issues/$PR/comments" -f body="$body" > /dev/null
78+
fi
79+
- if: inputs.fail == 'true' && steps.run.outputs.code != '0'
80+
shell: bash
81+
run: |
82+
echo "traceperf: more calls per request than ${{ inputs.baseline }}, or the run failed (see the report above)"
83+
exit 1
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"version": 1,
3+
"routes": {
4+
"GET /orders": {
5+
"requests": 10,
6+
"calls": {
7+
"SELECT * FROM items WHERE order_id IN (?)": 1,
8+
"SELECT * FROM orders LIMIT 12": 1
9+
}
10+
}
11+
},
12+
"problems": []
13+
}

‎src/__tests__/run.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,3 +412,21 @@ describe('config file and login', () => {
412412
}
413413
});
414414
});
415+
416+
describe('markdownReport', () => {
417+
const { markdownReport } = jest.requireActual('../run');
418+
it('leads with the verdict, lists baseline changes, folds the timing findings', () => {
419+
const md = markdownReport({
420+
trees: 30, findings: [{ kind: 'repeated', root: 'GET /orders', ms: 40, message: 'GET /orders called q 12 times', fix: 'batch it' }],
421+
baseline: { regressions: ['GET /orders: q 1 → 12 per request (an N+1?)'], improvements: [], notes: [] }, baselineFile: 'b.json',
422+
load: { requests: 30, durationMs: 300, statuses: { 200: 30 }, errors: 0, latency: {} },
423+
});
424+
expect(md).toMatch(/^### traceperf: ✖ more calls per request than the baseline\n\n- ✖ GET \/orders: q 1 → 12/);
425+
expect(md).toContain('<details><summary>Why it was slow');
426+
expect(md).toContain(' - fix: batch it');
427+
expect(md).toContain('30 requests in 0.3 s (100/s), 30× 200');
428+
expect(markdownReport({ trees: 5, findings: [], baseline: { regressions: [], improvements: [], notes: [] }, baselineFile: 'b.json' }))
429+
.toMatch(/^### traceperf: ✔ no new queries or calls\n\n- ✔ same calls per request on every route/);
430+
expect(markdownReport({ trees: 5, findings: [] })).toMatch(/^### traceperf: no issues found/);
431+
});
432+
});

‎src/run.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ export interface RunArgs {
3434
/** Request headers, e.g. { authorization: 'Bearer ...' } */
3535
headers?: Record<string, string>;
3636
html?: string;
37+
/** Write a Markdown summary here (pull-request comments, job summaries) */
38+
markdown?: string;
3739
/** Write call counts per route here (a baseline for --baseline) */
3840
saveBaseline?: string;
3941
/** Compare call counts per route with this baseline; exit 1 on more calls or new problems */
@@ -65,6 +67,7 @@ export const RUN_USAGE = ` traceperf run <app.js> [app args] [options]
6567
--html out.html write an HTML report
6668
--output f.jsonl keep the raw request trees
6769
--json print stats and findings as JSON
70+
--markdown f.md write a Markdown summary (for a PR comment or job summary)
6871
--save-baseline f.json save calls per request for each route (commit it)
6972
--baseline f.json compare with it; exit 1 if a route makes more calls
7073
(e.g. an N+1) or a new problem appears. Counts, not
@@ -252,6 +255,7 @@ function parseArgv(argv: string[], partial = false, start?: RunArgs): RunArgs |
252255
break;
253256
}
254257
case '--json': args.json = true; break;
258+
case '--markdown': if (!(args.markdown = argv[++i])) return '--markdown expects a file'; break;
255259
case '--save-baseline': if (!(args.saveBaseline = argv[++i])) return '--save-baseline expects a file'; break;
256260
case '--baseline': if (!(args.baseline = argv[++i])) return '--baseline expects a file'; break;
257261
case '--no-profile': args.profile = false; break;
@@ -856,6 +860,9 @@ export async function runApp(args: RunArgs, deps: RunDeps): Promise<number> {
856860
if (args.html && summary.trees) {
857861
fs.writeFileSync(args.html, deps.report(text, { html: true, findings: summary.findings, title: `traceperf run: ${args.command.join(' ')}` }));
858862
}
863+
if (args.markdown) {
864+
fs.writeFileSync(args.markdown, markdownReport({ load, findings: summary.findings, baseline: baselineReport, baselineFile: args.baseline, baselineError, trees: summary.trees || 0 }));
865+
}
859866
if (args.json) {
860867
deps.out(JSON.stringify({ load, trees: summary.trees, findings: summary.findings, ...(baselineReport ? { baseline: baselineReport } : {}) }, null, 2));
861868
} else if (!summary.trees) {
@@ -887,6 +894,39 @@ export async function runApp(args: RunArgs, deps: RunDeps): Promise<number> {
887894
return !summary.trees && exitCode ? 1 : 0;
888895
}
889896

897+
/** A short Markdown summary for a pull-request comment or a CI job summary */
898+
export function markdownReport(r: {
899+
load?: LoadResult; findings: Finding[]; trees: number;
900+
baseline?: { regressions: string[]; improvements: string[]; notes: string[] }; baselineFile?: string; baselineError?: string;
901+
}): string {
902+
const out: string[] = [];
903+
const failed = !!r.baselineError || !!(r.baseline && r.baseline.regressions.length);
904+
out.push(`### traceperf: ${failed ? '✖ more calls per request than the baseline' : r.baseline ? '✔ no new queries or calls' : `${r.findings.length ? `${r.findings.length} finding${r.findings.length === 1 ? '' : 's'}` : 'no issues found'}`}`, '');
905+
if (r.baselineError) out.push(r.baselineError, '');
906+
if (r.baseline) {
907+
const { regressions, improvements, notes } = r.baseline;
908+
for (const x of regressions) out.push(`- ✖ ${x}`);
909+
for (const x of improvements) out.push(`- ✔ ${x}`);
910+
for (const x of notes) out.push(`- ${x}`);
911+
if (!regressions.length && !improvements.length) out.push('- ✔ same calls per request on every route');
912+
out.push('', `<sub>Compared with \`${r.baselineFile}\`: calls per request from one-at-a-time requests, so the result does not depend on the machine.</sub>`, '');
913+
}
914+
if (r.findings.length) {
915+
out.push('<details><summary>Why it was slow (timings from this CI machine)</summary>', '');
916+
for (const f of r.findings) {
917+
out.push(`- **${f.kind}**: ${f.message.replace(/\n/g, ' ')}`);
918+
if (f.fix) out.push(` - fix: ${f.fix}`);
919+
}
920+
out.push('', '</details>', '');
921+
}
922+
if (r.load) {
923+
const rate = r.load.requests / Math.max(r.load.durationMs / 1000, 1e-3);
924+
const statuses = Object.entries(r.load.statuses).map(([s, n]) => `${n}× ${s}`).join(', ');
925+
out.push(`<sub>${r.load.requests} requests in ${(r.load.durationMs / 1000).toFixed(1)} s (${rate.toFixed(0)}/s)${statuses ? `, ${statuses}` : ''}; ${r.trees} request trees</sub>`);
926+
}
927+
return out.join('\n') + '\n';
928+
}
929+
890930
function waitForPort(fs: any, file: string, exited: () => boolean, timeoutMs: number): Promise<number | undefined> {
891931
const deadline = Date.now() + timeoutMs;
892932
return new Promise(resolve => {

0 commit comments

Comments
 (0)