Skip to content

[codex] stream AWS ELBv2 collector results#116

Draft
antdewei wants to merge 2 commits into
antgroup:mainfrom
antdewei:codex/aws-elbv2-streaming-timeout
Draft

[codex] stream AWS ELBv2 collector results#116
antdewei wants to merge 2 commits into
antgroup:mainfrom
antdewei:codex/aws-elbv2-streaming-timeout

Conversation

@antdewei

@antdewei antdewei commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Change AWS ELBv2 collection from batch-then-push to streaming push per load balancer.
  • Stream the standalone ELB Listener resource as listeners are discovered instead of collecting all listeners before sending.
  • Add deterministic streaming regression tests that would fail if the collector waits for every load balancer before emitting the first result.
  • Add ELBv2 progress logs for load-balancer pages, processed load balancers, listener count, listener errors, and collection duration.

Root cause

PR #114 added per-load-balancer DescribeListeners calls, but the AWS ELBv2 collector still built the entire region result set before sending anything to regionCh. 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=1
  • go test ./collector/elasticloadbalancing ./collector ./platform
  • go test ./... in collector/aws
  • git diff --check

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.
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