From 681d20083a302769f1c774041807af29ba3db521 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Yann=20C=C3=A9zard?= <eesprit@users.noreply.github.com>
Date: Fri, 21 Jul 2017 23:25:17 +0200
Subject: [PATCH] Only report cpu usage for online cpus in docker input (#3035)

---
 Godeps                               |  2 +-
 plugins/inputs/docker/docker.go      | 17 +++++++++++++++--
 plugins/inputs/docker/docker_test.go | 19 +++++++++++++++++--
 testutil/accumulator.go              | 22 ++++++++++++++++++++++
 4 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/Godeps b/Godeps
index b538624ad..a4f2725a2 100644
--- a/Godeps
+++ b/Godeps
@@ -11,7 +11,7 @@ github.com/couchbase/go-couchbase bfe555a140d53dc1adf390f1a1d4b0fd4ceadb28
 github.com/couchbase/gomemcached 4a25d2f4e1dea9ea7dd76dfd943407abf9b07d29
 github.com/couchbase/goutils 5823a0cbaaa9008406021dc5daf80125ea30bba6
 github.com/davecgh/go-spew 346938d642f2ec3594ed81d874461961cd0faa76
-github.com/docker/docker b89aff1afa1f61993ab2ba18fd62d9375a195f5d
+github.com/docker/docker f5ec1e2936dcbe7b5001c2b817188b095c700c27
 github.com/eapache/go-resiliency b86b1ec0dd4209a588dc1285cdd471e73525c0b3
 github.com/eapache/go-xerial-snappy bb955e01b9346ac19dc29eb16586c90ded99a98c
 github.com/eapache/queue 44cc805cf13205b55f69e14bcb69867d1ae92f98
diff --git a/plugins/inputs/docker/docker.go b/plugins/inputs/docker/docker.go
index ca7b70f9f..eeeefd2e6 100644
--- a/plugins/inputs/docker/docker.go
+++ b/plugins/inputs/docker/docker.go
@@ -446,7 +446,16 @@ func gatherContainerStats(
 	cputags["cpu"] = "cpu-total"
 	acc.AddFields("docker_container_cpu", cpufields, cputags, now)
 
-	for i, percpu := range stat.CPUStats.CPUUsage.PercpuUsage {
+	// If we have OnlineCPUs field, then use it to restrict stats gathering to only Online CPUs
+	// (https://github.com/moby/moby/commit/115f91d7575d6de6c7781a96a082f144fd17e400)
+	var percpuusage []uint64
+	if stat.CPUStats.OnlineCPUs > 0 {
+		percpuusage = stat.CPUStats.CPUUsage.PercpuUsage[:stat.CPUStats.OnlineCPUs]
+	} else {
+		percpuusage = stat.CPUStats.CPUUsage.PercpuUsage
+	}
+
+	for i, percpu := range percpuusage {
 		percputags := copyTags(tags)
 		percputags["cpu"] = fmt.Sprintf("cpu%d", i)
 		fields := map[string]interface{}{
@@ -527,7 +536,11 @@ func calculateCPUPercent(stat *types.StatsJSON) float64 {
 	systemDelta := float64(stat.CPUStats.SystemUsage) - float64(stat.PreCPUStats.SystemUsage)
 
 	if systemDelta > 0.0 && cpuDelta > 0.0 {
-		cpuPercent = (cpuDelta / systemDelta) * float64(len(stat.CPUStats.CPUUsage.PercpuUsage)) * 100.0
+		if stat.CPUStats.OnlineCPUs > 0 {
+			cpuPercent = (cpuDelta / systemDelta) * float64(stat.CPUStats.OnlineCPUs) * 100.0
+		} else {
+			cpuPercent = (cpuDelta / systemDelta) * float64(len(stat.CPUStats.CPUUsage.PercpuUsage)) * 100.0
+		}
 	}
 	return cpuPercent
 }
diff --git a/plugins/inputs/docker/docker_test.go b/plugins/inputs/docker/docker_test.go
index 45797df5c..29b0af8c8 100644
--- a/plugins/inputs/docker/docker_test.go
+++ b/plugins/inputs/docker/docker_test.go
@@ -141,14 +141,29 @@ func TestDockerGatherContainerStats(t *testing.T) {
 		"container_id": "123456789",
 	}
 	acc.AssertContainsTaggedFields(t, "docker_container_cpu", cpu1fields, cputags)
+
+	// Those tagged filed should not be present because of offline CPUs
+	cputags["cpu"] = "cpu2"
+	cpu2fields := map[string]interface{}{
+		"usage_total":  uint64(0),
+		"container_id": "123456789",
+	}
+	acc.AssertDoesNotContainsTaggedFields(t, "docker_container_cpu", cpu2fields, cputags)
+
+	cputags["cpu"] = "cpu3"
+	cpu3fields := map[string]interface{}{
+		"usage_total":  uint64(0),
+		"container_id": "123456789",
+	}
+	acc.AssertDoesNotContainsTaggedFields(t, "docker_container_cpu", cpu3fields, cputags)
 }
 
 func testStats() *types.StatsJSON {
 	stats := &types.StatsJSON{}
 	stats.Read = time.Now()
 	stats.Networks = make(map[string]types.NetworkStats)
-
-	stats.CPUStats.CPUUsage.PercpuUsage = []uint64{1, 1002}
+	stats.CPUStats.OnlineCPUs = 2
+	stats.CPUStats.CPUUsage.PercpuUsage = []uint64{1, 1002, 0, 0}
 	stats.CPUStats.CPUUsage.UsageInUsermode = 100
 	stats.CPUStats.CPUUsage.TotalUsage = 500
 	stats.CPUStats.CPUUsage.UsageInKernelmode = 200
diff --git a/testutil/accumulator.go b/testutil/accumulator.go
index e46476a8b..fdca47b07 100644
--- a/testutil/accumulator.go
+++ b/testutil/accumulator.go
@@ -258,6 +258,28 @@ func (a *Accumulator) AssertContainsTaggedFields(
 	assert.Fail(t, msg)
 }
 
+func (a *Accumulator) AssertDoesNotContainsTaggedFields(
+	t *testing.T,
+	measurement string,
+	fields map[string]interface{},
+	tags map[string]string,
+) {
+	a.Lock()
+	defer a.Unlock()
+	for _, p := range a.Metrics {
+		if !reflect.DeepEqual(tags, p.Tags) {
+			continue
+		}
+
+		if p.Measurement == measurement {
+			assert.Equal(t, fields, p.Fields)
+			msg := fmt.Sprintf("found measurement %s with tags %v which should not be there", measurement, tags)
+			assert.Fail(t, msg)
+		}
+	}
+	return
+}
+
 func (a *Accumulator) AssertContainsFields(
 	t *testing.T,
 	measurement string,