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 flag to remove uptime and n_users #5734

Closed
wants to merge 1 commit into from

Conversation

ehlerst
Copy link
Contributor

@ehlerst ehlerst commented Apr 17, 2019

This PR allows setting of flags to turn them off for those who wish.

Required for all PRs:

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

#5398
#5283
#3837
more...

Flags set to true

system,host=some-clever-name load1=2.18,load5=3.31,load15=3.45,n_cpus=8i 1555475800000000000

Flags set to false (still the default)

system,host=some-clever-name uptime_format=" 0:34" 1555475750000000000
system,host=some-clever-name uptime=2095i 1555475750000000000
system,host=some-clever-name load1=2.36,load5=3.5,load15=3.52,n_cpus=8i,n_users=1i 1555475750000000000

@@ -8,8 +8,12 @@ and number of users logged in. It is similar to the unix `uptime` command.
```toml
# Read metrics about system load & uptime
[[inputs.system]]
# no configuration
## Uncomment the following line to disable uptime collection
# skip_uptime = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make this a filter. I think just an exclude filter is sufficient. Allowed values would be uptime and users.

filter.NewIncludeExcludeFilter([]string{}, SomethingExclude)

The docker input plugin uses filters and may be a good reference.

}

var SystemsampleConfig = `

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this blank line.

@glinton glinton added feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin area/system labels Apr 17, 2019
@danielnelson
Copy link
Contributor

I feel like this is too many options for such a small plugin.

The only remaining issue with utmp that I know of is when it doesn't exist. I think it would be a good change to skip the error logging if os.IsNotExist(err) and add a note to the README that it requires this file.

With uptime, I haven't seen any isses at all, if its just something you don't want you can add it to the fielddrop option:

[[inputs.system]]
  fielddrop = ["uptime*"]

@ehlerst ehlerst mentioned this pull request Apr 17, 2019
1 task
@ehlerst
Copy link
Contributor Author

ehlerst commented Apr 17, 2019

I like @danielnelson idea better, #5742 to replace this.

@ehlerst ehlerst closed this Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/system 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