Metricbeat: store only top N processes by CPU/memory #4127

tsg · 2017-04-27T13:08:57Z

This adds the option to only report on the top N processes by CPU and/or memory. It is useful because storing metrics about each and every process from every host can be fairly expensive from the storage point of view. Previously it was possible to filter processes by name, which was useful if one knew in advance which are the most interesting processes. This adds a new option which should be quite convenient in practice, because the number of per-process documents gets limited while still allowing to display the top processes.

Closes #4126.

Configuration wise it looks like this:

  process.include_top_n:
    by_cpu: 5      # include top 5 processes by CPU
    by_memory: 5   # include top 5 processes by memory

Remaining TODOs:

unit tests
document the new settings

elasticmachine · 2017-04-27T13:40:40Z

metricbeat/module/system/process/helper.go

+	CmdLine     string `json:"cmdline"`
+	Cwd         string `json:"cwd"`
+	Mem         sigar.ProcMem
+	Cpu         sigar.ProcTime


[golint] _{reported by reviewdog 🐶}
struct field Cpu should be CPU

elasticmachine · 2017-04-27T13:40:40Z

metricbeat/module/system/process/helper.go

@@ -216,7 +221,7 @@ func getProcState(b byte) string {
 	return "unknown"
 }

-func (procStats *ProcStats) GetProcessEvent(process *Process, last *Process) common.MapStr {
+func (procStats *ProcStats) GetProcessEvent(process *Process) common.MapStr {


[golint] _{reported by reviewdog 🐶}
exported method ProcStats.GetProcessEvent should have comment or be unexported

I unexported this one, i don't think it needs to be exported.

elasticmachine · 2017-04-27T13:40:40Z

metricbeat/module/system/process/helper.go

+	return procs, nil
+}
+
+func (p *ProcStats) includeTopProcesses(processes []Process) []Process {


[golint] _{reported by reviewdog 🐶}
receiver name p should be consistent with previous receiver name procStats for ProcStats

tsg · 2017-04-28T14:02:20Z

As a follow up, I added in #4112 a point to add new metrics for the "total number of processes" and similar, and update the dashboards to use these metrics instead of aggregating them based on the row process values.

ruflin · 2017-04-28T14:01:37Z

metricbeat/module/system/_meta/config.full.yml

@@ -36,6 +36,21 @@
  # if true, exports the CPU usage in ticks, together with the percentage values
  #cpu_ticks: false

+  # These options allow you to filter out all processes that are not


Perhaps add a note that it is a union?

I added a note at the section start.

tsg · 2017-05-02T09:24:43Z

Added CHANGELOG entry, cleaned up and rebased to master.

ruflin · 2017-05-02T12:34:18Z

metricbeat/module/system/process/helper_test.go

 	"testing"
 	"time"

 	"github.com/elastic/beats/libbeat/common"
 	"github.com/elastic/gosigar"
+	sigar "github.com/elastic/gosigar"


it's tests but it seems like gosigar is imported twicw?

This adds the option to only report on the top N processes by CPU and/or memory. It is useful because storing metrics about each and every process from every host can be fairly expensive from the storage point of view. Previously it was possible to filter processes by name, which was useful if one knew in advance which are the most interesting processes. This adds a new option which should be quite convenient in practice, because the number of per-process documents gets limited while still allowing to display the top processes. Closes elastic#4126.

andrewkroh · 2017-05-02T13:09:50Z

metricbeat/docs/modules/system.asciidoc

+*`process.include_top_n`*:: These options allow you to filter out all processes
+that are not in the top N by CPU or memory, in order to reduce the number of
+documents created. If both the `by_cpu` and `by_memory` options are used, the
+reunion of the two tops is included.


s/reunion of the two tops/union of the two sets/?

andrewkroh · 2017-05-02T13:18:00Z

metricbeat/module/system/process/process.go

@@ -32,17 +32,31 @@ type MetricSet struct {
 	cacheCmdLine bool
 }

+// includeTopConfig is the configuration for the "top N processes
+// filtering" feature
+type includeTopConfig struct {


For some reason I thought the struct needed to be exported in order for go-ucfg to work on it via reflection. Researching further, I guess since Go 1.6 this isn't necessary.

andrewkroh · 2017-05-02T13:21:58Z

metricbeat/module/system/process/helper.go

@@ -336,7 +339,7 @@ func (procStats *ProcStats) GetProcStats() ([]common.MapStr, error) {
 		return nil, err
 	}

-	processes := []common.MapStr{}
+	processes := []Process{}


It's best to declare the empty slice as var processes []Process because it avoids the allocation if the slice goes unused. https://github.com/golang/go/wiki/CodeReviewComments#declaring-empty-slices

andrewkroh · 2017-05-02T13:24:45Z

metricbeat/module/system/process/helper.go

+	processes = procStats.includeTopProcesses(processes)
+	logp.Debug("processes", "Filtered top processes down to %d processes", len(processes))
+
+	procs := []common.MapStr{}


We know the size a priori, so this could do the full allocation at once with procs := make([]common.MapStr, 0, len(processes)).

andrewkroh · 2017-05-02T13:30:29Z

metricbeat/module/system/process/helper.go

+		return processes
+	}
+
+	result := []Process{}


Same issue with the empty slice declaration.

Follow up for elastic#4127 to address comments.

tsg · 2017-05-02T14:14:50Z

@andrewkroh thanks, I addressed your comments in #4173

Follow up for #4127 to address comments.

tsg added in progress Pull request is currently in progress. Metricbeat Metricbeat review docs and removed docs labels Apr 27, 2017

elasticmachine reviewed Apr 27, 2017

View reviewed changes

tsg removed the in progress Pull request is currently in progress. label Apr 28, 2017

tsg mentioned this pull request Apr 28, 2017

Metricbeat: reduce disk usage in default configuration #4112

Closed

9 tasks

ruflin approved these changes Apr 28, 2017

View reviewed changes

tsg force-pushed the top_n_processes branch 2 times, most recently from 6b72ad0 to 1eb3167 Compare May 2, 2017 09:23

ruflin reviewed May 2, 2017

View reviewed changes

tsg force-pushed the top_n_processes branch from 1eb3167 to 5e142c0 Compare May 2, 2017 12:39

ruflin merged commit 4660a9b into elastic:master May 2, 2017

andrewkroh reviewed May 2, 2017

View reviewed changes

tsg pushed a commit to tsg/beats that referenced this pull request May 2, 2017

Follow up addressing comments in elastic#4127

3d690d6

Follow up for elastic#4127 to address comments.

tsg mentioned this pull request May 2, 2017

Follow up addressing comments in #4127 #4173

Merged

andrewkroh pushed a commit that referenced this pull request May 2, 2017

Follow up addressing comments in #4127 (#4173)

70729fa

Follow up for #4127 to address comments.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metricbeat: store only top N processes by CPU/memory #4127

Metricbeat: store only top N processes by CPU/memory #4127

tsg commented Apr 27, 2017 •

edited

Loading

elasticmachine Apr 27, 2017

elasticmachine Apr 27, 2017

tsg Apr 28, 2017

elasticmachine Apr 27, 2017

tsg commented Apr 28, 2017

ruflin Apr 28, 2017

tsg May 2, 2017

tsg commented May 2, 2017

ruflin May 2, 2017

andrewkroh May 2, 2017

andrewkroh May 2, 2017

andrewkroh May 2, 2017

andrewkroh May 2, 2017

andrewkroh May 2, 2017

tsg commented May 2, 2017

Metricbeat: store only top N processes by CPU/memory #4127

Metricbeat: store only top N processes by CPU/memory #4127

Conversation

tsg commented Apr 27, 2017 • edited Loading

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

tsg commented Apr 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsg commented May 2, 2017

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

Choose a reason for hiding this comment

tsg commented May 2, 2017

tsg commented Apr 27, 2017 •

edited

Loading