fix: do not accumulate if pod is not there anymore #333 (#348)

* do not accumulate if pod is not there anymore * write simple test and return nil, for better checking * Update metrics/accumulator.go --------- Co-authored-by: Tyler Helmuth <[email protected]>
honeycombio · Mar 13, 2023 · 4660da8 · 4660da8
1 parent fef6119
commit 4660da8
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 1 deletion.
diff --git a/metrics/accumulator.go b/metrics/accumulator.go
@@ -59,6 +59,12 @@ func (a *MetricDataAccumulator) podStats(podResource *Resource, s stats.PodStats
 	if !a.metricGroupsToCollect[PodMetricGroup] {
 		return
 	}
+
+	// Metatdata can be nil if pod is terminated before metadata is fetched.
+	// no metrics are needed.
+	if podResource.PodMetadata == nil {
+		return
+	}
 
 	a.accumulate(
 		podResource,

diff --git a/metrics/metadata.go b/metrics/metadata.go
@@ -50,7 +50,7 @@ func (m *Metadata) GetPodMetadataByUid(uid types.UID) (*PodMetadata, error) {
 	logrus.WithFields(logrus.Fields{
 		"podUid": uid,
 	}).Error("Metadata: Pod not found")
-	return &PodMetadata{}, errors.New("pod not found")
+	return nil, errors.New("pod not found")
 }
 
 type NodeMetadata struct {

diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go
@@ -360,6 +360,20 @@ func TestNodeStats(t *testing.T) {
 	assert.Equal(t, 1, len(data.Resource.Labels))
 }
 
+func TestPodStatsOnDeadPod(t *testing.T) {
+	summary, metadata := createMockSourceAssets(true, true, nil, true)
+	acc := createMockAccumulator(metadata, ValidMetricGroups)
+
+	podStats := getPodStatsByUID(summary.Pods, "5997ad9b-1d2a-43cf-ab57-a98d8796dc34")
+	node := getNodeResource(summary.Node, metadata)
+	pod := getPodResource(node, podStats, metadata)
+	pod.PodMetadata = nil
+
+	acc.podStats(pod, podStats)
+	assert.Equal(t, 0, len(acc.Data))
+
+}
+
 func TestPodStats(t *testing.T) {
 	summary, metadata := createMockSourceAssets(true, true, nil, true)
 	acc := createMockAccumulator(metadata, ValidMetricGroups)