feat(inputs.procstat): Allow multiple selection criteria

[srebhan]

Summary

This PR adds the possibility to specify multiple filter expressions for the process list. Each filter is independent of the other ones, intra-criterion lists are combined using logical or whereas different criteria are combined via logical and.

[[inputs.procstat]]
  [[inputs.procstat.filter]]
    name = "shells"
    patterns = ['fish']
    users = ["sv*"]

Checklist

• No AI generated code was used in this PR

Related issues

resolves #6174

[tguenneguez]

He,

Haven't test it, but It seems correct overall.

I doesn't see how the "pgrep" mode work ?
I think this new implementation will only be available with "native" mode ? Why have a double code ?

The Filter should have more criterias :
exe (the name of binary (/proc//exe)), see https://pkg.go.dev/github.com/shirou/gopsutil/v3/process#Process.Exe
name (content of process (/proc//comm)), see https://pkg.go.dev/github.com/shirou/gopsutil/v3/process#Process.Name

It's difficult because de multifilter is only available for :
name, patterns and users
In your code it's also possible with other filter, but it doesn't should work ?

It's strange to agregate multi filter values in the procstat metrics ?
If I understand your implementation with this conf :
[[inputs.procstat]]
[[inputs.procstat.filter]]
name = "crond"
[[inputs.procstat.filter]]
name = "sshd"
You will have procstat metrics wich agregate metrics of 2 types off processus. It's strange ?

Thanks
Thomas

[srebhan]

@tguenneguez I would love to get some help from you to implement the missing filters. This was meant as a proof-of-concept and is not finished yet... So please feel free to push to my PR!

Regarding pgrep, I'm not sure it even makes sense to add this here. The processes package of gopsutil is available for all supported platforms AFAICS so what is the benefit for doing it in two different ways?

[tguenneguez]

@tguenneguez I would love to get some help from you to implement the missing filters. This was meant as a proof-of-concept and is not finished yet... So please feel free to push to my PR!

Regarding pgrep, I'm not sure it even makes sense to add this here. The processes package of gopsutil is available for all supported platforms AFAICS so what is the benefit for doing it in two different ways?

100% agree with you.
I will see for help you to imprement good feature

[tguenneguez]

Before complete your job, there are things to clarify.

The "name" should be add by [inputs.procstat.tags].
I think it's not a good idea to add this feature in your code...

I also think you will have in one part :
pid_files, systemd_units, cgroups, supervisor_units, win_services
And in the other hand, filters.

Indeed, the first filters are already very restrictive and I don't think there is any point in combining them. Besides, the implementation does use "pid_finder".
So I would leave these first criteria aside.

In filter, I would only implement the filters:
name
exe
pattern (that will be renamed in "cmdLine")
user

Name is the content of /proc//comm
Exe is the link of /proc//exe

In Filters, it seem's to define some filters as a liste...
As the filters are cumulative, I don't think it makes sense to be able to put lists.
Wath is the meaning of :
[[inputs.procstat.filter]]
pattern = ["myproc", "system"]
users = ["root", "cft"]
Should it return processes that match all the criteria, one of the criteria, a pattern and a user, ...?

looking forward to reading you
Thomas

[srebhan]

Thanks @tguenneguez for your comments! I'm trying to answer one-by-one:

The "name" should be add by [inputs.procstat.tags].
I think it's not a good idea to add this feature in your code...

The user has to have some ability to find out which of the potentially multiple filters produced the data, so I think we need to add the filter name. If you don't like filter for the tag name, I'm open for suggestions, but you cannot do this via [inputs.procstat.tags] as those are the same for all filters.

I also think you will have in one part :
pid_files, systemd_units, cgroups, supervisor_units, win_services
And in the other hand, filters.

Why do you want to add this limitation? Imagine you want to monitor 4 different systemd units but only certain child processes or different users. In your scheme, this means four different plugin instances for no good reason...

Indeed, the first filters are already very restrictive and I don't think there is any point in combining them. Besides, the implementation does use "pid_finder".
So I would leave these first criteria aside.

This might be true for your use-cases but I know that people are looking for combining those to monitor only partial aspects...

[...] Name is the content of /proc//comm
Exe is the link of /proc//exe

I'm not against having name and exe, but please also think about other operating systems such as Windows or MacOS.

In Filters, it seem's to define some filters as a liste...
As the filters are cumulative, I don't think it makes sense to be able to put lists.
Wath is the meaning of :
[[inputs.procstat.filter]]
pattern = ["myproc", "system"]
users = ["root", "cft"]
Should it return processes that match all the criteria, one of the criteria, a pattern and a user, ...?

In fact, all filters are lists! Each entry, i.e. pattern or users is a or concatenated list while the entries are combined via and. So your example means the process matches (pattern == "myproc" || pattern == "system") && (user == "root" || user == "cft")). Feel free to improve the documentation!

I hope this clarifies some points!?

[powersj]

@tguenneguez have you had a chance to look over @srebhan's response above? Were you going to update your existing PR based on this?

[tguenneguez]

@tguenneguez have you had a chance to look over @srebhan's response above? Were you going to update your existing PR based on this?

I will see when I will have time but not for the moment sorry

[powersj]

Looks good, the new findByPidFiles where you verify if the PID exists would be good for #15017.

Made a couple small changes to sample conf & readme.

[DStrand1]

LGTM! Will leave to you to merge after resolving the review comments

[telegraf-tiger]

Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1%)

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	darwin_arm64.tar.gz	windows_arm64.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz	windows_i386.zip
armhf.deb	i386.rpm	freebsd_armv7.tar.gz
i386.deb	ppc64le.rpm	freebsd_i386.tar.gz
mips.deb	riscv64.rpm	linux_amd64.tar.gz
mipsel.deb	s390x.rpm	linux_arm64.tar.gz
ppc64el.deb	x86_64.rpm	linux_armel.tar.gz
riscv64.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_riscv64.tar.gz
		linux_s390x.tar.gz