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

Bugfix/procstat adds new procstat metric about pgrep search #4307

Merged
merged 15 commits into from
Jun 19, 2018

Conversation

maxunt
Copy link
Contributor

@maxunt maxunt commented Jun 15, 2018

Required for all PRs:

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

if err != nil {
return "", fmt.Errorf("Error running %s: %s", path, err)
//return "", fmt.Errorf("Error running %s: %s", path, err)
return "", fmt.Errorf("Error executing command: %v %v, err: %s", path, args, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep only the old error message.

@@ -369,3 +370,39 @@ func TestGather_cgroupPIDs(t *testing.T) {
assert.Equal(t, []PID{1234, 5678}, pids)
assert.Equal(t, td, tags["cgroup"])
}

func TestExitStatus(t *testing.T) {
out, err := exec.Command("pgrep", "-G", "sys").Output()
Copy link
Contributor

Choose a reason for hiding this comment

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

This command could return different results on different systems, so it could create random failures, perhaps you can make the needed error by hand. Also, it tests a function in internal/internal.go so it should be placed in internal/internal_test.go


func TestNoPgrepResults(t *testing.T) {
p := Procstat{
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.

Same comment as above for this and the next test. You should be able to use testPgrep in this same file to fake the results.


func TestExitStatus(t *testing.T) {
out, err := exec.Command("pgrep", "-G", "sys").Output()
t.Logf("cmd out: %v", out)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove any logging in the test cases, use assertions to document results.

@danielnelson danielnelson added this to the 1.8.0 milestone Jun 19, 2018
@danielnelson danielnelson added feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin area/procstat labels Jun 19, 2018
@danielnelson danielnelson merged commit 3920667 into master Jun 19, 2018
@danielnelson danielnelson deleted the bugfix/procstat branch June 19, 2018 18:47
mkboudreau added a commit to mkboudreau/telegraf that referenced this pull request Jun 22, 2018
* origin: (39 commits)
  Document path tag in tail input
  Update changelog
  Added path tag to tail input plugin (influxdata#4292)
  Run windows tests with -short
  Fix postfix input handling of multi-level queues (influxdata#4333)
  Update changelog
  Add support for comma in logparser timestamp format (influxdata#4311)
  Update vendoring to dep from gdm (influxdata#4314)
  Update changelog
  Add new measurement with results of pgrep lookup to procstat input (influxdata#4307)
  Update changelog
  Add valuecounter aggregator plugin (influxdata#3523)
  Update changelog
  Update docker input documentation for container status
  Add container status tag to docker input (influxdata#4259)
  Drop CI support for Go 1.8 (influxdata#4309)
  Update changelog
  Fix selection of tags under nested objects in the JSON parser (influxdata#4284)
  Update changelog
  Add owner tag on partitions in burrow input (influxdata#4281)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/procstat 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.

None yet

2 participants