Skip unneeded query when getting CVE chart

[sgress454]

Related issue: Resolves #45720

Details

When requesting CVE chart data, we were making a call to get the set of tracked CVEs to filter the data by. Currently we're only collecting data for the tracked CVEs, so there's no reason to make this call at all.

When we add more filtering options and start collecting more data, we'll need a call like this again, and will likely need to start caching the results. Otherwise it's a multi-second cost per query on large deployments.

Checklist for submitter

If some of the following don't apply, delete the relevant line.

• Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
  See Changes files for more information.

Testing

• Added/updated automated tests
  • removed some outdated tests
  • replaced with a test that checks that when an entity filter returns no items (an empty, rather than nil slice) we get empty buckets returned rather than getting data for all entities. This is a regression test for when we add filtering back.
• QA'd all new/changed functionality manually
  • validated that chart still loads and shows the same data.
  • tried it on a load test env and saw dramatic API request time improvement

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Optimized CVE chart data retrieval by eliminating redundant queries, reducing unnecessary database operations and improving performance.
  • Fixed entity ID filtering logic to correctly handle edge cases and prevent unintended filter interactions across metrics.

• Tests

  • Added test coverage for chart data queries with empty entity filters.
  • Improved test isolation to ensure metric-specific filtering behavior is properly separated.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

_{Tip: disable this comment in your organization's Code Review settings.}

[Copilot]

Pull request overview

Removes an unnecessary TrackedCriticalCVEs query from the CVE chart data path since the system currently only collects data for tracked CVEs (making the filter redundant), and adds/updates tests to pin the storage-layer empty-slice contract for when filtering is reintroduced.

Changes:

• Drop the TrackedCriticalCVEs lookup in Service.GetChartData for the cve metric, leaving entityIDs as nil (no filter).
• Remove the now-obsolete service tests that verified filter propagation, and add a MySQL-layer regression test ensuring a non-nil empty entityIDs slice yields zero-valued buckets.
• Add a changes file entry.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File	Description
server/chart/internal/service/service.go	Removes the TrackedCriticalCVEs call so entityIDs stays nil for the CVE metric.
server/chart/internal/service/service_test.go	Deletes the two service tests that exercised the removed filter path.
server/chart/internal/mysql/data_test.go	Adds a storage-layer regression test for the non-nil empty entityIDs contract.
changes/45720-remove-unneeded-query	User-visible changelog entry.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Walkthrough

This PR removes an unneeded TrackedCriticalCVEs() query from the CVE chart data retrieval pipeline. The service layer's GetChartData method previously contained a special case for CVE metrics that resolved entity IDs from storage; this special case is removed and entityIDs is now left as nil for all metrics. Supporting test coverage validates the storage-layer semantics (nil = no filter, empty non-nil slice = match nothing), and service-layer tests are updated to remove CVE-specific assertions and add validation that uptime metrics do not trigger the removed query.

Possibly related PRs

• fleetdm/fleet#45247: Refactors tracked CVE scoping by moving TrackedCriticalCVEs and AffectedHostIDsByCVE calls into the CVE collection path rather than the chart-data retrieval path.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: removing an unneeded query call when fetching CVE chart data.
Linked Issues check	✅ Passed	The PR successfully removes the TrackedCriticalCVEs() call [#45720], verifies correctness through tests, and includes changes file documentation.
Out of Scope Changes check	✅ Passed	All changes directly support the primary objective of removing the unneeded TrackedCriticalCVEs query and validating correctness through targeted tests.
Description check	✅ Passed	The PR description adequately covers the change objective, implementation rationale, testing approach, and manual QA validation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sgress454/45720-skip-tracking-cve-call-on-read-path

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sgress454]

This test isn't strictly related to the removed code, but the test that's being removed in this PR indirectly tested the behavior of "asking the chart to display data for an empty set of entity IDs" which was now completely untested. It's worth adding the regression test for it here since we'll add more filtering soon and it'd be easy to forget.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.78%. Comparing base (91b3dd6) to head (0b90786).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #45813      +/-   ##
==========================================
+ Coverage   66.76%   66.78%   +0.02%     
==========================================
  Files        2747     2748       +1     
  Lines      219605   219825     +220     
  Branches    10882    10882              
==========================================
+ Hits       146610   146803     +193     
- Misses      59741    59756      +15     
- Partials    13254    13266      +12     

Flag	Coverage Δ
backend	68.60% <ø> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.