test(otel/multi_efa): assert per-device EFA pod correlation - #751
Open
samehkhalil wants to merge 1 commit into
Open
test(otel/multi_efa): assert per-device EFA pod correlation#751samehkhalil wants to merge 1 commit into
samehkhalil wants to merge 1 commit into
Conversation
…vs unclaimed) Add TestMultiEFAClaimedVsUnclaimedCorrelation to the multi-EFA suite. On the 2-EFA node, efaburn (replicas: 1) claims one device and leaves the other unclaimed, so the test asserts the claimed device correlates to exactly one pod and the remaining unclaimed device carries no pod attributes. It also flags a single device correlated to multiple pods. This closes a coverage gap: the existing multi_efa tests check device count, ENIs, ports, and that at least one series is correlated, but none detect the EFA multi-device correlation collapse — where the resource-level pod promotion (without a groupbyattrs/efa split) attributes every EFA device on the node, including unclaimed ones, to a single pod. Verified failing on the current chart and passing with the groupbyattrs/efa fix. No topology change is required: the multi-EFA node already exposes two EFA devices with efaburn claiming one, which yields the claimed/unclaimed split.
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
1 participant
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.
Description of the issue
The
multi_efasuite validates the EFA receiver surface (device count, uniqueENIs, ports, all metrics per device) and that at least one series is correlated
to a pod, but nothing asserts which device maps to which pod. As a result it
cannot catch the EFA multi-device correlation collapse: when the OTel Container
Insights pipeline promotes per-datapoint pod identity onto the shared node-level
resource without a
groupbyattrs/efasplit first, every EFA device on the node —including unclaimed ones — is attributed to a single pod.
The existing assertions all still pass under that collapse:
TestMultiEFACorrelatedCountonly requires>= 1correlated series.TestMultiEFACorrelatedPodLabelschecks the first efaburn-labeled series.Description of changes
Adds
TestMultiEFAClaimedVsUnclaimedCorrelationtotest/otel/multi_efa/. On theexisting 2-EFA node,
efaburn(replicas: 1) claims one device and leaves theother unclaimed. The test asserts:
and flags a single device attributed to multiple pods.
No fixture/topology change is required — the multi-EFA node already exposes two
devices with efaburn claiming one, which yields the claimed/unclaimed split. Small
sorted-key helpers were added to
helpers_test.gofor readable failure messages.License
By submitting this pull request, I confirm that you can use, modify, copy, and
redistribute this contribution, under the terms of your choice.
Tests
gofmtclean;go vet -tags integration ./test/otel/multi_efa/clean; testbinary compiles (
go test -tags integration -c).Validated on a live 2-EFA
c6in.32xlargenode in both directions:groupbyattrs/efa): FAIL —expected 1 correlated (claimed) EFA device(s), got 2: [rdmap0s30 rdmap0s31]; all pre-existingmulti_efatests still passed (confirming they don't catch it).groupbyattrs/efafix: PASS — 1 claimed, 1 unclaimed; whole suite green.Dependency: this test only passes once the
groupbyattrs/efafix is present inthe deployed chart (helm-charts). https://github.com/aws-observability/helm-charts/pull/356/commits It should merge after that chart change reaches
the branch this suite deploys (
helm_chart_branch, defaultmain).Not exercised: a fresh
terraform applyofotel-multi-efawas not run; thetest was validated against a hand-provisioned 2-EFA cluster with the chart deployed
both ways.