Skip to content

feat!: add summary to coder ping #14762

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

Merged
merged 4 commits into from
Sep 25, 2024

Conversation

ethanndickson
Copy link
Member

@ethanndickson ethanndickson commented Sep 23, 2024

Closes #14752.

This is a breaking change, as the default behavior of coder ping is now to ping indefinitely, until an interrupt is detected (alike many other ping commands, e.g. iputils ping). Previously, coder ping performed 10 pings by default. The -n argument can still be used to perform a fixed number of pings.

All diagnostics are also now written to stderr, instead of stdout.

New format with diagnostics included:

$ coder ping dev
✔ preferred DERP region: 10015 (Australia Fly.io (Sydney))
✔ sent local data to Coder networking coordinator
✔ received remote agent data from Coder networking coordinator
    preferred DERP region: 999 (Council Bluffs, Iowa)
    endpoints: xxx.xx.xxx.xxx:13337, xxx.xx.x.x:13337, xxx.xx.x.x:13337
✔ Wireguard handshake 0s ago

❗ The DERP map is not configured to use STUN
   https://coder.com/docs/@v2.15.0/networking/troubleshooting#no-stun-servers

Possible client-side issues with direct connection:

 - Client IP address is within an AWS range (AWS uses hard NAT)
   https://coder.com/docs/@v2.15.0/networking/troubleshooting#endpoint-dependent-nat-hard-nat

Possible agent-side issues with direct connections:

 - Agent IP address is within an AWS range (AWS uses hard NAT)
   https://coder.com/docs/@v2.15.0/networking/troubleshooting#endpoint-dependent-nat-hard-nat

pong from dev proxied via DERP(Council Bluffs, Iowa) in 209ms
pong from dev proxied via DERP(Council Bluffs, Iowa) in 209ms
pong from dev proxied via DERP(Council Bluffs, Iowa) in 208ms
pong from dev proxied via DERP(Council Bluffs, Iowa) in 209ms
pong from dev proxied via DERP(Council Bluffs, Iowa) in 209ms
<SIGINT>
❗ You are connected via a DERP relay, not directly (p2p)
https://coder.com/docs/@v2.15.0/networking/troubleshooting#common-problems-with-direct-connections
--------------------------------------------------------------------------------
WORKSPACE  TOTAL  SUCCESSFUL  MIN           AVG          MAX           STDDEV    
dev            5           5  208.407931ms  208.49958ms  208.544751ms  47.328µs  

(These links will be broken until the next release, as the troubleshooting page wasn't included in v2.15.0)

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @ethanndickson and the rest of your teammates on Graphite Graphite

@ethanndickson ethanndickson force-pushed the 09-23-feat_add_summary_to_coder_ping_ branch 5 times, most recently from 6fefb45 to 02d2b5a Compare September 24, 2024 05:49
@ethanndickson ethanndickson marked this pull request as ready for review September 24, 2024 05:53
Copy link
Member

@deansheather deansheather left a comment

Choose a reason for hiding this comment

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

I didn't read the diagnostics in your post but I think all diagnostics need to go at the end rather than some first and some afterwards.

@ethanndickson ethanndickson force-pushed the 09-23-feat_add_summary_to_coder_ping_ branch 2 times, most recently from c7a014b to d696bef Compare September 24, 2024 12:53
@ethanndickson ethanndickson force-pushed the 09-23-feat_add_summary_to_coder_ping_ branch from d696bef to d1040d5 Compare September 24, 2024 13:20
Comment on lines +67 to +71
if s.Successful > 0 {
s.Avg = ptr.Ref(time.Duration(s.latencySum / float64(s.Successful) * float64(time.Second)))
}
if s.Successful > 1 {
s.Variance = ptr.Ref(time.Duration((s.m2 / float64(s.Successful-1)) * float64(time.Second)))
}
Copy link
Member

Choose a reason for hiding this comment

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

This can be calculated on every addResult call no?

@deansheather
Copy link
Member

Wait, in your updated description you're dumping whether it's ping/p2p first? You can't log that until after you've finished pinging

@ethanndickson
Copy link
Member Author

ethanndickson commented Sep 24, 2024

❗ You are connected via a DERP relay, not directly (p2p)
https://coder.com/docs/@v2.15.0/networking/troubleshooting#common-problems-with-direct-connections

Was the only diagnostic left at the end when you said:

I didn't read the diagnostics in your post but I think all diagnostics need to go at the end rather than some first and some afterwards.

So I moved it up!

We're doing the silent ping ammar suggested to determine whether or not we'll show diagnostics at all, which is what I used to print it - but I'm definitely in favour of keeping it at the end.

@ethanndickson ethanndickson force-pushed the 09-23-feat_add_summary_to_coder_ping_ branch from d1040d5 to 0efd6ed Compare September 24, 2024 14:47
@ethanndickson ethanndickson merged commit b7c574f into main Sep 25, 2024
28 checks passed
@ethanndickson ethanndickson deleted the 09-23-feat_add_summary_to_coder_ping_ branch September 25, 2024 03:24
@github-actions github-actions bot locked and limited conversation to collaborators Sep 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

coder ping UX concerns
3 participants