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.

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

Check if a process is running using it's name #5768

Merged

Conversation

fuegas
Copy link
Contributor

@fuegas fuegas commented Apr 25, 2019

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

This PR relates to #5746. When you provide a pid_file that exists, has a (int32) number in it of a non-existing process, it will report the process as running right from the start of the agent:

> procstat_lookup,host=Kita.local,pid_finder=pgrep,pidfile=/tmp/process.pid,result=success pid_count=1i,result_code=0i,running=1i 1556141065000000000

A process is marked running if the procs map has at least one item:

"running":     len(procs),

This map is build/filled by updateProcesses:

info, ok := prevInfo[pid]
if ok {
  procs[pid] = info
}
...
proc, err := p.createProcess(pid)
if err != nil {
  // No problem; process may have ended after we found it
  continue
}
procs[pid] = proc

Because the PID is specified by a pid_file we have a valid PID (int32). This results in a valid Process from p.createProcess(pid), regardless whether the process exists or not. This Process is inserted into the procs map resulting in a map that has 1 or more entries. So if you provide a pid_file that has a number in it, procstat will always return running=1i.

In this PR I assume processes without name are rare and the chance that you want to monitor a process with no name is even rarer, a process without (or empty) name is seen as not running.

Assuming a processes without name is rare and the chance that you want to monitor a process with no name is even rarer, a process without (or empty) name is seen as not running.
@sgtsquiggs
Copy link
Contributor

Is this really necessary?

updateProcesses depends on gopsutil's NewProcess to check if the process exists, which achieves this by attempting to open /proc/<pid>.

@fuegas
Copy link
Contributor Author

fuegas commented Apr 25, 2019

That is true for the Linux adapter. However, on Darwin, FreeBSD, OpenBSD and Windows the process is only created as a struct and no checks are done. So on those platforms the process from a pidfile always shows running=1i regardless of the actual state.

@sgtsquiggs
Copy link
Contributor

That is unfortunate. For linux/bsd platforms we would confirm that the process exists via

err := process.Signal(syscall.Signal(0))

assuming process is type os.Process.

For windows, os.FindProcess actually checks to see if the process exists.

@danielnelson danielnelson added the fix pr to fix corresponding bug label Apr 26, 2019
@danielnelson danielnelson added this to the 1.11.0 milestone Apr 29, 2019
@danielnelson danielnelson merged commit cb4387d into influxdata:master Apr 29, 2019
@fuegas fuegas deleted the verify-pidfile-process-exists branch May 20, 2019 08:56
hwaastad pushed a commit to hwaastad/telegraf that referenced this pull request Jun 13, 2019
bitcharmer pushed a commit to bitcharmer/telegraf that referenced this pull request Oct 18, 2019
athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants