diff --git a/wgengine/magicsock/derp.go b/wgengine/magicsock/derp.go index 931934e76..913a1f447 100644 --- a/wgengine/magicsock/derp.go +++ b/wgengine/magicsock/derp.go @@ -691,8 +691,8 @@ func (c *connBind) receiveDERP(buffs [][]byte, sizes []int, eps []conn.Endpoint) continue } metricRecvDataPacketsDERP.Add(1) - c.metrics.inboundPacketsTotal.Add(pathLabel{Path: PathDERP}, 1) - c.metrics.inboundBytesTotal.Add(pathLabel{Path: PathDERP}, int64(n)) + c.metrics.inboundPacketsDERPTotal.Add(1) + c.metrics.inboundBytesDERPTotal.Add(int64(n)) sizes[0] = n eps[0] = ep return 1, nil diff --git a/wgengine/magicsock/endpoint.go b/wgengine/magicsock/endpoint.go index ec926a14f..9a4cdf00c 100644 --- a/wgengine/magicsock/endpoint.go +++ b/wgengine/magicsock/endpoint.go @@ -965,17 +965,15 @@ func (de *endpoint) send(buffs [][]byte) error { txBytes += len(b) } - var label pathLabel switch { case udpAddr.Addr().Is4(): - label = pathLabel{Path: PathDirectIPv4} + de.c.metrics.outboundPacketsIPv4Total.Add(int64(len(buffs))) + de.c.metrics.outboundBytesIPv4Total.Add(int64(txBytes)) case udpAddr.Addr().Is6(): - label = pathLabel{Path: PathDirectIPv6} + de.c.metrics.outboundPacketsIPv6Total.Add(int64(len(buffs))) + de.c.metrics.outboundBytesIPv6Total.Add(int64(txBytes)) } - de.c.metrics.outboundPacketsTotal.Add(label, int64(len(buffs))) - de.c.metrics.outboundBytesTotal.Add(label, int64(txBytes)) - // TODO(raggi): needs updating for accuracy, as in error conditions we may have partial sends. if stats := de.c.stats.Load(); err == nil && stats != nil { stats.UpdateTxPhysical(de.nodeAddr, udpAddr, txBytes) @@ -995,8 +993,8 @@ func (de *endpoint) send(buffs [][]byte) error { if stats := de.c.stats.Load(); stats != nil { stats.UpdateTxPhysical(de.nodeAddr, derpAddr, txBytes) } - de.c.metrics.outboundPacketsTotal.Add(pathLabel{Path: PathDERP}, int64(len(buffs))) - de.c.metrics.outboundBytesTotal.Add(pathLabel{Path: PathDERP}, int64(txBytes)) + de.c.metrics.outboundPacketsDERPTotal.Add(int64(len(buffs))) + de.c.metrics.outboundBytesDERPTotal.Add(int64(txBytes)) if allOk { return nil } diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 9d262f8da..48ecfeecd 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -10,6 +10,7 @@ import ( "bytes" "context" "errors" + "expvar" "fmt" "io" "net" @@ -33,7 +34,6 @@ import ( "tailscale.com/health" "tailscale.com/hostinfo" "tailscale.com/ipn/ipnstate" - tsmetrics "tailscale.com/metrics" "tailscale.com/net/connstats" "tailscale.com/net/netcheck" "tailscale.com/net/neterror" @@ -98,22 +98,35 @@ type pathLabel struct { Path Path } +// metrics in wgengine contains the usermetrics counters for magicsock, it +// is however a bit special. All them metrics are labeled, but looking up +// the metric everytime we need to record it has an overhead, and includes +// a lock in MultiLabelMap. The metrics are therefore instead created with +// wgengine and the underlying expvar.Int is stored to be used directly. type metrics struct { // inboundPacketsTotal is the total number of inbound packets received, // labeled by the path the packet took. - inboundPacketsTotal *tsmetrics.MultiLabelMap[pathLabel] - - // outboundPacketsTotal is the total number of outbound packets sent, - // labeled by the path the packet took. - outboundPacketsTotal *tsmetrics.MultiLabelMap[pathLabel] + inboundPacketsIPv4Total *expvar.Int + inboundPacketsIPv6Total *expvar.Int + inboundPacketsDERPTotal *expvar.Int // inboundBytesTotal is the total number of inbound bytes received, // labeled by the path the packet took. - inboundBytesTotal *tsmetrics.MultiLabelMap[pathLabel] + inboundBytesIPv4Total *expvar.Int + inboundBytesIPv6Total *expvar.Int + inboundBytesDERPTotal *expvar.Int + + // outboundPacketsTotal is the total number of outbound packets sent, + // labeled by the path the packet took. + outboundPacketsIPv4Total *expvar.Int + outboundPacketsIPv6Total *expvar.Int + outboundPacketsDERPTotal *expvar.Int // outboundBytesTotal is the total number of outbound bytes sent, // labeled by the path the packet took. - outboundBytesTotal *tsmetrics.MultiLabelMap[pathLabel] + outboundBytesIPv4Total *expvar.Int + outboundBytesIPv6Total *expvar.Int + outboundBytesDERPTotal *expvar.Int } // A Conn routes UDP packets and actively manages a list of its endpoints. @@ -542,32 +555,7 @@ func NewConn(opts Options) (*Conn, error) { UseDNSCache: true, } - c.metrics = &metrics{ - inboundBytesTotal: usermetric.NewMultiLabelMapWithRegistry[pathLabel]( - opts.Metrics, - "tailscaled_inbound_bytes_total", - "counter", - "Counts the number of bytes received from other peers", - ), - inboundPacketsTotal: usermetric.NewMultiLabelMapWithRegistry[pathLabel]( - opts.Metrics, - "tailscaled_inbound_packets_total", - "counter", - "Counts the number of packets received from other peers", - ), - outboundBytesTotal: usermetric.NewMultiLabelMapWithRegistry[pathLabel]( - opts.Metrics, - "tailscaled_outbound_bytes_total", - "counter", - "Counts the number of bytes sent to other peers", - ), - outboundPacketsTotal: usermetric.NewMultiLabelMapWithRegistry[pathLabel]( - opts.Metrics, - "tailscaled_outbound_packets_total", - "counter", - "Counts the number of packets sent to other peers", - ), - } + c.metrics = registerMetrics(opts.Metrics) if d4, err := c.listenRawDisco("ip4"); err == nil { c.logf("[v1] using BPF disco receiver for IPv4") @@ -586,6 +574,74 @@ func NewConn(opts Options) (*Conn, error) { return c, nil } +// registerMetrics wires up the metrics for wgengine, instead of +// registering the label metric directly, the underlying expvar is exposed. +// See metrics for more info. +func registerMetrics(reg *usermetric.Registry) *metrics { + pathDirectV4 := pathLabel{Path: PathDirectIPv4} + pathDirectV6 := pathLabel{Path: PathDirectIPv6} + pathDERP := pathLabel{Path: PathDERP} + inboundPacketsTotal := usermetric.NewMultiLabelMapWithRegistry[pathLabel]( + reg, + "tailscaled_inbound_packets_total", + "counter", + "Counts the number of packets received from other peers", + ) + inboundBytesTotal := usermetric.NewMultiLabelMapWithRegistry[pathLabel]( + reg, + "tailscaled_inbound_bytes_total", + "counter", + "Counts the number of bytes received from other peers", + ) + outboundPacketsTotal := usermetric.NewMultiLabelMapWithRegistry[pathLabel]( + reg, + "tailscaled_outbound_packets_total", + "counter", + "Counts the number of packets sent to other peers", + ) + outboundBytesTotal := usermetric.NewMultiLabelMapWithRegistry[pathLabel]( + reg, + "tailscaled_outbound_bytes_total", + "counter", + "Counts the number of bytes sent to other peers", + ) + m := metrics{ + inboundPacketsIPv4Total: &expvar.Int{}, + inboundPacketsIPv6Total: &expvar.Int{}, + inboundPacketsDERPTotal: &expvar.Int{}, + + inboundBytesIPv4Total: &expvar.Int{}, + inboundBytesIPv6Total: &expvar.Int{}, + inboundBytesDERPTotal: &expvar.Int{}, + + outboundPacketsIPv4Total: &expvar.Int{}, + outboundPacketsIPv6Total: &expvar.Int{}, + outboundPacketsDERPTotal: &expvar.Int{}, + + outboundBytesIPv4Total: &expvar.Int{}, + outboundBytesIPv6Total: &expvar.Int{}, + outboundBytesDERPTotal: &expvar.Int{}, + } + + inboundPacketsTotal.Set(pathDirectV4, m.inboundPacketsIPv4Total) + inboundPacketsTotal.Set(pathDirectV6, m.inboundPacketsIPv6Total) + inboundPacketsTotal.Set(pathDERP, m.inboundPacketsDERPTotal) + + inboundBytesTotal.Set(pathDirectV4, m.inboundBytesIPv4Total) + inboundBytesTotal.Set(pathDirectV6, m.inboundBytesIPv6Total) + inboundBytesTotal.Set(pathDERP, m.inboundBytesDERPTotal) + + outboundPacketsTotal.Set(pathDirectV4, m.outboundPacketsIPv4Total) + outboundPacketsTotal.Set(pathDirectV6, m.outboundPacketsIPv6Total) + outboundPacketsTotal.Set(pathDERP, m.outboundPacketsDERPTotal) + + outboundBytesTotal.Set(pathDirectV4, m.outboundBytesIPv4Total) + outboundBytesTotal.Set(pathDirectV6, m.outboundBytesIPv6Total) + outboundBytesTotal.Set(pathDERP, m.outboundBytesDERPTotal) + + return &m +} + // InstallCaptureHook installs a callback which is called to // log debug information into the pcap stream. This function // can be called with a nil argument to uninstall the capture @@ -1210,11 +1266,11 @@ func (c *Conn) sendUDP(ipp netip.AddrPort, b []byte) (sent bool, err error) { switch { case ipp.Addr().Is4(): - c.metrics.outboundPacketsTotal.Add(pathLabel{Path: PathDirectIPv4}, 1) - c.metrics.outboundBytesTotal.Add(pathLabel{Path: PathDirectIPv4}, int64(len(b))) + c.metrics.outboundPacketsIPv4Total.Add(1) + c.metrics.outboundBytesIPv4Total.Add(int64(len(b))) case ipp.Addr().Is6(): - c.metrics.outboundPacketsTotal.Add(pathLabel{Path: PathDirectIPv6}, 1) - c.metrics.outboundBytesTotal.Add(pathLabel{Path: PathDirectIPv6}, int64(len(b))) + c.metrics.outboundPacketsIPv6Total.Add(1) + c.metrics.outboundBytesIPv6Total.Add(int64(len(b))) } } } @@ -1357,9 +1413,9 @@ func (c *Conn) receiveIPv4() conn.ReceiveFunc { return c.mkReceiveFunc(&c.pconn4, c.health.ReceiveFuncStats(health.ReceiveIPv4), func(i int64) { metricRecvDataPacketsIPv4.Add(i) - c.metrics.inboundPacketsTotal.Add(pathLabel{Path: PathDirectIPv4}, i) + c.metrics.inboundPacketsIPv4Total.Add(i) }, func(i int64) { - c.metrics.inboundBytesTotal.Add(pathLabel{Path: PathDirectIPv4}, i) + c.metrics.inboundBytesIPv4Total.Add(i) }) } @@ -1368,9 +1424,9 @@ func (c *Conn) receiveIPv6() conn.ReceiveFunc { return c.mkReceiveFunc(&c.pconn6, c.health.ReceiveFuncStats(health.ReceiveIPv6), func(i int64) { metricRecvDataPacketsIPv6.Add(i) - c.metrics.inboundPacketsTotal.Add(pathLabel{Path: PathDirectIPv6}, i) + c.metrics.inboundPacketsIPv6Total.Add(i) }, func(i int64) { - c.metrics.inboundBytesTotal.Add(pathLabel{Path: PathDirectIPv6}, i) + c.metrics.inboundBytesIPv6Total.Add(i) }) } diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index 993132008..62b27f75a 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -10,7 +10,6 @@ import ( "crypto/tls" "encoding/binary" "errors" - "expvar" "fmt" "io" "math/rand" @@ -1207,10 +1206,18 @@ func testTwoDevicePing(t *testing.T, d *devices) { } func (c *Conn) resetMetricsForTest() { - c.metrics.inboundBytesTotal.ResetAllForTest() - c.metrics.inboundPacketsTotal.ResetAllForTest() - c.metrics.outboundBytesTotal.ResetAllForTest() - c.metrics.outboundPacketsTotal.ResetAllForTest() + c.metrics.inboundBytesIPv4Total.Set(0) + c.metrics.inboundPacketsIPv4Total.Set(0) + c.metrics.outboundBytesIPv4Total.Set(0) + c.metrics.outboundPacketsIPv4Total.Set(0) + c.metrics.inboundBytesIPv6Total.Set(0) + c.metrics.inboundPacketsIPv6Total.Set(0) + c.metrics.outboundBytesIPv6Total.Set(0) + c.metrics.outboundPacketsIPv6Total.Set(0) + c.metrics.inboundBytesDERPTotal.Set(0) + c.metrics.inboundPacketsDERPTotal.Set(0) + c.metrics.outboundBytesDERPTotal.Set(0) + c.metrics.outboundPacketsDERPTotal.Set(0) } func assertConnStatsAndUserMetricsEqual(t *testing.T, ms *magicStack) { @@ -1239,33 +1246,15 @@ func assertConnStatsAndUserMetricsEqual(t *testing.T, ms *magicStack) { } } - var metricIPv4RxBytes, metricIPv4TxBytes, metricDERPRxBytes, metricDERPTxBytes int64 - var metricIPv4RxPackets, metricIPv4TxPackets, metricDERPRxPackets, metricDERPTxPackets int64 + metricIPv4RxBytes := ms.conn.metrics.inboundBytesIPv4Total.Value() + metricIPv4RxPackets := ms.conn.metrics.inboundPacketsIPv4Total.Value() + metricIPv4TxBytes := ms.conn.metrics.outboundBytesIPv4Total.Value() + metricIPv4TxPackets := ms.conn.metrics.outboundPacketsIPv4Total.Value() - if m, ok := ms.conn.metrics.inboundBytesTotal.Get(pathLabel{Path: PathDirectIPv4}).(*expvar.Int); ok { - metricIPv4RxBytes = m.Value() - } - if m, ok := ms.conn.metrics.outboundBytesTotal.Get(pathLabel{Path: PathDirectIPv4}).(*expvar.Int); ok { - metricIPv4TxBytes = m.Value() - } - if m, ok := ms.conn.metrics.inboundBytesTotal.Get(pathLabel{Path: PathDERP}).(*expvar.Int); ok { - metricDERPRxBytes = m.Value() - } - if m, ok := ms.conn.metrics.outboundBytesTotal.Get(pathLabel{Path: PathDERP}).(*expvar.Int); ok { - metricDERPTxBytes = m.Value() - } - if m, ok := ms.conn.metrics.inboundPacketsTotal.Get(pathLabel{Path: PathDirectIPv4}).(*expvar.Int); ok { - metricIPv4RxPackets = m.Value() - } - if m, ok := ms.conn.metrics.outboundPacketsTotal.Get(pathLabel{Path: PathDirectIPv4}).(*expvar.Int); ok { - metricIPv4TxPackets = m.Value() - } - if m, ok := ms.conn.metrics.inboundPacketsTotal.Get(pathLabel{Path: PathDERP}).(*expvar.Int); ok { - metricDERPRxPackets = m.Value() - } - if m, ok := ms.conn.metrics.outboundPacketsTotal.Get(pathLabel{Path: PathDERP}).(*expvar.Int); ok { - metricDERPTxPackets = m.Value() - } + metricDERPRxBytes := ms.conn.metrics.inboundBytesDERPTotal.Value() + metricDERPRxPackets := ms.conn.metrics.inboundPacketsDERPTotal.Value() + metricDERPTxBytes := ms.conn.metrics.outboundBytesDERPTotal.Value() + metricDERPTxPackets := ms.conn.metrics.outboundPacketsDERPTotal.Value() assertEqual(t, "derp bytes inbound", physDERPRxBytes, metricDERPRxBytes) assertEqual(t, "derp bytes outbound", physDERPTxBytes, metricDERPTxBytes)