Procstat should support multiple search criteria

[kohonen]

Relevant telegraf.conf:

[[inputs.procstat]]
  exe = "java"
  user = "logstash|elasticsearch"
  interval = "1m"
  pid_tag = true

[[inputs.procstat]]
  exe = "influxd"
  user = "influxdb"
  interval = "1m"
  pid_tag = false

[[inputs.procstat]]
  exe = "node"
  user = "kibana"
  interval = "1m"

System info:

telegraf-1.11.1-1.x86_64
influxdb-1.7.7-1.x86_64

CentOS Linux release 7.6.1810 (Core)

Steps to reproduce:

the telegraf github page show way more fields and tags for the procstat_lookup measurement than we can see. tried to change pid_tag = true/false, no success

Expected behavior:

we need at least the 'user' tag in order to differentiate between the 2 java processes (elasticsearch+logstash)

procstat_lookup
tags:
- exe
- pid_finder
- pid_file
- pattern
- prefix
- user
- systemd_unit
- cgroup
- win_service
- result

fields:
- pid_count (int)
- running (int)
- result_code (int, success = 0, lookup_error = 1)

Actual behavior:

> select * from procstat_lookup
name: procstat_lookup
time                 exe     host               pid_count pid_finder result  result_code running
----                 ---     ----               --------- ---------- ------  ----------- -------
2019-07-26T08:26:00Z influxd MYHOSTNAME 1         pgrep      success 0           1
2019-07-26T08:26:00Z java    MYHOSTNAME 2         pgrep      success 0           2
2019-07-26T08:26:00Z node    MYHOSTNAME 1         pgrep      success 0           1

[danielnelson]

I should mention that the plugin right now only supports lookup using a single search criteria, so if you set both exe and user only exe is taking effect. Lookup on multiple attributes would be a good feature request though.

On the tags though, we should update the plugin so that the lookup tags on procstat_lookup match with the procstat measurement.

[danielnelson]

Tried fixing this today and it ended up being a result of procstat using only one search criteria. I'm going to convert this issue into a feature request for performing a logical AND across all the set search terms.

[mossad-zika]

hello?

[tguenneguez]

It would be nice to have this filtering capability.

[tguenneguez]

Hello,

I'm trying to implemente something to be able to search process by 2 criterias.
But after analysing how the plugin work, i realise that it doesn't search process with the same criteras if you are un natif mode or with pgrep.
In detail :

Mode	exe	pattern
pgrep	/proc/(pid)/stat (max 16 chars)	/proc/(pid)/cmdline (max 4096 chars)
natif	link /proc/(pid)/exe	/proc/(pid)/cmdline

I would like do a good implementation of this feature without breaking change but it's verry difficult.
pgrep isn't good for long args commands like java.
native isn't good for script because the exe isn't the process name.
There is a confusion between the exe and the process name.
The natif mode should search for exe in /proc/(pid)/stat not in the /proc/(pid)/exe...

I think that this feature will do :

Mode	exe	name	pattern
pgrep	/proc/(pid)/stat (max 16 chars)	/proc/(pid)/stat (max 16 chars)	/proc/(pid)/cmdline (max 4096 chars)
natif	link /proc/(pid)/exe	/proc/(pid)/stat (max 16 chars)	/proc/(pid)/cmdline

Thanks for ideas to do a good feature...
Thomas

[julien64140]

Hello,
I have the same problem with telegraf. Could you develop a new feature?
Thanks

[reminder84]

Same issue here, i am waiting for a release.

[srebhan]

@kohonen, @reminder84, @julien64140, @tguenneguez please test the binary in PR #14948 with the new filter definition. Let me know if this is what you expect!?!?

[tguenneguez]

Good morning,

Sorry, but I just read the code already.
In the readme, I would add that for filters:

• are based on native filtering and not on pgrep.
• each are cumulative.
  Specify that on Windows process_names and executables are the same

I would replace "wildcards" with "glob" ?

Why use "process_names" and not "names" ?
In this way I will replace name by identifier.

I find your code very well done and I can't wait to test it.
I'm on vacation tomorrow evening so it will be when I get back.

Thanks
Thomas

[srebhan]

@tguenneguez would you care to clarify the readme? I guess I'm a bit blind due to working on this for quite some time, so I'm likely not seeing ambiguous text segments...

Why use "process_names" and not "names" ?

I used this to be more specific as "names" is very general. Can you come up with some better naming?

In this way I will replace name by identifier.

Sorry I'm not getting this... :-(

[tguenneguez]

@tguenneguez would you care to clarify the readme? I guess I'm a bit blind due to working on this for quite some time, so I'm likely not seeing ambiguous text segments...

Why use "process_names" and not "names" ?

I used this to be more specific as "names" is very general. Can you come up with some better naming?

I started from the principle that we are on a plugin which supervises the processes so if we put the name, it is logically that of a process.
I also understand your thoughts which are also very valid.

In this way I will replace name by identifier.

Sorry I'm not getting this... :-(

the second part is to say if we change process_names to names, then name becomes ambiguous. hence the idea of renaming it by identifying

[srebhan]

@tguenneguez I merged the initial support, feel free to create a PR to improve things!