Skip to content

fix(fleetcontrol): paginate fleet search across all pages - #1874

Open
ShaneLillieNR wants to merge 3 commits into
mainfrom
fix/fleetcontrol-search-pagination
Open

fix(fleetcontrol): paginate fleet search across all pages#1874
ShaneLillieNR wants to merge 3 commits into
mainfrom
fix/fleetcontrol-search-pagination

Conversation

@ShaneLillieNR

@ShaneLillieNR ShaneLillieNR commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

fleetcontrol fleet search --name-equals/--name-contains only inspected the first page of GetEntitySearch results, so the client-side name filter silently missed any fleet that wasn't on page 1 (confirmed live: a known fleet absent from a 100-entity first page, found instantly via a name-scoped query against the general entitySearch API).

searchFleetEntities now loops on NextCursor (capped at 1000 pages as a safety net against a misbehaving cursor) and accumulates entities across every page before applying the name filter. This intentionally does not expose a --next-cursor flag the way fleet members list does - a name search's entire point is "does this exist," and the caller has no way to know which page a match might be on, so partial/manual paging would defeat the feature.

Depended on newrelic/newrelic-client-go#1442, which fixed the same bug one layer down: GetEntitySearch's cursor parameter was a no-op because the generated query never declared/passed $cursor. That PR has merged and shipped in client-go v2.93.2 - go.mod now points at the real release instead of the temporary commit-pinned replace directive, so this is ready for review.

Test plan

  • internal/fleetcontrol/fleet_management_search_test.go: 5 unit tests against a mock NerdGraph server - match on page 1, match only on page 2+ (the regression test for this bug), no match across all pages, error on a later page propagates (no partial results), --name-contains spanning multiple pages.
  • Verified the page-2 regression test isn't vacuous: temporarily capped the loop at one iteration and confirmed that test fails, then restored the fix.
  • make test-unit - 526 tests passing. gofmt / golangci-lint clean.
  • Live-verified against a real account (~800 fleets) with the actual built binary: unfiltered search returns all 798 fleets with no duplication, and --name-equals finds fleets at page 2+ that a pre-fix build returns null for. Details in the PR comments.

fleet search --name-equals/--name-contains only inspected the first
page of GetEntitySearch results, so the client-side name filter
silently missed any fleet that wasn't on page 1 (confirmed live: a
known fleet absent from a 100-entity first page, found instantly via
a name-scoped query against the general entitySearch API).

searchFleetEntities now loops on NextCursor (capped at 1000 pages as
a safety net against a misbehaving cursor) and accumulates entities
across every page before applying the name filter.

Depends on newrelic/newrelic-client-go#1442, which fixes the same bug
one layer down: GetEntitySearch's cursor parameter was a no-op because
the generated query never declared/passed $cursor. go.mod currently
points at that PR's commit via a temporary replace directive so this
fix can be reviewed and tested end-to-end before #1442 merges and
ships a release - opening as a draft for that reason. The replace
directive needs to be swapped for a released client-go version, and
this PR marked ready, once that lands.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ShaneLillieNR

ShaneLillieNR commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator Author

Live end-to-end verification of this fix, using the built newrelic-cli binary against a real account (~800 fleets), not just the unit tests in this PR:

With this fix (built against newrelic/newrelic-client-go#1442's commit via the temporary go.mod replace directive):

  • Unfiltered fleetcontrol fleet search returns 798 fleets, all 798 IDs unique — confirms the pagination loop walks every page (~8 pages at 100/page) with no overlap or duplication.
  • --name-equals correctly finds fleets that only exist beyond page 1 — tested one at result index 150 and the very last one at index 797.

Without this fix (same live account, same two fleet names, built from main with unpatched newrelic-client-go v2.91.0 in a throwaway worktree): both --name-equals queries returned null. That's the exact bug being fixed — silently wrong empty results for any fleet not on page 1, no error, no warning.

@pranav-new-relic pranav-new-relic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM (as discussed over the call). Thanks for fixing this :)

@ShaneLillieNR
ShaneLillieNR marked this pull request as ready for review August 20, 2026 18:01
@ShaneLillieNR
ShaneLillieNR marked this pull request as draft August 20, 2026 18:18
ShaneLillieNR and others added 2 commits August 24, 2026 07:40
newrelic/newrelic-client-go#1442 merged and shipped in v2.93.2, which
carries the GetEntitySearch cursor fix this PR depends on. Drop the
temporary replace directive and point go.mod at the real release.

Re-verified against the same live account/fleets used for the earlier
before/after check - unchanged now that a real released version is in
place.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ShaneLillieNR
ShaneLillieNR marked this pull request as ready for review August 24, 2026 22:18
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.

2 participants