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

Fix percentile calculations and assign Deadline correctly #8839

Merged
merged 1 commit into from
Feb 9, 2021
Merged

Fix percentile calculations and assign Deadline correctly #8839

merged 1 commit into from
Feb 9, 2021

Conversation

sspaink
Copy link
Contributor

@sspaink sspaink commented Feb 8, 2021

Required for all PRs:

  • Has appropriate unit tests.

#8805

  • Sort the RTTS slice before calculating the percentile, updated a unit test to verify this
  • Changed inputs.ping "Deadline" config to be assigned to go-pings "Timeout"

go-ping doesn't support "timeout to wait for ping response (-W)" yet, but there is an open issue go-ping/ping#63

@sspaink sspaink requested a review from ssoroka February 8, 2021 23:18
telegraf-tiger[bot]
telegraf-tiger bot approved these changes Feb 8, 2021
View reviewed changes
Copy link
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

🤝 ✅ CLA has been signed. Thank you!

@sspaink sspaink changed the title Sort and timeout is deadline Fix percentile calculations and assign Deadline correctly Feb 8, 2021
srebhan
srebhan approved these changes Feb 9, 2021
View reviewed changes
Copy link
Contributor

@srebhan srebhan 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 to me.
Just a hint: You could also use the quantile aggregator (#8594) but now we have this in here... :-(

@srebhan srebhan self-assigned this Feb 9, 2021
@srebhan srebhan added area/ping fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. labels Feb 9, 2021
@sspaink sspaink merged commit aa6dc79 into influxdata:master Feb 9, 2021
ssoroka pushed a commit that referenced this pull request Feb 17, 2021
@sspaink @ssoroka
Sort and timeout is deadline (#8839)
a196a82 
(cherry picked from commit aa6dc79)
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
@sspaink
Sort and timeout is deadline (influxdata#8839)
b954244
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srebhan srebhan srebhan approved these changes

@ivorybilled ivorybilled ivorybilled approved these changes

@telegraf-tiger telegraf-tiger[bot] telegraf-tiger approved these changes

@ssoroka ssoroka Awaiting requested review from ssoroka

Assignees

@srebhan srebhan

Labels
area/ping fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sspaink @srebhan @ivorybilled