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 support for glob patterns in net input plugin #3140

Merged

Conversation

bobmshannon
Copy link
Contributor

An approach for fixing #1073.

Adds support for specifying golang glob-style patterns.

Example

[[inputs.net]]
   interfaces = ["en*", "utun[0-1]", "lo0"]
$ ./telegraf --input-filter net --config telegraf.conf --test
* Plugin: inputs.net, Collection 1
> net,interface=lo0,host=mac bytes_sent=103240327i,packets_sent=227866i,drop_in=0i,drop_out=0i,bytes_recv=103240327i,packets_recv=227866i,err_in=0i,err_out=0i 1503126134000000000
> net,interface=en0,host=macbook packets_sent=13568823i,packets_recv=23715254i,err_in=0i,err_out=0i,drop_out=3193i,bytes_sent=2411560838i,bytes_recv=30527805468i,drop_in=0i 1503126134000000000
> net,interface=en1,host=macbook bytes_sent=0i,drop_out=0i,bytes_recv=0i,packets_sent=0i,packets_recv=0i,err_in=0i,err_out=0i,drop_in=0i 1503126134000000000
> net,interface=en2,host=macbook bytes_recv=0i,packets_sent=0i,packets_recv=0i,err_in=0i,err_out=0i,drop_in=0i,bytes_sent=0i,drop_out=0i 1503126134000000000
> net,interface=utun0,host=macbook bytes_sent=268i,packets_sent=3i,err_in=0i,drop_in=0i,drop_out=0i,bytes_recv=0i,packets_recv=0i,err_out=0i 1503126134000000000
> net,interface=utun1,host=macbook bytes_sent=22156i,packets_recv=115i,drop_in=0i,drop_out=0i,bytes_recv=20275i,packets_sent=108i,err_in=0i,err_out=0i 1503126134000000000

Required for all PRs:

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

@danielnelson danielnelson added this to the 1.5.0 milestone Aug 21, 2017
@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Aug 21, 2017
@ljagiello
Copy link
Contributor

@danielnelson any chance this PR will be merge before 1.5.0?

@danielnelson
Copy link
Contributor

I think we can get this in, looking over the code I assume the code causing the performance issue is caused by calling net.InterfaceByName(io.Name), which is only called if the interfaces option is empty. So by adding the ability to use globs we bypass that code? If either of you are experiencing the performance issue can you verify my assumption?

Godeps Outdated
@@ -27,6 +27,7 @@ github.com/go-ole/go-ole be49f7c07711fcb603cff39e1de7c67926dc0ba7
github.com/google/go-cmp f94e52cad91c65a63acc1e75d4be223ea22e99bc
github.com/gorilla/mux 392c28fe23e1c45ddba891b0320b3b5df220beea
github.com/go-sql-driver/mysql 2e00b5cd70399450106cec6431c2e2ce3cae5034
github.com/gobwas/glob bea32b9cd2d6f55753d94a28e959b13f0244797a
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have this (line 21)

"github.com/influxdata/telegraf"
"github.com/influxdata/telegraf/plugins/inputs"
)

type NetIOStats struct {
ps PS
patterns []glob.Glob
Copy link
Contributor

Choose a reason for hiding this comment

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

Use filter.Filter here

@bobmshannon
Copy link
Contributor Author

bobmshannon commented Nov 28, 2017

I think the performance problem can currently be mitigated by explicitly specifying a list of interfaces, it's just that support for glob patterns makes doing so less verbose and more adapting to different environments (e.g. eth* vs eth0,eth1,eth2,eth3,...)

@ljagiello
Copy link
Contributor

@bobmshannon It would be super cool if you could add blacklist, I would love to blacklist all calico interfaces.

@bobmshannon
Copy link
Contributor Author

@danielnelson I have addressed the PR comments, but it looks like the build may have flaked.

@ljagiello A blacklist sounds like an interesting idea. I'm not sure I can get that in this PR but it may be worth opening a separate issue for. @danielnelson, what do you think about such an option?

@danielnelson
Copy link
Contributor

We can add the blacklist, usually we have a pair of options like interface_include interface_exclude. We could add this in a backwards compatible way by appending the interfaces option to interface_include.

@danielnelson
Copy link
Contributor

I think the tests are failing because the branch is so far behind master. Failures are in unrelated code though so I don't mind merging.

@danielnelson danielnelson merged commit beb9d75 into influxdata:master Nov 29, 2017
aromeyer pushed a commit to aromeyer/telegraf that referenced this pull request May 19, 2018
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