Merge pull request #25 from thelastbackspace/feat/github-action

GitHub Action and run --markdown

Author: thelastbackspace

.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:

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@<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.
 - `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.
 

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:
+
+<!-- docs:skip -->
+```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 })`.

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 '<!-- traceperf-action -->\n%s' "$(cat "$REPORT")")"
+        id="$(gh api "repos/$REPO/issues/$PR/comments" --paginate --jq '.[] | select(.body | startswith("<!-- traceperf-action -->")) | .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

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": []
+}

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('<details><summary>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/);
+  });
+});

src/run.ts

@@ -34,6 +34,8 @@ export interface RunArgs {
   /** Request headers, e.g. { authorization: 'Bearer ...' } */
   headers?: Record<string, string>;
   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.js> [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<number> {
   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<number> {
   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('', `<sub>Compared with \`${r.baselineFile}\`: calls per request from one-at-a-time requests, so the result does not depend on the machine.</sub>`, '');
+  }
+  if (r.findings.length) {
+    out.push('<details><summary>Why it was slow (timings from this CI machine)</summary>', '');
+    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('', '</details>', '');
+  }
+  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(`<sub>${r.load.requests} requests in ${(r.load.durationMs / 1000).toFixed(1)} s (${rate.toFixed(0)}/s)${statuses ? `, ${statuses}` : ''}; ${r.trees} request trees</sub>`);
+  }
+  return out.join('\n') + '\n';
+}
+
 function waitForPort(fs: any, file: string, exited: () => boolean, timeoutMs: number): Promise<number | undefined> {
   const deadline = Date.now() + timeoutMs;
   return new Promise(resolve => {