Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[metricbeat] Add memory.stat data to the docker/memory metricset #12916

Merged
merged 6 commits into from
Jul 29, 2019
Merged

[metricbeat] Add memory.stat data to the docker/memory metricset #12916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
15f29a8
add memory.stat data to the docker/memory metricset
fearful-symmetry Jul 15, 2019
88debdc
make object name stats, so it's more consistant
fearful-symmetry Jul 15, 2019
5b0b030
fix tests
fearful-symmetry Jul 15, 2019
c73214b
fix mapping
fearful-symmetry Jul 17, 2019
91f7e2d
update data.json
fearful-symmetry Jul 23, 2019
7922e08
Add changelog entry
fearful-symmetry Jul 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Collect client provided name for rabbitmq connection. {issue}12851[12851] {pull}12852[12852]
- Add support to load default aws config file to get credentials. {pull}12727[12727] {issue}12708[12708]
- Add statistic option into cloudwatch metricset. {issue}12370[12370] {pull}12840[12840]
- Add cgroup memory stats to docker/memory metricset {pull}12916[12916]

*Packetbeat*

Expand Down
10 changes: 10 additions & 0 deletions metricbeat/docs/fields.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5371,6 +5371,16 @@ Memory metrics.



*`docker.memory.stats.*`*::
+
--
Raw memory stats from the cgroups memory.stat interface


type: object

--

[float]
=== commit

Expand Down
2 changes: 1 addition & 1 deletion metricbeat/module/docker/fields.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

76 changes: 43 additions & 33 deletions metricbeat/module/docker/memory/_meta/data.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,51 +1,61 @@
{
"@timestamp": "2017-10-12T08:05:34.853Z",
"agent": {
"hostname": "host.example.com",
"name": "host.example.com"
},
"container": {
"id": "cc78e58acfda4501105dc4de8e3ae218f2da616213e6e3af168c40103829302a",
"id": "aa41902101351f415e6e983b0673c0ba715dd4bc316bd5fc0ebd6fcf94287f86",
"image": {
"name": "metricbeat_elasticsearch"
"name": "redis:latest"
},
"name": "metricbeat_elasticsearch_1_df866b3a7b3d",
"name": "amazing_cohen",
"runtime": "docker"
},
"docker": {
"container": {
"labels": {
"com_docker_compose_config-hash": "e3e0a2c6e5d1afb741bc8b1ecb09cda0395886b7a3e5084a9fd110be46d70f78",
"com_docker_compose_container-number": "1",
"com_docker_compose_oneoff": "False",
"com_docker_compose_project": "metricbeat",
"com_docker_compose_service": "elasticsearch",
"com_docker_compose_slug": "df866b3a7b3d50c0802350cbe58ee5b34fa32b7f6ba7fe9e48cde2c12dd0201d",
"com_docker_compose_version": "1.23.1",
"license": "Elastic License",
"org_label-schema_build-date": "20181006",
"org_label-schema_license": "GPLv2",
"org_label-schema_name": "elasticsearch",
"org_label-schema_schema-version": "1.0",
"org_label-schema_url": "https://www.elastic.co/products/elasticsearch",
"org_label-schema_vcs-url": "https://github.com/elastic/elasticsearch-docker",
"org_label-schema_vendor": "Elastic",
"org_label-schema_version": "6.5.1"
}
},
"memory": {
"fail": {
"count": 0
},
"limit": 16571719680,
"limit": 2095878144,
"rss": {
"pct": 0.02170185104168984,
"total": 359636992
"pct": 0.0004025882909345325,
"total": 843776
},
"stats": {
"active_anon": 421888,
"active_file": 36864,
"cache": 86016,
"dirty": 0,
"hierarchical_memory_limit": 9223372036854771712,
"hierarchical_memsw_limit": 9223372036854771712,
"inactive_anon": 421888,
"inactive_file": 49152,
"mapped_file": 53248,
"pgfault": 1587,
"pgmajfault": 1,
"pgpgin": 2426,
"pgpgout": 2199,
"rss": 843776,
"rss_huge": 0,
"total_active_anon": 421888,
"total_active_file": 36864,
"total_cache": 86016,
"total_dirty": 0,
"total_inactive_anon": 421888,
"total_inactive_file": 49152,
"total_mapped_file": 53248,
"total_pgfault": 1587,
"total_pgmajfault": 1,
"total_pgpgin": 2426,
"total_pgpgout": 2199,
"total_rss": 843776,
"total_rss_huge": 0,
"total_unevictable": 0,
"total_writeback": 0,
"unevictable": 0,
"writeback": 0
},
"usage": {
"max": 521326592,
"pct": 0.029796111057558028,
"total": 493772800
"max": 7860224,
"pct": 0.000672283359618831,
"total": 1409024
}
}
},
Expand Down
6 changes: 6 additions & 0 deletions metricbeat/module/docker/memory/_meta/fields.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
Memory metrics.
release: ga
fields:
- name: stats.*
type: object
object_type: long
object_type_mapping_type: "*"
description: >
Raw memory stats from the cgroups memory.stat interface
- name: commit
type: group
description: >
Expand Down
1 change: 1 addition & 0 deletions metricbeat/module/docker/memory/data.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func eventMapping(r mb.ReporterV2, memoryData *MemoryData) {
}
} else {
fields = common.MapStr{
"stats": memoryData.Stats,
"fail": common.MapStr{
"count": memoryData.Failcnt,
},
Expand Down
3 changes: 3 additions & 0 deletions metricbeat/module/docker/memory/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ type MemoryData struct {
TotalRssP float64
Usage uint64
UsageP float64
//Raw stats from the cgroup subsystem
Stats map[string]uint64
//Windows-only memory stats
Commit uint64
CommitPeak uint64
Expand Down Expand Up @@ -70,6 +72,7 @@ func (s *MemoryService) getMemoryStats(myRawStat docker.Stat, dedot bool) Memory
TotalRssP: float64(totalRSS) / float64(myRawStat.Stats.MemoryStats.Limit),
Usage: myRawStat.Stats.MemoryStats.Usage,
UsageP: float64(myRawStat.Stats.MemoryStats.Usage) / float64(myRawStat.Stats.MemoryStats.Limit),
Stats: myRawStat.Stats.MemoryStats.Stats,
//Windows memory statistics
Commit: myRawStat.Stats.MemoryStats.Commit,
CommitPeak: myRawStat.Stats.MemoryStats.CommitPeak,
Expand Down
3 changes: 3 additions & 0 deletions metricbeat/module/docker/memory/memory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ func TestMemoryService_GetMemoryStats(t *testing.T) {
},
}
expectedFields := common.MapStr{
"stats": map[string]uint64{
"total_rss": 5,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, according to the naming conventions, this field should be called rss.count https://www.elastic.co/guide/en/beats/devguide/current/event-conventions.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much do we want to fiddle with the stuff being scraped from the kernel interface? The full list of total_* events is generated programmatically by the kernel.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like, this is what's happening in the kernel:

for (i = 0; i < ARRAY_SIZE(memcg1_events); i++)
		seq_printf(m, "total_%s %llu\n", memcg1_event_names[i],
			   (u64)memcg_events(memcg, i));

	for (i = 0; i < NR_LRU_LISTS; i++)
		seq_printf(m, "total_%s %llu\n", mem_cgroup_lru_names[i],
			   (u64)memcg_page_state(memcg, NR_LRU_BASE + i) *
			   PAGE_SIZE);

Seems like a potential lost cause to figure out what possible fields we would need to re-parse?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having that Docker doesn't document these fields and is just passing them in raw I think that we could fail if we try to convert them. Could you update data.json? we should see how this looks by default there.

Copy link
Member

@jsoriano jsoriano Jul 19, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just another opinion. I think that for this case it'd be fine to copy the stats map, as they are values taken directly from the OS, and I don't think they are going to change a lot. Also they seem to follow a good enough format (lowercase, snake case...).

But it is true that we risk to have fields explosion or other problems if some fancy feature is merged into the kernel.

Some things that we could do to improve this:

  • Apply an schema to these memory stats, so we control what variables we collect, and potentially we adapt them to some other schema as Mario proposed (although I am fine with keeping the naming given by docker/OS)
  • Explicitly document the fields we know (even if we leave the wildcard to catch new fields)

And +1 to have a data.json to have a better view of how it looks. It could be also nice to try it on Windows, to see if it has its own set of variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apply an schema to these memory stats

I initially thought of that, but schema.Apply seemed a bit like overkill considering everything is already a uint64. Should we just not use the type conversion part of it?

Explicitly document the fields we know (even if we leave the wildcard to catch new fields)

How would that work? Is there some way to document this besides fields.yml ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passing this in raw sounds reasonable enough, worst case scenario at some point we discover this generates too much data, which seems unlikely anyway.

Updating data.json will help to give a good example of what to expect there

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicitly document the fields we know (even if we leave the wildcard to catch new fields)

How would that work? Is there some way to document this besides fields.yml ?

Yeah, I mean to add to fields.yml the fields we know, but as they look like quite low level metrics, I am ok too with documenting the docker.memory.stats group only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yah, the fact that they're so low level but also consistently typed makes me not too worried about them. I don't think there's any downside to the wildcard yml entry from the perspective of mapping, is there?

},
"fail": common.MapStr{
"count": memorystats.MemoryStats.Failcnt,
},
Expand Down