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

Support IPv6 in the ping plugin #4703

Merged
merged 26 commits into from
Oct 2, 2018
Merged

Conversation

Jaeyo
Copy link
Contributor

@Jaeyo Jaeyo commented Sep 15, 2018

Required for all PRs:

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

close #2159
by using arguments options, use can determine packet size himself. so it can close #2830

@danielnelson
Copy link
Contributor

The way we've been planning to implement this is with a new option to override the ping command used as well as a way to set arguments directly. If you needed to do some ipv4 and some ipv6 urls then you would define the plugin twice.

[[inputs.ping]]
  urls = ["www.example.org"]
  binary = "ping6"
  arguments = ["-6"]

Do you think you could change around the options like this?

@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Sep 18, 2018
@Jaeyo Jaeyo changed the title Support IPv6 in the ping plugin [WIP] Support IPv6 in the ping plugin Sep 19, 2018
@Jaeyo
Copy link
Contributor Author

Jaeyo commented Sep 20, 2018

@danielnelson so the existing options for ping input (ex. ping_interval, timeout, deadline) will be replaced by "arguments"?

@danielnelson
Copy link
Contributor

Yes, I think what would be best is if when arguments is empty we us the options that currently exist, otherwise we only use the arguments and ignore ping_interval, timeout, etc.

@Jaeyo Jaeyo changed the title [WIP] Support IPv6 in the ping plugin Support IPv6 in the ping plugin Sep 25, 2018
@Jaeyo Jaeyo changed the title Support IPv6 in the ping plugin [WIP] Support IPv6 in the ping plugin Sep 25, 2018
@Jaeyo Jaeyo changed the title [WIP] Support IPv6 in the ping plugin Support IPv6 in the ping plugin Sep 26, 2018
@Jaeyo
Copy link
Contributor Author

Jaeyo commented Sep 26, 2018

@danielnelson can you review this?

Copy link
Contributor

@danielnelson danielnelson left a comment

Choose a reason for hiding this comment

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

Looks good from a quick glance. @glinton Can you do a more indepth review?

plugins/inputs/ping/ping_windows.go Outdated Show resolved Hide resolved
plugins/inputs/ping/ping.go Outdated Show resolved Hide resolved
plugins/inputs/ping/ping.go Outdated Show resolved Hide resolved
plugins/inputs/ping/ping.go Outdated Show resolved Hide resolved
plugins/inputs/ping/ping_windows.go Outdated Show resolved Hide resolved
@danielnelson danielnelson added this to the 1.9.0 milestone Oct 2, 2018
@danielnelson danielnelson merged commit 86b2145 into influxdata:master Oct 2, 2018
rgitzel pushed a commit to rgitzel/telegraf that referenced this pull request Oct 17, 2018
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
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.

Support packet size in the ping plugin Support IPv6 in the ping plugin
3 participants