Fix neuron per core multi runtime - #2263
Conversation
On a node with more than one Neuron runtime, roughly half of every per-core
Neuron metric is silently replaced by zero before it leaves the agent. A
NeuronCore pinned at 75% utilization reports 0.
Measured on an inf2.xlarge (2 NeuronCores, one process pinned per core).
neuron-monitor's own /metrics is correct -- it emits the full
(neuroncore x runtime_tag) cross-product with only the matching pairs non-zero:
neuroncore_utilization_ratio{neuroncore="0",runtime_tag="...core0"} 0.7523 <- real
neuroncore_utilization_ratio{neuroncore="1",runtime_tag="...core0"} 0.0
neuroncore_utilization_ratio{neuroncore="0",runtime_tag="...core1"} 0.0
neuroncore_utilization_ratio{neuroncore="1",runtime_tag="...core1"} 0.7517 <- real
A debug exporter on the live pipeline shows what the agent then produces: ONE
ResourceMetrics containing FOUR datapoints with only TWO distinct attribute
sets --
aws.neuron.core=0 Value: 75.154815 <- real
aws.neuron.core=1 Value: 0.000000
aws.neuron.core=0 Value: 0.000000 <- same identity, shadows the real one
aws.neuron.core=1 Value: 75.363968
Cause, in transform/cw_k8s_ci_v0_neuron_promote:
1. It runs in `context: datapoint` but writes `resource.attributes`. Resource
attributes are per-ResourceMetrics, so with N runtimes in one RM the
statement executes N times and the last write wins -- which is why a single
arbitrary runtime tag ends up labelling the whole node.
2. It then deletes `runtime_tag` from the datapoint. That was the only
attribute distinguishing the real reading from the other runtime's zero, so
two datapoints collapse to one identity with different values. Nothing
downstream reconciles them (awsattributelimit early-returns below 150
attributes; batch only concatenates), so both go over the wire and one is
arbitrarily dropped at ingest.
Blast radius is wider than utilization: neuron_runtime_memory_used_bytes has 4
series upstream (2 memory_location x 2 runtime_tag) and arrives as 2, so one
runtime's memory is dropped as well.
Fix: make `runtime_tag` a groupbyattrs grouping key so each runtime gets its own
ResourceMetrics. That removes the key from the datapoint as a side effect, so
each datapoint identity stays unique and the last-write-wins clobber becomes a
no-op. The promote then moves to `context: resource`, which is where the
attribute now lives -- and is how the DCGM pipeline already does it
(containerinsights/dcgm.yaml:66-83).
The six pod-identity statements are dropped because they are dead code:
groupbyattrs has already deleted those keys from the datapoint by then, so
`attributes["k8s.pod.name"]` is always nil and they never fire.
Verified on the live cluster: core 0 goes from 0 to 75.18, and CloudWatch now
carries 4 correctly-labelled series instead of 2.
Trade-off: this publishes the full cross-product, so series count grows
multiplicatively with runtime count (2 -> 4 here; a 32-core trn1 with 4 runtimes
goes from 32 to 128 series per metric). The alternative is to aggregate the
runtime dimension away in-agent, which keeps the count flat but loses per-runtime
attribution; that trade-off is rejected here because the PromQL surface can carry
the runtime dimension and a runtime legitimately spans multiple cores.
Note: the legacy EMF path already handles this correctly, via
gpuattributes/internal/awsneuron_metric_modifier.go -- it flattens runtime_tag
to "DEFAULT" then aggregates per core with max (commit dc672ff). The OTel
rewrite reimplemented synthesis and scaling but not the aggregation. There is no
multi-runtime test case in plugins/processors/awsneuron/processor_test.go, which
is why this shipped; adding pipeline-level coverage is recommended follow-up.
… loss
Builds the real groupbyattrs + promote processors from the embedded neuron.yaml
and feeds them the shape neuron-monitor emits with two runtimes on one node
(2 cores x 2 runtimes, non-owning pairs at zero, all four sharing one pod).
Four tests:
KeepsEveryCorePerRuntime all four (core, tag, value) triples survive
SeparatesRuntimesIntoResources one ResourceMetrics per runtime
PromotesPodIdentity pod/ns/container on resource, off datapoints --
pins the six promote statements the fix removed,
which groupbyattrs already performs
PreFixConfigLosesData negative control: the datapoint-context promote
collapses to one tag and two datapoints per core
Config is read from the embedded template, not restated, so a revert fails the
test. Verified by mutation: removing `- runtime_tag` from the groupbyattrs keys in
neuron.yaml fails three of the four. Runs in ~25ms with no cluster or Neuron
hardware, which is the gap that let the original defect ship -- it needed >=2
runtimes on one node to appear and no test environment had that.
| // collapsed onto one identity and a core pinned at 75% reported 0. | ||
| // | ||
| // TestNeuronMultiRuntimePreFixConfigLosesData pins that failure mode, so the | ||
| // assertions below are known to discriminate rather than merely pass. |
There was a problem hiding this comment.
This reads like an artifact from the fix you've done rather than future-understandable preface for a set of tests... LEts refactor it to focus the intention of the tests rather than the historical bug
There was a problem hiding this comment.
Fair — rewritten to lead with the invariant the tests protect rather than the history of how it broke. The context: datapoint/last-write-wins mechanism moved onto the negative control, which is the one test genuinely about that configuration shape, so it's local to where it's load-bearing.
Also renamed PreFixConfigLosesData → CollapsedConfigLosesData, since "pre-fix" ages the same way the comment did, and gave PromotesPodIdentity the same treatment — it described itself as guarding "the six promote statements removed by the fix" instead of stating that pod identity must land on the resource and not stay on the datapoint.
| // renderedNeuronProcessors returns the processors block of the shipped neuron.yaml. | ||
| // Read from the embedded template rather than restated, so reverting the config | ||
| // fails this test. | ||
| func renderedNeuronProcessors(t *testing.T) map[string]any { |
There was a problem hiding this comment.
Hmm I wonder if this is really a substitute for the integration tests? Do we have other tests like this? Or what are you trying to cover here that you can't cover with an integration test?
There was a problem hiding this comment.
Not a substitute — complementary, and there is precedent for the pattern: translator/translate/otel/processor/metricsdecorator/translator_unix_test.go:52 (TestMetricDecoration) does the same thing with the same imports, and its own comment describes it as "using a test TransformProcessor yaml configuration, starting the processor and having it consume test metrics." One deliberate difference: it loads a testdata yaml, whereas this renders the shipped neuron.yaml off the //go:embeded template, so reverting the real config fails this test.
Three things the integration test structurally can't do:
-
The negative control can only live here. CollapsedConfigLosesData runs the broken config to prove the assertions discriminate rather than merely pass. You can't deploy a deliberately broken agent in CI.
-
The integration test can't distinguish a collapse from a single-runtime node. I hit this when I reverted the agent on a live cluster to check the integration tests actually fail: the collapse destroys runtime_tag itself, so exactly one tag survives per node — structurally identical to a node that genuinely runs one runtime. The integration test has to bail out with a "check both of these causes" message rather than assert. Here the input is fixed, so it asserts an exact four-series expectation.
-
It fails on a config revert, in this repo's build. The integration test only ever sees whatever config happens to be deployed, and only if the cluster fixture is right.
The integration side is in a companion PR against amazon-cloudwatch-agent-test — that's the layer that proves it on real inf2 silicon.
… the bug Review feedback on #2263: the file's preface read as a post-mortem of the fix rather than something a future reader could use. Lead with the invariant every (core, runtime) reading must satisfy; keep the collapsed-config mechanism on the negative control, which is the one test that is genuinely about that shape. Rename PreFixConfigLosesData -> CollapsedConfigLosesData so the name describes the configuration rather than when it existed. No assertions changed. Also drop an internal cluster name from a comment.
Binary Size Reportlinux/amd64
linux/arm64
windows/amd64
Investigating size changesUse go-size-analyzer to compare binaries: GOEXPERIMENT=jsonv2 go install github.com/Zxilly/go-size-analyzer/cmd/gsa@latest
gsa diff --old <baseline-binary> --new <new-binary> |
2 participants
Description of the issue
On a node running more than one Neuron runtime, roughly half of every per-core Neuron
metric is silently replaced by zero. A NeuronCore pinned at 75% utilization reports 0.
transform/cw_k8s_ci_v0_neuron_promoteruns incontext: datapointbut writesresource.attributes. Resource attributes are per-ResourceMetrics, so with N runtimesin one RM the statement executes N times and the last write wins. It then deletes
runtime_tagfrom the datapoint — the only attribute distinguishing one runtime's realreading for a core from another runtime's zero for that same core — so two datapoints
collapse onto one identity with different values.
A debug exporter on the pipeline shows the agent emitting four datapoints with only
two distinct attribute sets:
Impact on a multi-runtime Neuron node: a saturated core renders as idle, a
low-utilization alarm fires falsely, and an idle-reclaim decision could reclaim a busy
core. Confirmed on
neuroncore_utilization_ratioand all fiveneuroncore_memory_usage_*families.Single-runtime nodes are unaffected, which is why this went unnoticed — it needs ≥2
Neuron runtimes on one node to appear.
neuron-monitoris not at fault. Scraping its own/metricsendpoint at the sameinstant shows it emitting the full
(core × runtime_tag)cross-product correctly, withonly the matching pairs non-zero.
Description of changes
Aggregation is not needed —
groupbyattrsalready does the right thing, it just wasn'tgiven the runtime dimension.
runtime_tagto thegroupbyattrs/cw_k8s_ci_v0_neuronkeys, so each runtimelands in its own
ResourceMetricsand the datapoints can no longer collide.transform/cw_k8s_ci_v0_neuron_promoteto acontext: resourcerename ofruntime_tag→aws.neuron.runtime.tag.The six
set/delete_keystatements fork8s.pod.name,k8s.namespace.nameandk8s.container.nameare removed as redundant: those keys are already in thegroupbyattrskeys, andgroupbyattrsmoves its grouping keys — seeprocessor/groupbyattrsprocessor/processor.go:so the promote was re-doing work already done one processor earlier, and its
datapoint-context write was the vehicle for the defect.
Net effect: 11 lines changed in
neuron.yamlplus the four regenerated golden configs.Trade-off
This publishes the full cross-product, so series count grows multiplicatively with
runtime count — 2 → 4 on a 2-core/2-runtime node; a 32-core
trn1with 4 runtimes goes32 → 128 series per per-core metric. This is the cardinality
neuron-monitoralreadyemits; the previous behaviour reduced it only by discarding data.
The alternative is to aggregate the runtime dimension away in-agent (
maxper core),which keeps the count flat but loses per-runtime attribution. Rejected here because the
PromQL/OTLP surface can carry the runtime dimension, and a runtime legitimately spans
multiple cores, so "which runtime is using this core" is a question this surface should
be able to answer.
Reviewers may want to weigh in on whether the cardinality growth is acceptable for large
trn1topologies. It has not been measured beyond a 2-core node.Scope
This does not affect the EMF / Container Insights path.
neuron.yaml's pipelineexports to the
forward/opentelemetryconnector, which is consumed only bymetrics/opentelemetry→otlphttp/metrics. In the combined v1+v2 config the solepipeline touching an EMF exporter is
logs/emf_logs(receiverudplog, one batchprocessor, no Neuron processors). The EMF Neuron path is a separate pipeline —
awscontainerinsightreceiver→gpuattributes→awsemf/containerinsights— and sharesno component instance with this one. Its own
max-per-core reduction ingpuattributes/internal/awsneuron_metric_modifier.gois unchanged and remains correctfor that surface, which does not publish
runtime_tagas a dimension.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
New unit tests —
translator/translate/otel/pipeline/opentelemetry/containerinsights/neuron_multiruntime_test.go.Instantiates the real
groupbyattrsandtransformprocessors from the//go:embededneuron.yaml, chains them, and feeds synthetic metrics in the multi-runtime shape(2 cores × 2 runtimes, non-owning pairs at zero, all four sharing one pod). No cluster or
Neuron hardware required; runs in ~25ms.
KeepsEveryCorePerRuntime(core, tag, value)triples survive — both 75.x readingsSeparatesRuntimesIntoResourcesResourceMetricsper runtimePromotesPodIdentityPreFixConfigLosesDataConfig is read from the embedded template rather than restated, so a revert fails the
test. Verified by mutation: removing
- runtime_tagfrom thegroupbyattrskeys inneuron.yamlfails three of the four.The negative control matters because it keeps the other three honest — if the collapse
ever stops reproducing,
PreFixConfigLosesDatastarts passing and tells us theassertions have gone vacuous.
Existing suites —
go test ./translator/tocwconfig/...and./translator/translate/otel/pipeline/opentelemetry/containerinsights/...pass. The fourgolden configs in this diff are the regenerated snapshots.
Live cluster — verified on a 2-core
inf2.xlargerunning two Neuron runtimes:75.23 under the other.
re-applying restored it. So the fix is doing the work, not a caching artifact.
including
TestNeuronPodName,TestNeuronNamespace,TestNeuronDeviceAttributesandTestNeuronNoPromotedDatapointKeys— the last of which specifically assertspod/namespace/container are not left on the datapoint, covering the six statements
removed here.
Integration test coverage for the multi-runtime case is a companion change to
amazon-cloudwatch-agent-test(adds a second co-located burn deployment so a node runstwo runtimes, plus 8 assertions on the resulting series shape). Worth noting that the
pre-existing integration tests could not catch this defect:
TestNeuronRuntimeTagInResourceScopepasses on the broken code because the collapsedresource still carries one tag, and
TestNeuronNoDuplicateSeriespasses because thecollision happens in-agent — the surface shows too few series rather than duplicated
ones. Cardinality is the signal, not duplication.
aws/amazon-cloudwatch-agent-test#749
Requirements
make fmtandmake fmt-sh— no changes producedmake lint— 0 issues (license and import-order checks pass)