frontend: apiDiscovery: Log every fallback or skipped fetch#6033
frontend: apiDiscovery: Log every fallback or skipped fetch#6033WasThatRudy wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: WasThatRudy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR improves observability of Kubernetes API discovery by adding console.debug breadcrumbs whenever aggregated discovery is unusable or legacy discovery fetches are skipped/fail, so “missing resources” scenarios have diagnosable console output (Fixes #4840).
Changes:
- Add debug logging when aggregated discovery responses are rejected or malformed, and when the code falls back to legacy discovery.
- Add debug logging for legacy
/apiand/apisfetch rejections (previously swallowed). - Add regression tests that pin the new logging behavior for aggregated and legacy failure modes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| frontend/src/lib/k8s/api/v2/apiDiscovery.tsx | Adds console.debug logs covering aggregated-discovery unusable results and legacy discovery fetch failures before falling back/skipping. |
| frontend/src/lib/k8s/api/v2/apiDiscovery.test.ts | Adds regression tests asserting the new debug logs are emitted for key fallback/skipped-fetch scenarios. |
Apply two patterns the maintainer review flagged on PR kubernetes-sigs#6033: codebase uses American English uniformly (`serialize`, `colorize`, `optimize` — zero `-ise` instances), and the duck-typed legacy/new CRD-API-group lookup was duplicated between getBaseObject() and makeKubeObjectNode() with identical bodies. Extract a `resolveCRDApiGroup` helper alongside the other spec helpers in `crdSpec.ts` so both call sites share one implementation and a future maintainer touching the legacy-plugin contract only has to change it in one place. Signed-off-by: Rudraksha Singh Sengar <rudraksharss@gmail.com>
6fd7078 to
2634eb9
Compare
| function fetchLegacyOrLogFailure( | ||
| path: '/api' | '/apis', | ||
| cluster: string | ||
| ): Promise<{ versions?: unknown[]; groups?: unknown[] } | null> { | ||
| return clusterFetch(path, { cluster }) | ||
| .then(res => res.json()) | ||
| .catch(err => { | ||
| console.debug(`Legacy ${path} discovery fetch or parse failed for cluster ${cluster}:`, err); | ||
| return null; | ||
| }); | ||
| } |
2634eb9 to
dcdc510
Compare
Three failure paths in cluster API discovery silently swallowed their cause: the outer try/catch around aggregated discovery, the two fulfilled-but-malformed payload checks, and the .catch(() => null) handlers around the legacy /api and /apis fetches. Without those logs a "missing resources in the UI" report was effectively undebuggable; nothing told the user that aggregated discovery had quietly fallen back to the legacy path or that the legacy fetch itself had failed. Add a console.debug breadcrumb on every fallback or skipped fetch. Bodies that arrive intact but with the wrong shape go through a summarizer that logs the value's type plus a capped list of top-level keys, so a real K8s discovery response (which can carry thousands of entries) doesn't flood the devtools and bury the actual failure signal. The summarizer iterates keys lazily with for...in so the bound is honest at the computation layer, not just at the log-output layer, and returns a discriminated PayloadSummary so call sites and tests can pattern-match instead of duck-typing. Add regression tests covering each failure mode and the bounded-summary contract (truncation, null body, array body), pinning the full discriminated shape so a future debug log carrying a keys field can't accidentally satisfy the assertion. Bug report: 4840. Signed-off-by: Rudraksha Singh Sengar <rudraksharss@gmail.com>
dcdc510 to
dd7d019
Compare
illume
left a comment
There was a problem hiding this comment.
Thanks for working on this.
Would you mind addressing the open Copilot review comments? Please mark each comment as resolved after addressing it.
|
@illume all the copilot comments have been resolved and the latest review has not generated any new comment |
Summary
Fixes #4840. Three catch blocks in
frontend/src/lib/k8s/api/v2/apiDiscovery.tsxsilently swallowed their errors:try/catchsetuseFallback = truewith no log line — a thrown error during aggregated discovery left no trace, and the legacy path was taken without any breadcrumb..catch(() => null)callbacks on the legacy/apiand/apisfetches dropped their rejection reasons on the floor; if either rejected, every dependent resource fetch was skipped with nothing in the console to explain why.Both kinds of failure surface as "resources missing in the UI" with zero console output, which makes the underlying network or RBAC condition extremely hard to diagnose.
Changes
All logging is
console.debugto match the existing logs already present on the per-group fetch path (processLegacyApiResourceListalready had debug logs from earlier work).catch (error)useFallbackonlyAggregated API discovery threw for cluster <name>with the error!apiAggregatedOk || !apisAggregatedOk)useFallbackonly/apifetch.catch(() => null)Legacy /api discovery fetch failed for cluster <name>with the error, then null/apisfetch.catch(() => null)Legacy /apis discovery fetch failed for cluster <name>with the error, then nullTest plan
apiDiscovery > logs the cause of every fallback or skipped fetch:/apirejection → log + rejection reason captured{ unexpected: 'shape' }) → unusable-payload log/apirejection → log + rejection reason captured/apisrejection → log + rejection reason capturedmockFetchError(status 500 / not-ok) which is exactly the malformed-payload path the new log targets — this is visible but not a failure.npm run tscclean.npx eslint -c .eslintrc.ci.cjs --max-warnings 0clean.Notes for the reviewer
console.debugis intentional — these are diagnostic breadcrumbs, not user-facing errors, and they match the level already used by the per-group fetch logs in the same function. Anyone investigating "missing resources" can flip devtools to Verbose and see exactly which call dropped.