diff --git a/.changes/pr-pipeline/perf-eam-call-attribution-4b9e2d17.md b/.changes/pr-pipeline/perf-eam-call-attribution-4b9e2d17.md new file mode 100644 index 00000000000..0f8aa77fe10 --- /dev/null +++ b/.changes/pr-pipeline/perf-eam-call-attribution-4b9e2d17.md @@ -0,0 +1,12 @@ +--- +bump: minor +--- + +### Added + +- A Node step that sets `API_BUDGET_SCRIPT` ends with its own api-calls notice, + `::notice title=api-calls::script= github-token=N lanes-app=N not-modified=M graphql=K`, counted + from the readings the rate guard appends for the job-end notice, so a job's total splits by step. + pr-merge's enable-auto-merge job names its eight counting steps (who, superseded, head-ref-map, + listing-carry, carry-run-cache, verify-answers, lane-records, enable-auto-merge). No request + changes. diff --git a/.github/actions/log-api-budget/count-requests.test.mjs b/.github/actions/log-api-budget/count-requests.test.mjs index 2a13a7c4085..849a98cca14 100644 --- a/.github/actions/log-api-budget/count-requests.test.mjs +++ b/.github/actions/log-api-budget/count-requests.test.mjs @@ -2,9 +2,11 @@ // sends in $RUNNER_TEMP/api-budget/, one reading line per answer, for // the job-end api-calls notice (api-budget.bash api_budget_emit_calls). import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; import { existsSync, readFileSync, rmSync } from "node:fs"; import { join } from "node:path"; import { test } from "node:test"; +import { fileURLToPath } from "node:url"; import { createGitHub } from "../check-trusted-trigger/check-trusted-trigger.mjs"; import { countRequests, createRateLimitLatch } from "./rate-guard.mjs"; import { countedFetch, createGitHubApi as addressFeedbackApi } from "../../scripts/pr-unblock/address-feedback.mjs"; @@ -183,3 +185,45 @@ for (const [name, run] of [ } })); } + +// A step's own api-calls line (API_BUDGET_SCRIPT): one child process sends +// two core reads, a 304 and a GraphQL query on the step's token and one read +// on a second token, so its line reads github-token=3 lanes-app=1 +// not-modified=1 graphql=1, and the job-end lines over the same files agree. +const STEP_CALLS = ` +import { createGitHub } from ${JSON.stringify(new URL("../check-trusted-trigger/check-trusted-trigger.mjs", import.meta.url).href)}; +const answer = (status, resource) => new Response(status === 304 ? null : "{}", { status, headers: { "x-ratelimit-limit": "5000", "x-ratelimit-remaining": "4990", "x-ratelimit-used": "10", "x-ratelimit-reset": "1792751600", "x-ratelimit-resource": resource } }); +const fetchImpl = async (url, init) => (init.headers["If-None-Match"] ? answer(304, "core") : url.endsWith("/graphql") ? answer(200, "graphql") : answer(200, "core")); +const own = createGitHub({ token: "own", fetchImpl, log: () => {} }); +await own("GET", "/repos/o/r/pulls/1"); +await own("GET", "/repos/o/r/pulls/2"); +await own("GET", "/repos/o/r/pulls/3", undefined, { "If-None-Match": "etag" }).catch(() => {}); +await own("POST", "/graphql", { query: "{ viewer { login } }" }); +await createGitHub({ token: "other", fetchImpl, log: () => {} })("GET", "/repos/o/r"); +`; +const runStep = (script) => { + const dir = tempDir("count-requests-step-"); + const env = { PATH: process.env.PATH, RUNNER_TEMP: dir, GH_TOKEN: "own", API_BUDGET_TOKEN: "github-token", API_BUDGET_OVERRIDE_TOKEN: "lanes-app" }; + try { + const step = spawnSync(process.execPath, ["--input-type=module", "-e", STEP_CALLS], { encoding: "utf8", env: script === undefined ? env : { ...env, API_BUDGET_SCRIPT: script } }); + assert.equal(step.status, 0, step.stderr); + const job = spawnSync("bash", ["-c", `. ${JSON.stringify(fileURLToPath(new URL("./api-budget.bash", import.meta.url)))}; api_budget_emit_calls github-token lanes-app`], { encoding: "utf8", env }); + return { step: step.stdout, job: job.stdout }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}; + +test("API_BUDGET_SCRIPT ends the process with its own api-calls line, which the job-end lines agree with", () => { + const { step, job } = runStep("who"); + assert.equal(step, "::notice title=api-calls::script=who github-token=3 lanes-app=1 not-modified=1 graphql=1\n"); + assert.equal(job, "::notice title=api-calls::github-token=3 not-modified=1 graphql=1\n::notice title=api-calls::lanes-app=1 not-modified=0 graphql=0\n"); +}); + +test("without API_BUDGET_SCRIPT, or with one that is not lower-case kebab case, the same calls print nothing", () => { + for (const script of [undefined, "Who", "who::x", ""]) { + const { step, job } = runStep(script); + assert.equal(step, "", String(script)); + assert.match(job, /^::notice title=api-calls::github-token=3 not-modified=1 graphql=1$/m); + } +}); diff --git a/.github/actions/log-api-budget/rate-guard.mjs b/.github/actions/log-api-budget/rate-guard.mjs index 430116729e7..89b22f34266 100644 --- a/.github/actions/log-api-budget/rate-guard.mjs +++ b/.github/actions/log-api-budget/rate-guard.mjs @@ -20,7 +20,7 @@ // change to a default waits for a person. // rate-guard.vectors.json pins classify's decisions for every port. -import { appendFileSync, mkdirSync, readFileSync } from "node:fs"; +import { appendFileSync, mkdirSync, readFileSync, writeSync } from "node:fs"; import { dirname, join } from "node:path"; import process from "node:process"; @@ -154,14 +154,55 @@ export function recordReading({ token = process.env.API_BUDGET_TOKEN, dir = proc if (!dir || !/^[a-z][a-z0-9-]{0,39}$/.test(token ?? "")) { return; } + const line = readingLine(status, headers); try { mkdirSync(join(dir, "api-budget"), { recursive: true }); - appendFileSync(join(dir, "api-budget", token), readingLine(status, headers)); + appendFileSync(join(dir, "api-budget", token), line); + scriptCalls.count(token, line); } catch { // A reading is never worth failing the job. } } +// This process's share of the job-end api-calls notice: the readings it +// appended, counted as api_budget_emit_calls counts the file (N its non-304 +// lines per token, M its 304s, K its graphql-resource lines). When +// API_BUDGET_SCRIPT names the step, the process ends with +// `::notice title=api-calls::script= github-token=N lanes-app=N +// not-modified=M graphql=K`, so a job's total splits by step. A child +// process's calls (babysit's Python) are its own, not in this line. +export function createScriptCalls() { + const tokens = { "github-token": 0, "lanes-app": 0 }; + let notModified = 0; + let graphql = 0; + return { + count(token, line) { + const fields = line.trim().split(" "); + if (fields[0] === "304") { + notModified += 1; + } else { + tokens[token] = (tokens[token] ?? 0) + 1; + } + graphql += fields[5] === "graphql" ? 1 : 0; + }, + line: (name) => + `::notice title=api-calls::script=${name} ${Object.entries(tokens) + .map(([token, n]) => `${token}=${n}`) + .join(" ")} not-modified=${notModified} graphql=${graphql}\n`, + }; +} +export const scriptCalls = createScriptCalls(); +const SCRIPT = process.env.API_BUDGET_SCRIPT ?? ""; +if (/^[a-z][a-z0-9-]{0,39}$/.test(SCRIPT)) { + process.on("exit", () => { + try { + writeSync(1, scriptCalls.line(SCRIPT)); + } catch { + // A count is never worth failing the job. + } + }); +} + // The bucket a request's reading goes under: token when the client pins one; // else API_BUDGET_OVERRIDE_TOKEN for a request whose bearer is not the // step's own token (GH_TOKEN, else GITHUB_TOKEN), as babysit_gh.py buckets a diff --git a/.github/scripts/pr-merge/workflow.test.mjs b/.github/scripts/pr-merge/workflow.test.mjs index f40d3f74531..f0d286614cb 100644 --- a/.github/scripts/pr-merge/workflow.test.mjs +++ b/.github/scripts/pr-merge/workflow.test.mjs @@ -647,14 +647,29 @@ for (const jobId of ["merge", "enable-auto-merge"]) { GITHUB_API_URL: `http://127.0.0.1:${server.address().port}`, NODE_OPTIONS: "--preserve-symlinks-main", }; + let stdout = ""; for (const name of ["Check who started the run", "Check for a newer run that does this work"]) { const step = stepOf(name); const run = /\n {8}run: (.*)/.exec(step)[1]; - const status = await new Promise((resolve) => - spawn("bash", ["-ec", run], { cwd: dir, env: { ...base, ...envOf(step) }, stdio: "ignore" }).on("close", resolve), - ); + const status = await new Promise((resolve) => { + const child = spawn("bash", ["-ec", run], { cwd: dir, env: { ...base, ...envOf(step) }, stdio: ["ignore", "pipe", "ignore"] }); + child.stdout.on("data", (chunk) => (stdout += chunk)); + child.on("close", resolve); + }); assert.equal(status, 0, name); } + // enable-auto-merge names each step (API_BUDGET_SCRIPT), so its job + // total splits by step: who's one read, superseded's two. The merge job + // names none. + assert.deepEqual( + stdout.match(/^::notice title=api-calls::script=.*$/gm) ?? [], + jobId === "enable-auto-merge" + ? [ + "::notice title=api-calls::script=who github-token=1 lanes-app=0 not-modified=0 graphql=0", + "::notice title=api-calls::script=superseded github-token=2 lanes-app=0 not-modified=0 graphql=0", + ] + : [], + ); assert.match(readFileSync(output, "utf8"), /^proceed=true$/m); assert.match(readFileSync(output, "utf8"), /^superseded=true$/m); const log = stepOf("Log the API budget at job end"); @@ -1484,3 +1499,14 @@ test("the bypass step runs only inside the cap after a check that judged, and th assert.match(config, /\n {2}full-run-cap-bypass: true\n/); assert.doesNotMatch(job("dispatch-full-run"), /steps\.bypass/); }); + +// Every enable-auto-merge step that counts its GitHub calls (API_BUDGET_TOKEN) +// names itself (API_BUDGET_SCRIPT, rate-guard.mjs scriptCalls), each a name of +// its own, so the job-end api-calls total splits by step. +test("enable-auto-merge: every step that counts its calls names itself once", () => { + const steps = job("enable-auto-merge").split(/\n {6}- name: /).filter((step) => /^ {10}API_BUDGET_TOKEN: /m.test(step)); + const names = steps.map((step) => /^ {10}API_BUDGET_SCRIPT: ([a-z][a-z0-9-]{0,39})$/m.exec(step)?.[1]); + assert.ok(steps.length > 0); + assert.ok(names.every(Boolean), names.join(" ")); + assert.equal(new Set(names).size, names.length); +}); diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index f8a0f429dfa..b5ef9508bc3 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -2044,6 +2044,7 @@ jobs: if: steps.kill.outputs.proceed == 'true' env: API_BUDGET_TOKEN: github-token + API_BUDGET_SCRIPT: who # The lane token holds no Actions permission. ACTIONS_TOKEN: ${{ github.token }} ACTOR_ID: ${{ github.actor_id }} @@ -2065,6 +2066,7 @@ jobs: timeout-minutes: 2 env: API_BUDGET_TOKEN: github-token + API_BUDGET_SCRIPT: superseded ACTIONS_TOKEN: ${{ github.token }} SUPERSEDED_JOB: enable-auto-merge TRUSTED_ACTORS_PATH: .base/.github/standards/trusted-actors/trusted-actors.json @@ -2105,6 +2107,7 @@ jobs: continue-on-error: true env: API_BUDGET_TOKEN: github-token + API_BUDGET_SCRIPT: head-ref-map ACTIONS_TOKEN: ${{ github.token }} HEAD_REFS_PATH: ${{ runner.temp }}/head-refs-in.json PR_MERGE_READS_PATH: ${{ runner.temp }}/pr-merge-reads.json @@ -2115,6 +2118,7 @@ jobs: continue-on-error: true env: API_BUDGET_TOKEN: github-token + API_BUDGET_SCRIPT: listing-carry ACTIONS_TOKEN: ${{ github.token }} BABYSIT_VERIFY_LISTING_CARRY: ${{ runner.temp }}/verify-listing-carry.json PR_MERGE_READS_PATH: ${{ runner.temp }}/pr-merge-reads.json @@ -2126,6 +2130,7 @@ jobs: timeout-minutes: 2 env: API_BUDGET_TOKEN: github-token + API_BUDGET_SCRIPT: carry-run-cache ACTIONS_TOKEN: ${{ github.token }} CARRY_TO: ${{ runner.temp }}/pr-merge-carried.json PR_MERGE_READS_PATH: ${{ runner.temp }}/pr-merge-reads.json @@ -2152,6 +2157,7 @@ jobs: timeout-minutes: 2 env: API_BUDGET_TOKEN: github-token + API_BUDGET_SCRIPT: verify-answers ACTIONS_TOKEN: ${{ github.token }} VERIFY_ANSWERS_PATH: ${{ runner.temp }}/verify-answers-in.json # The arm step's skip records, from the same artifact. @@ -2191,6 +2197,7 @@ jobs: timeout-minutes: 2 env: API_BUDGET_TOKEN: github-token + API_BUDGET_SCRIPT: lane-records ACTIONS_TOKEN: ${{ github.token }} LANE_RECORDS_PATH: ${{ runner.temp }}/lane-records.json PR_MERGE_READS_PATH: ${{ runner.temp }}/pr-merge-reads.json @@ -2235,6 +2242,7 @@ jobs: # the Actions reads that carry it. API_BUDGET_TOKEN: lanes-app API_BUDGET_OVERRIDE_TOKEN: github-token + API_BUDGET_SCRIPT: enable-auto-merge run: | set -euo pipefail config=.base/docs/conventions/pr-pipeline.yaml