[codex] stream AWS ELBv2 collector results#116
Draft
antdewei wants to merge 2 commits into
Draft
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the AWS Elastic Load Balancing collector to use a more efficient iteration pattern. It introduces a forEachELB helper function to handle pagination and execute callbacks for each load balancer, replacing the previous method of collecting all items into a slice. The GetELBDetail and GetELBListenerDetail functions were updated to use this pattern, and an unused import was removed. I have no feedback to provide.
Center-Sun
added a commit
to Center-Sun/CloudRec
that referenced
this pull request
Jun 1, 2026
… forms The AWS collector package had no tests; this adds 0->1 deterministic regression coverage for the streaming invariant established in 8295d1b / 5f3db2f / 65e81d7. Rather than test all ~20 streamed collectors, one representative per pagination form is covered, and each test file documents which sibling collectors it represents and which streaming links it asserts. Forms and representatives: manual pagination token -> elasticloadbalancing/ELBv2 (represents ec2, ecr, efs, fsx, rds, route53/domain, s3, wafv2, iam); SDK paginator -> lambda (represents acm, eks, kms, secretsmanager, sns, sqs, route53/resourcerecordset); ARN list then batched describe -> ecs (sole); un-paginated list -> opensearch (sole). To make the primary-list loops injectable, each collector gains a narrow *API interface whose signatures mirror the SDK so the concrete client still satisfies it, and GetXDetail is split into a thin wrapper plus a streamX(ctx, api, res) helper. The streaming loops move verbatim, so there is no runtime behavior change. Each test drives a fake client that blocks a chosen call after its first invocation and asserts the first result still reaches res within 1s, proving two properties without time.Sleep: (a) per-item enrich streams, and (b) per-page/per-batch streams (the invariant added in 65e81d7 that upstream antgroup#116 did not cover). opensearch has no pagination, so instead of (b) it asserts failure isolation: a domain whose DescribeDomain fails is skipped without panicking, locking the nil-dereference fix in 65e81d7. elbv2 ports antgroup#116's property-(a) fakes and adds the multi-page cases. Reverse-validated: reverting either streaming invariant to a build-slice-then-push pattern, or removing opensearch's nil guard, makes the matching test fail.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root cause
PR #114 added per-load-balancer
DescribeListenerscalls, but the AWS ELBv2 collector still built the entire region result set before sending anything toregionCh. In regions with many ALB/NLB resources, that left the framework consumer idle long enough to hit the hard-coded 30s timeout, close the channel, and drop the whole region.Validation
go test ./collector/elasticloadbalancing -run 'TestStreamELB' -count=1go test ./collector/elasticloadbalancing ./collector ./platformgo test ./...incollector/awsgit diff --check