cpu: hide small drops of the processor time counters (#2766)

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

docs/collector.cpu.md

@@ -33,6 +33,12 @@ These metrics are available on all versions of Windows:
 | `windows_cpu_processor_utility_total`            | Processor Utility Total is a newer, more accurate measure of CPU utilization, in particular handling modern CPUs with variant CPU frequencies. The rate of this counter divided by the rate of `windows_cpu_processor_rtc_total` should provide an accurate view of CPU utilisation on modern systems, as observed in Task Manager. | counter | `core`          |
 | `windows_cpu_processor_privileged_utility_total` | Processor Privileged Utility Total, when used in a similar fashion to `windows_cpu_processor_utility_total` will show the portion of CPU utilization which is happening in privileged mode.                                                                                                                                         | counter | `core`          |
 
+### Counter decreases
+Windows sometimes reports a per-processor time one clock tick (1/64 s) lower than in the previous sample.
+`rate()` would read such a drop as a counter reset and show a spike.
+The collector keeps the value of the previous scrape for `windows_cpu_time_seconds_total` and
+`windows_cpu_cstate_seconds_total` until the counter catches up again. Drops of 1 second or more are passed through.
+
 ### Example metric
 Show frequency of host CPU cores
 ```

internal/collector/cpu/AGENTS.md

@@ -13,6 +13,10 @@ Hyper-V guest with 12 logical processors in one processor group.
 - `% C1/C2/C3 Time`, `% DPC Time`, `% Idle Time`, `% Interrupt Time`,
   `% Privileged Time` and `% User Time` are `PERF_100NSEC_TIMER`. Their raw
   value is cumulative 100 ns ticks; `internal/pdh` scales them to seconds.
+  These raw values can drop by one clock tick (1/64 s) between two samples
+  ([#1383](https://github.com/prometheus-community/windows_exporter/issues/1383)),
+  so `Collect` holds the previous value of `time_seconds_total` and
+  `cstate_seconds_total` for drops below `maxTimeDrop` (1 s).
 - `% Processor Time` and `% Priority Time` are `PERF_100NSEC_TIMER_INV`. The
   raw value is cumulative idle (non-busy) time in 100 ns ticks, not busy time,
   and `internal/pdh` does not scale this type to seconds. In the sample, the

internal/collector/cpu/cpu.go

@@ -33,6 +33,12 @@ import (
 
 const Name = "cpu"
 
+// maxTimeDrop is the largest decrease of a processor time counter, in
+// seconds, that Collect hides. Windows sometimes reports a per-processor time
+// one clock tick (1/64 s) lower than in the previous sample. A larger drop is
+// passed through as a counter reset.
+const maxTimeDrop = 1.0
+
 type Config struct{}
 
 //nolint:gochecknoglobals
@@ -48,6 +54,8 @@ type Collector struct {
 
 	processorRTCValues   map[string]utils.Counter
 	processorMPerfValues map[string]utils.Counter
+	// timeValues are the processor times of the last scrape by core and mode.
+	timeValues map[timeKey]float64
 
 	logicalProcessors          *prometheus.Desc
 	cStateSecondsTotal         *prometheus.Desc
@@ -94,6 +102,20 @@ func (c *Collector) Close() error {
 func (c *Collector) Build(logger *slog.Logger, _ *mi.Session) error {
 	c.mu = sync.Mutex{}
 
+	c.buildDescriptors()
+
+	var err error
+
+	c.perfDataCollector, err = pdh.NewCollector[perfDataCounterValues](logger.With(slog.String("collector", Name)), pdh.CounterTypeRaw, "Processor Information", pdh.InstancesAll)
+	if err != nil {
+		return fmt.Errorf("failed to create Processor Information collector: %w", err)
+	}
+
+	return nil
+}
+
+// buildDescriptors creates the descriptors and the per-core state.
+func (c *Collector) buildDescriptors() {
 	c.logicalProcessors = prometheus.NewDesc(
 		prometheus.BuildFQName(types.Namespace, Name, "logical_processor"),
 		"Total number of logical processors",
@@ -181,19 +203,11 @@ func (c *Collector) Build(logger *slog.Logger, _ *mi.Session) error {
 
 	c.processorRTCValues = map[string]utils.Counter{}
 	c.processorMPerfValues = map[string]utils.Counter{}
-
-	var err error
-
-	c.perfDataCollector, err = pdh.NewCollector[perfDataCounterValues](logger.With(slog.String("collector", Name)), pdh.CounterTypeRaw, "Processor Information", pdh.InstancesAll)
-	if err != nil {
-		return fmt.Errorf("failed to create Processor Information collector: %w", err)
-	}
-
-	return nil
+	c.timeValues = map[timeKey]float64{}
 }
 
 func (c *Collector) Collect(ch chan<- prometheus.Metric, _ time.Duration) error {
-	c.mu.Lock() // Lock is needed to prevent concurrent map access to c.processorRTCValues
+	c.mu.Lock() // Lock is needed to prevent concurrent access to the per-core state maps.
 	defer c.mu.Unlock()
 
 	err := c.perfDataCollector.Collect(&c.perfDataObject)
@@ -201,8 +215,18 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, _ time.Duration) error
 		return fmt.Errorf("failed to collect Processor Information metrics: %w", err)
 	}
 
+	c.publish(ch)
+
+	return nil
+}
+
+// publish sends the metrics of c.perfDataObject and updates the per-core
+// state. The caller holds c.mu.
+func (c *Collector) publish(ch chan<- prometheus.Metric) {
 	var coreCount float64
 
+	timeValues := make(map[timeKey]float64, len(c.timeValues))
+
 	for _, coreData := range c.perfDataObject {
 		core := coreData.Name
 		coreCount++
@@ -232,56 +256,56 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, _ time.Duration) error
 		ch <- prometheus.MustNewConstMetric(
 			c.cStateSecondsTotal,
 			prometheus.CounterValue,
-			coreData.C1TimeSeconds,
+			c.monotonicTime(timeValues, core, "c1", coreData.C1TimeSeconds),
 			core, "c1",
 		)
 
 		ch <- prometheus.MustNewConstMetric(
 			c.cStateSecondsTotal,
 			prometheus.CounterValue,
-			coreData.C2TimeSeconds,
+			c.monotonicTime(timeValues, core, "c2", coreData.C2TimeSeconds),
 			core, "c2",
 		)
 
 		ch <- prometheus.MustNewConstMetric(
 			c.cStateSecondsTotal,
 			prometheus.CounterValue,
-			coreData.C3TimeSeconds,
+			c.monotonicTime(timeValues, core, "c3", coreData.C3TimeSeconds),
 			core, "c3",
 		)
 
 		ch <- prometheus.MustNewConstMetric(
 			c.timeTotal,
 			prometheus.CounterValue,
-			coreData.IdleTimeSeconds,
+			c.monotonicTime(timeValues, core, "idle", coreData.IdleTimeSeconds),
 			core, "idle",
 		)
 
 		ch <- prometheus.MustNewConstMetric(
 			c.timeTotal,
 			prometheus.CounterValue,
-			coreData.InterruptTimeSeconds,
+			c.monotonicTime(timeValues, core, "interrupt", coreData.InterruptTimeSeconds),
 			core, "interrupt",
 		)
 
 		ch <- prometheus.MustNewConstMetric(
 			c.timeTotal,
 			prometheus.CounterValue,
-			coreData.DpcTimeSeconds,
+			c.monotonicTime(timeValues, core, "dpc", coreData.DpcTimeSeconds),
 			core, "dpc",
 		)
 
 		ch <- prometheus.MustNewConstMetric(
 			c.timeTotal,
 			prometheus.CounterValue,
-			coreData.PrivilegedTimeSeconds,
+			c.monotonicTime(timeValues, core, "privileged", coreData.PrivilegedTimeSeconds),
 			core, "privileged",
 		)
 
 		ch <- prometheus.MustNewConstMetric(
 			c.timeTotal,
 			prometheus.CounterValue,
-			coreData.UserTimeSeconds,
+			c.monotonicTime(timeValues, core, "user", coreData.UserTimeSeconds),
 			core, "user",
 		)
 
@@ -363,11 +387,32 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, _ time.Duration) error
 		)
 	}
 
+	c.timeValues = timeValues
+
 	ch <- prometheus.MustNewConstMetric(
 		c.logicalProcessors,
 		prometheus.GaugeValue,
 		coreCount,
 	)
+}
 
-	return nil
+type timeKey struct {
+	core, mode string
+}
+
+// monotonicTime returns the processor time of the core and mode, but at least
+// the value of the last scrape if it dropped by less than maxTimeDrop. Rate
+// functions read every decrease of a counter as a reset, so a drop of one
+// clock tick would show up as a spike. The returned value is stored in
+// timeValues.
+func (c *Collector) monotonicTime(timeValues map[timeKey]float64, core, mode string, value float64) float64 {
+	key := timeKey{core: core, mode: mode}
+
+	if last, ok := c.timeValues[key]; ok && value < last && last-value < maxTimeDrop {
+		value = last
+	}
+
+	timeValues[key] = value
+
+	return value
 }

internal/collector/cpu/cpu_internal_test.go

@@ -0,0 +1,109 @@
+// 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 cpu
+
+import (
+	"testing"
+
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/stretchr/testify/require"
+)
+
+// collectFunc adapts publish to a prometheus.Collector for the registry.
+type collectFunc func(ch chan<- prometheus.Metric)
+
+func (f collectFunc) Describe(ch chan<- *prometheus.Desc) { prometheus.DescribeByCollect(f, ch) }
+
+func (f collectFunc) Collect(ch chan<- prometheus.Metric) { f(ch) }
+
+// scrapeTimes publishes the rows and returns the gathered windows_cpu_time_seconds_total
+// values by core and mode.
+func scrapeTimes(t *testing.T, c *Collector, rows ...perfDataCounterValues) map[timeKey]float64 {
+	t.Helper()
+
+	c.perfDataObject = rows
+
+	registry := prometheus.NewPedanticRegistry()
+	require.NoError(t, registry.Register(collectFunc(c.publish)))
+
+	families, err := registry.Gather()
+	require.NoError(t, err)
+
+	values := map[timeKey]float64{}
+
+	for _, family := range families {
+		if family.GetName() != "windows_cpu_time_seconds_total" {
+			continue
+		}
+
+		for _, metric := range family.GetMetric() {
+			key := timeKey{}
+
+			for _, label := range metric.GetLabel() {
+				switch label.GetName() {
+				case "core":
+					key.core = label.GetValue()
+				case "mode":
+					key.mode = label.GetValue()
+				}
+			}
+
+			values[key] = metric.GetCounter().GetValue()
+		}
+	}
+
+	return values
+}
+
+func TestMonotonicTime(t *testing.T) {
+	t.Parallel()
+
+	c := New(nil)
+	c.buildDescriptors()
+
+	privileged := func(values map[timeKey]float64, core string) float64 {
+		return values[timeKey{core: core, mode: "privileged"}]
+	}
+
+	// The values of issue #1383: the counter dropped by one clock tick.
+	for _, step := range []struct {
+		value, want float64
+	}{
+		{4653.78125, 4653.78125},
+		{4653.8125, 4653.8125},
+		{4653.796875, 4653.8125}, // A drop of one clock tick is hidden.
+		{4653.796875, 4653.8125},
+		{4653.84375, 4653.84375},                             // The counter continues once it caught up.
+		{4653.84375 - maxTimeDrop + 0.015625, 4653.84375},    // Just under maxTimeDrop is held.
+		{4653.84375 - maxTimeDrop, 4653.84375 - maxTimeDrop}, // A drop of maxTimeDrop is a reset.
+		{10, 10}, // A larger drop is a reset.
+	} {
+		values := scrapeTimes(t, c, perfDataCounterValues{Name: "0,0", PrivilegedTimeSeconds: step.value, UserTimeSeconds: 5})
+		require.InDelta(t, step.want, privileged(values, "0,0"), 0, "value %v", step.value)
+		require.InDelta(t, 5.0, values[timeKey{core: "0,0", mode: "user"}], 0)
+	}
+
+	// Cores are independent, and a core missing for one scrape loses its state.
+	scrapeTimes(t, c, perfDataCounterValues{Name: "0,0", PrivilegedTimeSeconds: 20}, perfDataCounterValues{Name: "0,1", PrivilegedTimeSeconds: 30})
+	values := scrapeTimes(t, c, perfDataCounterValues{Name: "0,0", PrivilegedTimeSeconds: 19.5})
+	require.InDelta(t, 20.0, privileged(values, "0,0"), 0)
+	require.NotContains(t, values, timeKey{core: "0,1", mode: "privileged"})
+
+	values = scrapeTimes(t, c, perfDataCounterValues{Name: "0,0", PrivilegedTimeSeconds: 20}, perfDataCounterValues{Name: "0,1", PrivilegedTimeSeconds: 29.5})
+	require.InDelta(t, 29.5, privileged(values, "0,1"), 0, "a core that returns starts without state")
+}