[chore] [receiver/k8s_cluster] Fix nil dereference in RecordSpecMetrics when container status is missing

receiver/k8sclusterreceiver/internal/container/containers.go

@@ -61,15 +61,17 @@ func RecordSpecMetrics(logger *zap.Logger, mb *metadata.MetricsBuilder, c corev1
 	}
 	e := metadata.NewK8sContainerEntity(stripContainerID(containerID))
 	e.SetK8sContainerName(c.Name)
-	if cs != nil && cs.LastTerminationState.Terminated != nil {
-		e.SetK8sContainerStatusLastTerminatedReason(cs.LastTerminationState.Terminated.Reason)
-	}
-	image, err := docker.ParseImageName(cs.Image)
-	if err != nil {
-		docker.LogParseError(err, cs.Image, logger)
-	} else {
-		e.SetContainerImageName(image.Repository)
-		e.SetContainerImageTag(image.Tag)
+	if cs != nil {
+		if cs.LastTerminationState.Terminated != nil {
+			e.SetK8sContainerStatusLastTerminatedReason(cs.LastTerminationState.Terminated.Reason)
+		}
+		image, err := docker.ParseImageName(cs.Image)
+		if err != nil {
+			docker.LogParseError(err, cs.Image, logger)
+		} else {
+			e.SetContainerImageName(image.Repository)
+			e.SetContainerImageTag(image.Tag)
+		}
 	}
 
 	e.SetK8sPodName(pod.Name)

receiver/k8sclusterreceiver/internal/container/containers_test.go

@@ -4,17 +4,161 @@
 package container
 
 import (
+	"path/filepath"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.opentelemetry.io/collector/pdata/pcommon"
+	"go.opentelemetry.io/collector/receiver/receivertest"
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+
+	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden"
+	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest/pmetrictest"
+	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver/internal/metadata"
 )
 
+var testPod = &corev1.Pod{
+	ObjectMeta: v1.ObjectMeta{
+		Name:      "test-pod",
+		Namespace: "test-namespace",
+		UID:       types.UID("test-pod-uid"),
+	},
+	Spec: corev1.PodSpec{
+		NodeName: "test-node",
+		Containers: []corev1.Container{
+			{
+				Name:  "test-container",
+				Image: "docker/test-image:v1.0",
+				Resources: corev1.ResourceRequirements{
+					Limits: corev1.ResourceList{
+						corev1.ResourceCPU:    resource.MustParse("2"),
+						corev1.ResourceMemory: resource.MustParse("512Mi"),
+					},
+					Requests: corev1.ResourceList{
+						corev1.ResourceCPU:    resource.MustParse("500m"),
+						corev1.ResourceMemory: resource.MustParse("256Mi"),
+					},
+				},
+			},
+		},
+	},
+}
+
+func TestRecordSpecMetrics(t *testing.T) {
+	tests := []struct {
+		name            string
+		containerStatus *corev1.ContainerStatus
+		metricsConfig   func(metadata.MetricsBuilderConfig) metadata.MetricsBuilderConfig
+		expectedFile    string
+	}{
+		{
+			name: "running container",
+			containerStatus: &corev1.ContainerStatus{
+				Name:         "test-container",
+				Image:        "docker/test-image:v1.0",
+				ContainerID:  "docker://abc123",
+				Ready:        true,
+				RestartCount: 2,
+				State: corev1.ContainerState{
+					Running: &corev1.ContainerStateRunning{},
+				},
+			},
+			expectedFile: "expected_running.yaml",
+		},
+		{
+			name: "terminated container",
+			containerStatus: &corev1.ContainerStatus{
+				Name:         "test-container",
+				Image:        "docker/test-image:v1.0",
+				ContainerID:  "docker://def456",
+				Ready:        false,
+				RestartCount: 5,
+				State: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{
+						Reason:   "OOMKilled",
+						ExitCode: 137,
+					},
+				},
+				LastTerminationState: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{
+						Reason: "OOMKilled",
+					},
+				},
+			},
+			metricsConfig: func(mbc metadata.MetricsBuilderConfig) metadata.MetricsBuilderConfig {
+				mbc.Metrics.K8sContainerStatusState.Enabled = true
+				mbc.Metrics.K8sContainerStatusReason.Enabled = true
+				return mbc
+			},
+			expectedFile: "expected_terminated.yaml",
+		},
+		{
+			name: "waiting container",
+			containerStatus: &corev1.ContainerStatus{
+				Name:         "test-container",
+				Image:        "docker/test-image:v1.0",
+				Ready:        false,
+				RestartCount: 3,
+				State: corev1.ContainerState{
+					Waiting: &corev1.ContainerStateWaiting{
+						Reason: "CrashLoopBackOff",
+					},
+				},
+			},
+			metricsConfig: func(mbc metadata.MetricsBuilderConfig) metadata.MetricsBuilderConfig {
+				mbc.Metrics.K8sContainerStatusState.Enabled = true
+				mbc.Metrics.K8sContainerStatusReason.Enabled = true
+				return mbc
+			},
+			expectedFile: "expected_waiting.yaml",
+		},
+		{
+			name:            "no matching container status",
+			containerStatus: nil,
+			expectedFile:    "expected_no_status.yaml",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			pod := testPod.DeepCopy()
+			if tt.containerStatus != nil {
+				pod.Status.ContainerStatuses = []corev1.ContainerStatus{*tt.containerStatus}
+			}
+
+			mbc := metadata.DefaultMetricsBuilderConfig()
+			if tt.metricsConfig != nil {
+				mbc = tt.metricsConfig(mbc)
+			}
+			mb := metadata.NewMetricsBuilder(mbc, receivertest.NewNopSettings(metadata.Type))
+			ts := pcommon.Timestamp(time.Now().UnixNano())
+			assert.NotPanics(t, func() {
+				RecordSpecMetrics(zap.NewNop(), mb, pod.Spec.Containers[0], pod, ts)
+			})
+			m := mb.Emit()
+
+			expectedFile := filepath.Join("testdata", tt.expectedFile)
+			// golden.WriteMetrics(t, expectedFile, m)
+			expected, err := golden.ReadMetrics(expectedFile)
+			require.NoError(t, err)
+			require.NoError(t, pmetrictest.CompareMetrics(expected, m,
+				pmetrictest.IgnoreTimestamp(),
+				pmetrictest.IgnoreStartTimestamp(),
+				pmetrictest.IgnoreResourceMetricsOrder(),
+				pmetrictest.IgnoreMetricsOrder(),
+				pmetrictest.IgnoreScopeMetricsOrder(),
+				pmetrictest.IgnoreMetricDataPointsOrder(),
+			))
+		})
+	}
+}
+
 func TestGetMetadata(t *testing.T) {
 	refTime := v1.Now()
 	pod := &corev1.Pod{

receiver/k8sclusterreceiver/internal/container/testdata/expected_no_status.yaml

@@ -0,0 +1,73 @@
+resourceMetrics:
+  - resource:
+      attributes:
+        - key: container.id
+          value:
+            stringValue: ""
+        - key: container.image.name
+          value:
+            stringValue: ""
+        - key: container.image.tag
+          value:
+            stringValue: ""
+        - key: k8s.container.name
+          value:
+            stringValue: test-container
+        - key: k8s.namespace.name
+          value:
+            stringValue: test-namespace
+        - key: k8s.node.name
+          value:
+            stringValue: test-node
+        - key: k8s.pod.name
+          value:
+            stringValue: test-pod
+        - key: k8s.pod.uid
+          value:
+            stringValue: test-pod-uid
+      entityRefs:
+        - descriptionKeys:
+            - k8s.container.name
+            - container.image.name
+            - container.image.tag
+          idKeys:
+            - container.id
+          type: k8s.container
+    schemaUrl: https://opentelemetry.io/schemas/1.18.0
+    scopeMetrics:
+      - metrics:
+          - description: Maximum resource limit set for the container. See https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#resourcerequirements-v1-core for details
+            gauge:
+              dataPoints:
+                - asDouble: 2
+                  startTimeUnixNano: "1000000"
+                  timeUnixNano: "2000000"
+            name: k8s.container.cpu_limit
+            unit: '{cpu}'
+          - description: Resource requested for the container. See https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#resourcerequirements-v1-core for details
+            gauge:
+              dataPoints:
+                - asDouble: 0.5
+                  startTimeUnixNano: "1000000"
+                  timeUnixNano: "2000000"
+            name: k8s.container.cpu_request
+            unit: '{cpu}'
+          - description: Maximum resource limit set for the container. See https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#resourcerequirements-v1-core for details
+            gauge:
+              dataPoints:
+                - asInt: "536870912"
+                  startTimeUnixNano: "1000000"
+                  timeUnixNano: "2000000"
+            name: k8s.container.memory_limit
+            unit: By
+          - description: Resource requested for the container. See https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#resourcerequirements-v1-core for details
+            gauge:
+              dataPoints:
+                - asInt: "268435456"
+                  startTimeUnixNano: "1000000"
+                  timeUnixNano: "2000000"
+            name: k8s.container.memory_request
+            unit: By
+        scope:
+          name: github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver
+          version: latest

receiver/k8sclusterreceiver/internal/container/testdata/expected_running.yaml

@@ -0,0 +1,88 @@
+resourceMetrics:
+  - resource:
+      attributes:
+        - key: container.id
+          value:
+            stringValue: abc123
+        - key: container.image.name
+          value:
+            stringValue: docker/test-image
+        - key: container.image.tag
+          value:
+            stringValue: v1.0
+        - key: k8s.container.name
+          value:
+            stringValue: test-container
+        - key: k8s.namespace.name
+          value:
+            stringValue: test-namespace
+        - key: k8s.node.name
+          value:
+            stringValue: test-node
+        - key: k8s.pod.name
+          value:
+            stringValue: test-pod
+        - key: k8s.pod.uid
+          value:
+            stringValue: test-pod-uid
+      entityRefs:
+        - descriptionKeys:
+            - k8s.container.name
+            - container.image.name
+            - container.image.tag
+          idKeys:
+            - container.id
+          type: k8s.container
+    schemaUrl: https://opentelemetry.io/schemas/1.18.0
+    scopeMetrics:
+      - metrics:
+          - description: Maximum resource limit set for the container. See https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#resourcerequirements-v1-core for details
+            gauge:
+              dataPoints:
+                - asDouble: 2
+                  startTimeUnixNano: "1000000"
+                  timeUnixNano: "2000000"
+            name: k8s.container.cpu_limit
+            unit: '{cpu}'
+          - description: Resource requested for the container. See https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#resourcerequirements-v1-core for details
+            gauge:
+              dataPoints:
+                - asDouble: 0.5
+                  startTimeUnixNano: "1000000"
+                  timeUnixNano: "2000000"
+            name: k8s.container.cpu_request
+            unit: '{cpu}'
+          - description: Maximum resource limit set for the container. See https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#resourcerequirements-v1-core for details
+            gauge:
+              dataPoints:
+                - asInt: "536870912"
+                  startTimeUnixNano: "1000000"
+                  timeUnixNano: "2000000"
+            name: k8s.container.memory_limit
+            unit: By
+          - description: Resource requested for the container. See https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#resourcerequirements-v1-core for details
+            gauge:
+              dataPoints:
+                - asInt: "268435456"
+                  startTimeUnixNano: "1000000"
+                  timeUnixNano: "2000000"
+            name: k8s.container.memory_request
+            unit: By
+          - description: Whether a container has passed its readiness probe (0 for no, 1 for yes)
+            gauge:
+              dataPoints:
+                - asInt: "1"
+                  startTimeUnixNano: "1000000"
+                  timeUnixNano: "2000000"
+            name: k8s.container.ready
+          - description: How many times the container has restarted in the recent past. This value is pulled directly from the K8s API and the value can go indefinitely high and be reset to 0 at any time depending on how your kubelet is configured to prune dead containers. It is best to not depend too much on the exact value but rather look at it as either == 0, in which case you can conclude there were no restarts in the recent past, or > 0, in which case you can conclude there were restarts in the recent past, and not try and analyze the value beyond that.
+            gauge:
+              dataPoints:
+                - asInt: "2"
+                  startTimeUnixNano: "1000000"
+                  timeUnixNano: "2000000"
+            name: k8s.container.restarts
+            unit: '{restart}'
+        scope:
+          name: github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8sclusterreceiver
+          version: latest