performancecounter: build metric descs once at Build (#2798)

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

docs/collector.performancecounter.md

@@ -179,7 +179,8 @@ Some Objects like `Memory` do not have instances to select from at all. In this
 
 The name of the label that holds the instance name. Optional and defaults to `object_instance`. Do not use `instance`: it collides with the target label that Prometheus adds.
 
-The label is only added if `instances` is set.
+The label is only added if `instances` is set. It must be a valid label name, otherwise the collector fails to start.
+A counter label with the same name takes precedence over the instance name.
 
 Some objects report the same instance name more than once in a sample, for example `Process` reports every `svchost` process as `svchost`.
 The first instance keeps its name, later ones get the suffix `#1`, `#2` and so on, skipping names that Windows reported itself, such as a network adapter `Ethernet#2`.
@@ -242,6 +243,7 @@ This key is optional.
 ##### labels
 
 Labels is a map of key-value pairs that will be added as labels to the metric.
+If a label or metric name is invalid, the collector fails to start.
 
 Counters of the same object that share a metric name must differ in their labels, like `state` in the schema example above.
 Otherwise, they would produce identical series and the collector fails to start.

internal/collector/performancecounter/performancecounter.go

@@ -35,6 +35,7 @@ import (
 	"github.com/prometheus-community/windows_exporter/internal/pdh"
 	"github.com/prometheus-community/windows_exporter/internal/types"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/common/model"
 	"go.yaml.in/yaml/v3"
 )
 
@@ -158,6 +159,17 @@ func (c *Collector) Build(logger *slog.Logger, _ *mi.Session) error {
 		}
 
 		names = append(names, object.Name)
+
+		if object.InstanceLabel == "" {
+			object.InstanceLabel = "object_instance"
+		}
+
+		if object.Instances != nil && !validLabelName(object.InstanceLabel) {
+			errs = append(errs, fmt.Errorf("object %s: %q is not a valid instance label", object.Name, object.InstanceLabel))
+
+			continue
+		}
+
 		counters := make([]Counter, 0, len(object.Counters))
 		counterNames := make([]string, 0, len(object.Counters))
 		object.valueIndex = make(map[string]int, len(object.Counters))
@@ -201,6 +213,12 @@ func (c *Collector) Build(logger *slog.Logger, _ *mi.Session) error {
 				continue
 			}
 
+			if err := object.buildDescs(&object.Counters[j]); err != nil {
+				errs = append(errs, fmt.Errorf("object %s: counter %s: %w", object.Name, counter.Name, err))
+
+				continue
+			}
+
 			counters = append(counters, counter)
 
 			object.valueIndex[counter.Name] = len(counterNames)
@@ -216,10 +234,6 @@ func (c *Collector) Build(logger *slog.Logger, _ *mi.Session) error {
 			errs = append(errs, fmt.Errorf("failed collector for %s: %w", object.Name, err))
 		}
 
-		if object.InstanceLabel == "" {
-			object.InstanceLabel = "object_instance"
-		}
-
 		object.collector = collector
 
 		c.objects = append(c.objects, object)
@@ -322,30 +336,46 @@ func (c *Collector) collectObject(ch chan<- prometheus.Metric, perfDataObject Ob
 				continue
 			}
 
-			labels := make(prometheus.Labels, len(counter.Labels)+1)
+			value := counter.multiplier.Apply(collectedCounterValue)
 
-			if perfDataObject.Instances != nil && row.Name != pdh.InstanceEmpty {
-				labels[perfDataObject.InstanceLabel] = row.Name
+			if counter.instanceDesc != nil && row.Name != pdh.InstanceEmpty {
+				ch <- prometheus.MustNewConstMetric(counter.instanceDesc, metricTypes[j], value, row.Name)
+			} else {
+				ch <- prometheus.MustNewConstMetric(counter.desc, metricTypes[j], value)
 			}
-
-			maps.Copy(labels, counter.Labels)
-
-			ch <- prometheus.MustNewConstMetric(
-				prometheus.NewDesc(
-					counter.Metric,
-					"windows_exporter: custom Performance Counter metric",
-					nil,
-					labels,
-				),
-				metricTypes[j],
-				counter.multiplier.Apply(collectedCounterValue),
-			)
 		}
 	}
 
 	return errors.Join(errs...)
 }
 
+// buildDescs creates the descs of counter once, so that a scrape doesn't
+// build one desc per series. The instance becomes a variable label; a
+// const label of the same name takes precedence over it.
+func (o *Object) buildDescs(counter *Counter) error {
+	const help = "windows_exporter: custom Performance Counter metric"
+
+	counter.desc = prometheus.NewDesc(counter.Metric, help, nil, counter.Labels)
+	if err := counter.desc.Err(); err != nil {
+		return err
+	}
+
+	counter.instanceDesc = nil
+
+	if _, ok := counter.Labels[o.InstanceLabel]; o.Instances == nil || ok {
+		return nil
+	}
+
+	counter.instanceDesc = prometheus.NewDesc(counter.Metric, help, []string{o.InstanceLabel}, counter.Labels)
+
+	return counter.instanceDesc.Err()
+}
+
+// validLabelName reports whether prometheus accepts name as label name.
+func validLabelName(name string) bool {
+	return model.UTF8Validation.IsValidLabelName(name) && !strings.HasPrefix(name, model.ReservedLabelPrefix)
+}
+
 // counterMetricTypes returns the metric type of every counter of the object.
 // An explicit type in the configuration takes precedence over the type that
 // PDH reports for the counter.

internal/collector/performancecounter/performancecounter_test_test.go

@@ -135,6 +135,26 @@ windows_performancecounter_collector_success\{collector="processor_information_f
 windows_performancecounter_processor_information_processor_time\{core="0,0",state="active"} [0-9.e+-]+
 windows_performancecounter_processor_information_processor_time\{core="0,0",state="idle"} [0-9.e+-]+
 .*`),
+		},
+		{
+			// A const label of the same name takes precedence over the instance label.
+			name:          "instance_label_overridden",
+			object:        "Processor Information",
+			counterType:   pdh.CounterTypeRaw,
+			instances:     []string{"_Total"},
+			instanceLabel: "core",
+			buildErr:      "",
+			counters:      []performancecounter.Counter{{Name: "% Processor Time", Metric: "windows_performancecounter_processor_information_processor_time", Labels: map[string]string{"core": "all"}}},
+			expectedMetrics: regexp.MustCompile(`^# HELP windows_performancecounter_collector_duration_seconds windows_exporter: Duration of an performancecounter child collection.
+# TYPE windows_performancecounter_collector_duration_seconds gauge
+windows_performancecounter_collector_duration_seconds\{collector="instance_label_overridden"} [0-9.e+-]+
+# HELP windows_performancecounter_collector_success windows_exporter: Whether a performancecounter child collector was successful.
+# TYPE windows_performancecounter_collector_success gauge
+windows_performancecounter_collector_success\{collector="instance_label_overridden"} 1
+# HELP windows_performancecounter_processor_information_processor_time windows_exporter: custom Performance Counter metric
+# TYPE windows_performancecounter_processor_information_processor_time counter
+windows_performancecounter_processor_information_processor_time\{core="all"} [0-9.e+-]+
+`),
 		},
 		{
 			name:            "",
@@ -328,6 +348,15 @@ func TestBuildInvalidConfig(t *testing.T) {
 			},
 			buildErr: "object processor: counters % Processor Time and % User Time produce identical series windows_processor_time",
 		},
+		{
+			name: "invalid instance label",
+			objects: []performancecounter.Object{
+				{Name: "processor", Object: "Processor Information", Instances: pdh.InstancesAll, InstanceLabel: "__core", Counters: []performancecounter.Counter{
+					{Name: "% Processor Time"},
+				}},
+			},
+			buildErr: `object processor: "__core" is not a valid instance label`,
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()

internal/collector/performancecounter/types.go

@@ -21,6 +21,7 @@ import (
 	"github.com/prometheus-community/windows_exporter/internal/config/yamlstring"
 	"github.com/prometheus-community/windows_exporter/internal/multiplier"
 	"github.com/prometheus-community/windows_exporter/internal/pdh"
+	"github.com/prometheus/client_golang/prometheus"
 	"go.yaml.in/yaml/v3"
 )
 
@@ -46,6 +47,11 @@ type Counter struct {
 
 	// Resolved at Build time.
 	multiplier multiplier.Multiplier
+	// desc describes the series without an instance label. instanceDesc
+	// adds the instance label as variable label; it is nil if the object
+	// has no instances or the counter sets the instance label itself.
+	desc         *prometheus.Desc
+	instanceDesc *prometheus.Desc
 }
 
 // https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/54691ebe11bb9ec32b4e35cd31fcb94a352de134/receiver/windowsperfcountersreceiver/README.md?plain=1#L150