ping and speedtest telemetry

coder · ethanndickson · Jul 3, 2024 · Jun 26, 2024 · Jun 26, 2024 · Jun 26, 2024
commit b9cd8fb7e019b49e273b1efddb568a330bbd8878
diff --git a/cli/speedtest.go b/cli/speedtest.go
@@ -186,6 +186,7 @@ func (r *RootCmd) speedtest() *serpent.Command {
 					outputResult.Intervals[i] = interval
 				}
 			}
+			_ = conn.Conn.SendSpeedtestTelemetry(outputResult.Overall.ThroughputMbits)
 			out, err := formatter.Format(inv.Context(), outputResult)
 			if err != nil {
 				return err

diff --git a/coderd/telemetry/telemetry.go b/coderd/telemetry/telemetry.go
@@ -1221,14 +1221,11 @@ func netcheckFromProto(proto *tailnetproto.Netcheck) Netcheck {
 
 		PreferredDERP: proto.PreferredDERP,
 
-		RegionLatency:   durationMapFromProto(proto.RegionLatency),
 		RegionV4Latency: durationMapFromProto(proto.RegionV4Latency),
 		RegionV6Latency: durationMapFromProto(proto.RegionV6Latency),
 
 		GlobalV4: netcheckIPFromProto(proto.GlobalV4),
 		GlobalV6: netcheckIPFromProto(proto.GlobalV6),
-
-		CaptivePortal: protoBool(proto.CaptivePortal),
 	}
 }
 

diff --git a/codersdk/workspacesdk/connector_internal_test.go b/codersdk/workspacesdk/connector_internal_test.go
@@ -323,7 +323,7 @@ func (*fakeDRPCClient) DRPCConn() drpc.Conn {
 }
 
 // PostTelemetry implements proto.DRPCTailnetClient.
-func (f *fakeDRPCClient) PostTelemetry(_ context.Context, in *proto.TelemetryRequest) (*proto.TelemetryResponse, error) {
+func (f *fakeDRPCClient) PostTelemetry(_ context.Context, _ *proto.TelemetryRequest) (*proto.TelemetryResponse, error) {
 	return nil, f.telemeteryErorr
 }
 

diff --git a/tailnet/conn.go b/tailnet/conn.go
@@ -448,7 +448,12 @@ func (c *Conn) Status() *ipnstate.Status {
 // Ping sends a ping to the Wireguard engine.
 // The bool returned is true if the ping was performed P2P.
 func (c *Conn) Ping(ctx context.Context, ip netip.Addr) (time.Duration, bool, *ipnstate.PingResult, error) {
-	return c.pingWithType(ctx, ip, tailcfg.PingDisco)
+	dur, p2p, pr, err := c.pingWithType(ctx, ip, tailcfg.PingDisco)
+	if err == nil {
+		// TODO(ethanndickson): Is this too often?
+		_ = c.sendPingTelemetry(dur, p2p)
+	}
+	return dur, p2p, pr, err
 }
 
 func (c *Conn) pingWithType(ctx context.Context, ip netip.Addr, pt tailcfg.PingType) (time.Duration, bool, *ipnstate.PingResult, error) {
@@ -525,7 +530,7 @@ func (c *Conn) AwaitReachable(ctx context.Context, ip netip.Addr) bool {
 		case <-completedCtx.Done():
 			// TODO(ethanndickson): For now, I'm interpreting 'connected' as when the
 			// agent is reachable.
-			// _ = c.connectedTelemetryEvent()
+			_ = c.sendConnectedTelemetry()
 			return true
 		case <-t.C:
 			// Pings can take a while, so we can run multiple
@@ -715,7 +720,24 @@ func (c *Conn) MagicsockServeHTTPDebug(w http.ResponseWriter, r *http.Request) {
 	c.magicConn.ServeHTTPDebug(w, r)
 }
 
-func (c *Conn) connectedTelemetryEvent() error {
+func (c *Conn) sendConnectedTelemetry() error {
+	if c.telemetrySink == nil {
+		return nil
+	}
+	e, err := c.newTelemetryEvent()
+	if err != nil {
+		return xerrors.Errorf("create telemetry event: %w", err)
+	}
+	e.Status = proto.TelemetryEvent_CONNECTED
+	c.telemetryWg.Add(1)
+	go func() {
+		defer c.telemetryWg.Done()
+		c.telemetrySink.SendTelemetryEvent(e)
+	}()
+	return nil
+}
+
+func (c *Conn) SendSpeedtestTelemetry(throughputMbits float64) error {
 	if c.telemetrySink == nil {
 		return nil
 	}
@@ -724,6 +746,30 @@ func (c *Conn) connectedTelemetryEvent() error {
 		return xerrors.Errorf("create telemetry event: %w", err)
 	}
 	e.Status = proto.TelemetryEvent_CONNECTED
+	e.ThroughputMbits = wrapperspb.Float(float32(throughputMbits))
+	c.telemetryWg.Add(1)
+	go func() {
+		defer c.telemetryWg.Done()
+		c.telemetrySink.SendTelemetryEvent(e)
+	}()
+	return nil
+}
+
+// nolint: revive
+func (c *Conn) sendPingTelemetry(latency time.Duration, p2p bool) error {
+	if c.telemetrySink == nil {
+		return nil
+	}
+	e, err := c.newTelemetryEvent()
+	if err != nil {
+		return xerrors.Errorf("create telemetry event: %w", err)
+	}
+	e.Status = proto.TelemetryEvent_CONNECTED
+	if p2p {
+		e.P2PLatency = durationpb.New(latency)
+	} else {
+		e.DerpLatency = durationpb.New(latency)
+	}
 	c.telemetryWg.Add(1)
 	go func() {
 		defer c.telemetryWg.Done()
@@ -754,14 +800,11 @@ func (c *Conn) newTelemetryEvent() (*proto.TelemetryEvent, error) {
 		Application:     "",
 		NodeIdRemote:    0,
 		P2PEndpoint:     &proto.TelemetryEvent_P2PEndpoint{},
-		ThroughputMbits: &wrapperspb.FloatValue{},
 		HomeDerp:        "",
 		ConnectionAge:   &durationpb.Duration{},
 		ConnectionSetup: &durationpb.Duration{},
-		P2PSetup:        &durationpb.Duration{},
-		// TODO: One of these two
-		DerpLatency: &durationpb.Duration{},
-		P2PLatency:  &durationpb.Duration{},
+		// TODO: We only calculate this in one place, do we really want it?
+		P2PSetup: &durationpb.Duration{},
 	}, nil
 }
 

diff --git a/tailnet/convert.go b/tailnet/convert.go
@@ -304,8 +304,8 @@ func NetInfoToProto(netInfo *tailcfg.NetInfo) *proto.Netcheck {
 	}
 	return &proto.Netcheck{
 		UDP:                   netInfo.WorkingUDP.EqualBool(true),
-		IPv6CanSend:           netInfo.WorkingIPv6.EqualBool(true),
 		OSHasIPv6:             netInfo.OSHasIPv6.EqualBool(true),
+		IPv6:                  netInfo.WorkingIPv6.EqualBool(true),
 		ICMPv4:                netInfo.WorkingICMPv4.EqualBool(true),
 		MappingVariesByDestIP: wrapperspb.Bool(netInfo.MappingVariesByDestIP.EqualBool(true)),
 		HairPinning:           wrapperspb.Bool(netInfo.HairPinning.EqualBool(true)),
@@ -315,14 +315,11 @@ func NetInfoToProto(netInfo *tailcfg.NetInfo) *proto.Netcheck {
 		PreferredDERP:         int64(netInfo.PreferredDERP),
 		RegionV4Latency:       rlv4,
 		RegionV6Latency:       rlv6,
-		// TODO: what's the most useful value to have here?
-		RegionLatency: make(map[int64]*durationpb.Duration),
-		// TODO: None of these are exposed by tailscale
-		IPv4CanSend:   false,
-		IPv4:          false,
-		IPv6:          false,
-		GlobalV4:      &proto.Netcheck_NetcheckIP{},
-		GlobalV6:      &proto.Netcheck_NetcheckIP{},
-		CaptivePortal: &wrapperspb.BoolValue{},
+		// TODO: These aren't yet exposed by Tailscale
+		IPv6CanSend: false,
+		IPv4CanSend: false,
+		IPv4:        false,
+		GlobalV4:    &proto.Netcheck_NetcheckIP{},
+		GlobalV6:    &proto.Netcheck_NetcheckIP{},
 	}
 }