Fix IIS collector panic caused by # characters in application pool names

Co-authored-by: jkroepke <1560587+jkroepke@users.noreply.github.com>

internal/collector/iis/iis.go

@@ -23,6 +23,7 @@ import (
 	"log/slog"
 	"regexp"
 	"slices"
+	"strconv"
 	"strings"
 	"time"
 
@@ -290,30 +291,75 @@ type collectorName interface {
 
 // deduplicateIISNames deduplicate IIS site names from various IIS perflib objects.
 //
-// E.G. Given the following list of site names, "Site_B" would be
-// discarded, and "Site_B#2" would be kept and presented as "Site_B" in the
-// Collector metrics.
-// [ "Site_A", "Site_B", "Site_C", "Site_B#2" ].
-func deduplicateIISNames[T collectorName](counterValues []T) {
-	indexes := make(map[string]int)
+// IIS perflib may report duplicate entries for the same site/app-pool when
+// instances are recycled; the duplicate carries a numeric suffix separated by
+// "#" (e.g. "Site_B#2"). In that case "Site_B" should be discarded and
+// "Site_B#2" kept – it will be presented as "Site_B" in the metrics.
+//
+// Application Pool names may legitimately contain a "#" character, so only a
+// trailing "#<digits>" pattern is treated as an IIS counter suffix.
+//
+// Example input:  [ "Site_A", "Site_B", "Site_C", "Site_B#2" ]
+// Example output: [ "Site_A", "Site_B#2", "Site_C" ]  (presented as "Site_B")
+func deduplicateIISNames[T collectorName](counterValues []T) []T {
+	type dedupeEntry struct {
+		resultIdx     int // index into result slice
+		numericSuffix int // -1 for base (no suffix), ≥0 for "#N" variants
+	}
 
-	// Ensure IIS entry with the highest suffix occurs last
+	// Sort by base name for a deterministic output order.
 	slices.SortFunc(counterValues, func(a, b T) int {
-		return strings.Compare(a.GetName(), b.GetName())
+		return strings.Compare(iisCounterBaseName(a.GetName()), iisCounterBaseName(b.GetName()))
 	})
 
-	// Use map to deduplicate IIS entries
-	for index, counterValue := range counterValues {
-		name := strings.Split(counterValue.GetName(), "#")[0]
-		if name == counterValue.GetName() {
-			continue
+	seen := make(map[string]dedupeEntry, len(counterValues))
+	result := make([]T, 0, len(counterValues))
+
+	for _, counterValue := range counterValues {
+		name := counterValue.GetName()
+		baseName := iisCounterBaseName(name)
+
+		// Determine the numeric suffix for this entry (-1 means "no suffix").
+		suffix := -1
+		if baseName != name {
+			// iisCounterBaseName guarantees the remainder after '#' is numeric.
+			if n, err := strconv.Atoi(name[len(baseName)+1:]); err == nil {
+				suffix = n
+			}
 		}
 
-		if originalIndex, ok := indexes[name]; !ok {
-			counterValues[originalIndex] = counterValue
-			counterValues = slices.Delete(counterValues, index, 1)
+		if e, ok := seen[baseName]; ok {
+			// Keep whichever entry carries the higher numeric suffix.
+			if suffix > e.numericSuffix {
+				result[e.resultIdx] = counterValue
+				seen[baseName] = dedupeEntry{e.resultIdx, suffix}
+			}
 		} else {
-			indexes[name] = index
+			seen[baseName] = dedupeEntry{len(result), suffix}
+			result = append(result, counterValue)
 		}
 	}
+
+	return result
+}
+
+// iisCounterBaseName strips a trailing IIS perflib numeric suffix ("#<digits>")
+// from name and returns the base name. If no such suffix is present, name is
+// returned unchanged.
+//
+// Only a trailing "#" followed exclusively by ASCII digits is treated as a
+// counter suffix, so "#" characters that are part of a legitimate application
+// name (e.g. "App#Pool") are left intact.
+func iisCounterBaseName(name string) string {
+	idx := strings.LastIndex(name, "#")
+	if idx < 0 {
+		return name
+	}
+
+	suffix := name[idx+1:]
+	if _, err := strconv.Atoi(suffix); err != nil || suffix == "" {
+		return name
+	}
+
+	return name[:idx]
 }

internal/collector/iis/iis_app_pool_was.go

@@ -173,7 +173,7 @@ func (c *Collector) collectAppPoolWAS(ch chan<- prometheus.Metric) error {
 		return fmt.Errorf("failed to collect APP_POOL_WAS metrics: %w", err)
 	}
 
-	deduplicateIISNames(c.perfDataObjectAppPoolWAS)
+	c.perfDataObjectAppPoolWAS = deduplicateIISNames(c.perfDataObjectAppPoolWAS)
 
 	for _, data := range c.perfDataObjectAppPoolWAS {
 		if c.config.AppExclude.MatchString(data.Name) || !c.config.AppInclude.MatchString(data.Name) {

internal/collector/iis/iis_dedup_test.go

@@ -0,0 +1,157 @@
+// 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 iis
+
+import (
+	"testing"
+)
+
+// testEntry is a minimal collectorName implementation used in tests.
+type testEntry struct {
+	Name string
+}
+
+func (e testEntry) GetName() string { return e.Name }
+
+func names(entries []testEntry) []string {
+	out := make([]string, len(entries))
+	for i, e := range entries {
+		out[i] = e.Name
+	}
+
+	return out
+}
+
+func mkEntries(ss ...string) []testEntry {
+	out := make([]testEntry, len(ss))
+	for i, s := range ss {
+		out[i] = testEntry{Name: s}
+	}
+
+	return out
+}
+
+func TestIISCounterBaseName(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		input string
+		want  string
+	}{
+		// Standard IIS counter suffixes – numeric only.
+		{"Site_B#2", "Site_B"},
+		{"Site_B#10", "Site_B"},
+		// Pool name legitimately contains '#' – not a numeric suffix.
+		{"App#Pool", "App#Pool"},
+		{"Pool#Name#2", "Pool#Name"},
+		// No '#' at all.
+		{"DefaultAppPool", "DefaultAppPool"},
+		// Edge-cases.
+		{"#2", ""},
+		{"Site#", "Site#"},   // empty suffix → not treated as counter
+		{"Site##2", "Site#"}, // double '#', last segment is numeric
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			t.Parallel()
+
+			got := iisCounterBaseName(tt.input)
+			if got != tt.want {
+				t.Errorf("iisCounterBaseName(%q) = %q, want %q", tt.input, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestDeduplicateIISNames(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name  string
+		input []testEntry
+		// wantNames is the set of Name fields present in the result (order-independent).
+		wantNames []string
+	}{
+		{
+			name:      "no duplicates",
+			input:     mkEntries("Site_A", "Site_B", "Site_C"),
+			wantNames: []string{"Site_A", "Site_B", "Site_C"},
+		},
+		{
+			name:      "one recycled instance",
+			input:     mkEntries("Site_A", "Site_B", "Site_C", "Site_B#2"),
+			wantNames: []string{"Site_A", "Site_B#2", "Site_C"},
+		},
+		{
+			name:      "multiple recycled suffixes – keep highest",
+			input:     mkEntries("Site_B", "Site_B#3", "Site_B#2"),
+			wantNames: []string{"Site_B#3"},
+		},
+		{
+			name:      "multiple recycled suffixes with high numbers – keep highest",
+			input:     mkEntries("Site_B", "Site_B#9", "Site_B#10"),
+			wantNames: []string{"Site_B#10"},
+		},
+		{
+			name:      "pool name with hash character (not a suffix)",
+			input:     mkEntries("App#Pool", "Other Pool"),
+			wantNames: []string{"App#Pool", "Other Pool"},
+		},
+		{
+			name:      "pool name with hash does not clash with recycled entry",
+			input:     mkEntries("App#Pool", "App#Pool#2"),
+			wantNames: []string{"App#Pool#2"},
+		},
+		{
+			name:      "space in pool name",
+			input:     mkEntries("My App Pool", "Default App Pool"),
+			wantNames: []string{"My App Pool", "Default App Pool"},
+		},
+		{
+			name:      "empty input",
+			input:     mkEntries(),
+			wantNames: []string{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := deduplicateIISNames(tt.input)
+			gotNames := names(got)
+
+			if len(gotNames) != len(tt.wantNames) {
+				t.Fatalf("deduplicateIISNames(%v) returned %v, want %v",
+					names(tt.input), gotNames, tt.wantNames)
+			}
+
+			wantSet := make(map[string]bool, len(tt.wantNames))
+			for _, n := range tt.wantNames {
+				wantSet[n] = true
+			}
+
+			for _, n := range gotNames {
+				if !wantSet[n] {
+					t.Errorf("unexpected name %q in result %v (want %v)", n, gotNames, tt.wantNames)
+				}
+			}
+		})
+	}
+}

internal/collector/iis/iis_http_service_request_queues.go

@@ -93,7 +93,7 @@ func (c *Collector) collectHttpServiceRequestQueues(ch chan<- prometheus.Metric)
 		return fmt.Errorf("failed to collect Http Service Request Queues metrics: %w", err)
 	}
 
-	deduplicateIISNames(c.perfDataObjectHttpServiceRequestQueues)
+	c.perfDataObjectHttpServiceRequestQueues = deduplicateIISNames(c.perfDataObjectHttpServiceRequestQueues)
 
 	for _, data := range c.perfDataObjectHttpServiceRequestQueues {
 		if strings.HasPrefix(data.Name, "---") {

internal/collector/iis/iis_w3svc_w3wp.go

@@ -20,7 +20,6 @@ package iis
 import (
 	"fmt"
 	"regexp"
-	"strings"
 
 	"github.com/prometheus-community/windows_exporter/internal/pdh"
 	"github.com/prometheus-community/windows_exporter/internal/types"
@@ -412,7 +411,7 @@ func (c *Collector) collectW3SVCW3WPv8(ch chan<- prometheus.Metric) error {
 		return fmt.Errorf("failed to collect APP_POOL_WAS metrics: %w", err)
 	}
 
-	deduplicateIISNames(c.perfDataObjectW3SVCW3WPV8)
+	c.perfDataObjectW3SVCW3WPV8 = deduplicateIISNames(c.perfDataObjectW3SVCW3WPV8)
 
 	for _, data := range c.perfDataObjectW3SVCW3WPV8 {
 		if c.config.AppExclude.MatchString(data.Name) || !c.config.AppInclude.MatchString(data.Name) {
@@ -422,17 +421,12 @@ func (c *Collector) collectW3SVCW3WPv8(ch chan<- prometheus.Metric) error {
 		// Extract the apppool name from the format <PID>_<NAME>
 		pid := workerProcessNameExtractor.ReplaceAllString(data.Name, "$1")
 
-		name := workerProcessNameExtractor.ReplaceAllString(data.Name, "$2")
+		name := iisCounterBaseName(workerProcessNameExtractor.ReplaceAllString(data.Name, "$2"))
 		if name == "" || c.config.AppExclude.MatchString(name) ||
 			!c.config.AppInclude.MatchString(name) {
 			continue
 		}
 
-		// Duplicate instances are suffixed # with an index number. These should be ignored
-		if strings.Contains(name, "#") {
-			continue
-		}
-
 		ch <- prometheus.MustNewConstMetric(
 			c.w3SVCW3WPRequestErrorsTotal,
 			prometheus.CounterValue,
@@ -511,23 +505,18 @@ func (c *Collector) collectW3SVCW3WPv7(ch chan<- prometheus.Metric) error {
 		return fmt.Errorf("failed to collect APP_POOL_WAS metrics: %w", err)
 	}
 
-	deduplicateIISNames(c.perfDataObjectW3SVCW3WP)
+	c.perfDataObjectW3SVCW3WP = deduplicateIISNames(c.perfDataObjectW3SVCW3WP)
 
 	for _, data := range c.perfDataObjectW3SVCW3WP {
 		// Extract the apppool name from the format <PID>_<NAME>
 		pid := workerProcessNameExtractor.ReplaceAllString(data.Name, "$1")
 
-		name := workerProcessNameExtractor.ReplaceAllString(data.Name, "$2")
+		name := iisCounterBaseName(workerProcessNameExtractor.ReplaceAllString(data.Name, "$2"))
 		if name == "" || c.config.AppExclude.MatchString(name) ||
 			!c.config.AppInclude.MatchString(name) {
 			continue
 		}
 
-		// Duplicate instances are suffixed # with an index number. These should be ignored
-		if strings.Contains(name, "#") {
-			continue
-		}
-
 		ch <- prometheus.MustNewConstMetric(
 			c.w3SVCW3WPThreads,
 			prometheus.GaugeValue,

internal/collector/iis/iis_web_service.go

@@ -249,7 +249,7 @@ func (c *Collector) collectWebService(ch chan<- prometheus.Metric) error {
 		return fmt.Errorf("failed to collect Web Service metrics: %w", err)
 	}
 
-	deduplicateIISNames(c.perfDataObjectWebService)
+	c.perfDataObjectWebService = deduplicateIISNames(c.perfDataObjectWebService)
 
 	for _, data := range c.perfDataObjectWebService {
 		if c.config.SiteExclude.MatchString(data.Name) || !c.config.SiteInclude.MatchString(data.Name) {