Fixed docker diskio bug due to reseting of map. (elastic#5582) (elast…

…ic#5698) * Fixed docker diskio bug due to reseting of map. Fixes elastic#5568
leweafan · Nov 27, 2017 · 809118b · 809118b
1 parent a1b9dd0
commit 809118b
Show file tree

Hide file tree

Showing 3 changed files with 120 additions and 3 deletions.
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
@@ -40,6 +40,8 @@ https://github.com/elastic/beats/compare/v6.0.0...master[Check the HEAD diff]
 
 *Metricbeat*
 
+- Fix map overwrite in docker diskio module. {issue}5582[5582]
+
 *Packetbeat*
 
 *Winlogbeat*

diff --git a/metricbeat/module/docker/diskio/diskio_test.go b/metricbeat/module/docker/diskio/diskio_test.go
@@ -1,14 +1,129 @@
 package diskio
 
 import (
+	"encoding/json"
 	"testing"
 	"time"
+
+	"github.com/elastic/beats/metricbeat/module/docker"
+
+	dc "github.com/fsouza/go-dockerclient"
 )
 
 var blkioService BLkioService
 var oldBlkioRaw = make([]BlkioRaw, 3)
 var newBlkioRaw = make([]BlkioRaw, 3)
 
+func TestDeltaMultipleContainers(t *testing.T) {
+	var apiContainer1 docker.Stat
+	var apiContainer2 docker.Stat
+	metrics := dc.BlkioStatsEntry{
+		Major: 123,
+		Minor: 123,
+		Op:    "Total",
+		Value: 123,
+	}
+	jsonContainers := `[
+     {
+             "Id": "8dfafdbc3a40",
+			 "Names": ["container"]
+     },{
+             "Id": "8dfafdbc3a41",
+			 "Names": ["container1"]
+     }]`
+	var containers []dc.APIContainers
+	err := json.Unmarshal([]byte(jsonContainers), &containers)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	apiContainer1.Stats.Read = time.Now()
+	apiContainer1.Container = containers[0]
+	apiContainer1.Stats.BlkioStats.IOServicedRecursive = append(apiContainer1.Stats.BlkioStats.IOServicedRecursive, metrics)
+	apiContainer2.Stats.Read = time.Now()
+	apiContainer2.Container = containers[1]
+	apiContainer2.Stats.BlkioStats.IOServicedRecursive = append(apiContainer2.Stats.BlkioStats.IOServicedRecursive, metrics)
+	dockerStats := []docker.Stat{apiContainer1, apiContainer2}
+	stats := blkioService.getBlkioStatsList(dockerStats)
+	totals := make([]float64, 2)
+	for _, stat := range stats {
+		totals[0] = stat.totals
+	}
+
+	dockerStats[0].Stats.BlkioStats.IOServicedRecursive[0].Value = 1000
+	dockerStats[0].Stats.Read = dockerStats[0].Stats.Read.Add(time.Second * 10)
+	dockerStats[1].Stats.BlkioStats.IOServicedRecursive[0].Value = 1000
+	dockerStats[1].Stats.Read = dockerStats[0].Stats.Read.Add(time.Second * 10)
+	stats = blkioService.getBlkioStatsList(dockerStats)
+	for _, stat := range stats {
+		totals[1] = stat.totals
+		if stat.totals < totals[0] {
+			t.Errorf("getBlkioStatsList(%v) => %v, want value bigger than %v", dockerStats, stat.totals, totals[0])
+		}
+	}
+
+	dockerStats[0].Stats.Read = dockerStats[0].Stats.Read.Add(time.Second * 15)
+	dockerStats[0].Stats.BlkioStats.IOServicedRecursive[0].Value = 2000
+	dockerStats[1].Stats.BlkioStats.IOServicedRecursive[0].Value = 2000
+	dockerStats[1].Stats.Read = dockerStats[0].Stats.Read.Add(time.Second * 15)
+	stats = blkioService.getBlkioStatsList(dockerStats)
+	for _, stat := range stats {
+		if stat.totals < totals[1] || stat.totals < totals[0] {
+			t.Errorf("getBlkioStatsList(%v) => %v, want value bigger than %v", dockerStats, stat.totals, totals[1])
+		}
+	}
+
+}
+
+func TestDeltaOneContainer(t *testing.T) {
+	var apiContainer docker.Stat
+	metrics := dc.BlkioStatsEntry{
+		Major: 123,
+		Minor: 123,
+		Op:    "Total",
+		Value: 123,
+	}
+	jsonContainers := `
+     {
+             "Id": "8dfafdbc3a40",
+			 "Names": ["container"]
+     }`
+	var containers dc.APIContainers
+	err := json.Unmarshal([]byte(jsonContainers), &containers)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	apiContainer.Stats.Read = time.Now()
+	apiContainer.Container = containers
+	apiContainer.Stats.BlkioStats.IOServicedRecursive = append(apiContainer.Stats.BlkioStats.IOServicedRecursive, metrics)
+	dockerStats := []docker.Stat{apiContainer}
+	stats := blkioService.getBlkioStatsList(dockerStats)
+	totals := make([]float64, 2)
+	for _, stat := range stats {
+		totals[0] = stat.totals
+	}
+
+	dockerStats[0].Stats.BlkioStats.IOServicedRecursive[0].Value = 1000
+	dockerStats[0].Stats.Read = dockerStats[0].Stats.Read.Add(time.Second * 10)
+	stats = blkioService.getBlkioStatsList(dockerStats)
+	for _, stat := range stats {
+		if stat.totals < totals[0] {
+			t.Errorf("getBlkioStatsList(%v) => %v, want value bigger than %v", dockerStats, stat.totals, totals[0])
+		}
+	}
+
+	dockerStats[0].Stats.BlkioStats.IOServicedRecursive[0].Value = 2000
+	dockerStats[0].Stats.Read = dockerStats[0].Stats.Read.Add(time.Second * 15)
+	stats = blkioService.getBlkioStatsList(dockerStats)
+	for _, stat := range stats {
+		if stat.totals < totals[1] || stat.totals < totals[0] {
+			t.Errorf("getBlkioStatsList(%v) => %v, want value bigger than %v", dockerStats, stat.totals, totals[1])
+		}
+	}
+
+}
+
 func TestWritePs(t *testing.T) {
 	oldWritePs := []uint64{220, 951, 0}
 	newWritePs := []uint64{120, 2951, 0}

diff --git a/metricbeat/module/docker/diskio/helper.go b/metricbeat/module/docker/diskio/helper.go
@@ -35,7 +35,9 @@ type BLkioService struct {
 
 func (io *BLkioService) getBlkioStatsList(rawStats []docker.Stat) []BlkioStats {
 	formattedStats := []BlkioStats{}
-
+	if io.BlkioSTatsPerContainer == nil {
+		io.BlkioSTatsPerContainer = make(map[string]BlkioRaw)
+	}
 	for _, myRawStats := range rawStats {
 		formattedStats = append(formattedStats, io.getBlkioStats(&myRawStats))
 	}
@@ -56,8 +58,6 @@ func (io *BLkioService) getBlkioStats(myRawStat *docker.Stat) BlkioStats {
 		myBlkioStats.reads = io.getReadPs(&oldBlkioStats, &newBlkioStats)
 		myBlkioStats.writes = io.getWritePs(&oldBlkioStats, &newBlkioStats)
 		myBlkioStats.totals = io.getTotalPs(&oldBlkioStats, &newBlkioStats)
-	} else {
-		io.BlkioSTatsPerContainer = make(map[string]BlkioRaw)
 	}
 
 	io.BlkioSTatsPerContainer[myRawStat.Container.ID] = newBlkioStats