Skip to content

chore: add additional network telemetry stats & events #13800

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.

Sign up for GitHub

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 17 commits into from
Jul 10, 2024
Merged

chore: add additional network telemetry stats & events #13800

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
6ee3bfa
chore: add additional network telemetry stats & events
ethanndickson Jul 8, 2024
6e0ec17
better connectionage
ethanndickson Jul 8, 2024
ae17bb3
send update events only when home derp changes
ethanndickson Jul 8, 2024
612ae5f
better remote node id
ethanndickson Jul 8, 2024
d2d9c39
use netmap callback
ethanndickson Jul 8, 2024
af36c37
fixup
ethanndickson Jul 8, 2024
3466ba6
fixup
ethanndickson Jul 8, 2024
dbffec4
test
ethanndickson Jul 8, 2024
b240507
set p2p setup
ethanndickson Jul 9, 2024
e2e4018
fixup
ethanndickson Jul 9, 2024
0bebad4
watch for conn changes
ethanndickson Jul 9, 2024
6a19849
fixup
ethanndickson Jul 9, 2024
11f2a2b
use latest p2p setup time
ethanndickson Jul 9, 2024
fe7252a
ensure latest derpmap
ethanndickson Jul 9, 2024
4b76942
nicer ping peer
ethanndickson Jul 9, 2024
6214e2a
fix race
ethanndickson Jul 9, 2024
60e88f7
relay -> udp addr
ethanndickson Jul 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
better remote node id
  • Loading branch information
@ethanndickson
ethanndickson committed Jul 8, 2024
commit 612ae5f45fbf14c94de40966ebe8d8c7b94ae045
3 changes: 1 addition & 2 deletions cli/ssh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"github.com/coder/coder/v2/codersdk/workspacesdk"
"github.com/coder/coder/v2/cryptorand"
"github.com/coder/coder/v2/pty"
"github.com/coder/coder/v2/tailnet"
"github.com/coder/retry"
"github.com/coder/serpent"
)
Expand Down Expand Up @@ -438,7 +437,7 @@ func (r *RootCmd) ssh() *serpent.Command {
}

err = sshSession.Wait()
conn.SendDisconnectedTelemetry(tailnet.TelemetryApplicationSSH)
conn.SendDisconnectedTelemetry()
if err != nil {
if exitErr := (&gossh.ExitError{}); errors.As(err, &exitErr) {
// Clear the error since it's not useful beyond
Expand Down
4 changes: 0 additions & 4 deletions codersdk/workspacesdk/agentconn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,3 @@ func (c *AgentConn) apiClient() *http.Client {
func (c *AgentConn) GetPeerDiagnostics() tailnet.PeerDiagnostics {
return c.Conn.GetPeerDiagnostics(c.opts.AgentID)
}

func (c *AgentConn) SendDisconnectedTelemetry(application string) {
c.Conn.SendDisconnectedTelemetry(c.agentAddress(), application)
}
19 changes: 6 additions & 13 deletions tailnet/conn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -728,14 +728,10 @@ func (c *Conn) SendConnectedTelemetry(ip netip.Addr, application string) {
if c.telemetrySink == nil {
return
}
c.telemetryStore.markConnected(time.Since(c.createdAt))
c.telemetryStore.markConnected(&ip, c.createdAt, application)
c.telemetryStore.updateRemoteNodeID(c.wireguardEngine)
e := c.newTelemetryEvent()
e.Status = proto.TelemetryEvent_CONNECTED
e.Application = application
pip, ok := c.wireguardEngine.PeerForIP(ip)
if ok {
e.NodeIdRemote = uint64(pip.Node.ID)
}
c.telemetryWg.Add(1)
go func() {
defer c.telemetryWg.Done()
Expand All @@ -749,6 +745,7 @@ func (c *Conn) sendUpdatedTelemetry() {
if c.telemetrySink == nil {
return
}
c.telemetryStore.updateRemoteNodeID(c.wireguardEngine)
e := c.newTelemetryEvent()
e.Status = proto.TelemetryEvent_CONNECTED
c.telemetryWg.Add(1)
Expand All @@ -758,17 +755,13 @@ func (c *Conn) sendUpdatedTelemetry() {
}()
}

func (c *Conn) SendDisconnectedTelemetry(ip netip.Addr, application string) {
func (c *Conn) SendDisconnectedTelemetry() {
if c.telemetrySink == nil {
return
}
e := c.newTelemetryEvent()
e.Status = proto.TelemetryEvent_DISCONNECTED
e.Application = application
pip, ok := c.wireguardEngine.PeerForIP(ip)
if ok {
e.NodeIdRemote = uint64(pip.Node.ID)
}
c.telemetryStore.updateRemoteNodeID(c.wireguardEngine)
c.telemetryWg.Add(1)
go func() {
defer c.telemetryWg.Done()
Expand All @@ -781,8 +774,8 @@ func (c *Conn) SendSpeedtestTelemetry(throughputMbits float64) {
return
}
e := c.newTelemetryEvent()
e.Status = proto.TelemetryEvent_CONNECTED
e.ThroughputMbits = wrapperspb.Float(float32(throughputMbits))
e.Status = proto.TelemetryEvent_CONNECTED
c.telemetryWg.Add(1)
go func() {
defer c.telemetryWg.Done()
Expand Down
30 changes: 27 additions & 3 deletions tailnet/telemetry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"google.golang.org/protobuf/types/known/timestamppb"
"google.golang.org/protobuf/types/known/wrapperspb"
"tailscale.com/tailcfg"
"tailscale.com/wgengine"

"github.com/coder/coder/v2/cryptorand"
"github.com/coder/coder/v2/tailnet/proto"
Expand All @@ -33,8 +34,13 @@ type TelemetryStore struct {
cleanNetCheck *proto.Netcheck
nodeID uint64
homeDerp int32
application string

// nil if not connected
connSetupTime *durationpb.Duration
connectedIP *netip.Addr
Copy link
Member Author

Choose a reason for hiding this comment

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

To support coderd and agent telemetry in the future, we'll need to change this to a map of peers to nodes/IPs.

// 0 if not connected
connectedNodeID uint64
}

func newTelemetryStore() (*TelemetryStore, error) {
Expand All @@ -58,19 +64,37 @@ func (b *TelemetryStore) newEvent() *proto.TelemetryEvent {
DerpMap: DERPMapToProto(b.cleanDerpMap),
LatestNetcheck: b.cleanNetCheck,
NodeIdSelf: b.nodeID,
NodeIdRemote: b.connectedNodeID,
HomeDerp: b.homeDerp,
ConnectionSetup: b.connSetupTime,
Application: b.application,

// TODO(ethanndickson):
P2PSetup: &durationpb.Duration{},
}
}

func (b *TelemetryStore) markConnected(connSetupTime time.Duration) {
func (b *TelemetryStore) markConnected(ip *netip.Addr, connCreatedAt time.Time, application string) {
b.mu.Lock()
defer b.mu.Unlock()

b.connSetupTime = durationpb.New(connSetupTime)
b.connSetupTime = durationpb.New(time.Since(connCreatedAt))
b.connectedIP = ip
b.application = application
}

func (b *TelemetryStore) updateRemoteNodeID(engine wgengine.Engine) {
b.mu.Lock()
defer b.mu.Unlock()

if b.connectedIP == nil {
return
}

pip, ok := engine.PeerForIP(*b.connectedIP)
if ok {
b.connectedNodeID = uint64(pip.Node.ID)
}
}

// Given a DERPMap, anonymise all IPs and hostnames.
Expand All @@ -97,7 +121,7 @@ func (b *TelemetryStore) updateDerpMap(cur *tailcfg.DERPMap) {
b.cleanDerpMap = cleanMap
}

// Update the telemetry store with the current node state.
// Update the telemetry store with the current self node state.
// Returns true if the home DERP has changed.
func (b *TelemetryStore) updateByNode(n *Node) bool {
b.mu.Lock()
Expand Down