Skip to content

[receiver/k8s_cluster] [chore] Fix nil dereference in setting image attributes#47538

Closed
kangyili wants to merge 3 commits into
open-telemetry:mainfrom
kangyili:kangyi/k8scluster-nil-pointer
Closed

[receiver/k8s_cluster] [chore] Fix nil dereference in setting image attributes#47538
kangyili wants to merge 3 commits into
open-telemetry:mainfrom
kangyili:kangyi/k8scluster-nil-pointer

Conversation

@kangyili

@kangyili kangyili commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Description

When a pod has no matching ContainerStatus entry for a given container (e.g. the container spec exists but kubelet hasn't reported status yet), cs is nil. The call to docker.ParseImageName(cs.Image) was not guarded by the existing cs != nil check, causing a panic at runtime.

The issue was found when testing with image otel/opentelemetry-collector-contrib-dev:latest

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x48 pc=0x7e95e57]

goroutine 541 [running]:
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver/internal/container.RecordSpecMetrics(_, _, {{0xc001b88138, 0x11}, {0x0, 0x0}, {0x0, 0x0, 0x0}, {0x0, ...}, ...}, ...)
	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver@v0.149.0/internal/container/containers.go:67 +0x1d7
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver/internal/pod.RecordMetrics(0xc001c7e700, 0xc000a48608, 0xc00207d908, 0xc0029dcf00?)
	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver@v0.149.0/internal/pod/pods.go:89 +0x3aa
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver/internal/collection.(*DataCollector).CollectMetricData.func1({0xd3cc680?, 0xc00207d908?})
	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver@v0.149.0/internal/collection/collector.go:67 +0x45
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver/internal/metadata.(*Store).ForEach(0xc0032e8da8?, {{0x0, 0x0}, {0xd51c6bc, 0x2}, {0xd51d0ed, 0x3}}, 0xc0004fbe28)
	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver@v0.149.0/internal/metadata/metadatastore.go:44 +0xfa
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver/internal/collection.(*DataCollector).CollectMetricData(0xc000000d20, {0xc002ab3180?, 0x0?, 0x1684d3a0?})
	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver@v0.149.0/internal/collection/collector.go:66 +0x108
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver.(*kubernetesReceiver).dispatchMetrics(0xc000000e00, {0xec3f350, 0xc002a987d0})
	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver@v0.149.0/receiver.go:162 +0x69
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver.(*kubernetesReceiver).startReceiver.func1()
	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver@v0.149.0/receiver.go:95 +0x233
created by github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver.(*kubernetesReceiver).startReceiver in goroutine 1
	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver@v0.149.0/receiver.go:64 +0x13c

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

Thank you for catching this before the release!

return 0, false
}

func TestRecordSpecMetrics(t *testing.T) {

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.

Thanks for the additional coverage. The preferred pattern in such tests is pkg/golden + pmetrictest.CompareMetrics. See the sibling pod package for reference. If that feels like too much, moving metricValue helper to receiver/k8sclusterreceiver/internal/testutils/metrics.go would at least keep it reusable.

@dmitryax dmitryax added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 10, 2026
@dmitryax dmitryax changed the title fix nil dereference in k8scluster receiver [receiver/k8s_cluster] [chore] Fix nil dereference in setting image attributes Apr 10, 2026
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

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.

This bug seems to be introduced in #46542 ind hasn't been released yet. So we don't need a changelog in this case

dmitryax added a commit that referenced this pull request Apr 10, 2026
…cs when container status is missing (#47554)

Replacing
#47538
as we need to merge it before the next release to avoid regression.

Guard `docker.ParseImageName` and `LastTerminationState` access under a
single cs != nil block to avoid a panic when a container spec exists but
kubelet has not yet reported its status.

Taking opportunity to increase test coverage of the
`receiver/k8sclusterreceiver/internal/container` package
@dmitryax

Copy link
Copy Markdown
Member

Fixed as part of #47554. Thank you, @kangyili!

@dmitryax dmitryax closed this Apr 10, 2026
AndrewCharlesHay pushed a commit to AndrewCharlesHay/opentelemetry-collector-contrib that referenced this pull request Apr 23, 2026
…cs when container status is missing (open-telemetry#47554)

Replacing
open-telemetry#47538
as we need to merge it before the next release to avoid regression.

Guard `docker.ParseImageName` and `LastTerminationState` access under a
single cs != nil block to avoid a panic when a container spec exists but
kubelet has not yet reported its status.

Taking opportunity to increase test coverage of the
`receiver/k8sclusterreceiver/internal/container` package
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

receiver/k8scluster Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants