Chore: Add Pipeline Obs averageRunTime #24979
Conversation
github-actions
bot
added
Ingestion
safe to test
|
TypeScript types have been updated based on the JSON schema changes in the PR |
|
TypeScript types have been updated based on the JSON schema changes in the PR |
Jest test CoverageUI tests summary
|
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| "Failed to find pipeline status for %s at %s", pipeline.getName(), timestamp)); | ||
| } | ||
|
|
||
| private Long calculateActualDuration(PipelineStatus pipelineStatus) { | ||
| if (pipelineStatus.getTaskStatus() == null || pipelineStatus.getTaskStatus().isEmpty()) { | ||
| return null; | ||
| } | ||
|
|
||
| Long minStartTime = | ||
| pipelineStatus.getTaskStatus().stream() | ||
| .map(task -> task.getStartTime()) | ||
| .filter(java.util.Objects::nonNull) | ||
| .min(Long::compare) | ||
| .orElse(null); | ||
|
|
||
| Long maxEndTime = | ||
| pipelineStatus.getTaskStatus().stream() | ||
| .map(task -> task.getEndTime()) | ||
| .filter(java.util.Objects::nonNull) | ||
| .max(Long::compare) | ||
| .orElse(null); | ||
|
|
There was a problem hiding this comment.
Details
The runtime calculation logic (finding min startTime and max endTime from task statuses) is duplicated in three places:
calculateActualDuration()(lines 425-446)calculateAverageRuntime()(lines 679-748)- The SQL CTEs in
CollectionDAO.java
The Java implementations have slightly different behaviors:
calculateActualDuration()filters startTime and endTime independentlycalculateAverageRuntime()requires both startTime AND endTime to be non-null for a task
This inconsistency could lead to different results for the same pipeline data. Consider extracting a shared utility method to ensure consistent calculation across all usages.
Suggested fix: Create a single utility method like calculateRuntimeFromTasks(List<Status> taskStatus) that both methods can use.
| pipelineStatus.getTaskStatus().stream() | ||
| .map(task -> task.getEndTime()) | ||
| .filter(java.util.Objects::nonNull) | ||
| .max(Long::compare) | ||
| .orElse(null); | ||
|
|
There was a problem hiding this comment.
💡 Edge Case: calculateActualDuration doesn't fallback to pipeline-level timing
Details
The calculateActualDuration() method returns null when task-level timing data is unavailable, but calculateAverageRuntime() falls back to pipeline-level timing (status.getEndTime() - status.getTimestamp()).
This inconsistency means:
- Duration filtering will exclude pipeline statuses that only have pipeline-level timing
- Average runtime calculation will include those same statuses
For consistency, consider adding the same fallback to calculateActualDuration():
if (minStartTime != null && maxEndTime != null && maxEndTime >= minStartTime) {
return maxEndTime - minStartTime;
}
// Fallback to pipeline-level timing
if (pipelineStatus.getEndTime() != null && pipelineStatus.getTimestamp() != null) {
return pipelineStatus.getEndTime() - pipelineStatus.getTimestamp();
}
return null;
| "Failed to find pipeline status for %s at %s", pipeline.getName(), timestamp)); | ||
| } | ||
|
|
||
| private Long calculateActualDuration(PipelineStatus pipelineStatus) { | ||
| if (pipelineStatus.getTaskStatus() == null || pipelineStatus.getTaskStatus().isEmpty()) { | ||
| return null; | ||
| } | ||
|
|
||
| Long minStartTime = | ||
| pipelineStatus.getTaskStatus().stream() | ||
| .map(task -> task.getStartTime()) | ||
| .filter(java.util.Objects::nonNull) | ||
| .min(Long::compare) | ||
| .orElse(null); | ||
|
|
||
| Long maxEndTime = | ||
| pipelineStatus.getTaskStatus().stream() | ||
| .map(task -> task.getEndTime()) | ||
| .filter(java.util.Objects::nonNull) | ||
| .max(Long::compare) | ||
| .orElse(null); | ||
|
|
There was a problem hiding this comment.
💡 Code Quality: Duplicated runtime calculation logic
Details
calculateActualDuration() and calculateAverageRuntime() both implement the same core logic: finding min(startTime) and max(endTime) from taskStatus. The implementations differ (stream-based vs imperative loop) which makes it harder to maintain consistency.
Consider extracting the common logic into a single shared method:
private Long calculateRuntimeFromTasks(List<Status> taskStatus) {
if (taskStatus == null || taskStatus.isEmpty()) return null;
Long minStart = taskStatus.stream()
.map(Status::getStartTime)
.filter(Objects::nonNull)
.min(Long::compare)
.orElse(null);
Long maxEnd = taskStatus.stream()
.map(Status::getEndTime)
.filter(Objects::nonNull)
.max(Long::compare)
.orElse(null);
return (minStart != null && maxEnd != null && maxEnd >= minStart)
? maxEnd - minStart : null;
}Both methods can then use this shared helper, reducing duplication and ensuring consistent behavior.
🔍 CI failure analysis for 096965e: Integration tests fail due to double /api prefix in PR code (fix by removing /api). Maven PostgreSQL and SonarCloud CI timed out after 3-4.5 hours. Playwright jobs failed due to infrastructure issues. Retry non-test failures.Integration Test Failures (Related to PR)Two CI jobs failed with identical test errors in
All three tests fail in both jobs:
Root CauseThe test code constructs API paths that include the SolutionThe fix should remove the Infrastructure Failures (Unrelated to PR)Maven Build Timeouts
Both jobs show the "Build with Maven" step stuck in Playwright CI Failures
Solution for Infrastructure FailuresAll infrastructure failures (Maven timeouts, Playwright network/workflow issues) should be resolved by retrying the failed jobs. Code Review
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | This comment will update automatically (Docs)
|
|
Describe your changes:
Chore Add Pipeline Obs averageRunTime
In
pipelines/name/fqnAPI we only send the latesttaskStatusType of change:
Checklist:
Fixes <issue-number>: <short explanation>Summary by Gitar
averageRunTimefield topipelineObservability.jsonfor tracking average pipeline execution duration in millisecondscalculateAverageRuntimeinPipelineRepository.javacomputes 30-day average using task-level timing with pipeline-level fallbackThis will update automatically on new commits.