Skip to content

Commit 9cc09db

Browse files
authored
fix(x2a): show correct phase duration and attempt count (#3238)
Use telemetry timestamps for phase duration instead of aggregated job-level times that include failed pod restarts. Add attemptCount and firstAttemptAt enrichment fields to the Job schema. Display attempts and total elapsed time as separate fields in PhaseDetails. Signed-off-by: Marek Libra <marek.libra@gmail.com>
1 parent c901f52 commit 9cc09db

21 files changed

Lines changed: 646 additions & 12 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-x2a-backend': patch
3+
'@red-hat-developer-hub/backstage-plugin-x2a-common': patch
4+
'@red-hat-developer-hub/backstage-plugin-x2a': patch
5+
---
6+
7+
Show attempt counter for repeated executions of phases.

workspaces/x2a/plugins/x2a-backend/src/schema/openapi.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,13 @@ components:
905905
commitId:
906906
type: string
907907
description: Git commit SHA produced by this job
908+
attemptCount:
909+
type: integer
910+
description: Number of restarts (attempts) for this projectId+moduleId+phase combination. Calculated value, not persisted in DB.
911+
firstAttemptAt:
912+
type: string
913+
format: date-time
914+
description: When the very first attempt for this phase was started.
908915

909916
ArtifactType:
910917
type: string

workspaces/x2a/plugins/x2a-backend/src/schema/openapi/generated/models/Job.model.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,12 @@ export interface Job {
6565
* Git commit SHA produced by this job
6666
*/
6767
commitId?: string;
68+
/**
69+
* Number of restarts (attempts) for this projectId+moduleId+phase combination. Calculated value, not persisted in DB.
70+
*/
71+
attemptCount?: number;
72+
/**
73+
* When the very first attempt for this phase was started.
74+
*/
75+
firstAttemptAt?: Date;
6876
}

workspaces/x2a/plugins/x2a-backend/src/schema/openapi/generated/router.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,6 +1298,15 @@ export const spec = {
12981298
"commitId": {
12991299
"type": "string",
13001300
"description": "Git commit SHA produced by this job"
1301+
},
1302+
"attemptCount": {
1303+
"type": "integer",
1304+
"description": "Number of restarts (attempts) for this projectId+moduleId+phase combination. Calculated value, not persisted in DB."
1305+
},
1306+
"firstAttemptAt": {
1307+
"type": "string",
1308+
"format": "date-time",
1309+
"description": "When the very first attempt for this phase was started."
13011310
}
13021311
}
13031312
},

workspaces/x2a/plugins/x2a-backend/src/services/X2ADatabaseService/index.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,18 @@ export class X2ADatabaseService implements X2ADatabaseServiceApi {
9494
this.#ruleOps = new RuleOperations(logger, dbClient);
9595
}
9696

97+
/**
98+
* Attaches attemptCount and firstAttemptAt to a Job object (mutates in place).
99+
*/
100+
private attachAttemptStats(
101+
job: Job | undefined,
102+
stats: { count: number; firstStartedAt: Date | undefined } | undefined,
103+
): void {
104+
if (!job || !stats) return;
105+
job.attemptCount = stats.count;
106+
job.firstAttemptAt = stats.firstStartedAt;
107+
}
108+
97109
/**
98110
* Enriches a project with migration plan and status (used by listProjects and getProject).
99111
*/
@@ -117,6 +129,14 @@ export class X2ADatabaseService implements X2ADatabaseServiceApi {
117129
);
118130

119131
project.initJob = removeSensitiveFromJob(lastInitJob);
132+
133+
if (project.initJob) {
134+
const stats = await this.#jobOps.getPhaseAttemptStats({
135+
projectId,
136+
phase: 'init',
137+
});
138+
this.attachAttemptStats(project.initJob, stats);
139+
}
120140
}
121141

