Skip to content

Commit d696bef

Browse files
ethanndicksonethanndickson
committed
single-pass variance
1 parent 98f33a0 commit d696bef

File tree

3 files changed

+76
-61
lines changed

3 files changed

+76
-61
lines changed

‎cli/cliui/agent.go

+6
Original file line numberDiff line numberDiff line change
@@ -458,5 +458,11 @@ func (d ConnDiags) splitDiagnostics() (general, client, agent []string) {
458458
agent = append(agent,
459459
fmt.Sprintf("Agent IP address is within an AWS range (AWS uses hard NAT)\n %s#endpoint-dependent-nat-hard-nat", d.TroubleshootingURL))
460460
}
461+
462+
if d.PingP2P {
463+
general = append(general, "✔ You are connected directly (p2p)")
464+
} else {
465+
general = append(general, fmt.Sprintf("❗ You are connected via a DERP relay, not directly (p2p)\n%s#common-problems-with-direct-connections", d.TroubleshootingURL))
466+
}
461467
return general, client, agent
462468
}

‎cli/ping.go

+32-47
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"errors"
66
"fmt"
77
"io"
8-
"math"
98
"net/http"
109
"net/netip"
1110
"strings"
@@ -36,48 +35,42 @@ type pingSummary struct {
3635
Min *time.Duration `table:"min"`
3736
Avg *time.Duration `table:"avg"`
3837
Max *time.Duration `table:"max"`
39-
StdDev *time.Duration `table:"stddev"`
38+
Variance *time.Duration `table:"variance"`
39+
latencySum float64
40+
runningAvg float64
41+
m2 float64
4042
}
4143

42-
func buildSummary(wsname string, r []*ipnstate.PingResult) *pingSummary {
43-
out := pingSummary{
44-
Workspace: wsname,
44+
func (s *pingSummary) addResult(r *ipnstate.PingResult) {
45+
s.Total++
46+
if r.Err != "" {
47+
return
4548
}
46-
totalLatency := 0.0
47-
latencies := make([]float64, 0, len(r))
48-
for _, pong := range r {
49-
out.Total++
50-
if pong.Err == "" {
51-
out.Successful++
52-
latencies = append(latencies, pong.LatencySeconds)
53-
totalLatency += pong.LatencySeconds
54-
}
49+
s.Successful++
50+
if s.Min == nil || r.LatencySeconds < s.Min.Seconds() {
51+
s.Min = ptr.Ref(time.Duration(r.LatencySeconds * float64(time.Second)))
5552
}
56-
if out.Successful > 0 {
57-
minLatency := latencies[0]
58-
maxLatency := latencies[0]
59-
varianceSum := 0.0
60-
avgLatency := totalLatency / float64(out.Successful)
61-
for _, l := range latencies {
62-
if l < minLatency {
63-
minLatency = l
64-
}
65-
if l > maxLatency {
66-
maxLatency = l
67-
}
68-
varianceSum += (l - avgLatency) * (l - avgLatency)
69-
}
70-
stddev := math.Sqrt(varianceSum / float64(out.Successful))
71-
out.StdDev = ptr.Ref(time.Duration(stddev * float64(time.Second)))
72-
out.Avg = ptr.Ref(time.Duration(avgLatency * float64(time.Second)))
73-
out.Max = ptr.Ref(time.Duration(maxLatency * float64(time.Second)))
74-
out.Min = ptr.Ref(time.Duration(minLatency * float64(time.Second)))
53+
if s.Max == nil || r.LatencySeconds > s.Min.Seconds() {
54+
s.Max = ptr.Ref(time.Duration(r.LatencySeconds * float64(time.Second)))
7555
}
76-
return &out
56+
s.latencySum += r.LatencySeconds
57+
58+
d := r.LatencySeconds - s.runningAvg
59+
s.runningAvg += d / float64(s.Successful)
60+
d2 := r.LatencySeconds - s.runningAvg
61+
s.m2 += d * d2
7762
}
7863

79-
func (p *pingSummary) Write(w io.Writer) {
80-
out, err := cliui.DisplayTable([]*pingSummary{p}, "", nil)
64+
// Write finalizes the summary and writes it
65+
func (s *pingSummary) Write(wsname string, w io.Writer) {
66+
s.Workspace = wsname
67+
if s.Successful > 0 {
68+
s.Avg = ptr.Ref(time.Duration(s.latencySum / float64(s.Successful) * float64(time.Second)))
69+
}
70+
if s.Successful > 1 {
71+
s.Variance = ptr.Ref(time.Duration((s.m2 / float64(s.Successful-1)) * float64(time.Second)))
72+
}
73+
out, err := cliui.DisplayTable([]*pingSummary{s}, "", nil)
8174
if err != nil {
8275
_, _ = fmt.Fprintf(w, "Failed to display ping summary: %v\n", err)
8376
return
@@ -194,10 +187,9 @@ func (r *RootCmd) ping() *serpent.Command {
194187
}
195188

196189
connDiags.Write(inv.Stdout)
197-
190+
results := &pingSummary{}
198191
n := 0
199192
start := time.Now()
200-
results := make([]*ipnstate.PingResult, 0)
201193
pingLoop:
202194
for {
203195
if n > 0 {
@@ -208,7 +200,7 @@ func (r *RootCmd) ping() *serpent.Command {
208200
ctx, cancel := context.WithTimeout(ctx, pingTimeout)
209201
dur, p2p, pong, err := conn.Ping(ctx)
210202
cancel()
211-
results = append(results, pong)
203+
results.addResult(pong)
212204
if err != nil {
213205
if xerrors.Is(err, context.DeadlineExceeded) {
214206
_, _ = fmt.Fprintf(inv.Stdout, "ping to %q timed out \n", workspaceName)
@@ -274,14 +266,7 @@ func (r *RootCmd) ping() *serpent.Command {
274266
}
275267
}
276268

277-
if didP2p {
278-
_, _ = fmt.Fprintf(inv.Stdout, "✔ You are connected directly (p2p)\n")
279-
} else {
280-
_, _ = fmt.Fprintf(inv.Stdout, "❗ You are connected via a DERP relay, not directly (p2p)\n%s#common-problems-with-direct-connections\n", connDiags.TroubleshootingURL)
281-
}
282-
283-
summary := buildSummary(workspaceName, results)
284-
summary.Write(inv.Stdout)
269+
results.Write(workspaceName, inv.Stdout)
285270

286271
return nil
287272
},

‎cli/ping_internal_test.go

+38-14
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
package cli
22

33
import (
4+
"io"
45
"testing"
56
"time"
67

78
"github.com/stretchr/testify/require"
89
"tailscale.com/ipn/ipnstate"
9-
10-
"github.com/coder/coder/v2/coderd/util/ptr"
1110
)
1211

1312
func TestBuildSummary(t *testing.T) {
@@ -34,18 +33,36 @@ func TestBuildSummary(t *testing.T) {
3433
},
3534
}
3635

37-
expected := &pingSummary{
38-
Workspace: "test",
39-
Total: 4,
40-
Successful: 3,
41-
Min: ptr.Ref(time.Duration(0.1 * float64(time.Second))),
42-
Avg: ptr.Ref(time.Duration(0.2 * float64(time.Second))),
43-
Max: ptr.Ref(time.Duration(0.3 * float64(time.Second))),
44-
StdDev: ptr.Ref(time.Duration(0.081649658 * float64(time.Second))),
36+
actual := pingSummary{}
37+
for _, r := range input {
38+
actual.addResult(r)
4539
}
40+
actual.Write("test", io.Discard)
41+
require.Equal(t, time.Duration(0.1*float64(time.Second)), *actual.Min)
42+
require.Equal(t, time.Duration(0.2*float64(time.Second)), *actual.Avg)
43+
require.Equal(t, time.Duration(0.3*float64(time.Second)), *actual.Max)
44+
require.Equal(t, time.Duration(0.009999999*float64(time.Second)), *actual.Variance)
45+
require.Equal(t, actual.Successful, 3)
46+
})
4647

47-
actual := buildSummary("test", input)
48-
require.Equal(t, expected, actual)
48+
t.Run("One", func(t *testing.T) {
49+
t.Parallel()
50+
input := []*ipnstate.PingResult{
51+
{
52+
LatencySeconds: 0.2,
53+
},
54+
}
55+
56+
actual := &pingSummary{}
57+
for _, r := range input {
58+
actual.addResult(r)
59+
}
60+
actual.Write("test", io.Discard)
61+
require.Equal(t, actual.Successful, 1)
62+
require.Equal(t, time.Duration(0.2*float64(time.Second)), *actual.Min)
63+
require.Equal(t, time.Duration(0.2*float64(time.Second)), *actual.Avg)
64+
require.Equal(t, time.Duration(0.2*float64(time.Second)), *actual.Max)
65+
require.Nil(t, actual.Variance)
4966
})
5067

5168
t.Run("NoLatency", func(t *testing.T) {
@@ -67,10 +84,17 @@ func TestBuildSummary(t *testing.T) {
6784
Min: nil,
6885
Avg: nil,
6986
Max: nil,
70-
StdDev: nil,
87+
Variance: nil,
88+
latencySum: 0,
89+
runningAvg: 0,
90+
m2: 0,
7191
}
7292

73-
actual := buildSummary("test", input)
93+
actual := &pingSummary{}
94+
for _, r := range input {
95+
actual.addResult(r)
96+
}
97+
actual.Write("test", io.Discard)
7498
require.Equal(t, expected, actual)
7599
})
76100
}

0 commit comments

Comments
 (0)