From 10415c58ca5f899577165101d6a964fda34ab26e Mon Sep 17 00:00:00 2001 From: kwojcicki Date: Tue, 14 Nov 2017 11:15:48 -0500 Subject: [PATCH 1/5] Fixed docker diskio bug due to reseting of map. --- .../module/docker/diskio/diskio_test.go | 118 ++++++++++++++++++ metricbeat/module/docker/diskio/helper.go | 6 +- 2 files changed, 121 insertions(+), 3 deletions(-) diff --git a/metricbeat/module/docker/diskio/diskio_test.go b/metricbeat/module/docker/diskio/diskio_test.go index 461a1fdfdae..10ce192b5de 100644 --- a/metricbeat/module/docker/diskio/diskio_test.go +++ b/metricbeat/module/docker/diskio/diskio_test.go @@ -1,14 +1,132 @@ 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 := make([]docker.Stat, 0) + dockerStats = append(dockerStats, apiContainer1) + dockerStats = append(dockerStats, apiContainer2) + stats := blkioService.getBlkioStatsList(dockerStats) + totals := make([]float64, 2, 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 := make([]docker.Stat, 0) + dockerStats = append(dockerStats, apiContainer) + stats := blkioService.getBlkioStatsList(dockerStats) + totals := make([]float64, 2, 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 index 11cf75feae0..6f71c6fdb54 100644 --- 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 From 49d07fd50a9855f114ab6ada8c836918f06629b8 Mon Sep 17 00:00:00 2001 From: kwojcicki Date: Wed, 15 Nov 2017 18:29:08 -0500 Subject: [PATCH 2/5] Updated changelog --- CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index d68f9989ae9..eb0ad333727 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -69,6 +69,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di - Fix the loading of 5.x dashboards. {issue}5277[5277] - Fix the fetching of process information when some data is missing under MacOS X. {issue}5337[5337] - Change `MySQL active connections` visualization title to `MySQL total connections`. {issue}4812[4812] +- Fix map overwrite in docker diskio module. {issue}5582[5582] *Packetbeat* From bcb4abcde8c5dd13bf9cb4ce882c0f7a0e9ca300 Mon Sep 17 00:00:00 2001 From: kwojcicki Date: Wed, 15 Nov 2017 18:36:46 -0500 Subject: [PATCH 3/5] Addressed hound comments --- metricbeat/module/docker/diskio/diskio_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/metricbeat/module/docker/diskio/diskio_test.go b/metricbeat/module/docker/diskio/diskio_test.go index 10ce192b5de..5f911ef37ed 100644 --- a/metricbeat/module/docker/diskio/diskio_test.go +++ b/metricbeat/module/docker/diskio/diskio_test.go @@ -43,9 +43,7 @@ func TestDeltaMultipleContainers(t *testing.T) { apiContainer2.Stats.Read = time.Now() apiContainer2.Container = containers[1] apiContainer2.Stats.BlkioStats.IOServicedRecursive = append(apiContainer2.Stats.BlkioStats.IOServicedRecursive, metrics) - dockerStats := make([]docker.Stat, 0) - dockerStats = append(dockerStats, apiContainer1) - dockerStats = append(dockerStats, apiContainer2) + dockerStats := []docker.Stat{apiContainer1, apiContainer2} stats := blkioService.getBlkioStatsList(dockerStats) totals := make([]float64, 2, 2) for _, stat := range stats { @@ -99,8 +97,7 @@ func TestDeltaOneContainer(t *testing.T) { apiContainer.Stats.Read = time.Now() apiContainer.Container = containers apiContainer.Stats.BlkioStats.IOServicedRecursive = append(apiContainer.Stats.BlkioStats.IOServicedRecursive, metrics) - dockerStats := make([]docker.Stat, 0) - dockerStats = append(dockerStats, apiContainer) + dockerStats := []docker.Stat{apiContainer} stats := blkioService.getBlkioStatsList(dockerStats) totals := make([]float64, 2, 2) for _, stat := range stats { From a48b048ef587d27f06f9a509126129bdff59b2b2 Mon Sep 17 00:00:00 2001 From: Andrew Kroh Date: Tue, 21 Nov 2017 09:44:10 -0500 Subject: [PATCH 4/5] Remove capacity from make call Specify `len` and `cap` is redundant. --- metricbeat/module/docker/diskio/diskio_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metricbeat/module/docker/diskio/diskio_test.go b/metricbeat/module/docker/diskio/diskio_test.go index 5f911ef37ed..8cff19b301a 100644 --- a/metricbeat/module/docker/diskio/diskio_test.go +++ b/metricbeat/module/docker/diskio/diskio_test.go @@ -45,7 +45,7 @@ func TestDeltaMultipleContainers(t *testing.T) { apiContainer2.Stats.BlkioStats.IOServicedRecursive = append(apiContainer2.Stats.BlkioStats.IOServicedRecursive, metrics) dockerStats := []docker.Stat{apiContainer1, apiContainer2} stats := blkioService.getBlkioStatsList(dockerStats) - totals := make([]float64, 2, 2) + totals := make([]float64, 2) for _, stat := range stats { totals[0] = stat.totals } From 9c7fef57651867241920aca24f256ef2eb828020 Mon Sep 17 00:00:00 2001 From: Andrew Kroh Date: Tue, 21 Nov 2017 09:44:58 -0500 Subject: [PATCH 5/5] Removing another capacity from make --- metricbeat/module/docker/diskio/diskio_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metricbeat/module/docker/diskio/diskio_test.go b/metricbeat/module/docker/diskio/diskio_test.go index 8cff19b301a..dca777994e1 100644 --- a/metricbeat/module/docker/diskio/diskio_test.go +++ b/metricbeat/module/docker/diskio/diskio_test.go @@ -99,7 +99,7 @@ func TestDeltaOneContainer(t *testing.T) { apiContainer.Stats.BlkioStats.IOServicedRecursive = append(apiContainer.Stats.BlkioStats.IOServicedRecursive, metrics) dockerStats := []docker.Stat{apiContainer} stats := blkioService.getBlkioStatsList(dockerStats) - totals := make([]float64, 2, 2) + totals := make([]float64, 2) for _, stat := range stats { totals[0] = stat.totals }