Commit 427a93d
authored
log-classifier: rules + fixtures from log sweeps, and fixes found by auditing matched verdicts (#8409)
Eight commits. The first four came from sampling logs the classifier
**missed**; the last four from sampling logs it **matched** — which
turned out to be far the more productive seam, because a rule that fires
on the *wrong line* still counts as "covered" and is invisible to every
coverage metric.
Measured on `main`, last 7 days: the landed ruleset classifies **95.2%**
of live red-job logs (95% CI 92.4–97.0; n=400 of 2944, verified by
re-running the engine over raw logs). The deployed lambda is at
**81.1%** — that ~14-point gap is deploy lag, and is worth more than
anything in this PR.
Throughout, the triage rule was: add a fixture only if the landed
classifier is *clearly wrong* on it, or it exercises something not
already represented. Most candidates were correctly skipped (deploy-lag
on the stale prod classifier, dead/rate-limited log blobs, or redundant
with existing fixtures). Prod's stored classification lags the landed
ruleset, so every candidate was re-derived against `ruleset.toml` rather
than trusted.
---
## Part 1 — from a first log sweep (`7d95f1cd9`)
New rules, where the real cause was mis- or un-classified:
- **`uv python download metadata fetch failed`** — a runner DNS/network
failure during `uv python install` was being masked by a benign CMake
feature-probe `ninja: build stopped`, so the surfaced line was a
meaningless "Build error". The new rule sits *above* the ninja rules and
captures only the stable phrase (the URL carries the uv version, so it's
excluded from the capture to keep grouping stable).
- **`Lintrunner general linter failure`** — a linter *tool crash*
(`General linter failure` / `Error (CODE) Linter failed`) is distinct
from a per-file lint violation. Previously only the far-away
`##[error]Linter failed` GHA catch-all fired, losing which linter died.
Captures the linter code so CLANGTIDY vs RUFF group separately.
Novel coverage for existing rules that had no fixture:
`python_importerror_name` and `python_importerror_missing_so` — two
ImportError flavors (bad symbol import; missing `.so` during `import
torch`).
Deliberately **not** added: a `received SIGINT` rule. That line is
emitted for **both** a timeout (a build hitting `timeout-minutes` is
SIGINT'd and reported "cancelled") **and** a real cancellation
(fail-fast / manual / concurrency). The candidate that surfaced it was
in fact a 280-minute cu130 build timeout, but the cause isn't
determinable from the log text alone, so the neutral GHA-error catch-all
is the honest ceiling here. Part 2 pins that ceiling with a fixture.
## Part 2 — from a second log sweep (`c8149594a`)
26 candidates triaged, 21 skipped: 4 dead S3 blobs (Azure
`BlobNotFound`), 1 job that had actually *succeeded*, rest deploy-lag or
redundant.
- **`pytest failure` (FAILURES section header)** — when a run prints no
`short test summary info`, the existing `^FAILED ...` rule can't fire
and the winner was a benign `PytestUnraisableExceptionWarning`
`AttributeError` raised in a **passing** test's `__del__`. Anchors on
the per-test header under `==== FAILURES ====`, always immediately above
the traceback.
Position matters beyond display: the matched line number centres the
±250-line window fed to the Bedrock summarizer (`main.rs`). `RERUNS`
prints before `FAILURES` and `evaluate_rule` takes each rule's **last**
match, so the real entry wins over rerun copies.
- **`AWS credentials could not be loaded`** — the last `##[error]` is a
`continue-on-error` S3-upload symptom ~2000 lines after the real
`configure-aws-credentials` failure.
- **`git checkout ref not found`** — a deleted `ciflow` tag, masked by
the later git exit-128 wrapper.
- **`ghstack old version - unexpected PR count in commit`** — generic
capture baked in the per-run commit sha, one group per run.
- `docker_manifest_unknown` — first fixture for an existing rule.
## Part 3 — SIGINT ceiling and ignore-list (`4f889eac6`)
Third sweep: 28 candidates, one fixture, no new rules. The null result
is the finding — two candidate categories are exhausted (documented so
future sweeps skip them): all 4 `no-match` job families were dead S3
blobs (8 of 10 across two sweeps), and all 3 `Python Test File
RuntimeError` candidates were already handled by `Test file timeout`
(7-for-7).
`gha_catchall_sigint.txt` is a guard. It pins the ignore-list dropping
**both** OSDC lines, and the bare `GHA error` catch-all shape which
nothing asserted. Its verdict is `##[error]‹received SIGINT›`
deliberately: that line is emitted for both a timeout and a real
cancellation and the log cannot distinguish them, so a future
over-confident SIGINT rule will fail this fixture.
## Part 4 — documented invariants (`7abbb0860`)
- **A fixture window must not change the verdict.** The harness
classifies the fixture *file*, so a window ending just after a matchable
line snaps `#=MATCH=#` onto it and encodes a verdict that disagrees with
production. This bit us: a pytest fixture trimmed to end on a `RERUNS`
header asserted the classifier lands on a rerun. Includes the `--full`
cross-check recipe.
- **Anchor position, not just text** — the matched line number centres
the Bedrock window.
## Part 5 — `Python Test SIG Code` could never fire (`2c9c67142`)
The rule required `Received Signal:` (capital S). `run_test.py:2119`
emits lowercase `Received signal:`. **The rule had never matched a
log.** Signal-killed test files fell through to the generic wrapper and
were reported as ordinary failures — 7 jobs last week: 4× SIGBUS, 1×
SIGIOT, 2× SIGINT. Now captures the file **and** the signal.
Checked for systemic breakage: all 97 rules recompiled
case-insensitively over 140 raw logs surfaces exactly one such rule —
this one. Isolated.
## Part 6 — pod failures grouped by pool (`2b26d2f14`)
`Runner pod failed to come online` captured only the phase tail, which
was `Pending: backoff timeout` for **39 of 39** pod failures in a week:
one group, no information, ~15% of all red jobs.
Only the `-<rand>-runner-<rand>-workflow` suffix is random; the prefix
is the runner pool:
```
26 mt-l-x86iavx512-8-64 1 mt-l-arm64g4-16-62
9 lf-l-x86iavx512-8-64 1 mt-l-x86aavx2-29-113-a10g
1 lf-l-arm64g4-16-62 1 mt-l-x86aavx2-45-167-a10g-4
```
**90% of pod failures are one pool shape**, previously impossible to
see. `\w+` can't cross a hyphen so the greedy prefix stops exactly at
the random segment; 39/39 parse. The old general pattern is kept below
as a fallback.
> Side note for whoever owns the fleet: if `x86iavx512-8-64` is failing
pod bringup at ~9× the rate of everything else, that's an infra problem
this grouping was hiding. n=39 from a 300-job sample — now confirmable
against the full week.
## Part 7 — new timeout signals (`40e49697c`)
Matches the two new shapes from pytorch/pytorch#191754, most specific
first:
1. `TIMED OUT: <nodeid>` → the nodeid (a test was in flight)
2. `... returning 124 (<file> <shard>/<n>)` → the file (hang during
import/collection)
3. `... returning 124` → unchanged, kept indefinitely for release
branches
Exit code 124 is reliable to key on: `wait_for_process` SIGINTs first
and returns the child's real exit code if it dies gracefully, so 124
only ever means an unkillable hang.
**Deviation from the PR author's writeup:** the shard is left *out* of
the capture in shape 2 — the same file hanging on 3/8 and 5/8 is one
problem and should aggregate; the shard is still readable on the line.
Tested in `src/main.rs`, not `fixtures/classify/`, because fixtures are
verbatim CI logs and neither shape exists in any log yet. **Replace with
real fixtures once the runner change lands and propagates.**
## Part 8 — the real cause of collection-time failures (`dd0b651a8`)
Re-aimed the audit at what a developer actually hits: **PR jobs** (not
`main`) and **test/build** failures (not infra). Both of my earlier
populations were wrong — `main` is inflated by trunk-only noise, and
volume-weighting sent half the sample at infra.
On PR jobs the top dev-facing rule is healthy: `Python unittest
failure`, 63% of dev-facing matches, always landing on a precise
`test/...::Class::test_name`. The **#2 rule was not**: `Python Test File
RuntimeError` (11%) surfaces run_test.py's `<file> N/N failed!` summary,
naming the file but not the cause.
All 16 sampled wrapper-only cases were the same thing — the file blew up
at import/collection before any test ran, with a good exception line
already in the log. **12 of the 16 shared one root cause**: an
unresolved merge-conflict marker committed into
```
torch/testing/_internal/common_methods_invocations.py:15375
>>>>>>> 597be33c510 (Add more cases in test_ops.py)
SyntaxError: invalid decimal literal
```
imported by much of the suite — so twelve jobs across a dozen unrelated
test files each reported a different useless key (`test_ops 3/4`,
`test_torch`, `lazy/test_ts_opinfo`, …) for a single one-line breakage.
The other 4 were a module-level `AssertionError` from `test_overrides`'
`__torch_function__` coverage check.
There was **no `SyntaxError` or `AssertionError` rule at all**. Added
both, *above* the wrapper so they beat it.
Deliberately did **not** demote the wrapper beneath the existing generic
exception rules instead: the wrapper line can itself read `RuntimeError:
test_ops 3/4 failed!`, so putting it under `Python RuntimeError` would
capture that whole line and make grouping strictly worse. Neither
promoted exception can match the wrapper's own text, so the change is
contained.
**All 16 now surface the real cause** (12 SyntaxError, 4 AssertionError)
instead of a test-file name.
Only these two exception types are promoted because that is what the
data showed; a collection failure can raise anything, and `ImportError`
/ `ValueError` / `TypeError` still sit below the wrapper. The principled
version is "any concrete exception beats a file-name summary" — worth
revisiting once more cases accumulate.
---
## Testing
`cargo test` green: 19 + 18 unit tests, 41 fixtures. Across the whole
branch exactly one existing fixture moved —
`infra_osdc_pod_failure.txt`, re-blessed to add the pool capture. Every
other change is regression-clean against all prior cases.
Fixtures are the marker-based regression format from #8394 — each
carries a `#=SOURCE=#` job link and a `#=MATCH=#` marker on the line the
classifier picks.
`pytest_failures_section.txt` is deliberately 663 lines: the benign
`__del__` confusers and the real `FAILURES` traceback sit ~550 lines
apart in the source log, and holding both in one contiguous real
fragment is what makes the fixture assert the actual production verdict
rather than an artifact of truncation.1 parent aafb741 commit 427a93d
17 files changed
Lines changed: 1685 additions & 9 deletions
File tree
- aws/lambda/log-classifier
- fixtures/classify
- src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
49 | 49 | | |
50 | 50 | | |
51 | 51 | | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
52 | 86 | | |
53 | 87 | | |
54 | 88 | | |
| |||
Lines changed: 62 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
Lines changed: 70 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
0 commit comments