122142
// Projects (facade enriches basic objects when needed)
@@ -348,6 +368,22 @@ export class X2ADatabaseService implements X2ADatabaseServiceApi {
348368
module.migrate = removeSensitiveFromJob(lastMigrateJobsOfModule[0]);
349369
module.publish = removeSensitiveFromJob(lastPublishJobsOfModule[0]);
350370

371+
// Attach attempt stats per phase
372+
const phases = ['analyze', 'migrate', 'publish'] as const;
373+
await Promise.all(
374+
phases.map(async phase => {
375+
const job = module[phase];
376+
if (job) {
377+
const stats = await this.#jobOps.getPhaseAttemptStats({
378+
projectId: module.projectId,
379+
moduleId: id,
380+
phase,
381+
});
382+
this.attachAttemptStats(job, stats);
383+
}
384+
}),
385+
);
386+
351387
return enrichModuleWithJobStatus(module, {
352388
analyze: module.analyze,
353389
migrate: module.migrate,
@@ -401,6 +437,10 @@ export class X2ADatabaseService implements X2ADatabaseServiceApi {
401437
),
402438
);
403439

440+
const attemptStatsMap = await this.#jobOps.batchPhaseAttemptStats({
441+
projectId,
442+
});
443+
404444
const response: Array<Module> = modules.map((module, idxModule) => {
405445
const analyze = removeSensitiveFromJob(
406446
lastAnalyzeJobsOfModules[idxModule][0],
@@ -411,6 +451,12 @@ export class X2ADatabaseService implements X2ADatabaseServiceApi {
411451
const publish = removeSensitiveFromJob(
412452
lastPublishJobsOfModules[idxModule][0],
413453
);
454+
455+
const moduleStats = attemptStatsMap.get(module.id);
456+
this.attachAttemptStats(analyze, moduleStats?.get('analyze'));
457+
this.attachAttemptStats(migrateJob, moduleStats?.get('migrate'));
458+
this.attachAttemptStats(publish, moduleStats?.get('publish'));
459+
414460
const lastJobs = { analyze, migrate: migrateJob, publish };
415461
return enrichModuleWithJobStatus(module, lastJobs);
416462
});

workspaces/x2a/plugins/x2a-backend/src/services/X2ADatabaseService/jobOperations.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,87 @@ export class JobOperations {
366366
return this.getJob({ id });
367367
}
368368

369+
/**
370+
* Returns attempt count and first-attempt timestamp for a single
371+
* (projectId, moduleId, phase) combination.
372+
*/
373+
async getPhaseAttemptStats({
374+
projectId,
375+
moduleId,
376+
phase,
377+
}: {
378+
projectId: string;
379+
moduleId?: string | null;
380+
phase: MigrationPhase;
381+
}): Promise<{ count: number; firstStartedAt: Date | undefined }> {
382+
const row = await this.#dbClient('jobs')
383+
.where('project_id', projectId)
384+
.modify(qb => {
385+
if (moduleId) {
386+
qb.where('module_id', moduleId);
387+
} else if (Phase.from(phase).isProjectPhase()) {
388+
qb.whereNull('module_id');
389+
}
390+
})
391+
.where('phase', phase)
392+
.select(
393+
this.#dbClient.raw('COUNT(*) as count'),
394+
this.#dbClient.raw('MIN(started_at) as first_started_at'),
395+
)
396+
.first();
397+
398+
const count = Number(row?.count ?? 0);
399+
const firstStartedAt = row?.first_started_at
400+
? new Date(row.first_started_at as string | Date)
401+
: undefined;
402+
403+
return { count, firstStartedAt };
404+
}
405+
406+
/**
407+
* Batch-fetches attempt stats for every (module_id, phase) pair in a
408+
* project. Returns a nested map: moduleId -> phase -> stats.
409+
* One SQL query regardless of the number of modules.
410+
*/
411+
async batchPhaseAttemptStats({
412+
projectId,
413+
}: {
414+
projectId: string;
415+
}): Promise<
416+
Map<
417+
string,
418+
Map<string, { count: number; firstStartedAt: Date | undefined }>
419+
>
420+
> {
421+
const rows: Array<Record<string, unknown>> = await this.#dbClient('jobs')
422+
.where('project_id', projectId)
423+
.whereNotNull('module_id')
424+
.groupBy('module_id', 'phase')
425+
.select(
426+
'module_id',
427+
'phase',
428+
this.#dbClient.raw('COUNT(*) as count'),
429+
this.#dbClient.raw('MIN(started_at) as first_started_at'),
430+
);
431+
432+
const result = new Map<
433+
string,
434+
Map<string, { count: number; firstStartedAt: Date | undefined }>
435+
>();
436+
for (const row of rows) {
437+
const modId = row.module_id as string;
438+
const phase = row.phase as string;
439+
if (!result.has(modId)) {
440+
result.set(modId, new Map());
441+
}
442+
result.get(modId)!.set(phase, {
443+
count: Number(row.count),
444+
firstStartedAt: new Date(row.first_started_at as string | Date),
445+
});
446+
}
447+
return result;
448+
}
449+
369450
async deleteJob({ id }: { id: string }): Promise<number> {
370451
this.#logger.info(`deleteJob called for id: ${id}`);
371452

workspaces/x2a/plugins/x2a-backend/src/services/X2ADatabaseService/jobs.test.ts

Lines changed: 148 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { mockCredentials } from '@backstage/backend-test-utils';
18-
1917
import {
2018
Artifact,
2119
toSorted,
2220
} from '@red-hat-developer-hub/backstage-plugin-x2a-common';
2321

22+
import { mockServices, mockCredentials } from '@backstage/backend-test-utils';
23+
2424
import {
2525
artifactsFromValues,
2626
createDatabase,
@@ -32,6 +32,7 @@ import {
3232
tearDownDatabases,
3333
} from '../../__testUtils__';
3434
import { delay } from '../../utils';
35+
import { JobOperations } from './jobOperations';
3536

3637
describe('X2ADatabaseService – jobs', () => {
3738
afterEach(async () => {
@@ -773,3 +774,148 @@ describe('X2ADatabaseService – jobs', () => {
773774
);
774775
});
775776
});
777+
778+
describe('JobOperations – phase attempt stats', () => {
779+
afterEach(async () => {
780+
await tearDownDatabases();
781+
});
782+
783+
describe('getPhaseAttemptStats', () => {
784+
it.each(supportedDatabaseIds)(
785+
'returns count and firstStartedAt for a single phase - %p',
786+
async databaseId => {
787+
const { client } = await createDatabase(databaseId);
788+
const service = createService(client);
789+
const jobOps = new JobOperations(mockServices.logger.mock(), client);
790+
const credentials = mockCredentials.user();
791+
const project = await service.createProject(
792+
{
793+
name: 'Test Project',
794+
description: 'D',
795+
...defaultProjectRepoFields,
796+
},
797+
{ credentials },
798+
);
799+
const module = await service.createModule({
800+
name: 'Mod',
801+
sourcePath: '/mod',
802+
projectId: project.id,
803+
});
804+
805+
const firstJob = await service.createJob({
806+
projectId: project.id,
807+
moduleId: module.id,
808+
phase: 'analyze',
809+
startedAt: new Date('2024-01-01T10:00:00Z'),
810+
});
811+
await service.createJob({
812+
projectId: project.id,
813+
moduleId: module.id,
814+
phase: 'analyze',
815+
startedAt: new Date('2024-01-01T11:00:00Z'),
816+
});
817+
await service.createJob({
818+
projectId: project.id,
819+
moduleId: module.id,
820+
phase: 'analyze',
821+
startedAt: new Date('2024-01-01T12:00:00Z'),
822+
});
823+
824+
const stats = await jobOps.getPhaseAttemptStats({
825+
projectId: project.id,
826+
moduleId: module.id,
827+
phase: 'analyze',
828+
});
829+
830+
expect(stats.count).toBe(3);
831+
expect(stats.firstStartedAt).toBeInstanceOf(Date);
832+
expect(stats.firstStartedAt!.toISOString()).toBe(
833+
firstJob.startedAt.toISOString(),
834+
);
835+
},
836+
LONG_TEST_TIMEOUT,
837+
);
838+
839+
it.each(supportedDatabaseIds)(
840+
'returns count 0 when no jobs exist - %p',
841+
async databaseId => {
842+
const { client } = await createDatabase(databaseId);
843+
const jobOps = new JobOperations(mockServices.logger.mock(), client);
844+
845+
const stats = await jobOps.getPhaseAttemptStats({
846+
projectId: nonExistentId,
847+
moduleId: nonExistentId,
848+
phase: 'analyze',
849+
});
850+
851+
expect(stats.count).toBe(0);
852+
expect(stats.firstStartedAt).toBeUndefined();
853+
},
854+
LONG_TEST_TIMEOUT,
855+
);
856+
});
857+
858+
describe('batchPhaseAttemptStats', () => {
859+
it.each(supportedDatabaseIds)(
860+
'returns stats grouped by module and phase - %p',
861+
async databaseId => {
862+
const { client } = await createDatabase(databaseId);
863+
const service = createService(client);
864+
const jobOps = new JobOperations(mockServices.logger.mock(), client);
865+
const credentials = mockCredentials.user();
866+
const project = await service.createProject(
867+
{
868+
name: 'Test Project',
869+
description: 'D',
870+
...defaultProjectRepoFields,
871+
},
872+
{ credentials },
873+
);
874+
const mod1 = await service.createModule({
875+
name: 'Mod1',
876+
sourcePath: '/mod1',
877+
projectId: project.id,
878+
});
879+
const mod2 = await service.createModule({
880+
name: 'Mod2',
881+
sourcePath: '/mod2',
882+
projectId: project.id,
883+
});
884+
885+
await service.createJob({
886+
projectId: project.id,
887+
moduleId: mod1.id,
888+
phase: 'analyze',
889+
});
890+
await delay(5);
891+
await service.createJob({
892+
projectId: project.id,
893+
moduleId: mod1.id,
894+
phase: 'analyze',
895+
});
896+
await delay(5);
897+
await service.createJob({
898+
projectId: project.id,
899+
moduleId: mod1.id,
900+
phase: 'migrate',
901+
});
902+
await delay(5);
903+
await service.createJob({
904+
projectId: project.id,
905+
moduleId: mod2.id,
906+
phase: 'analyze',
907+
});
908+
909+
const statsMap = await jobOps.batchPhaseAttemptStats({
910+
projectId: project.id,
911+
});
912+
913+
expect(statsMap.get(mod1.id)?.get('analyze')?.count).toBe(2);
914+
expect(statsMap.get(mod1.id)?.get('migrate')?.count).toBe(1);
915+
expect(statsMap.get(mod2.id)?.get('analyze')?.count).toBe(1);
916+
expect(statsMap.get(mod2.id)?.get('migrate')).toBeUndefined();
917+
},
918+
LONG_TEST_TIMEOUT,
919+
);
920+
});
921+
});

0 commit comments

Comments
 (0)