Skip to content

Fix percentile calculations and assign Deadline correctly#8839

Merged
sspaink merged 1 commit intoinfluxdata:masterfrom
sspaink:pingreg
Feb 9, 2021
Merged

Fix percentile calculations and assign Deadline correctly#8839
sspaink merged 1 commit intoinfluxdata:masterfrom
sspaink:pingreg

Conversation

@sspaink
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

@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

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

@ssoroka ssoroka Awaiting requested review from ssoroka

+1 more reviewer

@ivorybilled ivorybilled ivorybilled approved these changes

Reviewers whose approvals may not affect merge requirements

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.

3 participants

@sspaink @srebhan @ivorybilled