derp: change packets_dropped metric to also have reason and kind labels (#14651)

Metrics currently exist for dropped packets by reason, and total
received packets by kind (e.g., `disco` or `other`), but relating these
two together to gleam information about the drop rate for specific
reasons on a per-kind basis is not currently possible.

Change `derp_packets_dropped` to use a `metrics.MultiLabelMap` to
track both the `reason` and `kind` in the same metric to allow for this
desired level of granularity.

Drop metrics that this makes unnecessary (namely `packetsDroppedReason`
and `packetsDroppedType`).

Updates https://github.com/tailscale/corp/issues/25489

Signed-off-by: Mario Minardi <mario@tailscale.com>

derp/derp_server.go

@@ -112,6 +112,14 @@ const (
 	disableFighters
 )
 
+// packetKind is the kind of packet being sent through DERP
+type packetKind string
+
+const (
+	packetKindDisco packetKind = "disco"
+	packetKindOther packetKind = "other"
+)
+
 type align64 [0]atomic.Int64 // for side effect of its 64-bit alignment
 
 // Server is a DERP server.
@@ -137,13 +145,6 @@ type Server struct {
 	packetsRecvDisco           *expvar.Int
 	packetsRecvOther           *expvar.Int
 	_                          align64
-	packetsDropped               expvar.Int
-	packetsDroppedReason         metrics.LabelMap
-	packetsDroppedReasonCounters []*expvar.Int // indexed by dropReason
-	packetsDroppedType           metrics.LabelMap
-	packetsDroppedTypeDisco      *expvar.Int
-	packetsDroppedTypeOther      *expvar.Int
-	_                            align64
 	packetsForwardedOut        expvar.Int
 	packetsForwardedIn         expvar.Int
 	peerGoneDisconnectedFrames expvar.Int // number of peer disconnected frames sent
@@ -351,6 +352,11 @@ type Conn interface {
 	SetWriteDeadline(time.Time) error
 }
 
+var packetsDropped = metrics.NewMultiLabelMap[dropReasonKindLabels](
+	"derp_packets_dropped",
+	"counter",
+	"DERP packets dropped by reason and by kind")
+
 // NewServer returns a new DERP server. It doesn't listen on its own.
 // Connections are given to it via Server.Accept.
 func NewServer(privateKey key.NodePrivate, logf logger.Logf) *Server {
@@ -364,8 +370,6 @@ func NewServer(privateKey key.NodePrivate, logf logger.Logf) *Server {
 		logf:                logf,
 		limitedLogf:         logger.RateLimitedFn(logf, 30*time.Second, 5, 100),
 		packetsRecvByKind:   metrics.LabelMap{Label: "kind"},
-		packetsDroppedReason: metrics.LabelMap{Label: "reason"},
-		packetsDroppedType:   metrics.LabelMap{Label: "type"},
 		clients:             map[key.NodePublic]*clientSet{},
 		clientsMesh:         map[key.NodePublic]PacketForwarder{},
 		netConns:            map[Conn]chan struct{}{},
@@ -381,38 +385,60 @@ func NewServer(privateKey key.NodePrivate, logf logger.Logf) *Server {
 		clock:               tstime.StdClock{},
 	}
 	s.initMetacert()
-	s.packetsRecvDisco = s.packetsRecvByKind.Get("disco")
+	s.packetsRecvDisco = s.packetsRecvByKind.Get(string(packetKindDisco))
-	s.packetsRecvOther = s.packetsRecvByKind.Get("other")
+	s.packetsRecvOther = s.packetsRecvByKind.Get(string(packetKindOther))
 
-	s.packetsDroppedReasonCounters = s.genPacketsDroppedReasonCounters()
+	genPacketsDroppedCounters()
-
-	s.packetsDroppedTypeDisco = s.packetsDroppedType.Get("disco")
-	s.packetsDroppedTypeOther = s.packetsDroppedType.Get("other")
 
 	s.perClientSendQueueDepth = getPerClientSendQueueDepth()
 	return s
 }
 
-func (s *Server) genPacketsDroppedReasonCounters() []*expvar.Int {
+func genPacketsDroppedCounters() {
-	getMetric := s.packetsDroppedReason.Get
+	initMetrics := func(reason dropReason) {
-	ret := []*expvar.Int{
+		packetsDropped.Add(dropReasonKindLabels{
-		dropReasonUnknownDest:      getMetric("unknown_dest"),
+			Kind:   string(packetKindDisco),
-		dropReasonUnknownDestOnFwd: getMetric("unknown_dest_on_fwd"),
+			Reason: string(reason),
-		dropReasonGoneDisconnected: getMetric("gone_disconnected"),
+		}, 0)
-		dropReasonQueueHead:        getMetric("queue_head"),
+		packetsDropped.Add(dropReasonKindLabels{
-		dropReasonQueueTail:        getMetric("queue_tail"),
+			Kind:   string(packetKindOther),
-		dropReasonWriteError:       getMetric("write_error"),
+			Reason: string(reason),
-		dropReasonDupClient:        getMetric("dup_client"),
+		}, 0)
 	}
-	if len(ret) != int(numDropReasons) {
+	getMetrics := func(reason dropReason) []expvar.Var {
+		return []expvar.Var{
+			packetsDropped.Get(dropReasonKindLabels{
+				Kind:   string(packetKindDisco),
+				Reason: string(reason),
+			}),
+			packetsDropped.Get(dropReasonKindLabels{
+				Kind:   string(packetKindOther),
+				Reason: string(reason),
+			}),
+		}
+	}
+
+	dropReasons := []dropReason{
+		dropReasonUnknownDest,
+		dropReasonUnknownDestOnFwd,
+		dropReasonGoneDisconnected,
+		dropReasonQueueHead,
+		dropReasonQueueTail,
+		dropReasonWriteError,
+		dropReasonDupClient,
+	}
+
+	for _, dr := range dropReasons {
+		initMetrics(dr)
+		m := getMetrics(dr)
+		if len(m) != 2 {
 			panic("dropReason metrics out of sync")
 		}
-	for i := range numDropReasons {
+
-		if ret[i] == nil {
+		if m[0] == nil || m[1] == nil {
 			panic("dropReason metrics out of sync")
 		}
 	}
-	return ret
 }
 
 // SetMesh sets the pre-shared key that regional DERP servers used to mesh
@@ -1152,31 +1178,36 @@ func (c *sclient) debugLogf(format string, v ...any) {
 	}
 }
 
-// dropReason is why we dropped a DERP frame.
+type dropReasonKindLabels struct {
-type dropReason int
+	Reason string // metric label corresponding to a given dropReason
+	Kind   string // either `disco` or `other`
+}
 
-//go:generate go run tailscale.com/cmd/addlicense -file dropreason_string.go go run golang.org/x/tools/cmd/stringer -type=dropReason -trimprefix=dropReason
+// dropReason is why we dropped a DERP frame.
+type dropReason string
 
 const (
-	dropReasonUnknownDest      dropReason = iota // unknown destination pubkey
+	dropReasonUnknownDest      dropReason = "unknown_dest"        // unknown destination pubkey
-	dropReasonUnknownDestOnFwd                   // unknown destination pubkey on a derp-forwarded packet
+	dropReasonUnknownDestOnFwd dropReason = "unknown_dest_on_fwd" // unknown destination pubkey on a derp-forwarded packet
-	dropReasonGoneDisconnected                   // destination tailscaled disconnected before we could send
+	dropReasonGoneDisconnected dropReason = "gone_disconnected"   // destination tailscaled disconnected before we could send
-	dropReasonQueueHead                          // destination queue is full, dropped packet at queue head
+	dropReasonQueueHead        dropReason = "queue_head"          // destination queue is full, dropped packet at queue head
-	dropReasonQueueTail                          // destination queue is full, dropped packet at queue tail
+	dropReasonQueueTail        dropReason = "queue_tail"          // destination queue is full, dropped packet at queue tail
-	dropReasonWriteError                         // OS write() failed
+	dropReasonWriteError       dropReason = "write_error"         // OS write() failed
-	dropReasonDupClient                          // the public key is connected 2+ times (active/active, fighting)
+	dropReasonDupClient        dropReason = "dup_client"          // the public key is connected 2+ times (active/active, fighting)
-	numDropReasons                               // unused; keep last
 )
 
 func (s *Server) recordDrop(packetBytes []byte, srcKey, dstKey key.NodePublic, reason dropReason) {
-	s.packetsDropped.Add(1)
+	labels := dropReasonKindLabels{
-	s.packetsDroppedReasonCounters[reason].Add(1)
+		Reason: string(reason),
+	}
 	looksDisco := disco.LooksLikeDiscoWrapper(packetBytes)
 	if looksDisco {
-		s.packetsDroppedTypeDisco.Add(1)
+		labels.Kind = string(packetKindDisco)
 	} else {
-		s.packetsDroppedTypeOther.Add(1)
+		labels.Kind = string(packetKindOther)
 	}
+	packetsDropped.Add(labels, 1)
+
 	if verboseDropKeys[dstKey] {
 		// Preformat the log string prior to calling limitedLogf. The
 		// limiter acts based on the format string, and we want to
@@ -2095,9 +2126,6 @@ func (s *Server) ExpVar() expvar.Var {
 	m.Set("accepts", &s.accepts)
 	m.Set("bytes_received", &s.bytesRecv)
 	m.Set("bytes_sent", &s.bytesSent)
-	m.Set("packets_dropped", &s.packetsDropped)
-	m.Set("counter_packets_dropped_reason", &s.packetsDroppedReason)
-	m.Set("counter_packets_dropped_type", &s.packetsDroppedType)
 	m.Set("counter_packets_received_kind", &s.packetsRecvByKind)
 	m.Set("packets_sent", &s.packetsSent)
 	m.Set("packets_received", &s.packetsRecv)

derp/dropreason_string.go

@@ -1,33 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-
-// Code generated by "stringer -type=dropReason -trimprefix=dropReason"; DO NOT EDIT.
-
-package derp
-
-import "strconv"
-
-func _() {
-	// An "invalid array index" compiler error signifies that the constant values have changed.
-	// Re-run the stringer command to generate them again.
-	var x [1]struct{}
-	_ = x[dropReasonUnknownDest-0]
-	_ = x[dropReasonUnknownDestOnFwd-1]
-	_ = x[dropReasonGoneDisconnected-2]
-	_ = x[dropReasonQueueHead-3]
-	_ = x[dropReasonQueueTail-4]
-	_ = x[dropReasonWriteError-5]
-	_ = x[dropReasonDupClient-6]
-	_ = x[numDropReasons-7]
-}
-
-const _dropReason_name = "UnknownDestUnknownDestOnFwdGoneDisconnectedQueueHeadQueueTailWriteErrorDupClientnumDropReasons"
-
-var _dropReason_index = [...]uint8{0, 11, 27, 43, 52, 61, 71, 80, 94}
-
-func (i dropReason) String() string {
-	if i < 0 || i >= dropReason(len(_dropReason_index)-1) {
-		return "dropReason(" + strconv.FormatInt(int64(i), 10) + ")"
-	}
-	return _dropReason_name[_dropReason_index[i]:_dropReason_index[i+1]]
-}