From 5e685999afae3bb989e4a9a1cfa2014aba79ac55 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 6 Jul 2026 12:44:07 -0700 Subject: [PATCH] fix(ci): make the CI timing report unflakeable (#59818) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'CI timing report' job is pure observability — it collects per-job/step durations from the GitHub API after the run and publishes an HTML gantt report + PR-vs-main timing diff. It gates nothing (all-checks-pass does not include it), yet it could redden a PR: the script makes dozens of paginated API calls with the shared repo GITHUB_TOKEN and had zero retry handling, so a single 403 (rate-limit burst when several PRs run CI concurrently) failed the job. Observed twice in a row on PR #59805. - api_get(): retry 403/429/5xx and connection errors with exponential backoff, honoring Retry-After / X-RateLimit-Reset (max 5 attempts, 120s cap). Non-transient statuses (404 etc.) still fail fast. - main(): exhausted retries raise TimingsUnavailable, caught to emit a degraded summary line + placeholder HTML artifact and exit 0 — a metrics collector must never fail the PR's checks. No timings JSON is written on the degraded path so an empty baseline can never be cached. - ci.yml: baseline-save steps on main skip gracefully when no JSON exists. Verified with a mocked urlopen harness: retry-then-success (3 attempts), exhausted-retries -> TimingsUnavailable, 404 fails fast without retry, degraded main() exits 0 with summary + placeholder and no JSON, and the --from-json happy path is unchanged. --- .github/workflows/ci.yml | 11 ++++- scripts/ci/timings_report.py | 88 ++++++++++++++++++++++++++++++++++-- 2 files changed, 92 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 595569a82fa..ab601201653 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -221,10 +221,17 @@ jobs: - name: Save baseline cache (main only) if: github.event_name == 'push' && github.ref == 'refs/heads/main' - run: cp ci-timings.json ci-timings-baseline.json + run: | + # Degraded runs (API rate-limited) produce no ci-timings.json — + # skip rather than fail, and never cache an empty baseline. + if [ -f ci-timings.json ]; then + cp ci-timings.json ci-timings-baseline.json + else + echo "No timings JSON this run — skipping baseline update" + fi - name: Upload baseline to cache (main only) - if: github.event_name == 'push' && github.ref == 'refs/heads/main' + if: github.event_name == 'push' && github.ref == 'refs/heads/main' && hashFiles('ci-timings-baseline.json') != '' uses: actions/cache/save@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 with: path: ci-timings-baseline.json diff --git a/scripts/ci/timings_report.py b/scripts/ci/timings_report.py index 20b4b598453..709472ee179 100644 --- a/scripts/ci/timings_report.py +++ b/scripts/ci/timings_report.py @@ -22,6 +22,7 @@ import argparse import json import os import sys +import time import urllib.error import urllib.parse import urllib.request @@ -30,6 +31,70 @@ from html import escape API_BASE = "https://api.github.com" +# Retry policy for GitHub API calls. The repo-scoped GITHUB_TOKEN shares a +# rate-limit budget across every concurrent workflow run; when several PRs +# run CI at once, this report job (which makes dozens of paginated calls) +# regularly hits 403 rate-limit responses. Those are transient — retry with +# backoff, honoring Retry-After / X-RateLimit-Reset when present. +_RETRY_STATUSES = {403, 429, 500, 502, 503, 504} +_MAX_ATTEMPTS = 5 +_MAX_RETRY_WAIT_S = 120.0 + + +class TimingsUnavailable(Exception): + """GitHub API data could not be collected (rate limit, outage, ...). + + This is a REPORT job — never a reason to fail the PR's checks. main() + catches this and exits 0 with a degraded summary. + """ + + +def _retry_wait_s(headers, attempt: int) -> float: + """Seconds to wait before the next attempt, honoring server hints.""" + retry_after = (headers.get("Retry-After") or "").strip() if headers else "" + if retry_after.isdigit(): + return min(float(retry_after), _MAX_RETRY_WAIT_S) + reset = (headers.get("X-RateLimit-Reset") or "").strip() if headers else "" + remaining = (headers.get("X-RateLimit-Remaining") or "").strip() if headers else "" + if remaining == "0" and reset.isdigit(): + return min(max(float(reset) - time.time(), 1.0), _MAX_RETRY_WAIT_S) + return min(2.0 ** attempt * 2.0, _MAX_RETRY_WAIT_S) # 4s, 8s, 16s, 32s + + +def _urlopen_with_retry(req: urllib.request.Request): + """urlopen with backoff on rate-limit/transient statuses. + + Returns (parsed_json, link_header). Raises TimingsUnavailable when + attempts are exhausted — callers treat that as "no report this run", + not a job failure. + """ + last_err: Exception | None = None + for attempt in range(1, _MAX_ATTEMPTS + 1): + try: + with urllib.request.urlopen(req) as resp: + return json.loads(resp.read()), resp.headers.get("Link", "") + except urllib.error.HTTPError as e: + last_err = e + if e.code not in _RETRY_STATUSES or attempt == _MAX_ATTEMPTS: + break + wait = _retry_wait_s(e.headers, attempt) + print(f"GitHub API {e.code} on {req.full_url} — " + f"retry {attempt}/{_MAX_ATTEMPTS - 1} in {wait:.0f}s", + file=sys.stderr) + time.sleep(wait) + except urllib.error.URLError as e: + last_err = e + if attempt == _MAX_ATTEMPTS: + break + wait = _retry_wait_s(None, attempt) + print(f"GitHub API connection error on {req.full_url} ({e.reason}) — " + f"retry {attempt}/{_MAX_ATTEMPTS - 1} in {wait:.0f}s", + file=sys.stderr) + time.sleep(wait) + raise TimingsUnavailable( + f"GitHub API unavailable after {_MAX_ATTEMPTS} attempts: {last_err}" + ) + # --------------------------------------------------------------------------- # GitHub API helpers @@ -42,6 +107,9 @@ def api_get(path: str, token: str, params: dict | None = None, For list endpoints, pass list_key to extract items from the paginated wrapper response (e.g. list_key='jobs' for {'total_count': N, 'jobs': [...]}). When list_key is omitted, a non-list response is returned as-is (single object). + + Transient failures (403 rate limit, 429, 5xx, connection errors) are + retried with backoff; exhausted retries raise TimingsUnavailable. """ url = f"{API_BASE}{path}" if params: @@ -55,9 +123,7 @@ def api_get(path: str, token: str, params: dict | None = None, "X-GitHub-Api-Version": "2022-11-28", "User-Agent": "ci-timings-report", }) - with urllib.request.urlopen(req) as resp: - data = json.loads(resp.read()) - link_header = resp.headers.get("Link", "") + data, link_header = _urlopen_with_retry(req) if list_key: results.extend(data.get(list_key, [])) @@ -748,8 +814,20 @@ def main(): repo = expect_env("GITHUB_REPOSITORY") run_id = expect_env("GITHUB_RUN_ID") head_sha = expect_env("GITHUB_SHA") - - timings = collect_timings(token, repo, run_id, head_sha) + try: + timings = collect_timings(token, repo, run_id, head_sha) + except TimingsUnavailable as e: + # Observability job: a missing report must never redden the PR. + # Emit a degraded summary + placeholder artifact and exit 0. + msg = f"CI timing data unavailable this run: {e}" + print(msg, file=sys.stderr) + with open(args.summary_out, "a", encoding="utf-8") as f: + f.write(f"\n> ⚠️ {msg}\n") + with open(args.output, "w", encoding="utf-8") as f: + f.write(f"

{escape(msg)}

\n") + # No JSON on purpose: an empty timings file must never be cached + # as the main baseline. + sys.exit(0) # Save JSON with open(args.json_out, "w", encoding="utf-8") as f: