Add Filesystem and Process Metricset to System Module #1081

ruflin · 2016-03-01T13:38:06Z

Add Filesystem Metricset with fields.yml and example doc
Add Process Metricset with fields.yml and example doc
Enhance template generation to support nested documents
Fix issue with type: string
Raise exception in template generation script if invalid type is used

tsg · 2016-03-01T13:42:24Z

metricbeat/module/system/cpu/cpu.go

+		"steal":    cpuStat.Stolen,
+		"user_p":   cpuStat.UserPercent,
+		"system_p": cpuStat.SystemPercent,
+	}


Could we somehow create the event (MapStr) in the topbeat code? Otherwise we have to remember adding the key here as well every time we add something to topbeat.

Yes, we should add some abstraction to Topbeat to also profit directly from things like addCPUPercentage etc: https://github.com/elastic/beats/blob/master/topbeat/beater/topbeat.go#L262

monicasarbu · 2016-03-01T14:34:39Z

LGTM

monicasarbu · 2016-03-01T14:42:23Z

@ruflin If we want to add per process statistics, are you planning to add a new module to Metricbeat or re-use/rename "system"?
I imagined that in the future Topbeat & Topbeat module in Metricbeat are the same thing, share the same code and export the same data.

ruflin · 2016-03-01T14:54:37Z

@monicasarbu I would definitively want to add it. I think topbeat and metricbeat should have feature parity. I would see all topbeat features under the module system (but we could also rename it). What I'm not sure yet how to call all the metricsets. For example belongs Disk I/O under filesystem or is it its own metricset? What about the per-process-stats? Should this be a metricset processes or it is part of cpu?

ruflin · 2016-03-22T19:46:03Z

metricbeat/module/system/filesystem/filesystem.go

+		system.AddFileSystemUsedPercentage(fsStat)
+
+		fsEvent := common.MapStr{
+			fsStat.DevName: system.GetFilesystemEvent(fsStat),


What do we want to have as a key here? Filesystem names can also have spaces etc.

@tsg Would be good to get your thoughts on this.

Unfortunately organizing the data like this is making it impossible to do top like widgets in Kibana. For example top processes by memory usage, top FS by disk usage, etc. So it's a question of how uniform we want to have the data model versus enabling different viz in Kibana.

This would mean that Topbeat is not strictly a subset of Metricbeat, because the data is organized differently. This could be OK, but we have to take a conscious decision about it.

Overall I find this model in which the fields names are not predictable less flexible on the data consumption part.

I agree that both implementations should be "almost" identical. I think that topbeat should send the status for processes and file system stats in one event instead of lots of sub events. This still doesn't solve the above problem how to do it in the best way. We could potentially use arrays (https://www.elastic.co/guide/en/elasticsearch/guide/current/complex-core-fields.html#object-arrays) but I have to check how this would work for visualisations.

I'm afraid arrays are also not a good option when it comes to visualizing in Kibana, see for e.g. elastic/kibana#998. Besides the visualization aspect, having ephemeral fields like PID is quite space inefficient, because it tends to create a lot of sparse doc values.

I was thinking the metric name will be something like filesystem.size and the device name would be a label, just like the host, for example. IMO putting the device name or PID into the metric name sends us back to the Graphite ways where this is the only way to put metadata in.

ruflin · 2016-04-25T10:57:28Z

This is currently blocked by finding the right data model for process and filesystem.

ruflin · 2016-04-26T20:00:15Z

libbeat/scripts/generate_template.py

        properties[field["name"]] = {
            "type": field.get("type")
        }
        if field["type"] == "keyword":
            properties[field["name"]]["ignore_above"] = \
                defaults.get("ignore_above", 1024)

-    elif field["type"] == "dict":
+    elif field["type"] in ["dict", "list"]:


@tsg @monicasarbu Packetbeat had a type "list". I assumed this is identical to dict?

Yeah, I'd say lets use only dict for now, for simplicity. At some point we might have to separate them.

Ok.

Can you briefly elaborate on how dict and list could be different?

I was thinking we'd use dict only for actual sub-dictionaries, and list for "arrays of dictionaries", like we have in DNS at the moment. The requirements are likely to be different, but at the moment dict without dict-type adds nothing to the template, so that works on anything :-).

ruflin · 2016-04-26T20:05:22Z

@tsg I completely rewrote / updated this PR. Have a look.

ruflin · 2016-04-26T20:06:12Z

metricbeat/module/system/process/process.go

+      "rtt": 20982,
+      "system-process": {
+        "processes": [
+          {


The reason I added the additional "processes" array is that this will allow us to store additional data in the metricset if needed without changing the structure.

tsg · 2016-04-26T20:50:08Z

libbeat/scripts/generate_template.py

+            properties[field.get("name")] = {"type": "nested", "properties": {}}
+            properties[field.get("name")]["properties"] = prop
+
+        dynamic_templates.extend(dynamic)


I wonder if dynamic templates work on nested documents. We don't need it now, but we should know if that's a limitation.

@tsg I would expect that we can use path_match for this: https://www.elastic.co/guide/en/elasticsearch/reference/current/dynamic-templates.html#path-match-unmatch But I didn't test it.

ruflin · 2016-04-27T14:13:20Z

libbeat/scripts/generate_template.py

@@ -240,7 +240,7 @@ def fill_field_properties(args, field, defaults, path):
            path = path + "." + field["name"]
        else:
            path = field["name"]
-        prop, dynamic = fill_section_properties(field, defaults, path)
+        prop, dynamic = fill_section_properties(args, field, defaults, path)


@tsg Seems like this one only affected metricbeat

ruflin · 2016-05-02T05:58:42Z

In a recent meeting we decided to do the following with the data structure:

Not use nested documents and send for each process / filesystem a separate document (as this is more or less anyway how ES stores nested docs). Correlate the docs which belong together (for example all processes) with an identifider
Have a metricset processes which sends all info for all processes and have a metricset process-info (or similar) which provides overview information over processes.
Same for Filesystem
It can be that the information of the two metricsets partially overlaps. Shared functionality should go into the module.

@andrewkroh is currently working on making it possible for a metricset to return multiple events. This PR will be updated as soon as these changes are in master.

ruflin · 2016-05-03T08:46:44Z

I updated this PR to send for each process and filesytem an event. This is now possible with the new metricset interfaces. In addition I added the fsstats metricset that contains the file system stats.

@andrewkroh @tsg Please have a look.

monicasarbu · 2016-05-03T12:55:27Z

metricbeat/module/system/fsstats/fsstats.go

+      "metricset": "filesystem",
+      "module": "system",
+      "rtt": 434,
+      "system-filesystem": {


Is there any reason to have system in the name of the object? Why not just filesystem? I am thinking once we will have conditions in generic filtering, then the name of the field available in the condition is a bit too long (e.g. system-filesystem.device_name). Also, I think a mixture of "-" and "_" is not a good idea.

@monicasarbu That is the namespacing we require. This is always $module-$metricset for all events.

andrewkroh · 2016-05-03T13:54:34Z

Looks like you need a doc.go placeholder in the fsstats and filesystem packages so that the package is not empty for the operating systems on which those metric sets are unavailable.

andrewkroh · 2016-05-03T13:56:19Z

Other than the cross-compile error, LGTM. My comments were just minor things that I can fix later if you want.

monicasarbu · 2016-05-03T13:57:10Z

topbeat/system/process.go

+		event := common.MapStr{
+			"@timestamp": common.Time(time.Now()),
+			"type":       "process",
+			"count":      1,


count: 1 is not longer exported.

Good one. I would have introduced count accidentially again.

ruflin · 2016-05-03T15:16:37Z

@monicasarbu @andrewkroh Cleaned up and pushed again.

andrewkroh · 2016-05-03T18:00:23Z

@ruflin The system/process package also needs a doc.go file.

* Add Filesystem Metricset with fields.yml and example doc * Add Fsstats Metricset with file system stats * Add Process Metricset with fields.yml and example doc * Enhance template generation to support nested documents * Fix issue with type: string * Raise exception in template generation script if invalid type is used

ruflin · 2016-05-03T18:21:22Z

@andrewkroh Fixed

andrewkroh · 2016-05-03T21:36:45Z

metricbeat/etc/fields.yml

+        `system-filesystem` contains local filesystem stats
+      fields:
+        - name: avail
+          type: integer


A lot of these are marked as integer but are marked as longs in Topbeat. Looks like they should long because their data types are either int64 or uint64.

ruflin added the discuss Issue needs further discussion. label Mar 1, 2016

tsg reviewed Mar 1, 2016
View reviewed changes

ruflin force-pushed the metricbeat-topbeat branch from 26cfdf1 to cc9fb92 Compare March 16, 2016 15:30

ruflin changed the title ~~[POC] Add Topbeat to Metricbeat~~ Add Topbeat to Metricbeat Mar 22, 2016

ruflin force-pushed the metricbeat-topbeat branch from cc9fb92 to aefe190 Compare March 22, 2016 19:45

ruflin reviewed Mar 22, 2016
View reviewed changes

ruflin force-pushed the metricbeat-topbeat branch from aefe190 to 9fb6387 Compare April 18, 2016 14:41

ruflin mentioned this pull request Apr 19, 2016

Add System module with CPU and Memory stats #1416

Merged

ruflin force-pushed the metricbeat-topbeat branch from 9fb6387 to ca2710d Compare April 22, 2016 15:18

ruflin added the blocked label Apr 25, 2016

ruflin added Metricbeat Metricbeat and removed discuss Issue needs further discussion. labels Apr 25, 2016

ruflin force-pushed the metricbeat-topbeat branch from ca2710d to 0271de8 Compare April 26, 2016 19:59

ruflin changed the title ~~Add Topbeat to Metricbeat~~ Add Topbeat to MetricbeatAdd Filesystem and Process Metricset to System Module Apr 26, 2016

ruflin reviewed Apr 26, 2016
View reviewed changes

ruflin changed the title ~~Add Topbeat to MetricbeatAdd Filesystem and Process Metricset to System Module~~ Add Filesystem and Process Metricset to System Module Apr 26, 2016

ruflin force-pushed the metricbeat-topbeat branch from 0271de8 to a4d7b0f Compare April 26, 2016 20:12

tsg reviewed Apr 26, 2016
View reviewed changes

ruflin force-pushed the metricbeat-topbeat branch from 8d790c6 to 7f6ce34 Compare April 27, 2016 14:12

ruflin reviewed Apr 27, 2016
View reviewed changes

ruflin force-pushed the metricbeat-topbeat branch from 7f6ce34 to 50cffb0 Compare April 28, 2016 13:22

ruflin force-pushed the metricbeat-topbeat branch 2 times, most recently from 5ff8db7 to b205dce Compare May 3, 2016 08:45

monicasarbu reviewed May 3, 2016
View reviewed changes

ruflin force-pushed the metricbeat-topbeat branch from b205dce to 8d67b2c Compare May 3, 2016 15:16

tsg mentioned this pull request May 3, 2016

Re-factor to fix high CPU usage on windows #1562

Merged

andrewkroh added enhancement review and removed blocked labels May 3, 2016

ruflin force-pushed the metricbeat-topbeat branch from 8d67b2c to 92ba33d Compare May 3, 2016 18:20

andrewkroh reviewed May 3, 2016
View reviewed changes

andrewkroh merged commit 8e71923 into elastic:master May 3, 2016

andrewkroh mentioned this pull request May 3, 2016

Add tests for the filesystem, fsstats, and process MetricSets. #1566

Merged

ruflin deleted the metricbeat-topbeat branch May 4, 2016 06:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Filesystem and Process Metricset to System Module #1081

Add Filesystem and Process Metricset to System Module #1081

ruflin commented Mar 1, 2016 •

edited

Loading

tsg Mar 1, 2016

ruflin Mar 1, 2016

monicasarbu commented Mar 1, 2016

monicasarbu commented Mar 1, 2016

ruflin commented Mar 1, 2016

ruflin Mar 22, 2016

ruflin Mar 23, 2016

tsg Mar 23, 2016

ruflin Mar 23, 2016

tsg Mar 23, 2016

ruflin commented Apr 25, 2016

ruflin Apr 26, 2016

tsg Apr 26, 2016

ruflin Apr 27, 2016

tsg Apr 27, 2016

ruflin commented Apr 26, 2016

ruflin Apr 26, 2016

tsg Apr 26, 2016

ruflin Apr 27, 2016

ruflin Apr 27, 2016

ruflin commented May 2, 2016

ruflin commented May 3, 2016

monicasarbu May 3, 2016

ruflin May 3, 2016

andrewkroh commented May 3, 2016

andrewkroh commented May 3, 2016

monicasarbu May 3, 2016

ruflin May 3, 2016

ruflin May 3, 2016

ruflin commented May 3, 2016

andrewkroh commented May 3, 2016

ruflin commented May 3, 2016

andrewkroh May 3, 2016

Add Filesystem and Process Metricset to System Module #1081

Add Filesystem and Process Metricset to System Module #1081

Conversation

ruflin commented Mar 1, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monicasarbu commented Mar 1, 2016

monicasarbu commented Mar 1, 2016

ruflin commented Mar 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Apr 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Apr 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented May 2, 2016

ruflin commented May 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewkroh commented May 3, 2016

andrewkroh commented May 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented May 3, 2016

andrewkroh commented May 3, 2016

ruflin commented May 3, 2016

Choose a reason for hiding this comment

ruflin commented Mar 1, 2016 •

edited

Loading