feat(query): populate timestamps for config analysis search#2025
Conversation
|
Warning Review limit reached
More reviews will be available in 12 minutes and 40 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Walkthrough
ChangesConfigAnalysis timestamp propagation
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
Benchstat (RLS)Base:
|
| Benchmark | Base | Head | Change | p-value |
|---|---|---|---|---|
RLS/Sample-15000/analysis_types/With_RLS-4 |
3.910m | 4.114m | +5.21% 🔴 | 0.002 |
RLS/Sample-15000/config_classes/With_RLS-4 |
137.4m | 140.7m | +2.43% | 0.002 |
RLS/Sample-15000/catalog_changes/Without_RLS-4 |
5.279m | 5.374m | +1.79% | 0.004 |
RLS/Sample-15000/config_changes/Without_RLS-4 |
5.340m | 5.378m | +0.71% | 0.026 |
✅ 4 improvement(s)
| Benchmark | Base | Head | Change | p-value |
|---|---|---|---|---|
RLS/Sample-15000/change_types/Without_RLS-4 |
5.424m | 5.261m | -3.01% | 0.002 |
RLS/Sample-15000/catalog_changes/With_RLS-4 |
145.6m | 142.8m | -1.89% | 0.002 |
RLS/Sample-15000/config_detail/Without_RLS-4 |
4.832m | 4.785m | -0.97% | 0.041 |
RLS/Sample-15000/config_summary/With_RLS-4 |
726.3m | 720.6m | -0.79% | 0.026 |
Failed: 1 benchmark(s) regressed by more than 5%:
RLS/Sample-15000/analysis_types/With_RLS-4: 3.910m -> 4.114m (+5.21%)
Full benchstat output
goos: linux
goarch: amd64
pkg: github.com/flanksource/duty/bench
cpu: AMD EPYC 7763 64-Core Processor
│ bench-base.txt │ bench-head.txt │
│ sec/op │ sec/op vs base │
RLS/Sample-15000/catalog_changes/Without_RLS-4 5.279m ± 1% 5.374m ± 1% +1.79% (p=0.004 n=6)
RLS/Sample-15000/catalog_changes/With_RLS-4 145.6m ± 1% 142.8m ± 0% -1.89% (p=0.002 n=6)
RLS/Sample-15000/config_changes/Without_RLS-4 5.340m ± 1% 5.378m ± 3% +0.71% (p=0.026 n=6)
RLS/Sample-15000/config_changes/With_RLS-4 144.4m ± 0% 144.8m ± 1% ~ (p=0.065 n=6)
RLS/Sample-15000/config_detail/Without_RLS-4 4.832m ± 1% 4.785m ± 1% -0.97% (p=0.041 n=6)
RLS/Sample-15000/config_detail/With_RLS-4 139.3m ± 1% 139.2m ± 1% ~ (p=0.699 n=6)
RLS/Sample-15000/config_names/Without_RLS-4 13.86m ± 2% 13.85m ± 2% ~ (p=0.589 n=6)
RLS/Sample-15000/config_names/With_RLS-4 140.8m ± 1% 140.4m ± 1% ~ (p=0.699 n=6)
RLS/Sample-15000/config_summary/Without_RLS-4 102.8m ± 2% 100.8m ± 5% ~ (p=0.394 n=6)
RLS/Sample-15000/config_summary/With_RLS-4 726.3m ± 1% 720.6m ± 1% -0.79% (p=0.026 n=6)
RLS/Sample-15000/configs/Without_RLS-4 8.886m ± 8% 8.297m ± 8% ~ (p=0.093 n=6)
RLS/Sample-15000/configs/With_RLS-4 139.2m ± 1% 138.3m ± 2% ~ (p=0.310 n=6)
RLS/Sample-15000/analysis_types/Without_RLS-4 3.931m ± 1% 3.923m ± 1% ~ (p=0.485 n=6)
RLS/Sample-15000/analysis_types/With_RLS-4 3.910m ± 1% 4.114m ± 2% +5.21% (p=0.002 n=6)
RLS/Sample-15000/analyzer_types/Without_RLS-4 3.766m ± 1% 3.751m ± 1% ~ (p=0.310 n=6)
RLS/Sample-15000/analyzer_types/With_RLS-4 3.762m ± 2% 3.788m ± 2% ~ (p=0.180 n=6)
RLS/Sample-15000/change_types/Without_RLS-4 5.424m ± 1% 5.261m ± 1% -3.01% (p=0.002 n=6)
RLS/Sample-15000/change_types/With_RLS-4 5.321m ± 2% 5.308m ± 4% ~ (p=0.699 n=6)
RLS/Sample-15000/config_classes/Without_RLS-4 4.150m ± 0% 4.122m ± 2% ~ (p=0.132 n=6)
RLS/Sample-15000/config_classes/With_RLS-4 137.4m ± 1% 140.7m ± 0% +2.43% (p=0.002 n=6)
RLS/Sample-15000/config_types/Without_RLS-4 4.827m ± 0% 4.877m ± 3% ~ (p=0.093 n=6)
RLS/Sample-15000/config_types/With_RLS-4 140.0m ± 1% 139.4m ± 1% ~ (p=0.093 n=6)
geomean 21.34m 21.27m -0.30%
Benchstat (Other)Base: ✅ 1 improvement(s)
Full benchstat output |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/query_resource_selector_test.go`:
- Around line 494-497: The test is asserting `UpdatedAt` from
`items.ConfigAnalysis[0]`, but `dummy.LogisticsDBRDSAnalysis` only seeds
`FirstObserved` and leaves `LastObserved` nil, so this case should not require
`UpdatedAt` unless the fixture is updated. Adjust
`tests/query_resource_selector_test.go` to keep this fixture-focused assertion
limited to `CreatedAt`, and if you need coverage for `UpdatedAt`, add or update
a separate fixture case using `dummy.LogisticsDBRDSAnalysis` (or the relevant
mapping path) with `LastObserved` explicitly set.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13fab33e-b8e8-4e84-94a9-e0bf76cc4a8c
📒 Files selected for processing (2)
query/resource_selector.gotests/query_resource_selector_test.go
SearchResources populated CreatedAt/UpdatedAt/DeletedAt for every resource kind except config analysis, so config insight searches always came back without timestamps even when Timestamps was requested. Map first_observed -> created_at and last_observed -> updated_at (the only timestamps config_analysis carries) via the existing setTimestamps helper.
The fixture leaves last_observed unset and relied on the column default to make updated_at non-nil, which was implicit. Seed a dedicated analysis with explicit first/last observed and assert the exact mapping instead.
6c25ecb to
ac733e0
Compare
Gavel resultsGavel exited with code . |
Problem
SearchResources(POST /resources/search) populatescreated_at/updated_at/deleted_aton the selected resources for every resource kind except config analysis. So config insight searches (e.g.faro catalog insights search) always came back without timestamps, even whenTimestamps: truewas requested — every other kind (configs, components, checks, changes, playbooks, connections) callssetTimestamps, theconfig_analysisbranch did not.Change
Add the missing
setTimestampscall in theconfig_analysisbranch.config_analysishas nocreated_at/updated_atcolumns — its only timestamps arefirst_observedandlast_observed— so we map:first_observed→created_atlast_observed→updated_atThis keeps the response shape uniform with the other resource kinds.
Tests
Extended the existing
SearchResourceSelectorstimestamp specs to cover config analysis:created_at/updated_atare set andfirst_observed < last_observed(confirming mapping direction).Timestampsis not set.Both pass against the embedded test DB.
Downstream
Unblocks the
faro catalog insights searchtimestamp fix in mission-control.Summary by CodeRabbit