Skip to content

Commit 99f4710

Browse files
dillon-zhengclaude
dillon-zheng
and
claude
authored
fix(insight): skip invalid prow jobs (#567)
## Summary - Skip Prow jobs that cannot be inserted into the prow_jobs table because required fields are missing or outside the DB enum - Log a small sample of skipped jobs so the crawler can finish successfully while still surfacing bad source records - Add Deno tests for missing status fields and invalid states; also fix the existing DSN parser test import ## Verification - npx --yes deno fmt insight/crawlers/ci/prow-jobs.ts insight/crawlers/ci/prow-jobs.test.ts - npx --yes deno test --allow-net insight/crawlers/ci/prow-jobs.test.ts - npx --yes deno lint insight/crawlers/ci/prow-jobs.ts insight/crawlers/ci/prow-jobs.test.ts - Read-only live check against https://prow.tidb.net: 8123 jobs fetched, 8120 insertable, 3 skipped due to missing status.state --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 15b244e commit 99f4710

2 files changed

Lines changed: 240 additions & 40 deletions

File tree

‎insight/crawlers/ci/prow-jobs.test.ts‎

Lines changed: 109 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,44 @@
1-
import { assertEquals, assertThrows } from "jsr:@std/assert";
2-
import { convertDsnToClientConfig } from "./prow-jobs.ts";
1+
import { assertEquals, assertThrows } from "jsr:@std/assert@1.0.19";
2+
import { convertDsnToClientConfig } from "../../db/utils.ts";
3+
import {
4+
filterInsertableJobs,
5+
invalidProwJobReason,
6+
type prowJobRun,
7+
} from "./prow-jobs.ts";
8+
9+
function makeProwJob(overrides: Partial<prowJobRun> = {}): prowJobRun {
10+
const job: prowJobRun = {
11+
kind: "ProwJob",
12+
metadata: {
13+
name: "valid-prow-job",
14+
namespace: "prow-test-pods",
15+
labels: {},
16+
},
17+
spec: {
18+
type: "periodic",
19+
agent: "kubernetes",
20+
cluster: "default",
21+
namespace: "prow-test-pods",
22+
job: "periodic-crawl-ci-run-data",
23+
report: true,
24+
},
25+
status: {
26+
state: "success",
27+
startTime: "2026-07-11T02:11:48Z",
28+
pendingTime: "2026-07-11T02:11:48Z",
29+
completionTime: "2026-07-11T02:14:10Z",
30+
url: "https://prow.tidb.net/view/gs/prow-tidb-logs/logs/example/1",
31+
},
32+
};
33+
34+
return {
35+
...job,
36+
...overrides,
37+
metadata: { ...job.metadata, ...overrides.metadata },
38+
spec: { ...job.spec, ...overrides.spec },
39+
status: { ...job.status, ...overrides.status },
40+
};
41+
}
342

443
Deno.test("should correctly parse a valid DSN", () => {
544
const dsn = "mysql://user:password@localhost:5432/database";
@@ -44,3 +83,71 @@ Deno.test("should correctly parse a DSN with special characters in user and pass
4483
db: "database",
4584
});
4685
});
86+
87+
Deno.test("filters prow jobs missing insert-required status fields", () => {
88+
const validJob = makeProwJob();
89+
const missingStateJob = makeProwJob({
90+
metadata: {
91+
name: "missing-state",
92+
namespace: "prow-test-pods",
93+
labels: {},
94+
},
95+
status: { state: null },
96+
});
97+
const missingStartTimeJob = makeProwJob({
98+
metadata: {
99+
name: "missing-start-time",
100+
namespace: "prow-test-pods",
101+
labels: {},
102+
},
103+
status: { startTime: null },
104+
});
105+
106+
const { insertableJobs, skippedJobs } = filterInsertableJobs([
107+
validJob,
108+
missingStateJob,
109+
missingStartTimeJob,
110+
]);
111+
112+
assertEquals(insertableJobs.map((job) => job.metadata.name), [
113+
"valid-prow-job",
114+
]);
115+
assertEquals(skippedJobs.map(({ reason }) => reason), [
116+
"missing status.state",
117+
"missing or invalid status.startTime",
118+
]);
119+
});
120+
121+
Deno.test("rejects prow jobs with states outside the database enum", () => {
122+
const job = makeProwJob({
123+
status: { state: "unknown-state" },
124+
});
125+
126+
assertEquals(
127+
invalidProwJobReason(job),
128+
"invalid status.state: unknown-state",
129+
);
130+
});
131+
132+
Deno.test("rejects prow jobs missing insert-required metadata and spec fields", () => {
133+
assertEquals(
134+
invalidProwJobReason(makeProwJob({ metadata: { namespace: "" } })),
135+
"missing metadata.namespace",
136+
);
137+
assertEquals(
138+
invalidProwJobReason(makeProwJob({ metadata: { name: "" } })),
139+
"missing metadata.name",
140+
);
141+
assertEquals(
142+
invalidProwJobReason(makeProwJob({ spec: { job: "" } })),
143+
"missing spec.job",
144+
);
145+
assertEquals(
146+
invalidProwJobReason(makeProwJob({ spec: { type: "" } })),
147+
"missing spec.type",
148+
);
149+
assertEquals(
150+
invalidProwJobReason(makeProwJob({ spec: { type: "weird" } })),
151+
"invalid spec.type: weird",
152+
);
153+
});

‎insight/crawlers/ci/prow-jobs.ts‎

Lines changed: 131 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,36 @@ import { parseArgs } from "jsr:@std/cli@1.0.14/parse-args";
22
import * as mysql from "https://deno.land/x/mysql@v2.12.1/mod.ts";
33
import { convertDsnToClientConfig } from "../../db/utils.ts";
44

5-
interface prowJobRun {
5+
const VALID_JOB_STATES = new Set([
6+
"triggered",
7+
"pending",
8+
"success",
9+
"failure",
10+
"error",
11+
"aborted",
12+
]);
13+
14+
const VALID_JOB_TYPES = new Set([
15+
"presubmit",
16+
"postsubmit",
17+
"batch",
18+
"periodic",
19+
]);
20+
21+
export interface prowJobRun {
622
kind: string;
723
metadata: {
8-
name: string;
9-
namespace: string;
10-
labels: Record<string, string>;
24+
name?: string;
25+
namespace?: string;
26+
labels?: Record<string, string>;
1127
};
1228
spec: {
13-
type: string;
14-
agent: string;
15-
cluster: string;
16-
namespace: string;
17-
job: string;
18-
report: boolean;
29+
type?: string;
30+
agent?: string;
31+
cluster?: string;
32+
namespace?: string;
33+
job?: string;
34+
report?: boolean;
1935
refs?: {
2036
org: string;
2137
repo: string;
@@ -33,11 +49,11 @@ interface prowJobRun {
3349
};
3450
};
3551
status: {
36-
state: string;
37-
startTime: string;
38-
pendingTime: string;
39-
completionTime: string;
40-
url: string;
52+
state?: string | null;
53+
startTime?: string | null;
54+
pendingTime?: string | null;
55+
completionTime?: string | null;
56+
url?: string | null;
4157
};
4258
}
4359

@@ -50,10 +66,63 @@ export async function fetchProwJobs(prowBaseUrl: string) {
5066
return data.items;
5167
}
5268

69+
function hasValidDate(value: string | null | undefined): value is string {
70+
if (!value) {
71+
return false;
72+
}
73+
return !Number.isNaN(new Date(value).getTime());
74+
}
75+
76+
export function invalidProwJobReason(job: prowJobRun): string | null {
77+
if (!job.metadata?.namespace) {
78+
return "missing metadata.namespace";
79+
}
80+
if (!job.metadata?.name) {
81+
return "missing metadata.name";
82+
}
83+
if (!job.spec?.job) {
84+
return "missing spec.job";
85+
}
86+
if (!job.spec?.type) {
87+
return "missing spec.type";
88+
}
89+
if (!VALID_JOB_TYPES.has(job.spec.type)) {
90+
return `invalid spec.type: ${job.spec.type}`;
91+
}
92+
if (!job.status?.state) {
93+
return "missing status.state";
94+
}
95+
if (!VALID_JOB_STATES.has(job.status.state)) {
96+
return `invalid status.state: ${job.status.state}`;
97+
}
98+
if (!hasValidDate(job.status.startTime)) {
99+
return "missing or invalid status.startTime";
100+
}
101+
return null;
102+
}
103+
104+
export function filterInsertableJobs(jobs: prowJobRun[]) {
105+
const insertableJobs: prowJobRun[] = [];
106+
const skippedJobs: { job: prowJobRun; reason: string }[] = [];
107+
108+
for (const job of jobs) {
109+
const reason = invalidProwJobReason(job);
110+
if (reason) {
111+
skippedJobs.push({ job, reason });
112+
continue;
113+
}
114+
insertableJobs.push(job);
115+
}
116+
117+
return { insertableJobs, skippedJobs };
118+
}
119+
53120
export async function createJobTable(client: mysql.Client, tableName: string) {
54121
// Validate table name to prevent SQL injection (only allow alphanumeric and underscores)
55122
if (!/^[a-zA-Z0-9_]+$/.test(tableName)) {
56-
throw new Error(`Invalid table name: ${tableName}. Only alphanumeric characters and underscores are allowed.`);
123+
throw new Error(
124+
`Invalid table name: ${tableName}. Only alphanumeric characters and underscores are allowed.`,
125+
);
57126
}
58127

59128
const sql = `
@@ -89,17 +158,22 @@ export async function createJobTable(client: mysql.Client, tableName: string) {
89158
export async function migrateJobTable(client: mysql.Client, tableName: string) {
90159
// Validate table name to prevent SQL injection (only allow alphanumeric and underscores)
91160
if (!/^[a-zA-Z0-9_]+$/.test(tableName)) {
92-
throw new Error(`Invalid table name: ${tableName}. Only alphanumeric characters and underscores are allowed.`);
161+
throw new Error(
162+
`Invalid table name: ${tableName}. Only alphanumeric characters and underscores are allowed.`,
163+
);
93164
}
94165

95166
// Check if table exists
96167
const tableExistsResult = await client.query(
97168
`SELECT COUNT(*) as count FROM information_schema.tables
98169
WHERE table_schema = DATABASE() AND table_name = ?`,
99-
[tableName]
170+
[tableName],
100171
);
101172

102-
if (!tableExistsResult || tableExistsResult.length === 0 || tableExistsResult[0].count === 0) {
173+
if (
174+
!tableExistsResult || tableExistsResult.length === 0 ||
175+
tableExistsResult[0].count === 0
176+
) {
103177
console.info(`Table ${tableName} does not exist, skipping migration`);
104178
return;
105179
}
@@ -108,41 +182,43 @@ export async function migrateJobTable(client: mysql.Client, tableName: string) {
108182
const columnsResult = await client.query(
109183
`SELECT COLUMN_NAME FROM information_schema.columns
110184
WHERE table_schema = DATABASE() AND table_name = ?`,
111-
[tableName]
185+
[tableName],
112186
);
113187

114188
const existingColumns = new Set(
115-
columnsResult.map((row: any) => row.COLUMN_NAME)
189+
columnsResult.map((row: { COLUMN_NAME: string }) => row.COLUMN_NAME),
116190
);
117191

118192
// Add retest column if it doesn't exist
119193
if (!existingColumns.has("retest")) {
120194
console.info(`Adding column 'retest' to table ${tableName}`);
121195
await client.execute(
122-
`ALTER TABLE \`${tableName}\` ADD COLUMN retest BOOLEAN DEFAULT NULL AFTER url`
196+
`ALTER TABLE \`${tableName}\` ADD COLUMN retest BOOLEAN DEFAULT NULL AFTER url`,
123197
);
124198
}
125199

126200
// Add author column if it doesn't exist
127201
if (!existingColumns.has("author")) {
128202
console.info(`Adding column 'author' to table ${tableName}`);
129203
await client.execute(
130-
`ALTER TABLE \`${tableName}\` ADD COLUMN author VARCHAR(128) AFTER retest`
204+
`ALTER TABLE \`${tableName}\` ADD COLUMN author VARCHAR(128) AFTER retest`,
131205
);
132206
}
133207

134208
// Add event_guid column if it doesn't exist
135209
if (!existingColumns.has("event_guid")) {
136210
console.info(`Adding column 'event_guid' to table ${tableName}`);
137211
await client.execute(
138-
`ALTER TABLE \`${tableName}\` ADD COLUMN event_guid VARCHAR(128) AFTER author`
212+
`ALTER TABLE \`${tableName}\` ADD COLUMN event_guid VARCHAR(128) AFTER author`,
139213
);
140214
}
141215

142216
console.info(`Migration completed for table ${tableName}`);
143217
}
144218

145219
function jobInsertValues(job: prowJobRun) {
220+
const labels = job.metadata.labels ?? {};
221+
146222
// Helper to parse the retest label into a nullable boolean
147223
const parseRetestLabel = (label: string | undefined): boolean | null => {
148224
if (label === "true") return true;
@@ -160,24 +236,24 @@ function jobInsertValues(job: prowJobRun) {
160236
};
161237

162238
return [
163-
job.metadata.namespace,
164-
job.metadata.name,
165-
job.spec.job,
166-
job.spec.type,
167-
job.status.state,
168-
new Date(job.status.startTime),
239+
job.metadata.namespace!,
240+
job.metadata.name!,
241+
job.spec.job!,
242+
job.spec.type!,
243+
job.status.state!,
244+
new Date(job.status.startTime!),
169245
job.status.completionTime ? new Date(job.status.completionTime) : null,
170-
job.metadata.labels["prow.k8s.io/is-optional"] === "true",
246+
labels["prow.k8s.io/is-optional"] === "true",
171247
job.spec.report || false,
172248
job.spec.refs?.org || null,
173249
job.spec.refs?.repo || null,
174250
job.spec.refs?.base_ref || null,
175-
job.metadata.labels["prow.k8s.io/refs.pull"] || null,
176-
job.metadata.labels["prow.k8s.io/context"] || null,
251+
labels["prow.k8s.io/refs.pull"] || null,
252+
labels["prow.k8s.io/context"] || null,
177253
job.status.url || null,
178-
parseRetestLabel(job.metadata.labels["prow.k8s.io/retest"]),
254+
parseRetestLabel(labels["prow.k8s.io/retest"]),
179255
getAuthor(),
180-
job.metadata.labels["event-GUID"] || null,
256+
labels["event-GUID"] || null,
181257
JSON.stringify(job.spec),
182258
JSON.stringify(job.status),
183259
];
@@ -189,8 +265,21 @@ export async function saveJobs(
189265
jobs: prowJobRun[],
190266
chunkSize = 100, // Default chunk size
191267
) {
192-
for (let i = 0; i < jobs.length; i += chunkSize) {
193-
const chunk = jobs.slice(i, i + chunkSize);
268+
const { insertableJobs, skippedJobs } = filterInsertableJobs(jobs);
269+
270+
if (skippedJobs.length > 0) {
271+
console.warn(
272+
`Skipping ${skippedJobs.length}/${jobs.length} prow jobs with incomplete data`,
273+
);
274+
for (const { job, reason } of skippedJobs.slice(0, 5)) {
275+
console.warn(
276+
`Skipped prow job ${job.metadata?.name ?? "<unknown>"}: ${reason}`,
277+
);
278+
}
279+
}
280+
281+
for (let i = 0; i < insertableJobs.length; i += chunkSize) {
282+
const chunk = insertableJobs.slice(i, i + chunkSize);
194283
const values = chunk.map((job) => jobInsertValues(job));
195284
const placeholders = values.map((value) =>
196285
`(${value.map(() => "?").join(", ")})`
@@ -212,7 +301,11 @@ export async function saveJobs(
212301
`;
213302
const flattenedValues = values.flat();
214303
await client.execute(sql, flattenedValues);
215-
console.info(`Saved ${i}/${jobs.length} jobs`);
304+
console.info(
305+
`Saved ${
306+
Math.min(i + chunkSize, insertableJobs.length)
307+
}/${insertableJobs.length} jobs`,
308+
);
216309
}
217310
}
218311

0 commit comments

Comments
 (0)