From 2ce6916a3fa330386d637ee87ee78310f75ca83a Mon Sep 17 00:00:00 2001 From: thelastbackspace <23155651+thelastbackspace@users.noreply.github.com> Date: Sat, 7 Nov 2026 10:28:35 +0100 Subject: [PATCH] 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). --- .github/workflows/ci.yml | 36 +++++++- CHANGELOG.md | 1 + README.md | 23 ++++- action.yml | 83 +++++++++++++++++++ .../clean-batched/perf-baseline.json | 13 +++ src/__tests__/run.test.ts | 18 ++++ src/run.ts | 40 +++++++++ 7 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 action.yml create mode 100644 examples/slow-apps/clean-batched/perf-baseline.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0585434..4ef8333 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -188,6 +188,40 @@ jobs: - run: node compat.js working-directory: test/compat + # The GitHub Action in action.yml, run against a local build: the batched + # app must pass its baseline, the N+1 version of it must fail. + action: + needs: build + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 22.x + - uses: actions/download-artifact@v4 + with: + name: dist + path: dist + - id: same + uses: ./ + with: + run: examples/slow-apps/clean-batched/server.js --path /orders -n 30 + baseline: examples/slow-apps/clean-batched/perf-baseline.json + version: ./ + comment: 'false' + - id: nplus1 + uses: ./ + with: + run: examples/slow-apps/n-plus-one/server.js --path /orders -n 30 + baseline: examples/slow-apps/clean-batched/perf-baseline.json + version: ./ + comment: 'false' + fail: 'false' + - name: The batched app passed and the N+1 one was caught + run: | + test "${{ steps.same.outputs.regressed }}" = false + test "${{ steps.nplus1.outputs.regressed }}" = true + # Bun and Deno run the ESM build (nesting, stats, exporters, browser entry). other-runtimes: needs: build @@ -215,7 +249,7 @@ jobs: run: test/runtimes/workerd.sh publish-npm: - needs: [build, test, runtime, browser, driver-compat, other-runtimes] + needs: [build, test, runtime, browser, driver-compat, action, other-runtimes] if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/v') runs-on: ubuntu-latest permissions: diff --git a/CHANGELOG.md b/CHANGELOG.md index b43b9ee..0da428d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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. ### Added +- **GitHub Action** (`uses: thelastbackspace/traceperf@`): 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. - `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. - `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. diff --git a/README.md b/README.md index a6b4ba1..6f0a73b 100644 --- a/README.md +++ b/README.md @@ -101,7 +101,7 @@ Checked on real apps, not only on the planted ones: Prisma's REST example, the R - `-c 10` concurrency, `-d 5s` duration or `-n 500` requests, `--method POST`, `--port 3000` (detected by default). - `-H "Authorization: Bearer $TOKEN"` (repeatable): request headers, for APIs behind a login. - `--probe 10`: requests per path sent one at a time before the load (0 for none). -- `--html report.html`, `--output calls.jsonl` (keep the trees), `--json`. +- `--html report.html`, `--output calls.jsonl` (keep the trees), `--json`, `--markdown summary.md` (for a PR comment or job summary). - `--warmup 3`: untimed requests per path first, so cold-start work doesn't look like a problem. - `--no-profile`: don't sample the CPU. - 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 - run: npx traceperf run dist/server.js --path /orders --path /users/1 --baseline perf-baseline.json ``` +Or use the action, which also comments on the pull request with what changed, and updates that one comment on every push: + + +```yaml +permissions: + pull-requests: write +steps: + - uses: actions/checkout@v4 + - run: npm ci && npm run build && npm run db:seed + - uses: thelastbackspace/traceperf@v1.9.0 + with: + run: dist/server.js --path /orders --path /users/1 # or leave out and use traceperf.config.json + baseline: perf-baseline.json +``` + +> ### traceperf: ✖ more calls per request than the baseline +> - ✖ GET /orders: SELECT * FROM items WHERE order_id = ? 0 → 12 per request (an N+1?) +> - ✖ new problem: N+1 in GET /orders + +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. + `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. 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 })`. diff --git a/action.yml b/action.yml new file mode 100644 index 0000000..8720426 --- /dev/null +++ b/action.yml @@ -0,0 +1,83 @@ +name: traceperf +description: Fail a pull request that adds database queries or HTTP calls per request (an N+1), and comment what changed +branding: + icon: activity + color: orange +inputs: + run: + 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.' + required: false + default: '' + baseline: + description: Committed baseline file (create it with `npx traceperf run ... --save-baseline perf-baseline.json`) + required: false + default: perf-baseline.json + comment: + description: Post (and update) one comment on the pull request with the result + required: false + default: 'true' + fail: + description: Fail the job when a route makes more calls per request than the baseline + required: false + default: 'true' + version: + description: traceperf version to run (npm version or range, or the path of a local build) + required: false + default: '1' + github-token: + description: Token for the comment (needs pull-requests write permission) + required: false + default: ${{ github.token }} +outputs: + regressed: + description: '"true" when a route makes more calls per request than the baseline' + value: ${{ steps.run.outputs.regressed }} +runs: + using: composite + steps: + - id: run + shell: bash + env: + TP_ARGS: ${{ inputs.run }} + TP_BASELINE: ${{ inputs.baseline }} + TP_VERSION: ${{ inputs.version }} + run: | + report="$RUNNER_TEMP/traceperf.md" + set +e + # shellcheck disable=SC2086 # the arguments are a word list on purpose + case "$TP_VERSION" in + # A local build (this repository's own CI tests the action with it) + ./*|/*) cli=(node "$TP_VERSION/dist/cli.js") ;; + *) cli=(npx --yes "traceperf@$TP_VERSION") ;; + esac + "${cli[@]}" run $TP_ARGS --baseline "$TP_BASELINE" --markdown "$report" + code=$? + set -e + if [ ! -s "$report" ]; then + printf '### traceperf: the run failed before it produced a report (exit %s)\n\nSee the job log.\n' "$code" > "$report" + fi + cat "$report" >> "$GITHUB_STEP_SUMMARY" + echo "report=$report" >> "$GITHUB_OUTPUT" + echo "code=$code" >> "$GITHUB_OUTPUT" + if [ "$code" -ne 0 ]; then echo "regressed=true" >> "$GITHUB_OUTPUT"; else echo "regressed=false" >> "$GITHUB_OUTPUT"; fi + - if: inputs.comment == 'true' && github.event_name == 'pull_request' + shell: bash + env: + GH_TOKEN: ${{ inputs.github-token }} + REPORT: ${{ steps.run.outputs.report }} + PR: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + run: | + # One comment per PR, updated on every push (found by its marker) + body="$(printf '\n%s' "$(cat "$REPORT")")" + id="$(gh api "repos/$REPO/issues/$PR/comments" --paginate --jq '.[] | select(.body | startswith("")) | .id' | head -n 1)" + if [ -n "$id" ]; then + gh api --method PATCH "repos/$REPO/issues/comments/$id" -f body="$body" > /dev/null + else + gh api --method POST "repos/$REPO/issues/$PR/comments" -f body="$body" > /dev/null + fi + - if: inputs.fail == 'true' && steps.run.outputs.code != '0' + shell: bash + run: | + echo "traceperf: more calls per request than ${{ inputs.baseline }}, or the run failed (see the report above)" + exit 1 diff --git a/examples/slow-apps/clean-batched/perf-baseline.json b/examples/slow-apps/clean-batched/perf-baseline.json new file mode 100644 index 0000000..fcdf185 --- /dev/null +++ b/examples/slow-apps/clean-batched/perf-baseline.json @@ -0,0 +1,13 @@ +{ + "version": 1, + "routes": { + "GET /orders": { + "requests": 10, + "calls": { + "SELECT * FROM items WHERE order_id IN (?)": 1, + "SELECT * FROM orders LIMIT 12": 1 + } + } + }, + "problems": [] +} diff --git a/src/__tests__/run.test.ts b/src/__tests__/run.test.ts index dfc2320..42ec458 100644 --- a/src/__tests__/run.test.ts +++ b/src/__tests__/run.test.ts @@ -412,3 +412,21 @@ describe('config file and login', () => { } }); }); + +describe('markdownReport', () => { + const { markdownReport } = jest.requireActual('../run'); + it('leads with the verdict, lists baseline changes, folds the timing findings', () => { + const md = markdownReport({ + trees: 30, findings: [{ kind: 'repeated', root: 'GET /orders', ms: 40, message: 'GET /orders called q 12 times', fix: 'batch it' }], + baseline: { regressions: ['GET /orders: q 1 → 12 per request (an N+1?)'], improvements: [], notes: [] }, baselineFile: 'b.json', + load: { requests: 30, durationMs: 300, statuses: { 200: 30 }, errors: 0, latency: {} }, + }); + expect(md).toMatch(/^### traceperf: ✖ more calls per request than the baseline\n\n- ✖ GET \/orders: q 1 → 12/); + expect(md).toContain('
Why it was slow'); + expect(md).toContain(' - fix: batch it'); + expect(md).toContain('30 requests in 0.3 s (100/s), 30× 200'); + expect(markdownReport({ trees: 5, findings: [], baseline: { regressions: [], improvements: [], notes: [] }, baselineFile: 'b.json' })) + .toMatch(/^### traceperf: ✔ no new queries or calls\n\n- ✔ same calls per request on every route/); + expect(markdownReport({ trees: 5, findings: [] })).toMatch(/^### traceperf: no issues found/); + }); +}); diff --git a/src/run.ts b/src/run.ts index 7068c8c..f22ef47 100644 --- a/src/run.ts +++ b/src/run.ts @@ -34,6 +34,8 @@ export interface RunArgs { /** Request headers, e.g. { authorization: 'Bearer ...' } */ headers?: Record; html?: string; + /** Write a Markdown summary here (pull-request comments, job summaries) */ + markdown?: string; /** Write call counts per route here (a baseline for --baseline) */ saveBaseline?: string; /** 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 args] [options] --html out.html write an HTML report --output f.jsonl keep the raw request trees --json print stats and findings as JSON + --markdown f.md write a Markdown summary (for a PR comment or job summary) --save-baseline f.json save calls per request for each route (commit it) --baseline f.json compare with it; exit 1 if a route makes more calls (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 | break; } case '--json': args.json = true; break; + case '--markdown': if (!(args.markdown = argv[++i])) return '--markdown expects a file'; break; case '--save-baseline': if (!(args.saveBaseline = argv[++i])) return '--save-baseline expects a file'; break; case '--baseline': if (!(args.baseline = argv[++i])) return '--baseline expects a file'; break; case '--no-profile': args.profile = false; break; @@ -856,6 +860,9 @@ export async function runApp(args: RunArgs, deps: RunDeps): Promise { if (args.html && summary.trees) { fs.writeFileSync(args.html, deps.report(text, { html: true, findings: summary.findings, title: `traceperf run: ${args.command.join(' ')}` })); } + if (args.markdown) { + fs.writeFileSync(args.markdown, markdownReport({ load, findings: summary.findings, baseline: baselineReport, baselineFile: args.baseline, baselineError, trees: summary.trees || 0 })); + } if (args.json) { deps.out(JSON.stringify({ load, trees: summary.trees, findings: summary.findings, ...(baselineReport ? { baseline: baselineReport } : {}) }, null, 2)); } else if (!summary.trees) { @@ -887,6 +894,39 @@ export async function runApp(args: RunArgs, deps: RunDeps): Promise { return !summary.trees && exitCode ? 1 : 0; } +/** A short Markdown summary for a pull-request comment or a CI job summary */ +export function markdownReport(r: { + load?: LoadResult; findings: Finding[]; trees: number; + baseline?: { regressions: string[]; improvements: string[]; notes: string[] }; baselineFile?: string; baselineError?: string; +}): string { + const out: string[] = []; + const failed = !!r.baselineError || !!(r.baseline && r.baseline.regressions.length); + 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'}`}`, ''); + if (r.baselineError) out.push(r.baselineError, ''); + if (r.baseline) { + const { regressions, improvements, notes } = r.baseline; + for (const x of regressions) out.push(`- ✖ ${x}`); + for (const x of improvements) out.push(`- ✔ ${x}`); + for (const x of notes) out.push(`- ${x}`); + if (!regressions.length && !improvements.length) out.push('- ✔ same calls per request on every route'); + out.push('', `Compared with \`${r.baselineFile}\`: calls per request from one-at-a-time requests, so the result does not depend on the machine.`, ''); + } + if (r.findings.length) { + out.push('
Why it was slow (timings from this CI machine)', ''); + for (const f of r.findings) { + out.push(`- **${f.kind}**: ${f.message.replace(/\n/g, ' ')}`); + if (f.fix) out.push(` - fix: ${f.fix}`); + } + out.push('', '
', ''); + } + if (r.load) { + const rate = r.load.requests / Math.max(r.load.durationMs / 1000, 1e-3); + const statuses = Object.entries(r.load.statuses).map(([s, n]) => `${n}× ${s}`).join(', '); + out.push(`${r.load.requests} requests in ${(r.load.durationMs / 1000).toFixed(1)} s (${rate.toFixed(0)}/s)${statuses ? `, ${statuses}` : ''}; ${r.trees} request trees`); + } + return out.join('\n') + '\n'; +} + function waitForPort(fs: any, file: string, exited: () => boolean, timeoutMs: number): Promise { const deadline = Date.now() + timeoutMs; return new Promise(resolve => {