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

Monitor process by pidfile or exe name #240

Closed
wants to merge 10 commits into from
Closed

Monitor process by pidfile or exe name #240

wants to merge 10 commits into from

Conversation

ranjib
Copy link
Contributor

@ranjib ranjib commented Oct 4, 2015

Process plugin allows monitoring individual processes specified by pid file or exe name. It will report back memory, cpu, context switch and io stats. name is used to prefix measurements.

[process]
[[process.specifications]]
  exe = "influxd"
  name = "influxd"
[[process.specifications]]
  pid_file = "/var/run/lxc/dnsmasq.pid"
  name = "dnsmasq"

@sparrc this was my exact requirement for #215 . After going through the nagios and collectd plugins i have realized this is bit different than the overall processes plugin, where a different type of metrics. I am planning to add global process metrics by a separate PR (still need some more details, i'll continue the discussion on the ticket)

@sparrc
Copy link
Contributor

sparrc commented Oct 5, 2015

@ranjib looks like a godep dependency issue with the build, try this:

$ go get github.com/tools/godep
$ godep save ./...

@sparrc
Copy link
Contributor

sparrc commented Oct 5, 2015

@ranjib this looks good, thank you for the contribution! 👍, two things:

  1. The name of a the plugin might conflict with a more general "process" plugin for aggregating sleep/zombie/active processes, similar to the collectd processes plugin. Maybe we can name this one something else, like procstat?
  2. I feel that the name field in the config is confusing, and could easily be confused for the name of the process. Let's rename that to something else, something along the lines of prefix or measurement_prefix

@ranjib
Copy link
Contributor Author

ranjib commented Oct 5, 2015

@sparrc im getting the following error with godep save
godep: cannot find package "github.com/prometheus/client_golang/extraction" in any of: /home/zenith/softwares/go1.4/src/github.com/prometheus/client_golang/extraction (from $GOROOT) /home/zenith/go/src/github.com/prometheus/client_golang/extraction (from $GOPATH) godep: cannot find package "github.com/prometheus/client_golang/model" in any of: /home/zenith/softwares/go1.4/src/github.com/prometheus/client_golang/model (from $GOROOT) /home/zenith/go/src/github.com/prometheus/client_golang/model (from $GOPATH) godep: error loading dependencies
i can run godep go build ./... and godep go test -short ./... without any issue :-/

} else {
return strconv.Atoi(strings.TrimSpace(string(pidString)))
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering how this would behave should pgrep return multiple PIDs. I tend to run multiple Python interpreters on our servers, and setting "python" as the executable makes pgrep return the PID for each one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arnimarj it will error out in that case. an strconv.Atoi call. this is as expected. i'll prefer pid file or master process' name for such scenarios.

@sparrc
Copy link
Contributor

sparrc commented Oct 5, 2015

@ranjib try this:

godep restore
godep save ./...

Or, if you could give me write access to your fork, I can push a change to this PR to fix it. Let me know what you prefer.

@sparrc
Copy link
Contributor

sparrc commented Oct 5, 2015

Could you also please write a README for this plugin? See #177

@ranjib
Copy link
Contributor Author

ranjib commented Oct 5, 2015

@sparrc

  • i'll add readme
  • i tried godep restore before, it was giving the same error. im still looking into,
  • i have granted your write access on my fork,
    thanks for taking time , really appreciate this :0)

@sparrc
Copy link
Contributor

sparrc commented Oct 5, 2015

@ranjib fixed, although...

I'm sorry, but I made some force pushes, I was in the zone and using my regular workflow on personal branches!

Hope it doesn't mess you up too much, let me know if I can lend a hand

@ranjib
Copy link
Contributor Author

ranjib commented Oct 5, 2015

@sparrc done.

@sparrc
Copy link
Contributor

sparrc commented Oct 5, 2015

@ranjib can you please make the README more detailed? You can use the CPU plugin as an example:
https://github.com/influxdb/telegraf/blob/master/plugins/system/CPU_README.md

Primarily I'd like to see documentation of the metrics that it will output.

@ranjib
Copy link
Contributor Author

ranjib commented Oct 6, 2015

@sparrc sure. i took the example from exec plugin.

@ranjib
Copy link
Contributor Author

ranjib commented Oct 7, 2015

@sparrc updated readme with measurement names.
also, i am not able to build telegraf due to this: shirou/gopsutil@3303647
can you update the shirou/gopsutil version.

@sparrc
Copy link
Contributor

sparrc commented Oct 7, 2015

thank you @ranjib for updating the README, is this ready to merge?

@ranjib
Copy link
Contributor Author

ranjib commented Oct 7, 2015

@sparrc i think so, i am hoping i dont have to rebase for the gopsutil changes.
i really want to understand why godeps is not working on my end.. is influxdb mailing is the right place to ask?

@sparrc
Copy link
Contributor

sparrc commented Oct 7, 2015

No I don't think so, the influxdb repo doesn't use godep in fact. You may want to file an issue in the godep project: https://github.com/tools/godep

@sparrc sparrc mentioned this pull request Oct 7, 2015
@sparrc sparrc closed this in 6827459 Oct 7, 2015
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants