From 6c83d1bbeb82282a9592061772ae27128b89b4e4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:46:51 +0000 Subject: [PATCH] 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 | 80 +++++++-- internal/collector/iis/iis_app_pool_was.go | 2 +- internal/collector/iis/iis_dedup_test.go | 157 ++++++++++++++++++ .../iis/iis_http_service_request_queues.go | 2 +- internal/collector/iis/iis_w3svc_w3wp.go | 19 +-- internal/collector/iis/iis_web_service.go | 2 +- 6 files changed, 227 insertions(+), 35 deletions(-) create mode 100644 internal/collector/iis/iis_dedup_test.go diff --git a/internal/collector/iis/iis.go b/internal/collector/iis/iis.go index 30237cfb..49b9cb43 100644 --- a/internal/collector/iis/iis.go +++ b/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 "#" 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 ("#") +// 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] } diff --git a/internal/collector/iis/iis_app_pool_was.go b/internal/collector/iis/iis_app_pool_was.go index 96f61619..f3661c16 100644 --- a/internal/collector/iis/iis_app_pool_was.go +++ b/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) { diff --git a/internal/collector/iis/iis_dedup_test.go b/internal/collector/iis/iis_dedup_test.go new file mode 100644 index 00000000..d7a73a28 --- /dev/null +++ b/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) + } + } + }) + } +} diff --git a/internal/collector/iis/iis_http_service_request_queues.go b/internal/collector/iis/iis_http_service_request_queues.go index c088924a..287bacc8 100644 --- a/internal/collector/iis/iis_http_service_request_queues.go +++ b/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, "---") { diff --git a/internal/collector/iis/iis_w3svc_w3wp.go b/internal/collector/iis/iis_w3svc_w3wp.go index 1ba193a0..6fa46996 100644 --- a/internal/collector/iis/iis_w3svc_w3wp.go +++ b/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 := 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 := 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, diff --git a/internal/collector/iis/iis_web_service.go b/internal/collector/iis/iis_web_service.go index a552ec8f..bae2fb22 100644 --- a/internal/collector/iis/iis_web_service.go +++ b/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) {