Repository: honeycombio/honeycomb-kubernetes-agent (go)

Issue:
CrashLoopBackOff on nil pointer deref
**Versions**
2.2.1

<!-- version of honeycomb-kubernetes-agent and any other relevant bits -->

**Steps to reproduce**

1. Unclear - but I've seen this twice today in dogfood, on different pods.

```
time="2021-12-29T18:20:04Z" level=info msg="Done tailing file" filePath=/var/log/pods/honeycomb-dogfood_basset-27346685-x7ztx_b9bf7535-88e7-486d-b60a-0ab718c596bd/basset/0.log
time="2021-12-29T18:21:00Z" level=info msg="Setting up watcher for pod" Name=basset-27346701-tz9hl UID=7b52bcb8-975c-4bc4-b277-ad3af9b98e1d
time="2021-12-29T18:21:00Z" level=info msg="starting watcher for pod" labelselector="app,app!=honeycomb-agent" name=basset-27346701-tz9hl namespace=honeycomb-dogfood uid=7b52bcb8-975c-4bc4-b277-ad3af9b98e1d
time="2021-12-29T18:21:02Z" level=info msg="Log pattern" log pattern="/var/log/pods/honeycomb-dogfood_basset-27346701-tz9hl_7b52bcb8-975c-4bc4-b277-ad3af9b98e1d/*/*"
time="2021-12-29T18:21:02Z" level=info msg="Initializing file handler" parser=nop path=/var/log/pods/honeycomb-dogfood_basset-27346701-tz9hl_7b52bcb8-975c-4bc4-b277-ad3af9b98e1d/basset/0.log
time="2021-12-29T18:21:02Z" level=info msg="no prior tail state found" detail="key not found" path=/var/log/pods/honeycomb-dogfood_basset-27346701-tz9hl_7b52bcb8-975c-4bc4-b277-ad3af9b98e1d/basset/0.log
time="2021-12-29T18:21:02Z" level=info msg="Tailing file" offset=0 path=/var/log/pods/honeycomb-dogfood_basset-27346701-tz9hl_7b52bcb8-975c-4bc4-b277-ad3af9b98e1d/basset/0.log
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x64c5a4]

goroutine 65 [running]:
github.com/honeycombio/honeycomb-kubernetes-agent/metrics.getPodResource(0x400057c140, 0x4000b0f320, 0x12, 0x4000b0f338, 0x11, 0x4000b9e2a0, 0x24, 0x0, 0xed95e9e0c, 0x1819560, ...)
	/home/circleci/project/metrics/resource.go:55 +0x2a4
github.com/honeycombio/honeycomb-kubernetes-agent/metrics.(*Processor).GenerateMetricsData(0x4000b8a070, 0x4002452000, 0x4000c0b4a0, 0x4000ae5620, 0x1, 0xa2c7d202020200a, 0x6d656d2220202020)
	/home/circleci/project/metrics/processor.go:41 +0x25c
github.com/honeycombio/honeycomb-kubernetes-agent/service.(*runnable).Run(0x40001cfd50, 0x400298bc10, 0x1)
	/home/circleci/project/service/runnable.go:82 +0x3c4
github.com/honeycombio/honeycomb-kubernetes-agent/interval.(*Runner).run(0x4000a72f40, 0x0, 0x0)
	/home/circleci/project/interval/interval_runner.go:72 +0xa4
github.com/honeycombio/honeycomb-kubernetes-agent/interval.(*Runner).Start(0x4000a72f40, 0x0, 0x0)
	/home/circleci/project/interval/interval_runner.go:50 +0x1e4
github.com/honeycombio/honeycomb-kubernetes-agent/service.(*Service).Start.func1(0x40001cfb90)
	/home/circleci/project/service/service.go:138 +0x2c
created by github.com/honeycombio/honeycomb-kubernetes-agent/service.(*Service).Start
	/home/circleci/project/service/service.go:137 +0x458
```

**Additional context**

Pull request:
fix: check for existence of optional CPU stats to prevent exceptions
<!--
Thank you for contributing to the project! 💜
Please make sure to:
- Chat with us first if this is a big change
  - Open a new issue (or comment on an existing one)
  - We want to make sure you don't spend time implementing something we might have to say No to
- Add unit tests
- Mention any relevant issues in the PR description (e.g. "Fixes #123")

Please see our [OSS process document](https://github.com/honeycombio/home/blob/main/honeycomb-oss-lifecycle-and-practices.md#) to get an idea of how we operate.
-->

## Which problem is this PR solving?

- fixes #204 

## Short description of the changes

- check for CPUStats because it's an optional field. In the issue reported, an error was thrown on `s.CPU.Time.Time`. Though `Time` is a required field on the `CPUStats` struct, the `CPUStats` are optional on the `NodeStats` struct. We can check for existence of CPUStats and only add if it exists.
- This PR checks for existence of CPU, as well as optional fields on CPUStats
- Current tests checked for proper handling of CPU Stats when available, but did not check for proper handling of omitted CPU Stats. I updated the second pod in `stats-summary.json` to have no CPU Stats, and added test to assert against it. 
- Other similar scenarios (volume, fs, etc) will be addressed in a separate PR.

Patch:
diff --git a/metrics/processor.go b/metrics/processor.go
index 18f81a7..634e893 100644
--- a/metrics/processor.go
+++ b/metrics/processor.go
@@ -122,9 +122,13 @@ func (p *Processor) UptimeMetrics(startTime time.Time) Metrics {
 }
 
 func (p *Processor) CpuMetrics(s *stats.CPUStats, limit float64) Metrics {
+	if s == nil {
+		return nil
+	}
 	// Convert nanoCores to seconds
 	var cpuUsage float64
 	nanoCores := s.UsageNanoCores
+
 	if nanoCores != nil {
 		cpuUsage = float64(*nanoCores) / 1000000000
 	}
diff --git a/metrics/resource.go b/metrics/resource.go
index 16f1ee8..a56db9b 100644
--- a/metrics/resource.go
+++ b/metrics/resource.go
@@ -20,22 +20,25 @@ func getNodeResource(s stats.NodeStats, metadata *Metadata) *Resource {
 	labels := map[string]string{
 		LabelNodeName: s.NodeName,
 	}
-
 	if metadata.IncludeNodeLabels {
 		nodeMetadata, err := metadata.GetNodeMetadataByName(s.NodeName)
 		if err == nil {
-            for k, v := range nodeMetadata.GetLabels() {
-                labels[PrefixLabel+k] = v
-            }
-        }
+			for k, v := range nodeMetadata.GetLabels() {
+				labels[PrefixLabel+k] = v
+			}
+		}
+	}
+	resource := &Resource{
+		Type:   "node",
+		Name:   s.NodeName,
+		Labels: labels,
 	}
 
-	return &Resource{
-		Type:      "node",
-		Name:      s.NodeName,
-		Labels:    labels,
-		Timestamp: s.CPU.Time.Time,
+	if s.CPU != nil {
+		resource.Timestamp = s.CPU.Time.Time
 	}
+
+	return resource
 }
 
 func getPodResource(node *Resource, s stats.PodStats, metadata *Metadata) *Resource {
@@ -45,27 +48,28 @@ func getPodResource(node *Resource, s stats.PodStats, metadata *Metadata) *Resou
 		LabelPodName:       s.PodRef.Name,
 	}
 
+	resource := &Resource{
+		Type:   "pod",
+		Name:   s.PodRef.Name,
+		Labels: labels,
+	}
+
+	if s.CPU != nil {
+		resource.Timestamp = s.CPU.Time.Time
+	}
+
 	podMetadata, err := metadata.GetPodMetadataByUid(types.UID(s.PodRef.UID))
 	if err != nil {
-		return &Resource{
-			Type:      "pod",
-			Name:      s.PodRef.Name,
-			Labels:    labels,
-			Timestamp: s.CPU.Time.Time,
-		}
+		return resource
 	}
 
 	status := podMetadata.GetStatus()
 	addAdditionalLabels(labels, node.Labels, podMetadata)
 
-	return &Resource{
-		Type:        "pod",
-		Name:        s.PodRef.Name,
-		Labels:      labels,
-		Timestamp:   s.CPU.Time.Time,
-		Status:      status,
-		PodMetadata: podMetadata,
-	}
+	resource.Status = status
+	resource.PodMetadata = podMetadata
+
+	return resource
 
 }
 
@@ -78,14 +82,17 @@ func getContainerResource(pod *Resource, s stats.ContainerStats) (*Resource, err
 
 	status := pod.PodMetadata.GetStatusForContainer(s.Name)
 
-	return &Resource{
+	resource := &Resource{
 		Type:        "container",
 		Name:        s.Name,
 		Labels:      labels,
-		Timestamp:   s.CPU.Time.Time,
 		Status:      status,
 		PodMetadata: pod.PodMetadata,
-	}, nil
+	}
+	if s.CPU != nil {
+		resource.Timestamp = s.CPU.Time.Time
+	}
+	return resource, nil
 }
 
 func getVolumeResource(pod *Resource, s stats.VolumeStats) *Resource {


Test patch:
diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go
index 92af40a..b903dec 100644
--- a/metrics/metrics_test.go
+++ b/metrics/metrics_test.go
@@ -202,6 +202,18 @@ func TestCpuMetrics(t *testing.T) {
 	assert.InDelta(t, 0.000001, 7.6691825, metrics[MeasureCpuUtilization].GetValue(), "CPU Utilization")
 }
 
+func TestCpuMetricsOptional(t *testing.T) {
+	summary, _ := createMockSourceAssets(true, false, nil, false)
+
+	p := NewMetricsProcessor(10*time.Second, logrus.StandardLogger())
+
+	metrics := p.CpuMetrics(summary.Pods[1].CPU, 0)
+
+	require.Equal(t, 0, len(metrics))
+	assert.Empty(t, metrics[MeasureCpuUsage])
+	assert.Empty(t, metrics[MeasureCpuUtilization])
+}
+
 func TestMemMetrics(t *testing.T) {
 	summary, _ := createMockSourceAssets(true, false, nil, false)
 
diff --git a/testdata/stats-summary.json b/testdata/stats-summary.json
index 531ff38..f8cf69a 100644
--- a/testdata/stats-summary.json
+++ b/testdata/stats-summary.json
@@ -348,10 +348,8 @@
      }
     }
    ],
-   "cpu": {
-    "time": "2020-09-16T03:30:56Z",
-    "usageNanoCores": 476961,
-    "usageCoreNanoSeconds": 656759728006
+   "test-empty-cpu": {
+    "time": "2020-09-16T03:30:56Z"
    },
    "memory": {
     "time": "2020-09-16T03:30:56Z",