dns: fix zone transfer counter mapping and count queued dynamic updates (#2718)

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

internal/collector/dns/AGENTS.md

@@ -20,7 +20,11 @@ secondary server, so zone transfer counters stayed 0.
 - `Total Query Received` equals `UDP Query Received` + `TCP Query Received`
   (830 = 820 + 10); `Total Response Sent` likewise (825 = 815 + 10).
 - `Recursive Send TimeOuts` and `Recursive TimeOut/sec` are a twin pair with
-  different names; the collector reads the `/sec` one.
+  different names; the collector reads `Recursive Send TimeOuts`.
+- `Dynamic Update Queued` is cumulative: it matches `Queue / Queued` in
+  `dnscmd /statistics 0x00000100` (and `Queued` in
+  `Get-DnsServerStatistics`), not the current `In Queue` value. Publish it as
+  a counter.
 - Eleven counters of the object have names that belong to other objects and
   no help text: `% User Time`, `Async Fast Reads/sec`,
   `Fast Read Not Possibles/sec`, `Fast Read Resource Misses/sec`,
@@ -28,6 +32,11 @@ secondary server, so zone transfer counters stayed 0.
   `Data Flush Pages/sec`, `Level 2 TLB Fills/sec`, `Enumerations Server/sec`
   and `Virtual Bytes`. `Get-Counter -ListSet DNS` also lists a counter named
   `176`. All of them stayed 0; the collector ignores them (`_` fields).
+- Compare live values with `dnscmd /statistics` or `Get-Counter "\DNS\*"`.
+  `ipconfig /registerdns` on a domain member raises the dynamic update
+  counters.
+- `collectPerfData` publishes one sample; test value mapping with it through
+  a registry (see `dns_internal_test.go`).
 
 ## Counter reference
 

internal/collector/dns/dns.go

@@ -158,6 +158,19 @@ func (c *Collector) Build(logger *slog.Logger, miSession *mi.Session) error {
 }
 
 func (c *Collector) buildMetricsCollector(logger *slog.Logger) error {
+	c.buildDescriptors()
+
+	var err error
+
+	c.perfDataCollector, err = pdh.NewCollector[perfDataCounterValues](logger.With(slog.String("collector", Name)), pdh.CounterTypeRaw, "DNS", pdh.InstancesAll)
+	if err != nil {
+		return fmt.Errorf("failed to create DNS collector: %w", err)
+	}
+
+	return nil
+}
+
+func (c *Collector) buildDescriptors() {
 	c.zoneTransferRequestsReceived = prometheus.NewDesc(
 		prometheus.BuildFQName(types.Namespace, Name, "zone_transfer_requests_received_total"),
 		"Number of zone transfer requests (AXFR/IXFR) received by the master DNS server",
@@ -166,7 +179,7 @@ func (c *Collector) buildMetricsCollector(logger *slog.Logger) error {
 	)
 	c.zoneTransferRequestsSent = prometheus.NewDesc(
 		prometheus.BuildFQName(types.Namespace, Name, "zone_transfer_requests_sent_total"),
-		"Number of zone transfer requests (AXFR/IXFR) sent by the secondary DNS server",
+		"Number of zone transfer requests (AXFR/IXFR) and zone transfer SOA requests sent by the secondary DNS server",
 		[]string{"qtype"},
 		nil,
 	)
@@ -201,14 +214,14 @@ func (c *Collector) buildMetricsCollector(logger *slog.Logger) error {
 		nil,
 	)
 	c.dynamicUpdatesQueued = prometheus.NewDesc(
-		prometheus.BuildFQName(types.Namespace, Name, "dynamic_updates_queued"),
+		prometheus.BuildFQName(types.Namespace, Name, "dynamic_updates_queued_total"),
 		"Number of dynamic updates queued by the DNS server",
 		nil,
 		nil,
 	)
 	c.dynamicUpdatesReceived = prometheus.NewDesc(
 		prometheus.BuildFQName(types.Namespace, Name, "dynamic_updates_received_total"),
-		"Number of secure update requests received by the DNS server",
+		"Number of dynamic update requests received by the DNS server that were no-operations (noop) or written to the database (written)",
 		[]string{"operation"},
 		nil,
 	)
@@ -297,15 +310,6 @@ func (c *Collector) buildMetricsCollector(logger *slog.Logger) error {
 		[]string{"name", "collection_name", "dns_server"},
 		nil,
 	)
-
-	var err error
-
-	c.perfDataCollector, err = pdh.NewCollector[perfDataCounterValues](logger.With(slog.String("collector", Name)), pdh.CounterTypeRaw, "DNS", pdh.InstancesAll)
-	if err != nil {
-		return fmt.Errorf("failed to create DNS collector: %w", err)
-	}
-
-	return nil
 }
 
 func (c *Collector) buildErrorStatsCollector(miSession *mi.Session) error {
@@ -357,59 +361,66 @@ func (c *Collector) collectMetrics(ch chan<- prometheus.Metric) error {
 		return fmt.Errorf("failed to collect DNS metrics: %w", types.ErrNoDataUnexpected)
 	}
 
+	c.collectPerfData(ch, c.perfDataObject[0])
+
+	return nil
+}
+
+// collectPerfData publishes the metrics of one sample of the DNS object.
+func (c *Collector) collectPerfData(ch chan<- prometheus.Metric, data perfDataCounterValues) {
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferRequestsReceived,
 		prometheus.CounterValue,
-		c.perfDataObject[0].AxfrRequestReceived,
+		data.AxfrRequestReceived,
 		"full",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferRequestsReceived,
 		prometheus.CounterValue,
-		c.perfDataObject[0].IxfrRequestReceived,
+		data.IxfrRequestReceived,
 		"incremental",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferRequestsSent,
 		prometheus.CounterValue,
-		c.perfDataObject[0].AxfrRequestSent,
+		data.AxfrRequestSent,
 		"full",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferRequestsSent,
 		prometheus.CounterValue,
-		c.perfDataObject[0].IxfrRequestSent,
+		data.IxfrRequestSent,
 		"incremental",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferRequestsSent,
 		prometheus.CounterValue,
-		c.perfDataObject[0].ZoneTransferSOARequestSent,
+		data.ZoneTransferSOARequestSent,
 		"soa",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferResponsesReceived,
 		prometheus.CounterValue,
-		c.perfDataObject[0].AxfrResponseReceived,
+		data.AxfrResponseReceived,
 		"full",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferResponsesReceived,
 		prometheus.CounterValue,
-		c.perfDataObject[0].IxfrResponseReceived,
+		data.IxfrResponseReceived,
 		"incremental",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferSuccessReceived,
 		prometheus.CounterValue,
-		c.perfDataObject[0].AxfrSuccessReceived,
+		data.AxfrSuccessReceived,
 		"full",
 		"tcp",
 	)
@@ -417,7 +428,7 @@ func (c *Collector) collectMetrics(ch chan<- prometheus.Metric) error {
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferSuccessReceived,
 		prometheus.CounterValue,
-		c.perfDataObject[0].IxfrTCPSuccessReceived,
+		data.IxfrTCPSuccessReceived,
 		"incremental",
 		"tcp",
 	)
@@ -425,7 +436,7 @@ func (c *Collector) collectMetrics(ch chan<- prometheus.Metric) error {
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferSuccessReceived,
 		prometheus.CounterValue,
-		c.perfDataObject[0].IxfrTCPSuccessReceived,
+		data.IxfrUDPSuccessReceived,
 		"incremental",
 		"udp",
 	)
@@ -433,204 +444,202 @@ func (c *Collector) collectMetrics(ch chan<- prometheus.Metric) error {
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferSuccessSent,
 		prometheus.CounterValue,
-		c.perfDataObject[0].AxfrSuccessSent,
+		data.AxfrSuccessSent,
 		"full",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferSuccessSent,
 		prometheus.CounterValue,
-		c.perfDataObject[0].IxfrSuccessSent,
+		data.IxfrSuccessSent,
 		"incremental",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.zoneTransferFailures,
 		prometheus.CounterValue,
-		c.perfDataObject[0].ZoneTransferFailure,
+		data.ZoneTransferFailure,
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memoryUsedBytes,
 		prometheus.GaugeValue,
-		c.perfDataObject[0].CachingMemory,
+		data.CachingMemory,
 		"caching",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memoryUsedBytes,
 		prometheus.GaugeValue,
-		c.perfDataObject[0].DatabaseNodeMemory,
+		data.DatabaseNodeMemory,
 		"database_node",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memoryUsedBytes,
 		prometheus.GaugeValue,
-		c.perfDataObject[0].NbStatMemory,
+		data.NbStatMemory,
 		"nbstat",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memoryUsedBytes,
 		prometheus.GaugeValue,
-		c.perfDataObject[0].RecordFlowMemory,
+		data.RecordFlowMemory,
 		"record_flow",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memoryUsedBytes,
 		prometheus.GaugeValue,
-		c.perfDataObject[0].TcpMessageMemory,
+		data.TcpMessageMemory,
 		"tcp_message",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memoryUsedBytes,
 		prometheus.GaugeValue,
-		c.perfDataObject[0].UdpMessageMemory,
+		data.UdpMessageMemory,
 		"udp_message",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.dynamicUpdatesReceived,
 		prometheus.CounterValue,
-		c.perfDataObject[0].DynamicUpdateNoOperation,
+		data.DynamicUpdateNoOperation,
 		"noop",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.dynamicUpdatesReceived,
 		prometheus.CounterValue,
-		c.perfDataObject[0].DynamicUpdateWrittenToDatabase,
+		data.DynamicUpdateWrittenToDatabase,
 		"written",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.dynamicUpdatesQueued,
-		prometheus.GaugeValue,
-		c.perfDataObject[0].DynamicUpdateQueued,
+		prometheus.CounterValue,
+		data.DynamicUpdateQueued,
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.dynamicUpdatesFailures,
 		prometheus.CounterValue,
-		c.perfDataObject[0].DynamicUpdateRejected,
+		data.DynamicUpdateRejected,
 		"rejected",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.dynamicUpdatesFailures,
 		prometheus.CounterValue,
-		c.perfDataObject[0].DynamicUpdateTimeOuts,
+		data.DynamicUpdateTimeOuts,
 		"timeout",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.notifyReceived,
 		prometheus.CounterValue,
-		c.perfDataObject[0].NotifyReceived,
+		data.NotifyReceived,
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.notifySent,
 		prometheus.CounterValue,
-		c.perfDataObject[0].NotifySent,
+		data.NotifySent,
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.recursiveQueries,
 		prometheus.CounterValue,
-		c.perfDataObject[0].RecursiveQueries,
+		data.RecursiveQueries,
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.recursiveQueryFailures,
 		prometheus.CounterValue,
-		c.perfDataObject[0].RecursiveQueryFailure,
+		data.RecursiveQueryFailure,
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.recursiveQuerySendTimeouts,
 		prometheus.CounterValue,
-		c.perfDataObject[0].RecursiveSendTimeOuts,
+		data.RecursiveSendTimeOuts,
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.queries,
 		prometheus.CounterValue,
-		c.perfDataObject[0].TcpQueryReceived,
+		data.TcpQueryReceived,
 		"tcp",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.queries,
 		prometheus.CounterValue,
-		c.perfDataObject[0].UdpQueryReceived,
+		data.UdpQueryReceived,
 		"udp",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.responses,
 		prometheus.CounterValue,
-		c.perfDataObject[0].TcpResponseSent,
+		data.TcpResponseSent,
 		"tcp",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.responses,
 		prometheus.CounterValue,
-		c.perfDataObject[0].UdpResponseSent,
+		data.UdpResponseSent,
 		"udp",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.unmatchedResponsesReceived,
 		prometheus.CounterValue,
-		c.perfDataObject[0].UnmatchedResponsesReceived,
+		data.UnmatchedResponsesReceived,
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.winsQueries,
 		prometheus.CounterValue,
-		c.perfDataObject[0].WinsLookupReceived,
+		data.WinsLookupReceived,
 		"forward",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.winsQueries,
 		prometheus.CounterValue,
-		c.perfDataObject[0].WinsReverseLookupReceived,
+		data.WinsReverseLookupReceived,
 		"reverse",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.winsResponses,
 		prometheus.CounterValue,
-		c.perfDataObject[0].WinsResponseSent,
+		data.WinsResponseSent,
 		"forward",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.winsResponses,
 		prometheus.CounterValue,
-		c.perfDataObject[0].WinsReverseResponseSent,
+		data.WinsReverseResponseSent,
 		"reverse",
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.secureUpdateFailures,
 		prometheus.CounterValue,
-		c.perfDataObject[0].SecureUpdateFailure,
+		data.SecureUpdateFailure,
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.secureUpdateReceived,
 		prometheus.CounterValue,
-		c.perfDataObject[0].SecureUpdateReceived,
+		data.SecureUpdateReceived,
 	)
-
-	return nil
 }
 
 func (c *Collector) collectErrorStats(ch chan<- prometheus.Metric, maxScrapeDuration time.Duration) error {

internal/collector/dns/dns_internal_test.go

@@ -0,0 +1,122 @@
+// 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 dns
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/prometheus/client_golang/prometheus"
+	dto "github.com/prometheus/client_model/go"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCollectPerfData(t *testing.T) {
+	t.Parallel()
+
+	data := perfDataCounterValues{
+		AxfrRequestSent:            1,
+		IxfrRequestSent:            2,
+		ZoneTransferSOARequestSent: 3,
+		AxfrSuccessReceived:        4,
+		IxfrTCPSuccessReceived:     5,
+		IxfrUDPSuccessReceived:     6,
+		DynamicUpdateQueued:        93,
+		RecursiveSendTimeOuts:      7,
+	}
+
+	c := New(nil)
+	c.buildDescriptors()
+
+	registry := prometheus.NewPedanticRegistry()
+	require.NoError(t, registry.Register(metricCollector{collector: c, data: data}))
+
+	families, err := registry.Gather()
+	require.NoError(t, err)
+
+	type sample struct {
+		metricType dto.MetricType
+		value      float64
+	}
+
+	got := make(map[string]sample)
+
+	for _, family := range families {
+		for _, metric := range family.GetMetric() {
+			labels := make([]string, 0, len(metric.GetLabel()))
+			for _, label := range metric.GetLabel() {
+				labels = append(labels, label.GetName()+"="+label.GetValue())
+			}
+
+			value := metric.GetGauge().GetValue()
+			if family.GetType() == dto.MetricType_COUNTER {
+				value = metric.GetCounter().GetValue()
+			}
+
+			got[family.GetName()+"{"+strings.Join(labels, ",")+"}"] = sample{metricType: family.GetType(), value: value}
+		}
+	}
+
+	for name, want := range map[string]sample{
+		"windows_dns_zone_transfer_requests_sent_total{qtype=full}":                        {dto.MetricType_COUNTER, 1},
+		"windows_dns_zone_transfer_requests_sent_total{qtype=incremental}":                 {dto.MetricType_COUNTER, 2},
+		"windows_dns_zone_transfer_requests_sent_total{qtype=soa}":                         {dto.MetricType_COUNTER, 3},
+		"windows_dns_zone_transfer_success_received_total{protocol=tcp,qtype=full}":        {dto.MetricType_COUNTER, 4},
+		"windows_dns_zone_transfer_success_received_total{protocol=tcp,qtype=incremental}": {dto.MetricType_COUNTER, 5},
+		"windows_dns_zone_transfer_success_received_total{protocol=udp,qtype=incremental}": {dto.MetricType_COUNTER, 6},
+		"windows_dns_dynamic_updates_queued_total{}":                                       {dto.MetricType_COUNTER, 93},
+		"windows_dns_recursive_query_send_timeouts_total{}":                                {dto.MetricType_COUNTER, 7},
+		"windows_dns_memory_used_bytes{area=caching}":                                      {dto.MetricType_GAUGE, 0},
+	} {
+		require.Contains(t, got, name)
+		require.Equal(t, want, got[name], name)
+	}
+
+	require.NotContains(t, got, "windows_dns_dynamic_updates_queued{}")
+}
+
+// TestPerfDataCounterNames guards against a field reading a counter of a different name.
+func TestPerfDataCounterNames(t *testing.T) {
+	t.Parallel()
+
+	for field, counter := range map[string]string{
+		"ZoneTransferSOARequestSent": "Zone Transfer SOA Request Sent",
+		"IxfrUDPSuccessReceived":     "IXFR UDP Success Received",
+		"IxfrTCPSuccessReceived":     "IXFR TCP Success Received",
+		"DynamicUpdateQueued":        "Dynamic Update Queued",
+		"RecursiveSendTimeOuts":      "Recursive Send TimeOuts",
+	} {
+		f, ok := reflect.TypeFor[perfDataCounterValues]().FieldByName(field)
+		require.True(t, ok, field)
+		require.Equal(t, counter, f.Tag.Get("perfdata"), field)
+	}
+}
+
+type metricCollector struct {
+	collector *Collector
+	data      perfDataCounterValues
+}
+
+func (c metricCollector) Describe(ch chan<- *prometheus.Desc) {
+	prometheus.DescribeByCollect(c, ch)
+}
+
+func (c metricCollector) Collect(ch chan<- prometheus.Metric) {
+	c.collector.collectPerfData(ch, c.data)
+}

internal/collector/dns/types.go

@@ -70,8 +70,8 @@ type perfDataCounterValues struct {
 	_                              float64 `perfdata:"Recursive Queries/sec"`
 	RecursiveQueryFailure          float64 `perfdata:"Recursive Query Failure"`
 	_                              float64 `perfdata:"Recursive Query Failure/sec"`
-	_                              float64 `perfdata:"Recursive Send TimeOuts"`
-	RecursiveSendTimeOuts          float64 `perfdata:"Recursive TimeOut/sec"`
+	RecursiveSendTimeOuts          float64 `perfdata:"Recursive Send TimeOuts"`
+	_                              float64 `perfdata:"Recursive TimeOut/sec"`
 	_                              float64 `perfdata:"Responses Suppressed"`
 	_                              float64 `perfdata:"Responses Suppressed/sec"`
 	SecureUpdateFailure            float64 `perfdata:"Secure Update Failure"`
@@ -103,8 +103,8 @@ type perfDataCounterValues struct {
 	WinsReverseResponseSent        float64 `perfdata:"WINS Reverse Response Sent"`
 	_                              float64 `perfdata:"WINS Reverse Response Sent/sec"`
 	ZoneTransferFailure            float64 `perfdata:"Zone Transfer Failure"`
-	ZoneTransferSOARequestSent     float64 `perfdata:"Zone Transfer Request Received"`
-	_                              float64 `perfdata:"Zone Transfer SOA Request Sent"`
+	_                              float64 `perfdata:"Zone Transfer Request Received"`
+	ZoneTransferSOARequestSent     float64 `perfdata:"Zone Transfer SOA Request Sent"`
 	_                              float64 `perfdata:"Zone Transfer Success"`
 }