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

added config flag to skip collection of network protocol metrics #3880

Merged
merged 1 commit into from
Mar 14, 2018
Merged

added config flag to skip collection of network protocol metrics #3880

merged 1 commit into from
Mar 14, 2018

Conversation

datwiz
Copy link
Contributor

@datwiz datwiz commented Mar 13, 2018

Added config flag to system net (inputs.net) to ignore collection of protocol stats.
Provides feature requested in #3879

Required for all PRs:

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

@danielnelson
Copy link
Contributor

Would you be interested in chaining down a list of protocols to the underlying call to gopsutil? Then we would have an option like protocols = ["udp", "tcp"] which eventually passes the list down to:

func ProtoCounters(protocols []string) ([]ProtoCountersStat, error) {

@datwiz
Copy link
Contributor Author

datwiz commented Mar 14, 2018

I can have a go at that. 2 questions:

  1. Would you see a protocols set option being used in place of an ignore_protocol_stats flag or in addition to a flag?
  2. If the set option is in addition to the ignore flag, would you like the work added into this PR or raising a new one?

@danielnelson
Copy link
Contributor

I guess we would need to have the boolean as well, otherwise we still can't disable them all and keep backwards compatibility. I was kind of hoping to avoid adding a boolean option here, since they often don't compose well as the plugin grows.

Let's scratch the idea of the protocols array, users can just use the fielddrop filtering option to achieve the same results.

@danielnelson danielnelson added this to the 1.6.0 milestone Mar 14, 2018
@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Mar 14, 2018
@danielnelson danielnelson merged commit f114f6a into influxdata:master Mar 14, 2018
@datwiz
Copy link
Contributor Author

datwiz commented Mar 14, 2018

My apologies. I'm new to telegraf and hadn't fully appreciated the tag and filtering capabilities.
In my case, all the protocol stats are attributed to the interface=all tag.
I've been able to achieve the same filtering, without a new config flag using a tag drop section.

# # Read metrics about network interface usage
[[inputs.net]]
  ## By default, telegraf gathers stats from any up interface (excluding loopback)
  ## Setting interfaces will tell it to gather these explicit interfaces,
  ## regardless of status.
  ##
  interfaces = ["eth0"]
  [inputs.net.tagdrop]
    interface = [ "all" ]

At least for my use case, this config flag isn't required.

@danielnelson
Copy link
Contributor

It hadn't occurred to me either that it would be so easy to filter. I think I will revert this change then since it is always better to have a smaller interface and we haven't had any other requests for this.

@datwiz datwiz deleted the ignore-net-protocol-stats branch March 14, 2018 04:35
@zabudskyi
Copy link

Would you be interested in adding option to gather stats from "interface to Internet" only? It is useful in case there are many hosts with different interface naming, numbering, etc.

@danielnelson
Copy link
Contributor

@zabudskyi How do you think we should pick which interfaces are to the internet?

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