Skip to content

fix(issues): preserve honest onset provenance - #1390

Open
nadaverell wants to merge 6 commits into
mainfrom
fix/rad-346-onset-provenance
Open

fix(issues): preserve honest onset provenance#1390
nadaverell wants to merge 6 commits into
mainfrom
fix/rad-346-onset-provenance

Conversation

@nadaverell

@nadaverell nadaverell commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • distinguish evidence-backed active time from resource creation and unknown snapshot timing
  • propagate condition, event, threshold, and tracker timestamps only when they support the current failure
  • expose grouped uncertainty without emitting redundant all-known coverage
  • describe first_seen as “active at least since,” never exact onset, across API docs, MCP, and UI
  • reject CEL age filters when any logical branch can treat unknown first_seen=0 as 1970
  • keep owner-condition timing explicitly workload-level and separate from a row's own active-time anchor

Fleet validation

  • compared binaries from main and this branch across all configured clusters
  • 17 of 23 clusters were reachable; AWS, GCP, and kind environments were represented
  • zero issue membership, severity, reason, message, cause, action, or top-200 ordering differences
  • 17,025 timing/provenance field differences, which is the intended surface of this PR
  • all-known grouped coverage is now omitted; on radar-test-prod, 0 of the first 1,000 rows carry redundant all-known coverage

Validation

  • make test
  • make tsc
  • make build
  • 34 focused issue UI tests
  • focused issue, filter, MCP catalog, and schema tests
  • Cursor Opus cross-review, findings fixed, convergence review clean
  • live EKS run against radar-test-prod (3,200 issues): API/UI healthy, zero browser console errors
  • live unsafe disjunctive first_seen filter rejected with HTTP 400 and actionable guidance; guarded filter returned HTTP 200
  • Playwright verified health regressed, active at least …, and the owner-workload attribution caveat in the production build

Risk and rollback

This is a broad read-path change: issue membership and diagnosis are unchanged, but timing fields are consumed by the UI, MCP agents, and CEL filters. Unknown onset is now explicit, and previously unguarded first_seen age filters fail loudly instead of returning misleading matches. Reverting this PR restores the prior timing behavior.

Follow-up to RAD-346.


Note

Medium Risk
Broad read-path change to timing fields used by the UI, MCP agents, and CEL filters; issue membership is unchanged but clients must handle unknown onset and stricter filter validation.

Overview
Issue timing is now evidence-backed end-to-end: first_seen is set only from explicit anchors (OnsetAt, condition lastTransitionTime, deletion time, trackers, etc.), not from resource age or compose-time guesses. Missing anchors surface as onset_unknown with resource_created_at kept as separate context, and grouped rows can carry onset_coverage when members disagree on whether onset is known.

Detectors and condition sources were updated to pass parsed transition times (including CAPI, Velero, Helm, gateway routes, CNPG) and to mark unknown onset when timestamps are absent or in the future. setDetectionOnset centralizes duration/OnsetUnknown derivation; problem normalization no longer fabricates onset from AgeSeconds/DurationSeconds.

Aggregation behavior uses foldIssueOnset for grouping and duplicate-env rollups, sorts unknown-onset issues by resource creation via issueSortAnchor, drops symptom→structural issue_timing donation, and clears group-wide timing when onset coverage is mixed. Public timestamps are normalized to UTC before filters run.

Consumers: MCP/docs describe first_seen as “active at least since”; CEL issue filters gain onset_unknown, onset_coverage_unknown, and resource_created_at, with compile-time rejection of age filters that use first_seen without guarding unknown onset (first_seen != 0, !onset_unknown, etc.).

Reviewed by Cursor Bugbot for commit 8a266b3. Bugbot is set up for automated code reviews on this repo. Configure here.

@nadaverell
nadaverell requested a review from hisco as a code owner August 9, 2026 01:54

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 04d2b62. Configure here.

Comment thread internal/k8s/detect.go Outdated
Comment thread internal/k8s/detect_env_history.go
nadaverell added a commit that referenced this pull request Aug 9, 2026
## Summary
- detect Gateways that reference a missing GatewayClass after a
two-minute reconciliation grace
- detect HTTPRoute, GRPCRoute, TCPRoute, and TLSRoute parentRefs that
target a missing Gateway
- require authoritative cluster or exact-namespace informer coverage
before asserting absence
- preserve unrelated Gateway controller conditions while deduplicating
exact structural echoes, including Envoy Gateway PortNotFound
- apply the same authority check to KEDA Rollout scaleTargetRefs so
partial caches cannot produce false missing-target issues

## Validation
- `make build`
- `make test`
- `make tsc`
- `go test ./internal/issues ./internal/k8s`
- `go test ./...` from `pkg/k8score/`
- live EKS smoke on `radar-test-nonprod`: grace suppression, both
findings present, target creation recovery, and fixture cleanup
- Playwright Issues-page smoke with both findings rendered and zero
console errors
- visual-test skipped: no UI delta

## Stack
- stacked on #1391
- #1391 is stacked on #1390

Linear: RAD-346

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes live issue detection for Gateway networking and dynamic-cache
“absence” semantics; incorrect authority or dedupe could hide real
problems or briefly miss issues during informer sync, but behavior is
heavily tested and biased toward silence when coverage is incomplete.
> 
> **Overview**
> Extends **Gateway API missing-reference detection** beyond route
backend Services: after a **2-minute grace**, it flags **Gateways** with
a non-existent `spec.gatewayClassName` and **routes** (`HTTPRoute`,
`GRPCRoute`, `TCPRoute`, `TLSRoute`) whose `parentRefs` point at a
**missing Gateway** (same- or cross-namespace). **Backend Service / port
/ ReferenceGrant** checks still require the Service lister; **topology**
checks (class + parent) run even when Services aren’t available.
> 
> **Issue taxonomy** maps `Missing GatewayClass` to
**gateway_not_ready** and `Missing Gateway parent` to
**gateway_route_invalid**; user-facing catalog copy is updated
accordingly.
> 
> **Dedupe** no longer drops every `ResolvedRefs:*` condition when any
structural missing-ref exists on the route. It only hides **matching**
controller echoes (e.g. backend missing → `BackendNotFound` /
`PortNotFound`; ReferenceGrant → `RefNotPermitted`). A **missing
parent** structural row does **not** suppress unrelated `ResolvedRefs`
conditions.
> 
> **Dynamic cache authority**: new `HasWatchedInSyncedNamespace` returns
“missing” only when the relevant informer has **synced** for that
namespace (including during informer scope replacement). **KEDA
`Rollout` scaleTargetRefs** use the same rule so partial watches don’t
emit false **missing scaleTargetRef** issues. Initial add-event
suppression is renamed/clarified so it isn’t confused with sync
authority.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
93ee479. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
nadaverell added a commit that referenced this pull request Aug 9, 2026
## Summary
- detect Gateways that reference a missing GatewayClass after a
two-minute reconciliation grace
- detect HTTPRoute, GRPCRoute, TCPRoute, and TLSRoute parentRefs that
target a missing Gateway
- require authoritative cluster or exact-namespace informer coverage
before asserting absence
- preserve unrelated Gateway controller conditions while deduplicating
exact structural echoes, including Envoy Gateway PortNotFound
- apply the same authority check to KEDA Rollout scaleTargetRefs so
partial caches cannot produce false missing-target issues

## Validation
- `make build`
- `make test`
- `make tsc`
- `go test ./internal/issues ./internal/k8s`
- `go test ./...` from `pkg/k8score/`
- live EKS smoke on `radar-test-nonprod`: grace suppression, both
findings present, target creation recovery, and fixture cleanup
- Playwright Issues-page smoke with both findings rendered and zero
console errors
- visual-test skipped: no UI delta

## Stack
- stacked on #1391
- #1391 is stacked on #1390

Linear: RAD-346

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes live issue detection for Gateway networking and dynamic-cache
“absence” semantics; incorrect authority or dedupe could hide real
problems or briefly miss issues during informer sync, but behavior is
heavily tested and biased toward silence when coverage is incomplete.
> 
> **Overview**
> Extends **Gateway API missing-reference detection** beyond route
backend Services: after a **2-minute grace**, it flags **Gateways** with
a non-existent `spec.gatewayClassName` and **routes** (`HTTPRoute`,
`GRPCRoute`, `TCPRoute`, `TLSRoute`) whose `parentRefs` point at a
**missing Gateway** (same- or cross-namespace). **Backend Service / port
/ ReferenceGrant** checks still require the Service lister; **topology**
checks (class + parent) run even when Services aren’t available.
> 
> **Issue taxonomy** maps `Missing GatewayClass` to
**gateway_not_ready** and `Missing Gateway parent` to
**gateway_route_invalid**; user-facing catalog copy is updated
accordingly.
> 
> **Dedupe** no longer drops every `ResolvedRefs:*` condition when any
structural missing-ref exists on the route. It only hides **matching**
controller echoes (e.g. backend missing → `BackendNotFound` /
`PortNotFound`; ReferenceGrant → `RefNotPermitted`). A **missing
parent** structural row does **not** suppress unrelated `ResolvedRefs`
conditions.
> 
> **Dynamic cache authority**: new `HasWatchedInSyncedNamespace` returns
“missing” only when the relevant informer has **synced** for that
namespace (including during informer scope replacement). **KEDA
`Rollout` scaleTargetRefs** use the same rule so partial watches don’t
emit false **missing scaleTargetRef** issues. Initial add-event
suppression is renamed/clarified so it isn’t confused with sync
authority.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
93ee479. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@nadaverell
nadaverell force-pushed the fix/rad-346-onset-provenance branch from a2fce69 to e2c2e3d Compare August 9, 2026 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant