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.

Sign up for GitHub

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

Jump to bottom

Support IPv6 in the ping plugin #4703

Merged
merged 26 commits into from
Oct 2, 2018
Merged

Support IPv6 in the ping plugin #4703

merged 26 commits into from
Oct 2, 2018

Conversation

Jaeyo
Copy link
Contributor

@Jaeyo Jaeyo commented Sep 15, 2018 •
edited
Loading

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 •
edited
Loading

@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?

danielnelson
danielnelson reviewed Sep 27, 2018
View reviewed changes
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
glinton
glinton suggested changes Sep 28, 2018
View reviewed changes
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
@Jaeyo @rgitzel
Add support for IPv6 in the ping plugin (influxdata#4703)
1cfdc07
@dirkaholic dirkaholic mentioned this pull request Feb 28, 2019
Closed
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
@Jaeyo @otherpirate
Add support for IPv6 in the ping plugin (influxdata#4703)
ab1bca2
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
@Jaeyo @otherpirate
Add support for IPv6 in the ping plugin (influxdata#4703)
7db0715
dupondje pushed a commit to dupondje/telegraf that referenced this pull request Apr 22, 2019
@Jaeyo
Add support for IPv6 in the ping plugin (influxdata#4703)
04363ca
athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
@Jaeyo
Add support for IPv6 in the ping plugin (influxdata#4703)
34808f1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielnelson danielnelson danielnelson left review comments

@glinton glinton glinton approved these changes

Assignees
No one assigned
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Milestone
1.9.0
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
@Jaeyo @danielnelson @glinton