feat(observability): scheduled-task Runs page + retries scoped to run lifetime#692
feat(observability): scheduled-task Runs page + retries scoped to run lifetime#692ThusharaSampath wants to merge 9 commits into
Conversation
|
Changeset detected — the following file(s) will be released with this PR: |
📝 WalkthroughWalkthroughAdds a permission-gated Runs tab for scheduled-task entities, with API clients and hooks for runs, retries, and pod logs. The UI supports URL-synchronized filters, pagination, expandable retry details, events, and logs. ChangesScheduled task observability
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ObservabilityRunsPage
participant useRuns
participant ObservabilityClient
participant Observer
participant RunRow
participant RetryRow
participant usePodLogs
User->>ObservabilityRunsPage: select filters or expand a run
ObservabilityRunsPage->>useRuns: fetch runs
useRuns->>ObservabilityClient: getRuns
ObservabilityClient->>Observer: POST scheduled-runs query
Observer-->>ObservabilityClient: return runs
ObservabilityClient-->>useRuns: resolve runs
useRuns-->>ObservabilityRunsPage: render RunsTable
RunRow->>ObservabilityClient: getRetries for run lifetime
ObservabilityClient->>Observer: POST retries query
Observer-->>RunRow: return retries
RunRow-->>RetryRow: render retry details
RetryRow->>usePodLogs: fetch expanded pod logs
usePodLogs->>ObservabilityClient: getPodLogs
ObservabilityClient->>Observer: POST pod-log query
Observer-->>RetryRow: return logs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
Pass the run's `startTime` and `completionTime` from `RunRow` through `useRetries` into `ObservabilityApi.getRetries`, and forward them as `startTime`/`endTime` on the backend retries query so the events fetch is scoped to a single run's window instead of the backend's 30-day default lookback. Motivation: the observer's kube-events-collector adapter caps a single retries call at 1000 events. On high-frequency CronJobs, a 30-day default window silently truncates data. Scoping to the run's own lifetime keeps the retries view accurate without paginating. Contract (from openchoreo/openchoreo#3933): - Both `startTime` and `endTime` provided -> that window is used. - Exactly one provided -> backend responds 400 (both-or-none). - Both omitted -> backend defaults to a 30-day lookback (unchanged behavior for callers that do not pass the pair). `useRetries` and `getRetries` only forward the pair when both values are truthy, so a still-running run (no `completionTime` yet) transparently falls back to the backend default rather than triggering a 400. Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
Catch up the Runs page (originally landed as "Triggers") with upstream refactors that moved / renamed helpers the page had been importing from locations that no longer exist. The branch's task-obs commits pre-dated these upstream moves, so `tsc` failed on stale imports. No behavioral change beyond adopting the new empty/forbidden/unavailable environment status pattern already used by Metrics, Alerts, etc. - ObservabilityRunsPage.tsx: swap `useGetEnvironmentsByNamespace` (gone) for `useProjectEnvironments` from `@openchoreo/backstage-plugin-react`, and render `EnvironmentsStatusNotice` for the non-`ok` statuses (`empty-pipeline` / `forbidden` / `unavailable`) instead of the old generic "No environments found" alert. - Runs/types.ts: inline the local `Environment` shape (id/name/resourceName) the Runs filter and URL sync depend on. Previously re-exported from `RuntimeLogs/types`, which no longer defines it; the upstream `Environment` has a different shape (name/displayName/namespace/dataPlaneRef) and is mapped onto this local one by `ObservabilityRunsPage`. - hooks/useRuns.ts: import `calculateTimeRange` from `@openchoreo/backstage-plugin-react` (its new home); the old `components/RuntimeLogs/utils` module was removed upstream. - hooks/useUrlFiltersForRuns.ts: point the `Environment` type import at the local `Runs/types` shape (`id`-based) it actually uses, not the removed `RuntimeLogs/types` re-export. Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
- Ran prettier over the 6 files the Lint job flagged (ObservabilityApi.ts, RetryRow.tsx, RunRow.tsx, RunsActions.tsx, useRetries.ts, useRuns.ts). No behavioural change. - Added .changeset/scheduled-task-runs-page.md so the Check-for-changeset job passes. Marks the openchoreo-observability plugin for a minor bump and references the backend PR openchoreo/openchoreo#3933. Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
4678ddd to
69b00ad
Compare
Covers the Runs-page API surface + supporting hooks that were pushing patch coverage under threshold on the retries-time-window PR: - src/api/ObservabilityApi.test.ts: add getRuns, getRetries, getPodLogs suites. getRetries includes explicit both/one-only/neither cases for the client-side both-or-none time-bounds guard, plus jobName encoding and response-shape defaults. - src/hooks/useRuns.test.ts: initial state, fetch success, error paths, refresh flow, and the stale-request guard on requestVersionRef. - src/hooks/useRetries.test.ts: skips call when jobName/namespace empty, bounds forwarding under both/one/neither, error paths, stale guard. - src/hooks/usePodLogs.test.ts: skips call when podName/component empty, option forwarding, error paths, stale guard. - src/hooks/useUrlFiltersForRuns.test.tsx: default filters, URL parsing, auto-select-first-env effect, updateFilters, resetFilters. Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
…PI tests - Prettier'd plugins/openchoreo-observability/src/api/ObservabilityApi.test.ts (Lint CI flagged it after the previous test-coverage push). - New RunsActions.test.tsx covers the pagination + refresh + disabled behaviour of the Runs page actions bar. Focused on real behaviour that breaks silently: Prev disabled at page 0, Next disabled on last page, Next disabled when totalCount is 0, endItem capped on the partial last page, "showing X-Y" hidden when totalCount is 0, disabled prop overrides pagination state. Together these bring patch coverage above the 52% threshold codecov flagged. Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugins/openchoreo-observability/src/components/Runs/ObservabilityRunsPage.tsx`:
- Around line 201-223: Update ObservabilityRunsPage to select the environment
before invoking useLogsPermission, passing selectedEnvironment.resourceName for
the environment-scoped check. Move the permission/loading gate below environment
selection, and prevent runs and pod-log data fetching until the permission check
completes and canViewLogs is true.
- Around line 82-105: The filter-change path in the ObservabilityRunsPage
useEffect must not retain runs from the previous environment or filter context.
Update the useRuns implementation used by fetchRuns so its _reset path clears
runs and totalCount before loading the new context, ensuring the rendering logic
around the runs list cannot display stale jobs after a filter change or failed
request.
In `@plugins/openchoreo-observability/src/components/Runs/RetryRow.tsx`:
- Around line 72-105: Update the log-fetch flow in RetryRow around usePodLogs
and the expanded useEffect so an active run without runCompletionTime computes
logsEndTime from Date.now() immediately before every fetchLogs call, rather than
reusing the render-time value. Preserve the fixed completion-time end bound for
completed runs and continue using the existing start-time window.
In `@plugins/openchoreo-observability/src/components/Runs/RunsActions.tsx`:
- Around line 39-43: Normalize the current page after a successful runs response
when it exceeds the calculated totalPages. Update filters.page to the last valid
page (totalPages minus one) and refetch using that page, preventing invalid
ranges such as “showing 1981-25”; preserve existing pagination behavior for
valid pages and add a test covering an out-of-range bookmarked page.
In `@plugins/openchoreo-observability/src/hooks/usePodLogs.ts`:
- Around line 30-41: Invalidate requests before incomplete-scope returns: in
plugins/openchoreo-observability/src/hooks/usePodLogs.ts lines 30-41, validate
projectName, increment requestVersionRef, and clear logs, loading, and error
state; in plugins/openchoreo-observability/src/hooks/useRetries.ts lines 40-50,
apply the same projectName validation, version increment, and
retry/loading/error cleanup; in
plugins/openchoreo-observability/src/hooks/useRuns.ts lines 43-55, increment the
request version and clear runs, total, loading, and error state before
returning.
In `@plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuns.ts`:
- Around line 59-104: Update updateFilters so a supplied page is ignored
whenever environmentId, timeRange, or sortOrder is present in the same update,
preserving the reset-to-first-page behavior for combined filter and page
changes. Keep standalone page updates unchanged, and add a test covering each
combined-update case to verify the stale page is not restored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ea6380e-7b34-4fed-8bd7-15d66cc0b5a6
📒 Files selected for processing (25)
.changeset/scheduled-task-runs-page.mdpackages/app/src/components/catalog/EntityPage.tsxplugins/openchoreo-observability/src/api/ObservabilityApi.test.tsplugins/openchoreo-observability/src/api/ObservabilityApi.tsplugins/openchoreo-observability/src/components/Runs/ObservabilityRunsPage.tsxplugins/openchoreo-observability/src/components/Runs/RetryRow.tsxplugins/openchoreo-observability/src/components/Runs/RunRow.tsxplugins/openchoreo-observability/src/components/Runs/RunsActions.test.tsxplugins/openchoreo-observability/src/components/Runs/RunsActions.tsxplugins/openchoreo-observability/src/components/Runs/RunsFilter.tsxplugins/openchoreo-observability/src/components/Runs/RunsTable.tsxplugins/openchoreo-observability/src/components/Runs/index.tsplugins/openchoreo-observability/src/components/Runs/styles.tsplugins/openchoreo-observability/src/components/Runs/types.tsplugins/openchoreo-observability/src/hooks/index.tsplugins/openchoreo-observability/src/hooks/usePodLogs.test.tsplugins/openchoreo-observability/src/hooks/usePodLogs.tsplugins/openchoreo-observability/src/hooks/useRetries.test.tsplugins/openchoreo-observability/src/hooks/useRetries.tsplugins/openchoreo-observability/src/hooks/useRuns.test.tsplugins/openchoreo-observability/src/hooks/useRuns.tsplugins/openchoreo-observability/src/hooks/useUrlFiltersForRuns.test.tsxplugins/openchoreo-observability/src/hooks/useUrlFiltersForRuns.tsplugins/openchoreo-observability/src/index.tsplugins/openchoreo-observability/src/plugin.ts
| useEffect(() => { | ||
| const currentFilters = { | ||
| environmentId: filters.environmentId, | ||
| timeRange: filters.timeRange, | ||
| sortOrder: filters.sortOrder, | ||
| page: filters.page, | ||
| }; | ||
| const filtersChanged = | ||
| previousFiltersRef.current === null || | ||
| JSON.stringify(previousFiltersRef.current) !== | ||
| JSON.stringify(currentFilters); | ||
|
|
||
| if ( | ||
| filters.environmentId && | ||
| selectedEnvironment && | ||
| namespace && | ||
| project && | ||
| componentName && | ||
| filtersChanged | ||
| ) { | ||
| fetchRuns(true); | ||
| setLastUpdated(new Date()); | ||
| previousFiltersRef.current = currentFilters; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Clear runs when their filter context changes.
At Line 102, fetchRuns(true) does not reset anything: the supplied useRuns implementation ignores _reset and retains runs and totalCount. Lines 187-194 therefore render old jobs with the newly selected environment identifiers; expanding one can query an old jobName against the new environment, and a failed request leaves that mismatch visible indefinitely.
Honor _reset in useRuns, or only render data tagged with the active filter context.
Also applies to: 176-194
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/openchoreo-observability/src/components/Runs/ObservabilityRunsPage.tsx`
around lines 82 - 105, The filter-change path in the ObservabilityRunsPage
useEffect must not retain runs from the previous environment or filter context.
Update the useRuns implementation used by fetchRuns so its _reset path clears
runs and totalCount before loading the new context, ensuring the rendering logic
around the runs list cannot display stale jobs after a filter change or failed
request.
| export const ObservabilityRunsPage = () => { | ||
| const { | ||
| canViewLogs, | ||
| loading: permissionLoading, | ||
| deniedTooltip, | ||
| } = useLogsPermission(); | ||
|
|
||
| if (permissionLoading) return <Progress />; | ||
|
|
||
| if (!canViewLogs) { | ||
| return ( | ||
| <EmptyState | ||
| missing="data" | ||
| title="Permission Denied" | ||
| description={ | ||
| <Box display="flex" alignItems="center" gridGap={8}> | ||
| <WarningIcon /> | ||
| {deniedTooltip} | ||
| </Box> | ||
| } | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Apply the logs permission check to the selected environment.
Line 206 calls useLogsPermission() without an environment, so its environment-scoped ABAC evaluation is skipped while runs and pod logs are subsequently requested for a selected environment. Move the permission gate below environment selection, pass selectedEnvironment.resourceName, and suppress data fetching until that check allows access.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/openchoreo-observability/src/components/Runs/ObservabilityRunsPage.tsx`
around lines 201 - 223, Update ObservabilityRunsPage to select the environment
before invoking useLogsPermission, passing selectedEnvironment.resourceName for
the environment-scoped check. Move the permission/loading gate below environment
selection, and prevent runs and pod-log data fetching until the permission check
completes and canViewLogs is true.
| const { logsStartTime, logsEndTime } = useMemo(() => { | ||
| const baseStart = retry.startTime || runStartTime; | ||
| const startMs = baseStart | ||
| ? new Date(baseStart).getTime() - 60 * 1000 | ||
| : Date.now() - 24 * 3600 * 1000; | ||
| const endMs = runCompletionTime | ||
| ? new Date(runCompletionTime).getTime() + 5 * 60 * 1000 | ||
| : Date.now(); | ||
| return { | ||
| logsStartTime: new Date(startMs).toISOString(), | ||
| logsEndTime: new Date(endMs).toISOString(), | ||
| }; | ||
| }, [retry.startTime, runStartTime, runCompletionTime]); | ||
|
|
||
| const { | ||
| logs, | ||
| loading: logsLoading, | ||
| error: logsError, | ||
| fetchLogs, | ||
| } = usePodLogs({ | ||
| podName: expanded ? retry.podName : '', | ||
| namespaceName, | ||
| projectName, | ||
| environmentName, | ||
| componentName, | ||
| startTime: logsStartTime, | ||
| endTime: logsEndTime, | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| if (expanded) { | ||
| fetchLogs(); | ||
| } | ||
| }, [expanded, fetchLogs]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Recompute the active-run log window for every fetch.
When runCompletionTime is absent, Line 79 captures Date.now() when the row renders. Expanding or refreshing later reuses that frozen logsEndTime, so logs produced afterward are never requested. Build the live end time immediately before each fetchLogs call while retaining the fixed completion bound for completed runs.
Also applies to: 171-180
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/openchoreo-observability/src/components/Runs/RetryRow.tsx` around
lines 72 - 105, Update the log-fetch flow in RetryRow around usePodLogs and the
expanded useEffect so an active run without runCompletionTime computes
logsEndTime from Date.now() immediately before every fetchLogs call, rather than
reusing the render-time value. Preserve the fixed completion-time end bound for
completed runs and continue using the existing start-time window.
| const page = filters.page; | ||
| const pageSize = RUNS_PAGE_SIZE; | ||
| const totalPages = Math.max(1, Math.ceil(totalCount / pageSize)); | ||
| const startItem = totalCount === 0 ? 0 : page * pageSize + 1; | ||
| const endItem = Math.min((page + 1) * pageSize, totalCount); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Normalize pages that exceed the current result count.
A bookmarked page or shrinking totalCount can produce Page 100 / 2 and showing 1981-25, while requiring repeated Prev clicks to recover. After a successful response, reset filters.page to the last valid page and refetch; add an out-of-range test.
Also applies to: 79-91
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/openchoreo-observability/src/components/Runs/RunsActions.tsx` around
lines 39 - 43, Normalize the current page after a successful runs response when
it exceeds the calculated totalPages. Update filters.page to the last valid page
(totalPages minus one) and refetch using that page, preventing invalid ranges
such as “showing 1981-25”; preserve existing pagination behavior for valid pages
and add a test covering an out-of-range bookmarked page.
| const fetchLogs = useCallback(async () => { | ||
| if ( | ||
| !options.podName || | ||
| !options.namespaceName || | ||
| !options.environmentName || | ||
| !options.componentName | ||
| ) { | ||
| setLogs([]); | ||
| return; | ||
| } | ||
|
|
||
| const version = ++requestVersionRef.current; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Invalidate pending requests before returning for incomplete query scope.
Each hook returns before advancing its request version, so an earlier response can commit data belonging to the previous scope.
plugins/openchoreo-observability/src/hooks/usePodLogs.ts#L30-L41: validateprojectName, increment the version, and clear log/loading/error state.plugins/openchoreo-observability/src/hooks/useRetries.ts#L40-L50: validateprojectName, increment the version, and clear retry/loading/error state.plugins/openchoreo-observability/src/hooks/useRuns.ts#L43-L55: increment the version and clear runs, total, loading, and error state.
📍 Affects 3 files
plugins/openchoreo-observability/src/hooks/usePodLogs.ts#L30-L41(this comment)plugins/openchoreo-observability/src/hooks/useRetries.ts#L40-L50plugins/openchoreo-observability/src/hooks/useRuns.ts#L43-L55
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/openchoreo-observability/src/hooks/usePodLogs.ts` around lines 30 -
41, Invalidate requests before incomplete-scope returns: in
plugins/openchoreo-observability/src/hooks/usePodLogs.ts lines 30-41, validate
projectName, increment requestVersionRef, and clear logs, loading, and error
state; in plugins/openchoreo-observability/src/hooks/useRetries.ts lines 40-50,
apply the same projectName validation, version increment, and
retry/loading/error cleanup; in
plugins/openchoreo-observability/src/hooks/useRuns.ts lines 43-55, increment the
request version and clear runs, total, loading, and error state before
returning.
| const updateFilters = useCallback( | ||
| (newFilters: Partial<RunsFilters>) => { | ||
| const newParams = new URLSearchParams(searchParams); | ||
|
|
||
| if (newFilters.environmentId !== undefined) { | ||
| if (newFilters.environmentId) { | ||
| newParams.set('env', newFilters.environmentId); | ||
| } else { | ||
| newParams.delete('env'); | ||
| } | ||
| // Reset page when environment changes | ||
| newParams.delete('page'); | ||
| } | ||
|
|
||
| if (newFilters.timeRange !== undefined) { | ||
| if (newFilters.timeRange === DEFAULT_TIME_RANGE) { | ||
| newParams.delete('timeRange'); | ||
| } else { | ||
| newParams.set('timeRange', newFilters.timeRange); | ||
| } | ||
| // Reset page when time range changes | ||
| newParams.delete('page'); | ||
| } | ||
|
|
||
| if (newFilters.sortOrder !== undefined) { | ||
| if (newFilters.sortOrder === 'desc') { | ||
| newParams.delete('sort'); | ||
| } else { | ||
| newParams.set('sort', newFilters.sortOrder); | ||
| } | ||
| // Reset page when sort changes | ||
| newParams.delete('page'); | ||
| } | ||
|
|
||
| if (newFilters.page !== undefined) { | ||
| if (newFilters.page === 0) { | ||
| newParams.delete('page'); | ||
| } else { | ||
| newParams.set('page', String(newFilters.page)); | ||
| } | ||
| } | ||
|
|
||
| setSearchParams(newParams, { replace: true }); | ||
| }, | ||
| [searchParams, setSearchParams], | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prevent combined filter updates from restoring a stale page.
For { environmentId, page }, { timeRange, page }, or { sortOrder, page }, the reset deletes page, then Lines 93–98 add it back. Ignore the supplied page whenever a page-resetting filter is present, and add a combined-update test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuns.ts` around
lines 59 - 104, Update updateFilters so a supplied page is ignored whenever
environmentId, timeRange, or sortOrder is present in the same update, preserving
the reset-to-first-page behavior for combined filter and page changes. Keep
standalone page updates unchanged, and add a test covering each combined-update
case to verify the stale page is not restored.
Purpose
Adds the Runs tab for scheduled-task components to the OpenChoreo observability plugin, and scopes the per-run retries fetch to the run's own lifetime.
The tab renders a hierarchical view — `Component → Runs (Jobs) → Retries (Pods) → per-retry logs` — over the two runs/retries endpoints introduced in the observer PR:
Approach
Related PRs / Issues
Checklist
Verification
Remarks
Summary by CodeRabbit
New Features
Bug Fixes
Tests