Use optional values for docker memory metrics (elastic#41449)

* use opt values for docker memory metrics * fix vars, comments * add changelog * spelling
rdner · Nov 1, 2024 · 249d0dc · 249d0dc
1 parent 3c16c29
commit 249d0dc
Show file tree

Hide file tree

Showing 5 changed files with 135 additions and 88 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -203,6 +203,8 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]
 - Use namespace for GetListMetrics when exists in AWS {pull}41022[41022]
 - Fix http server helper SSL config. {pull}39405[39405]
 - Fix Kubernetes metadata sometimes not being present after startup {pull}41216[41216]
+- Do not report non-existant 0 values for RSS metrics in docker/memory {pull}41449[41449]
+
 
 *Osquerybeat*
 

diff --git a/metricbeat/module/docker/memory/_meta/data.json b/metricbeat/module/docker/memory/_meta/data.json
@@ -1,83 +1,88 @@
 {
     "@timestamp": "2017-10-12T08:05:34.853Z",
     "container": {
-        "id": "23ce6f1b53181ea3db0611fe4de36f0ebf1c0a37cb8272e028cac06240dafbe0",
+        "id": "3ebfd3aebc686af21efccc552aabceb4303b70ef4bc2be7fffbb616000f824b4",
         "image": {
-            "name": "docker.elastic.co/beats/elastic-agent:7.15.0-SNAPSHOT"
+            "name": "docker.elastic.co/elastic-agent/elastic-agent-complete:8.15.3"
         },
-        "name": "elastic-package-stack_elastic-agent_1",
+        "memory": {
+            "usage": 0.0012028823743718271
+        },
+        "name": "elastic-package-stack-elastic-agent-1",
         "runtime": "docker"
     },
     "docker": {
         "container": {
             "labels": {
-                "com_docker_compose_config-hash": "8e3d03827946685d53a2f171a126c397a3278da18ecd68a970cba9131160c52c",
+                "com_docker_compose_config-hash": "34d3699c997ecee19f466a3a5be2c73b86a5f4a89c362301412d9cc03e41d62d",
                 "com_docker_compose_container-number": "1",
+                "com_docker_compose_depends_on": "fleet-server:service_healthy:false",
+                "com_docker_compose_image": "sha256:37bd3034b35d10da7c806226eb2956b6c998745da9dc15ed3e920d214a59bcec",
                 "com_docker_compose_oneoff": "False",
                 "com_docker_compose_project": "elastic-package-stack",
+                "com_docker_compose_project_config_files": "/home/alexk/.elastic-package/profiles/default/stack/docker-compose.yml",
+                "com_docker_compose_project_working_dir": "/home/alexk/.elastic-package/profiles/default/stack",
                 "com_docker_compose_service": "elastic-agent",
-                "com_docker_compose_version": "1.28.6",
-                "description": "Agent manages other beats based on configuration provided.",
-                "io_k8s_description": "Agent manages other beats based on configuration provided.",
+                "com_docker_compose_version": "2.29.2",
+                "description": "Elastic Agent - single, unified way to add monitoring for logs, metrics, and other types of data to a host.",
+                "io_k8s_description": "Elastic Agent - single, unified way to add monitoring for logs, metrics, and other types of data to a host.",
                 "io_k8s_display-name": "Elastic-Agent image",
                 "license": "Elastic License",
                 "maintainer": "infra@elastic.co",
                 "name": "elastic-agent",
-                "org_label-schema_build-date": "2021-07-28T09:55:40Z",
+                "org_label-schema_build-date": "2024-10-10T10:08:37Z",
                 "org_label-schema_license": "Elastic License",
                 "org_label-schema_name": "elastic-agent",
                 "org_label-schema_schema-version": "1.0",
-                "org_label-schema_url": "https://www.elastic.co/beats/elastic-agent",
-                "org_label-schema_vcs-ref": "16108a69f9f437c00cb6125c57bbc01c4eb805bb",
-                "org_label-schema_vcs-url": "github.com/elastic/beats/v7",
+                "org_label-schema_url": "https://www.elastic.co/elastic-agent",
+                "org_label-schema_vcs-ref": "61975895b1409449db21ddca0405e7b71bfc1c46",
+                "org_label-schema_vcs-url": "github.com/elastic/elastic-agent",
                 "org_label-schema_vendor": "Elastic",
-                "org_label-schema_version": "7.15.0-SNAPSHOT",
-                "org_opencontainers_image_created": "2021-07-28T09:55:40Z",
+                "org_label-schema_version": "8.15.3",
+                "org_opencontainers_image_created": "2024-10-10T10:08:37Z",
                 "org_opencontainers_image_licenses": "Elastic License",
+                "org_opencontainers_image_ref_name": "ubuntu",
                 "org_opencontainers_image_title": "Elastic-Agent",
                 "org_opencontainers_image_vendor": "Elastic",
+                "org_opencontainers_image_version": "20.04",
                 "release": "1",
                 "summary": "elastic-agent",
-                "url": "https://www.elastic.co/beats/elastic-agent",
+                "url": "https://www.elastic.co/elastic-agent",
                 "vendor": "Elastic",
-                "version": "7.15.0-SNAPSHOT"
+                "version": "8.15.3"
             }
         },
         "memory": {
             "fail": {
                 "count": 0
             },
-            "limit": 67514433536,
-            "rss": {
-                "pct": 0,
-                "total": 0
-            },
+            "limit": 33537363968,
             "stats": {
-                "active_anon": 270336,
-                "active_file": 135168,
-                "anon": 246484992,
-                "anon_thp": 4194304,
-                "file": 325484544,
+                "active_anon": 10223616,
+                "active_file": 19795968,
+                "anon": 19140608,
+                "anon_thp": 0,
+                "file": 36466688,
                 "file_dirty": 0,
-                "file_mapped": 170582016,
+                "file_mapped": 585728,
                 "file_writeback": 0,
-                "inactive_anon": 250257408,
-                "inactive_file": 325619712,
-                "kernel_stack": 2703360,
-                "pgactivate": 62898,
+                "inactive_anon": 8921088,
+                "inactive_file": 16670720,
+                "kernel_stack": 212992,
+                "pgactivate": 4810,
                 "pgdeactivate": 0,
-                "pgfault": 2150971515,
-                "pglazyfree": 207999,
+                "pgfault": 703936010,
+                "pglazyfree": 0,
                 "pglazyfreed": 0,
-                "pgmajfault": 0,
-                "pgrefill": 0,
-                "pgscan": 0,
-                "pgsteal": 0,
+                "pgmajfault": 729,
+                "pgrefill": 170270,
+                "pgscan": 12002,
+                "pgsteal": 10536,
                 "shmem": 0,
-                "slab": 8112800,
-                "slab_reclaimable": 5753632,
-                "slab_unreclaimable": 2359168,
-                "sock": 200704,
+                "slab": 513312,
+                "slab_reclaimable": 194680,
+                "slab_unreclaimable": 318632,
+                "sock": 0,
                 "thp_collapse_alloc": 0,
                 "thp_fault_alloc": 0,
                 "unevictable": 0,
@@ -87,8 +92,8 @@
             },
             "usage": {
                 "max": 0,
-                "pct": 0.0039415723433138695,
-                "total": 266113024
+                "pct": 0.0012028823743718271,
+                "total": 40341504
             }
         }
     },

diff --git a/metricbeat/module/docker/memory/data.go b/metricbeat/module/docker/memory/data.go
@@ -51,16 +51,21 @@ func eventMapping(r mb.ReporterV2, memoryData *MemoryData) {
 				"count": memoryData.Failcnt,
 			},
 			"limit": memoryData.Limit,
-			"rss": mapstr.M{
-				"total": memoryData.TotalRss,
-				"pct":   memoryData.TotalRssP,
-			},
 			"usage": mapstr.M{
 				"total": memoryData.Usage,
 				"pct":   memoryData.UsageP,
 				"max":   memoryData.MaxUsage,
 			},
 		}
+		if memoryData.TotalRss.Exists() {
+			fields["rss"] = mapstr.M{
+				"total": memoryData.TotalRss.ValueOr(0),
+			}
+			if memoryData.TotalRssP.Exists() {
+				fields.Put("rss.pct", memoryData.TotalRssP.ValueOr(0))
+			}
+		}
+
 		// Add container ECS fields
 		_, _ = rootFields.Put("container.memory.usage", memoryData.UsageP)
 	}

diff --git a/metricbeat/module/docker/memory/helper.go b/metricbeat/module/docker/memory/helper.go
@@ -20,6 +20,7 @@ package memory
 import (
 	"github.com/elastic/beats/v7/libbeat/common"
 	"github.com/elastic/beats/v7/metricbeat/module/docker"
+	"github.com/elastic/elastic-agent-libs/opt"
 )
 
 // MemoryData contains parsed container memory info
@@ -29,8 +30,8 @@ type MemoryData struct {
 	Failcnt   uint64
 	Limit     uint64
 	MaxUsage  uint64
-	TotalRss  uint64
-	TotalRssP float64
+	TotalRss  opt.Uint
+	TotalRssP opt.Float
 	Usage     uint64
 	UsageP    float64
 	//Raw stats from the cgroup subsystem
@@ -61,7 +62,7 @@ func (s *MemoryService) getMemoryStatsList(containers []docker.Stat, dedot bool)
 }
 
 func (s *MemoryService) getMemoryStats(myRawStat docker.Stat, dedot bool) MemoryData {
-	totalRSS := myRawStat.Stats.MemoryStats.Stats["total_rss"]
+	totalRSS, rssOK := myRawStat.Stats.MemoryStats.Stats["total_rss"]
 
 	// Emulate newer docker releases and exclude cache values from memory usage
 	// See here for a little more context. usage - cache won't work, as it includes shared mappings that can't be dropped
@@ -79,14 +80,12 @@ func (s *MemoryService) getMemoryStats(myRawStat docker.Stat, dedot bool) Memory
 	}
 
 	memUsage = myRawStat.Stats.MemoryStats.Usage - fileUsage
-	return MemoryData{
+	memData := MemoryData{
 		Time:      common.Time(myRawStat.Stats.Read),
 		Container: docker.NewContainer(myRawStat.Container, dedot),
 		Failcnt:   myRawStat.Stats.MemoryStats.Failcnt,
 		Limit:     myRawStat.Stats.MemoryStats.Limit,
 		MaxUsage:  myRawStat.Stats.MemoryStats.MaxUsage,
-		TotalRss:  totalRSS,
-		TotalRssP: float64(totalRSS) / float64(myRawStat.Stats.MemoryStats.Limit),
 		Usage:     memUsage,
 		UsageP:    float64(memUsage) / float64(myRawStat.Stats.MemoryStats.Limit),
 		Stats:     myRawStat.Stats.MemoryStats.Stats,
@@ -95,4 +94,13 @@ func (s *MemoryService) getMemoryStats(myRawStat docker.Stat, dedot bool) Memory
 		CommitPeak:        myRawStat.Stats.MemoryStats.CommitPeak,
 		PrivateWorkingSet: myRawStat.Stats.MemoryStats.PrivateWorkingSet,
 	}
+	// the RSS metrics are cgv1 only.
+	if rssOK {
+		memData.TotalRss = opt.UintWith(totalRSS)
+		if myRawStat.Stats.MemoryStats.Limit != 0 {
+			memData.TotalRssP = opt.FloatWith(float64(totalRSS) / float64(myRawStat.Stats.MemoryStats.Limit))
+		}
+
+	}
+	return memData
 }
diff --git a/metricbeat/module/docker/memory/memory_test.go b/metricbeat/module/docker/memory/memory_test.go
@@ -22,43 +22,68 @@ import (
 	"time"
 
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	mbtest "github.com/elastic/beats/v7/metricbeat/mb/testing"
 	"github.com/elastic/beats/v7/metricbeat/module/docker"
 	"github.com/elastic/elastic-agent-libs/mapstr"
 )
 
+var defaultContainerID = "containerID"
+
+var defaultLabels = map[string]string{
+	"label1":     "val1",
+	"label2":     "val2",
+	"label2.foo": "val3",
+}
+
+var defaultContainerStats = types.Container{
+	ID:         defaultContainerID,
+	Image:      "image",
+	Command:    "command",
+	Created:    123789,
+	Status:     "Up",
+	SizeRw:     123,
+	SizeRootFs: 456,
+	Names:      []string{"/name1", "name1/fake"},
+	Labels:     defaultLabels,
+}
+
+func TestMemStatsV2(t *testing.T) {
+	// Test to make sure we don't report any RSS metrics where they don't exist
+	memoryService := &MemoryService{}
+	memorystats := getMemoryStats(time.Now(), 1, false)
+
+	memoryRawStats := docker.Stat{}
+	memoryRawStats.Container = &defaultContainerStats
+	memoryRawStats.Stats = memorystats
+
+	rawStats := memoryService.getMemoryStats(memoryRawStats, false)
+	require.False(t, rawStats.TotalRss.Exists())
+	require.False(t, rawStats.TotalRssP.Exists())
+
+	r := &mbtest.CapturingReporterV2{}
+	eventMapping(r, &rawStats)
+	events := r.GetEvents()
+	require.NotContains(t, "rss", events[0].MetricSetFields)
+
+}
+
 func TestMemoryService_GetMemoryStats(t *testing.T) {
-	//Container  + dockerstats
-	containerID := "containerID"
-	labels := map[string]string{
-		"label1":     "val1",
-		"label2":     "val2",
-		"label2.foo": "val3",
-	}
-	container := types.Container{
-		ID:         containerID,
-		Image:      "image",
-		Command:    "command",
-		Created:    123789,
-		Status:     "Up",
-		SizeRw:     123,
-		SizeRootFs: 456,
-		Names:      []string{"/name1", "name1/fake"},
-		Labels:     labels,
-	}
+
 	memoryService := &MemoryService{}
-	memorystats := getMemoryStats(time.Now(), 1)
+	memorystats := getMemoryStats(time.Now(), 1, true)
 
 	memoryRawStats := docker.Stat{}
-	memoryRawStats.Container = &container
+	memoryRawStats.Container = &defaultContainerStats
 	memoryRawStats.Stats = memorystats
 
 	totalRSS := memorystats.MemoryStats.Stats["total_rss"]
 	expectedRootFields := mapstr.M{
 		"container": mapstr.M{
-			"id":   containerID,
+			"id":   defaultContainerID,
 			"name": "name1",
 			"image": mapstr.M{
 				"name": "image",
@@ -113,30 +138,30 @@ func TestMemoryService_GetMemoryStats(t *testing.T) {
 
 func TestMemoryServiceBadData(t *testing.T) {
 
-	badMemStats := types.StatsJSON{
-		Stats: types.Stats{
+	badMemStats := container.StatsResponse{
+		Stats: container.Stats{
 			Read:        time.Now(),
-			MemoryStats: types.MemoryStats{}, //Test for cases where this is empty
+			MemoryStats: container.MemoryStats{}, //Test for cases where this is empty
 		},
 	}
 
 	memoryService := &MemoryService{}
-	memoryRawStats := []docker.Stat{docker.Stat{Stats: badMemStats}}
+	memoryRawStats := []docker.Stat{{Stats: badMemStats}}
 	rawStats := memoryService.getMemoryStatsList(memoryRawStats, false)
 	assert.Len(t, rawStats, 0)
 
 }
 
 func TestMemoryMath(t *testing.T) {
-	memStats := types.StatsJSON{
-		Stats: types.Stats{
+	memStats := container.StatsResponse{
+		Stats: container.Stats{
 			Read: time.Now(),
-			PreCPUStats: types.CPUStats{
-				CPUUsage: types.CPUUsage{
+			PreCPUStats: container.CPUStats{
+				CPUUsage: container.CPUUsage{
 					TotalUsage: 200,
 				},
 			},
-			MemoryStats: types.MemoryStats{
+			MemoryStats: container.MemoryStats{
 				Limit: 5,
 				Usage: 5000,
 				Stats: map[string]uint64{
@@ -149,18 +174,18 @@ func TestMemoryMath(t *testing.T) {
 
 	memoryService := &MemoryService{}
 	memoryRawStats := []docker.Stat{
-		docker.Stat{Stats: memStats, Container: &types.Container{Names: []string{"test-container"}, Labels: map[string]string{}}},
+		{Stats: memStats, Container: &types.Container{Names: []string{"test-container"}, Labels: map[string]string{}}},
 	}
 	rawStats := memoryService.getMemoryStatsList(memoryRawStats, false)
 	assert.Equal(t, float64(800), rawStats[0].UsageP) // 5000-900 /5
 }
 
-func getMemoryStats(read time.Time, number uint64) types.StatsJSON {
+func getMemoryStats(read time.Time, number uint64, rssExists bool) container.StatsResponse {
 
-	myMemoryStats := types.StatsJSON{
-		Stats: types.Stats{
+	myMemoryStats := container.StatsResponse{
+		Stats: container.Stats{
 			Read: read,
-			MemoryStats: types.MemoryStats{
+			MemoryStats: container.MemoryStats{
 				MaxUsage: number,
 				Usage:    number * 2,
 				Failcnt:  number * 3,
@@ -170,7 +195,9 @@ func getMemoryStats(read time.Time, number uint64) types.StatsJSON {
 		},
 	}
 
-	myMemoryStats.MemoryStats.Stats["total_rss"] = number * 5
+	if rssExists {
+		myMemoryStats.MemoryStats.Stats["total_rss"] = number * 5
+	}
 
 	return myMemoryStats
 }