net: match PDH adapter names, format full MAC addresses, reuse buffers (#2689)

Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>

docs/collector.net.md

@@ -5,8 +5,7 @@ The net collector exposes metrics about network interfaces
 |||
 -|-
 Metric name prefix  | `net`
-Data source         | Perflib
-Classes             | [`Win32_PerfRawData_Tcpip_NetworkInterface`](https://technet.microsoft.com/en-us/security/aa394340(v=vs.80))
+Data source         | Performance counters (`Network Interface`), [`GetAdaptersAddresses`](https://learn.microsoft.com/en-us/windows/win32/api/iphlpapi/nf-iphlpapi-getadaptersaddresses)
 Enabled by default? | Yes
 
 ## Flags
@@ -27,6 +26,12 @@ Comma-separated list of collectors to use. Defaults to all, if not specified. Su
 
 Default: `metrics,nic_info`
 
+## Interface names
+
+The `nic` label is the instance name of the adapter in the `Network Interface` performance counter object. It is the adapter description with `(` and `)` replaced by `[` and `]`, and `#`, `/` and `\` replaced by `_`, for example `Intel[R] PRO_1000 MT Network Connection _2`. The include and exclude expressions match this name.
+
+The `mac` label of `windows_net_nic_info` is empty for adapters without a physical address, like the loopback adapter.
+
 ## Metrics
 
 | Name                                           | Description                                                                                                             | Type    | Labels                         |
@@ -59,7 +64,7 @@ rate(windows_net_bytes_sent_total{instance="localhost"}[2m])
 ## Useful queries
 Get total utilisation of network interface as a percentage
 ```
-rate(windows_net_bytes_total{instance="localhost", nic="Microsoft_Hyper_V_Network_Adapter__1"}[2m]) / windows_net_current_bandwidth_bytes{instance="localhost", nic="Microsoft_Hyper_V_Network_Adapter__1"} * 100
+rate(windows_net_bytes_total{instance="localhost", nic="Microsoft Hyper-V Network Adapter _2"}[2m]) / windows_net_current_bandwidth_bytes{instance="localhost", nic="Microsoft Hyper-V Network Adapter _2"} * 100
 ```
 
 ## Alerting examples

internal/collector/net/net.go

@@ -29,6 +29,7 @@ import (
 	"unsafe"
 
 	"github.com/alecthomas/kingpin/v2"
+	"github.com/prometheus-community/windows_exporter/internal/labelcache"
 	"github.com/prometheus-community/windows_exporter/internal/metriccache"
 	"github.com/prometheus-community/windows_exporter/internal/mi"
 	"github.com/prometheus-community/windows_exporter/internal/pdh"
@@ -60,6 +61,12 @@ var ConfigDefaults = Config{
 	},
 }
 
+// nicNameReplacer converts an adapter description into the instance name of
+// the adapter in the Network Interface performance counter object.
+//
+//nolint:gochecknoglobals
+var nicNameReplacer = strings.NewReplacer("(", "[", ")", "]", "#", "_", "/", "_", "\\", "_")
+
 // A Collector is a Prometheus Collector for Perflib Network Interface metrics.
 type Collector struct {
 	config Config
@@ -67,6 +74,15 @@ type Collector struct {
 	perfDataCollector *pdh.Collector[perfDataCounterValues]
 	perfDataObject    []perfDataCounterValues
 
+	// labels keeps the label pairs of the Network Interface series of the last
+	// scrape. Their values change on every scrape, but their labels stay the
+	// same while the adapters do.
+	labels labelcache.Cache
+
+	// adapterAddressesBuf holds the result of GetAdaptersAddresses. It is
+	// reused between scrapes, so its size also seeds the next call.
+	adapterAddressesBuf []byte
+
 	bytesReceivedTotal       *prometheus.Desc
 	bytesSentTotal           *prometheus.Desc
 	bytesTotal               *prometheus.Desc
@@ -182,6 +198,8 @@ func (c *Collector) GetName() string {
 func (c *Collector) Close() error {
 	c.perfDataCollector.Close()
 	c.nicMetricCache.Reset()
+	c.labels.Reset()
+	c.adapterAddressesBuf = nil
 
 	return nil
 }
@@ -294,6 +312,7 @@ func (c *Collector) Build(logger *slog.Logger, _ *mi.Session) error {
 
 	// Cached metrics refer to the descriptors of an earlier Build.
 	c.nicMetricCache.Reset()
+	c.labels.Reset()
 
 	var err error
 
@@ -337,97 +356,100 @@ func (c *Collector) collect(ch chan<- prometheus.Metric) error {
 		return fmt.Errorf("failed to collect Network Information metrics: %w", err)
 	}
 
+	scrape := c.labels.Begin()
+	defer scrape.Commit()
+
 	for _, data := range c.perfDataObject {
 		if c.config.NicExclude.MatchString(data.Name) || !c.config.NicInclude.MatchString(data.Name) {
 			continue
 		}
 
 		// Counters
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.bytesReceivedTotal,
 			prometheus.CounterValue,
 			data.BytesReceivedPerSec,
 			data.Name,
 		)
 
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.bytesSentTotal,
 			prometheus.CounterValue,
 			data.BytesSentPerSec,
 			data.Name,
 		)
 
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.bytesTotal,
 			prometheus.CounterValue,
 			data.BytesTotalPerSec,
 			data.Name,
 		)
 
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.outputQueueLength,
 			prometheus.GaugeValue,
 			data.OutputQueueLength,
 			data.Name,
 		)
 
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.packetsOutboundDiscarded,
 			prometheus.CounterValue,
 			data.PacketsOutboundDiscarded,
 			data.Name,
 		)
 
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.packetsOutboundErrors,
 			prometheus.CounterValue,
 			data.PacketsOutboundErrors,
 			data.Name,
 		)
 
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.packetsTotal,
 			prometheus.CounterValue,
 			data.PacketsPerSec,
 			data.Name,
 		)
 
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.packetsReceivedDiscarded,
 			prometheus.CounterValue,
 			data.PacketsReceivedDiscarded,
 			data.Name,
 		)
 
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.packetsReceivedErrors,
 			prometheus.CounterValue,
 			data.PacketsReceivedErrors,
 			data.Name,
 		)
 
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.packetsReceivedTotal,
 			prometheus.CounterValue,
 			data.PacketsReceivedPerSec,
 			data.Name,
 		)
 
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.packetsReceivedUnknown,
 			prometheus.CounterValue,
 			data.PacketsReceivedUnknown,
 			data.Name,
 		)
 
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.packetsSentTotal,
 			prometheus.CounterValue,
 			data.PacketsSentPerSec,
 			data.Name,
 		)
 
-		ch <- prometheus.MustNewConstMetric(
+		ch <- scrape.MustNewConstMetric(
 			c.currentBandwidth,
 			prometheus.GaugeValue,
 			data.CurrentBandwidth/8,
@@ -439,34 +461,30 @@ func (c *Collector) collect(ch chan<- prometheus.Metric) error {
 }
 
 func (c *Collector) collectNICInfo(ch chan<- prometheus.Metric) error {
-	nicAdapterAddresses, err := adapterAddresses()
+	var (
+		nicAdapters *windows.IpAdapterAddresses
+		err         error
+	)
+
+	nicAdapters, c.adapterAddressesBuf, err = adapterAddresses(c.adapterAddressesBuf)
 	if err != nil {
 		return err
 	}
 
-	convertNicName := strings.NewReplacer("(", "[", ")", "]", "#", "_")
-
 	scrape := c.nicMetricCache.Begin()
 
 	var addresses []byte
 
-	for _, nicAdapter := range nicAdapterAddresses {
+	for nicAdapter := nicAdapters; nicAdapter != nil; nicAdapter = nicAdapter.Next {
 		friendlyName := windows.UTF16PtrToString(nicAdapter.FriendlyName)
-		nicName := convertNicName.Replace(windows.UTF16PtrToString(nicAdapter.Description))
+		nicName := nicNameReplacer.Replace(windows.UTF16PtrToString(nicAdapter.Description))
 
 		if c.config.NicExclude.MatchString(nicName) ||
 			!c.config.NicInclude.MatchString(nicName) {
 			continue
 		}
 
-		macAddress := fmt.Sprintf("%02X:%02X:%02X:%02X:%02X:%02X",
-			nicAdapter.PhysicalAddress[0],
-			nicAdapter.PhysicalAddress[1],
-			nicAdapter.PhysicalAddress[2],
-			nicAdapter.PhysicalAddress[3],
-			nicAdapter.PhysicalAddress[4],
-			nicAdapter.PhysicalAddress[5],
-		)
+		macAddress := formatPhysicalAddress(nicAdapter)
 
 		addresses = addresses[:0]
 
@@ -593,43 +611,57 @@ func (c *Collector) nicMetrics(nicName string, nicAdapter *windows.IpAdapterAddr
 	return metrics
 }
 
-// adapterAddresses returns a list of IP adapter and address
-// structures. The structure contains an IP adapter and flattened
-// multiple IP addresses including unicast, anycast and multicast
-// addresses.
-func adapterAddresses() ([]*windows.IpAdapterAddresses, error) {
-	var b []byte
+// formatPhysicalAddress returns the physical address of nicAdapter as
+// colon-separated upper case hex bytes. It is empty for adapters without one,
+// like the loopback adapter.
+func formatPhysicalAddress(nicAdapter *windows.IpAdapterAddresses) string {
+	const hexDigits = "0123456789ABCDEF"
 
-	l := uint32(15000) // recommended initial size
+	n := min(int(nicAdapter.PhysicalAddressLength), len(nicAdapter.PhysicalAddress))
+	if n == 0 {
+		return ""
+	}
+
+	b := make([]byte, 0, 3*n-1)
+
+	for i, octet := range nicAdapter.PhysicalAddress[:n] {
+		if i > 0 {
+			b = append(b, ':')
+		}
+
+		b = append(b, hexDigits[octet>>4], hexDigits[octet&0x0F])
+	}
+
+	return string(b)
+}
+
+// adapterAddresses returns the linked list of IP adapter addresses of the
+// system, without multicast and DNS server addresses. The list lives in the
+// returned buffer, which the caller passes to the next call for reuse. The
+// list is nil if the system has no adapters.
+func adapterAddresses(b []byte) (*windows.IpAdapterAddresses, []byte, error) {
+	const flags = windows.GAA_FLAG_SKIP_MULTICAST | windows.GAA_FLAG_SKIP_DNS_SERVER
+
+	l := uint32(max(len(b), 15000)) // 15000 is the recommended initial size.
 
 	for {
-		b = make([]byte, l)
+		if len(b) < int(l) {
+			b = make([]byte, l)
+		}
 
-		const flags = windows.GAA_FLAG_SKIP_MULTICAST | windows.GAA_FLAG_SKIP_DNS_SERVER
+		l = uint32(len(b))
 
 		err := windows.GetAdaptersAddresses(windows.AF_UNSPEC, flags, 0, (*windows.IpAdapterAddresses)(unsafe.Pointer(&b[0])), &l)
 		if err == nil {
-			if l == 0 {
-				return nil, nil
-			}
-
-			break
+			return (*windows.IpAdapterAddresses)(unsafe.Pointer(&b[0])), b, nil
 		}
 
-		if !errors.Is(err, windows.ERROR_BUFFER_OVERFLOW) {
-			return nil, os.NewSyscallError("getadaptersaddresses", err)
+		if errors.Is(err, windows.ERROR_NO_DATA) {
+			return nil, b, nil
 		}
 
-		if l <= uint32(len(b)) {
-			return nil, os.NewSyscallError("getadaptersaddresses", err)
+		if !errors.Is(err, windows.ERROR_BUFFER_OVERFLOW) || l <= uint32(len(b)) {
+			return nil, b, os.NewSyscallError("getadaptersaddresses", err)
 		}
 	}
-
-	var addresses []*windows.IpAdapterAddresses
-
-	for address := (*windows.IpAdapterAddresses)(unsafe.Pointer(&b[0])); address != nil; address = address.Next {
-		addresses = append(addresses, address)
-	}
-
-	return addresses, nil
 }

internal/collector/net/net_bench_test.go

@@ -18,18 +18,37 @@
 package net_test
 
 import (
+	"log/slog"
+	"sync"
 	"testing"
 
-	"github.com/alecthomas/kingpin/v2"
 	"github.com/prometheus-community/windows_exporter/internal/collector/net"
-	"github.com/prometheus-community/windows_exporter/internal/utils/testutils"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+// BenchmarkCollector builds the collector from its config instead of
+// testutils.FuncBenchmarkCollector, which doesn't parse the flags and so
+// leaves every sub collector disabled.
 func BenchmarkCollector(b *testing.B) {
-	// PrinterInclude is not set in testing context (kingpin flags not parsed), causing the collector to skip all interfaces.
-	localNicInclude := ".+"
+	c := net.New(nil)
+	require.NoError(b, c.Build(slog.New(slog.DiscardHandler), nil))
 
-	testutils.FuncBenchmarkCollector(b, net.Name, net.NewWithFlags, func(app *kingpin.Application) {
-		app.GetFlag("collector.net.nic-include").StringVar(&localNicInclude)
+	metrics := make(chan prometheus.Metric)
+
+	var wg sync.WaitGroup
+	wg.Go(func() {
+		for range metrics {
+		}
 	})
+	b.Cleanup(func() {
+		close(metrics)
+		wg.Wait()
+		assert.NoError(b, c.Close())
+	})
+
+	for b.Loop() {
+		require.NoError(b, c.Collect(metrics, 0))
+	}
 }

internal/collector/net/net_internal_test.go

@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: Apache-2.0
+//
+// Copyright The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build windows
+
+package net
+
+import (
+	"testing"
+
+	"golang.org/x/sys/windows"
+)
+
+func TestFormatPhysicalAddress(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name    string
+		address []byte
+		want    string
+	}{
+		{name: "none", address: nil, want: ""},
+		{name: "ethernet", address: []byte{0x00, 0x15, 0x5d, 0x0a, 0xbc, 0xef}, want: "00:15:5D:0A:BC:EF"},
+		{name: "eui-64", address: []byte{0, 0, 0, 0, 0, 0, 0, 0xe0}, want: "00:00:00:00:00:00:00:E0"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			var adapter windows.IpAdapterAddresses
+
+			adapter.PhysicalAddressLength = uint32(copy(adapter.PhysicalAddress[:], tc.address))
+
+			if got := formatPhysicalAddress(&adapter); got != tc.want {
+				t.Errorf("formatPhysicalAddress() = %q, want %q", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestNICNameReplacer(t *testing.T) {
+	t.Parallel()
+
+	// The Network Interface counter object replaces these characters in the
+	// instance names of the adapters.
+	got := nicNameReplacer.Replace(`Intel(R) PRO/1000 MT Network Connection #2 a\b`)
+	want := "Intel[R] PRO_1000 MT Network Connection _2 a_b"
+
+	if got != want {
+		t.Errorf("nicNameReplacer.Replace() = %q, want %q", got, want)
+	}
+}