Repository: honeycombio/honeycomb-kubernetes-agent
Pull request #262 (2022-06-06 12:30:13)

bug: Only return cpu.utilization if a limit was provided
<!--
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?
It's not possible to calculate CPU utilisation without a limit. The current implementation still adds the cpu.utilization metric to the returned map but with a value of 0. This PR ensures the `cpu.utilization` metric is only returned when a limit is provided.

## Short description of the changes
- Create metrics map and add cpu usage, only add cpu util if a limit was provided
- Add unit test case where limit is 0 and cpu.util metric is not returned


Patch:
diff --git a/metrics/processor.go b/metrics/processor.go
index eaabd8c..db136e5 100644
--- a/metrics/processor.go
+++ b/metrics/processor.go
@@ -128,16 +128,17 @@ func (p *Processor) CpuMetrics(s *stats.CPUStats, limit float64) Metrics {
 		cpuUsage = float64(*nanoCores) / 1000000000
 	}
 
+	metrics := Metrics{
+		MeasureCpuUsage: &Metric{Type: MetricTypeFloat, FloatValue: &cpuUsage},
+	}
+
 	// if resource limits defined, get utilization
-	var cpuUtilization float64
 	if limit > 0 {
-		cpuUtilization = (cpuUsage / limit) * 100
+		util := (cpuUsage / limit) * 100
+		metrics[MeasureCpuUtilization] = &Metric{Type: MetricTypeFloat, FloatValue: &util}
 	}
 
-	return Metrics{
-		MeasureCpuUsage:       &Metric{Type: MetricTypeFloat, FloatValue: &cpuUsage},
-		MeasureCpuUtilization: &Metric{Type: MetricTypeFloat, FloatValue: &cpuUtilization},
-	}
+	return metrics
 }
 
 func (p *Processor) MemMetrics(s *stats.MemoryStats, limit float64) Metrics {


Test patch:
diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go
index 9a61b98..c791ecb 100644
--- a/metrics/metrics_test.go
+++ b/metrics/metrics_test.go
@@ -200,6 +200,17 @@ func TestCpuMetrics(t *testing.T) {
 	assert.InDelta(t, 0.000001, 7.6691825, metrics[MeasureCpuUtilization].GetValue(), "CPU Utilization")
 }
 
+func TestCpuMetricsWithoutLimit(t *testing.T) {
+	summary, _ := createMockSourceAssets(true, false, nil, false)
+
+	p := NewMetricsProcessor(10 * time.Second)
+	metrics := p.CpuMetrics(summary.Pods[0].CPU, 0)
+
+	assert.Equal(t, 1, len(metrics))
+	assert.Equal(t, 0.015338365, metrics[MeasureCpuUsage].GetValue(), "CPU Usage")
+	assert.Nil(t, metrics[MeasureCpuUtilization])
+}
+
 func TestCpuMetricsOptional(t *testing.T) {
 	summary, _ := createMockSourceAssets(true, false, nil, false)
 
@@ -334,7 +345,7 @@ func TestNodeStats(t *testing.T) {
 	assert.Equal(t, 0, len(data.Resource.Status))
 	assert.Equal(t, 9, len(data.Resource.Labels))
 	assert.Equal(t, "us-east-1", data.Resource.Labels[PrefixLabel+"topology.kubernetes.io/region"])
-	assert.Equal(t, 17, len(data.Metrics))
+	assert.Equal(t, 16, len(data.Metrics))
 	assert.Equal(t, float64(388954406)/1000000000, data.Metrics[MeasureCpuUsage].GetValue())
 
 	// without includeNodeLabels
diff --git a/tailer/tailer_test.go b/tailer/tailer_test.go
index d92b885..5b299a7 100644
--- a/tailer/tailer_test.go
+++ b/tailer/tailer_test.go
@@ -154,6 +154,7 @@ func TestPathWatching(t *testing.T) {
 	assert.Equal(t, 2, len(handlerFactory.handlers))
 	for k, h := range handlerFactory.handlers {
 		fmt.Println(k)
+		fmt.Println(h.lines)
 		assert.Equal(t, 2, len(h.lines))
 	}
 	assert.Equal(t, len(watcher.tailers), 0)