[Heartbeat] Rootless pings #13795
[Heartbeat] Rootless pings #13795
Conversation
Test creation Changelog entry Removed unnecessary comment Windows fix Windows allows users to run icmp formatting log detailing formatting Review changes Travis Ci debugging autopep8 travis ci log analyzing removing logs Adding note
|
Pinging @elastic/uptime |
| if l.conn4 == nil && l.conn6 == nil { | ||
| if unprivilegedPossible { | ||
| var buffer bytes.Buffer | ||
| path, _ := os.Executable() |
There was a problem hiding this comment.
Not checking for the error?
There was a problem hiding this comment.
I don't think there's much to do with the error here. We're already reporting an error, so the worst case is we can't print the path. WDYT the behavior here should be?
There was a problem hiding this comment.
Did not check the code when extacly we would get an error. Was thinking to have the error as part of the message below if there is one. But I'm find with what you have now. If the path is empty, it will just print an empty line.
| @@ -202,6 +228,7 @@ func (l *icmpLoop) ping( | |||
| timeout time.Duration, | |||
| interval time.Duration, | |||
| ) (time.Duration, int, error) { | |||
| debugf("INVOKE PING") | |||
| @@ -272,6 +299,8 @@ func (l *icmpLoop) ping( | |||
| if !success { | |||
| return 0, requests, timeoutError{} | |||
| } | |||
|
|
|||
| debugf("PING DONE %v", rtt) | |||
|
@ruflin thanks for the quick review. Removed the stray |
| if l.conn4 == nil && l.conn6 == nil { | ||
| if unprivilegedPossible { | ||
| var buffer bytes.Buffer | ||
| path, _ := os.Executable() |
There was a problem hiding this comment.
Did not check the code when extacly we would get an error. Was thinking to have the error as part of the message below if there is one. But I'm find with what you have now. If the path is empty, it will just print an empty line.
We missed adding these docs in elastic#13795
We missed adding these docs in #13795
We missed adding these docs in elastic#13795 (cherry picked from commit 2c94e72)
We missed adding these docs in elastic#13795 (cherry picked from commit 2c94e72)
We missed adding these docs in elastic#13795 (cherry picked from commit 129fb3f)
This PR attempts to make #11610 by @leopucci merge-able (e.g. CI passes). The code looked good, but I think there are some remaining nits. This also changes the way errors are reported to make it cleaner and more testable.
Takes over for #13349 as well.