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

Add running field to procstat_lookup #5069

Merged
merged 3 commits into from
Dec 12, 2018
Merged

Conversation

danielnelson
Copy link
Contributor

This field indicates the number of processes that were actually found, as opposed to pid_count which tells you how many pids were found and may not be what you are after with some query methods.

closes #5043

Required for all PRs:

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

@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Nov 30, 2018
@danielnelson danielnelson added this to the 1.10.0 milestone Nov 30, 2018
@danielnelson
Copy link
Contributor Author

@LTKH Can you give this changeset a shot and let me know if it works out okay?

@LTKH
Copy link

LTKH commented Nov 30, 2018

@danielnelson Tested. If the pid file is missing, running returns nothing. If it is possible to make running always return zero, even if the pid file is not found?

@danielnelson
Copy link
Contributor Author

Yeah I can do that, good catch.

@danielnelson
Copy link
Contributor Author

I made a few changes, it is always showing the running count now but I also added a result tag+result_code field for the status of the lookup. If the lookup fails the metric will still be emitted as well.

@LTKH
Copy link

LTKH commented Nov 30, 2018

@danielnelson Tested. Works perfect! Thank you.

@danielnelson danielnelson requested a review from glinton November 30, 2018 23:58
@@ -93,6 +93,7 @@ func (p *Procstat) Gather(acc telegraf.Accumulator) error {
case "pgrep":
p.createPIDFinder = NewPgrep
default:
p.PidFinder = "pgrep"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this fit better in the init function, as it seems to be a standard location for object defaults?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried this out, it seems good but then it moves it away from the setting of p.createPIDFinder, which it would need updated alongside of. We could set that in the init function too, but then we would need to introduce a initialized boolean or such. We can rework this when we add constructor functions for the plugins (part of the config prototype).

@danielnelson danielnelson merged commit cf2b85f into master Dec 12, 2018
@danielnelson danielnelson deleted the procstat-lookup-running branch December 12, 2018 03:11
trevorwhitney pushed a commit to trevorwhitney/telegraf that referenced this pull request Feb 14, 2019
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
dupondje pushed a commit to dupondje/telegraf that referenced this pull request Apr 22, 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants