Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit4f97578

Browse files
ethanndicksonEdwardAngert
authored andcommitted
chore(vpn): send ping results over tunnel (#18200)
Closes#17982.The purpose of this PR is to expose network latency via the API used by Coder Desktop.This PR has the tunnel ping all known agents every 5 seconds, in order to produce an instance of:```protomessage LastPing {// latency is the RTT of the ping to the agent.google.protobuf.Duration latency = 1;// did_p2p indicates whether the ping was sent P2P, or over DERP.bool did_p2p = 2;// preferred_derp is the human readable name of the preferred DERP region,// or the region used for the last ping, if it was sent over DERP.string preferred_derp = 3;// preferred_derp_latency is the last known latency to the preferred DERP// region. Unset if the region does not appear in the DERP map.optional google.protobuf.Duration preferred_derp_latency = 4;}```The contents of this message are stored and included on all subsequent upsertions of the agent. Note that we upsert existing agents every 5 seconds to update the `last_handshake` value.On the desktop apps, this message will be used to produce a tooltip similar to that of the VS Code extension:<img width="495" alt="image" src="https://github.com/user-attachments/assets/d8b65f3d-f536-4c64-9af9-35c1a42c92d2" />(wording not final)Unlike the VS Code extension, we omit:- The Latency of *all* available DERP regions. It seems not ideal to send a copy of this entire map for every online agent, and it certainly doesn't make sense for it to be on the `Agent` or `LastPing` message. If we do want to expose this info on Coder Desktop, we should consider how best to do so; maybe we want to include it on a more generic `Netcheck` message.- The current throughput (Bytes up/down). This is out of scope of the linked issue, and is non-trivial to implement. I'm also not sure of the value given the frequency we're doing these status updates (every 5 seconds).If we want to expose it, it'll be in a separate PR.<img width="343" alt="image" src="https://github.com/user-attachments/assets/8447d03b-9721-4111-8ac1-332d70a1e8f1" />
1 parentf46810a commit4f97578

File tree

12 files changed

+973
-318
lines changed

12 files changed

+973
-318
lines changed

‎cli/ssh.go‎

Lines changed: 8 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"path/filepath"
1717
"regexp"
1818
"slices"
19-
"strconv"
2019
"strings"
2120
"sync"
2221
"time"
@@ -31,7 +30,6 @@ import (
3130
"golang.org/x/term"
3231
"golang.org/x/xerrors"
3332
"gvisor.dev/gvisor/pkg/tcpip/adapters/gonet"
34-
"tailscale.com/tailcfg"
3533
"tailscale.com/types/netlogtype"
3634

3735
"cdr.dev/slog"
@@ -40,11 +38,13 @@ import (
4038
"github.com/coder/coder/v2/cli/cliui"
4139
"github.com/coder/coder/v2/cli/cliutil"
4240
"github.com/coder/coder/v2/coderd/autobuild/notify"
41+
"github.com/coder/coder/v2/coderd/util/maps"
4342
"github.com/coder/coder/v2/coderd/util/ptr"
4443
"github.com/coder/coder/v2/codersdk"
4544
"github.com/coder/coder/v2/codersdk/workspacesdk"
4645
"github.com/coder/coder/v2/cryptorand"
4746
"github.com/coder/coder/v2/pty"
47+
"github.com/coder/coder/v2/tailnet"
4848
"github.com/coder/quartz"
4949
"github.com/coder/retry"
5050
"github.com/coder/serpent"
@@ -1456,28 +1456,6 @@ func collectNetworkStats(ctx context.Context, agentConn *workspacesdk.AgentConn,
14561456
}
14571457
node:=agentConn.Node()
14581458
derpMap:=agentConn.DERPMap()
1459-
derpLatency:=map[string]float64{}
1460-
1461-
// Convert DERP region IDs to friendly names for display in the UI.
1462-
forrawRegion,latency:=rangenode.DERPLatency {
1463-
regionParts:=strings.SplitN(rawRegion,"-",2)
1464-
regionID,err:=strconv.Atoi(regionParts[0])
1465-
iferr!=nil {
1466-
continue
1467-
}
1468-
region,found:=derpMap.Regions[regionID]
1469-
if!found {
1470-
// It's possible that a workspace agent is using an old DERPMap
1471-
// and reports regions that do not exist. If that's the case,
1472-
// report the region as unknown!
1473-
region=&tailcfg.DERPRegion{
1474-
RegionID:regionID,
1475-
RegionName:fmt.Sprintf("Unnamed %d",regionID),
1476-
}
1477-
}
1478-
// Convert the microseconds to milliseconds.
1479-
derpLatency[region.RegionName]=latency*1000
1480-
}
14811459

14821460
totalRx:=uint64(0)
14831461
totalTx:=uint64(0)
@@ -1491,27 +1469,20 @@ func collectNetworkStats(ctx context.Context, agentConn *workspacesdk.AgentConn,
14911469
uploadSecs:=float64(totalTx)/dur.Seconds()
14921470
downloadSecs:=float64(totalRx)/dur.Seconds()
14931471

1494-
// Sometimes the preferred DERP doesn't match the one we're actually
1495-
// connected with. Perhaps because the agent prefers a different DERP and
1496-
// we're using that server instead.
1497-
preferredDerpID:=node.PreferredDERP
1498-
ifpingResult.DERPRegionID!=0 {
1499-
preferredDerpID=pingResult.DERPRegionID
1500-
}
1501-
preferredDerp,ok:=derpMap.Regions[preferredDerpID]
1502-
preferredDerpName:=fmt.Sprintf("Unnamed %d",preferredDerpID)
1503-
ifok {
1504-
preferredDerpName=preferredDerp.RegionName
1505-
}
1472+
preferredDerpName:=tailnet.ExtractPreferredDERPName(pingResult,node,derpMap)
1473+
derpLatency:=tailnet.ExtractDERPLatency(node,derpMap)
15061474
if_,ok:=derpLatency[preferredDerpName];!ok {
15071475
derpLatency[preferredDerpName]=0
15081476
}
1477+
derpLatencyMs:=maps.Map(derpLatency,func(dur time.Duration)float64 {
1478+
returnfloat64(dur)/float64(time.Millisecond)
1479+
})
15091480

15101481
return&sshNetworkStats{
15111482
P2P:p2p,
15121483
Latency:float64(latency.Microseconds())/1000,
15131484
PreferredDERP:preferredDerpName,
1514-
DERPLatency:derpLatency,
1485+
DERPLatency:derpLatencyMs,
15151486
UploadBytesSec:int64(uploadSecs),
15161487
DownloadBytesSec:int64(downloadSecs),
15171488
},nil

‎coderd/database/db2sdk/db2sdk.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,6 @@ func ListLazy[F any, T any](convert func(F) T) func(list []F) []T {
4747
}
4848
}
4949

50-
funcMap[Kcomparable,Fany,Tany](paramsmap[K]F,convertfunc(F)T)map[K]T {
51-
into:=make(map[K]T)
52-
fork,item:=rangeparams {
53-
into[k]=convert(item)
54-
}
55-
returninto
56-
}
57-
5850
typeExternalAuthMetastruct {
5951
Authenticatedbool
6052
ValidateErrorstring

‎coderd/util/maps/maps.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ import (
66
"golang.org/x/exp/constraints"
77
)
88

9+
funcMap[Kcomparable,Fany,Tany](paramsmap[K]F,convertfunc(F)T)map[K]T {
10+
into:=make(map[K]T)
11+
fork,item:=rangeparams {
12+
into[k]=convert(item)
13+
}
14+
returninto
15+
}
16+
917
// Subset returns true if all the keys of a are present
1018
// in b and have the same values.
1119
// If the corresponding value of a[k] is the zero value in

‎tailnet/derpmap.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@ import (
88
"net/http"
99
"os"
1010
"strconv"
11+
"strings"
12+
"time"
1113

1214
"golang.org/x/xerrors"
15+
"tailscale.com/ipn/ipnstate"
1316
"tailscale.com/tailcfg"
1417
)
1518

@@ -152,6 +155,49 @@ regionLoop:
152155
returnderpMap,nil
153156
}
154157

158+
funcExtractPreferredDERPName(pingResult*ipnstate.PingResult,node*Node,derpMap*tailcfg.DERPMap)string {
159+
// Sometimes the preferred DERP doesn't match the one we're actually
160+
// connected with. Perhaps because the agent prefers a different DERP and
161+
// we're using that server instead.
162+
preferredDerpID:=node.PreferredDERP
163+
ifpingResult.DERPRegionID!=0 {
164+
preferredDerpID=pingResult.DERPRegionID
165+
}
166+
preferredDerp,ok:=derpMap.Regions[preferredDerpID]
167+
preferredDerpName:=fmt.Sprintf("Unnamed %d",preferredDerpID)
168+
ifok {
169+
preferredDerpName=preferredDerp.RegionName
170+
}
171+
172+
returnpreferredDerpName
173+
}
174+
175+
// ExtractDERPLatency extracts a map of derp region names to their latencies
176+
funcExtractDERPLatency(node*Node,derpMap*tailcfg.DERPMap)map[string]time.Duration {
177+
latencyMs:=make(map[string]time.Duration)
178+
179+
// Convert DERP region IDs to friendly names for display in the UI.
180+
forrawRegion,latency:=rangenode.DERPLatency {
181+
regionParts:=strings.SplitN(rawRegion,"-",2)
182+
regionID,err:=strconv.Atoi(regionParts[0])
183+
iferr!=nil {
184+
continue
185+
}
186+
region,found:=derpMap.Regions[regionID]
187+
if!found {
188+
// It's possible that a workspace agent is using an old DERPMap
189+
// and reports regions that do not exist. If that's the case,
190+
// report the region as unknown!
191+
region=&tailcfg.DERPRegion{
192+
RegionID:regionID,
193+
RegionName:fmt.Sprintf("Unnamed %d",regionID),
194+
}
195+
}
196+
latencyMs[region.RegionName]=time.Duration(latency*float64(time.Second))
197+
}
198+
returnlatencyMs
199+
}
200+
155201
// CompareDERPMaps returns true if the given DERPMaps are equivalent. Ordering
156202
// of slices is ignored.
157203
//

‎tailnet/derpmap_test.go‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"testing"
1111

1212
"github.com/stretchr/testify/require"
13+
"tailscale.com/ipn/ipnstate"
1314
"tailscale.com/tailcfg"
1415

1516
"github.com/coder/coder/v2/tailnet"
@@ -162,3 +163,111 @@ func TestNewDERPMap(t *testing.T) {
162163
require.ErrorContains(t,err,"DERP map has no DERP nodes")
163164
})
164165
}
166+
167+
funcTestExtractDERPLatency(t*testing.T) {
168+
t.Parallel()
169+
170+
derpMap:=&tailcfg.DERPMap{
171+
Regions:map[int]*tailcfg.DERPRegion{
172+
1: {
173+
RegionID:1,
174+
RegionName:"Region One",
175+
Nodes: []*tailcfg.DERPNode{
176+
{Name:"node1",RegionID:1},
177+
},
178+
},
179+
2: {
180+
RegionID:2,
181+
RegionName:"Region Two",
182+
Nodes: []*tailcfg.DERPNode{
183+
{Name:"node2",RegionID:2},
184+
},
185+
},
186+
},
187+
}
188+
189+
t.Run("Basic",func(t*testing.T) {
190+
t.Parallel()
191+
node:=&tailnet.Node{
192+
DERPLatency:map[string]float64{
193+
"1-node1":0.05,
194+
"2-node2":0.1,
195+
},
196+
}
197+
latencyMs:=tailnet.ExtractDERPLatency(node,derpMap)
198+
require.EqualValues(t,50,latencyMs["Region One"].Milliseconds())
199+
require.EqualValues(t,100,latencyMs["Region Two"].Milliseconds())
200+
require.Len(t,latencyMs,2)
201+
})
202+
203+
t.Run("UnknownRegion",func(t*testing.T) {
204+
t.Parallel()
205+
node:=&tailnet.Node{
206+
DERPLatency:map[string]float64{
207+
"999-node999":0.2,
208+
},
209+
}
210+
latencyMs:=tailnet.ExtractDERPLatency(node,derpMap)
211+
require.EqualValues(t,200,latencyMs["Unnamed 999"].Milliseconds())
212+
require.Len(t,latencyMs,1)
213+
})
214+
215+
t.Run("InvalidRegionFormat",func(t*testing.T) {
216+
t.Parallel()
217+
node:=&tailnet.Node{
218+
DERPLatency:map[string]float64{
219+
"invalid":0.3,
220+
"1-node1":0.05,
221+
"abc-node":0.15,
222+
},
223+
}
224+
latencyMs:=tailnet.ExtractDERPLatency(node,derpMap)
225+
require.EqualValues(t,50,latencyMs["Region One"].Milliseconds())
226+
require.Len(t,latencyMs,1)
227+
require.NotContains(t,latencyMs,"invalid")
228+
require.NotContains(t,latencyMs,"abc-node")
229+
})
230+
231+
t.Run("EmptyInput",func(t*testing.T) {
232+
t.Parallel()
233+
node:=&tailnet.Node{
234+
DERPLatency:map[string]float64{},
235+
}
236+
latencyMs:=tailnet.ExtractDERPLatency(node,derpMap)
237+
require.Empty(t,latencyMs)
238+
})
239+
}
240+
241+
funcTestExtractPreferredDERPName(t*testing.T) {
242+
t.Parallel()
243+
derpMap:=&tailcfg.DERPMap{
244+
Regions:map[int]*tailcfg.DERPRegion{
245+
1: {RegionName:"New York"},
246+
2: {RegionName:"London"},
247+
},
248+
}
249+
250+
t.Run("UsesPingRegion",func(t*testing.T) {
251+
t.Parallel()
252+
pingResult:=&ipnstate.PingResult{DERPRegionID:2}
253+
node:=&tailnet.Node{PreferredDERP:1}
254+
result:=tailnet.ExtractPreferredDERPName(pingResult,node,derpMap)
255+
require.Equal(t,"London",result)
256+
})
257+
258+
t.Run("UsesNodePreferred",func(t*testing.T) {
259+
t.Parallel()
260+
pingResult:=&ipnstate.PingResult{DERPRegionID:0}
261+
node:=&tailnet.Node{PreferredDERP:1}
262+
result:=tailnet.ExtractPreferredDERPName(pingResult,node,derpMap)
263+
require.Equal(t,"New York",result)
264+
})
265+
266+
t.Run("UnknownRegion",func(t*testing.T) {
267+
t.Parallel()
268+
pingResult:=&ipnstate.PingResult{DERPRegionID:99}
269+
node:=&tailnet.Node{PreferredDERP:1}
270+
result:=tailnet.ExtractPreferredDERPName(pingResult,node,derpMap)
271+
require.Equal(t,"Unnamed 99",result)
272+
})
273+
}

‎vpn/client.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@ import (
55
"net/http"
66
"net/netip"
77
"net/url"
8+
"time"
89

910
"golang.org/x/xerrors"
1011

12+
"tailscale.com/ipn/ipnstate"
1113
"tailscale.com/net/dns"
1214
"tailscale.com/net/netmon"
15+
"tailscale.com/tailcfg"
1316
"tailscale.com/wgengine/router"
1417

1518
"github.com/google/uuid"
@@ -27,6 +30,9 @@ import (
2730
typeConninterface {
2831
CurrentWorkspaceState() (tailnet.WorkspaceUpdate,error)
2932
GetPeerDiagnostics(peerID uuid.UUID) tailnet.PeerDiagnostics
33+
Ping(ctx context.Context,agentID uuid.UUID) (time.Duration,bool,*ipnstate.PingResult,error)
34+
Node()*tailnet.Node
35+
DERPMap()*tailcfg.DERPMap
3036
Close()error
3137
}
3238

@@ -38,6 +44,10 @@ type vpnConn struct {
3844
updatesCtrl*tailnet.TunnelAllWorkspaceUpdatesController
3945
}
4046

47+
func (c*vpnConn)Ping(ctx context.Context,agentID uuid.UUID) (time.Duration,bool,*ipnstate.PingResult,error) {
48+
returnc.Conn.Ping(ctx,tailnet.TailscaleServicePrefix.AddrFromUUID(agentID))
49+
}
50+
4151
func (c*vpnConn)CurrentWorkspaceState() (tailnet.WorkspaceUpdate,error) {
4252
returnc.updatesCtrl.CurrentState()
4353
}

‎vpn/speaker_internal_test.go‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ func TestMain(m *testing.M) {
2323
goleak.VerifyTestMain(m,testutil.GoleakOptions...)
2424
}
2525

26+
constexpectedHandshake="codervpn tunnel 1.2\n"
27+
2628
// TestSpeaker_RawPeer tests the speaker with a peer that we simulate by directly making reads and
2729
// writes to the other end of the pipe. There should be at least one test that does this, rather
2830
// than use 2 speakers so that we don't have a bug where we don't adhere to the stated protocol, but
@@ -48,8 +50,6 @@ func TestSpeaker_RawPeer(t *testing.T) {
4850
errCh<-err
4951
}()
5052

51-
expectedHandshake:="codervpn tunnel 1.1\n"
52-
5353
b:=make([]byte,256)
5454
n,err:=mp.Read(b)
5555
require.NoError(t,err)
@@ -157,8 +157,6 @@ func TestSpeaker_OversizeHandshake(t *testing.T) {
157157
errCh<-err
158158
}()
159159

160-
expectedHandshake:="codervpn tunnel 1.1\n"
161-
162160
b:=make([]byte,256)
163161
n,err:=mp.Read(b)
164162
require.NoError(t,err)
@@ -210,7 +208,6 @@ func TestSpeaker_HandshakeInvalid(t *testing.T) {
210208
_,err=mp.Write([]byte(tc.handshake))
211209
require.NoError(t,err)
212210

213-
expectedHandshake:="codervpn tunnel 1.1\n"
214211
b:=make([]byte,256)
215212
n,err:=mp.Read(b)
216213
require.NoError(t,err)
@@ -248,8 +245,6 @@ func TestSpeaker_CorruptMessage(t *testing.T) {
248245
errCh<-err
249246
}()
250247

251-
expectedHandshake:="codervpn tunnel 1.1\n"
252-
253248
b:=make([]byte,256)
254249
n,err:=mp.Read(b)
255250
require.NoError(t,err)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp