From b5ce53fdac3c6925d916f79c85fabf877f03200a Mon Sep 17 00:00:00 2001
From: Ben Reedy <breed808@breed808.com>
Date: Thu, 3 Dec 2020 20:20:28 +1000
Subject: [PATCH] Merge mssql and dfsr expandEnabledCollectors func

Move to common collector.go and add function test.

Signed-off-by: Ben Reedy <breed808@breed808.com>
---
 collector/collector.go      | 20 ++++++++++++++++++++
 collector/collector_test.go | 34 ++++++++++++++++++++++++++++++++++
 collector/dfsr.go           | 19 +------------------
 collector/mssql.go          | 19 ++-----------------
 4 files changed, 57 insertions(+), 35 deletions(-)
 create mode 100644 collector/collector_test.go

diff --git a/collector/collector.go b/collector/collector.go
index 089b5674..f7973ed3 100644
--- a/collector/collector.go
+++ b/collector/collector.go
@@ -2,6 +2,7 @@ package collector
 
 import (
 	"fmt"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -128,3 +129,22 @@ func find(slice []string, val string) bool {
 	}
 	return false
 }
+
+// Used by more complex collectors where user input specifies enabled child collectors.
+// Splits provided child collectors and deduplicate.
+func expandEnabledChildCollectors(enabled string) []string {
+	separated := strings.Split(enabled, ",")
+	unique := map[string]bool{}
+	for _, s := range separated {
+		if s != "" {
+			unique[s] = true
+		}
+	}
+	result := make([]string, 0, len(unique))
+	for s := range unique {
+		result = append(result, s)
+	}
+	// Ensure result is ordered, to prevent test failure
+	sort.Strings(result)
+	return result
+}
diff --git a/collector/collector_test.go b/collector/collector_test.go
new file mode 100644
index 00000000..bafeb8f9
--- /dev/null
+++ b/collector/collector_test.go
@@ -0,0 +1,34 @@
+package collector
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestExpandChildCollectors(t *testing.T) {
+	cases := []struct {
+		name           string
+		input          string
+		expectedOutput []string
+	}{
+		{
+			name:           "simple",
+			input:          "testing1,testing2,testing3",
+			expectedOutput: []string{"testing1", "testing2", "testing3"},
+		},
+		{
+			name:           "duplicate",
+			input:          "testing1,testing2,testing2,testing3",
+			expectedOutput: []string{"testing1", "testing2", "testing3"},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			output := expandEnabledChildCollectors(c.input)
+			if !reflect.DeepEqual(output, c.expectedOutput) {
+				t.Errorf("Output mismatch, expected %+v, got %+v", c.expectedOutput, output)
+			}
+		})
+	}
+}
diff --git a/collector/dfsr.go b/collector/dfsr.go
index b331b16d..aefeafcf 100644
--- a/collector/dfsr.go
+++ b/collector/dfsr.go
@@ -4,7 +4,6 @@ package collector
 
 import (
 	"errors"
-	"strings"
 	"sync"
 	"time"
 
@@ -88,22 +87,6 @@ type dfsrCollectorMap map[string]dfsrCollectorFunc
 
 type dfsrCollectorFunc func(ctx *ScrapeContext, ch chan<- prometheus.Metric) (*prometheus.Desc, error)
 
-// Split provided perflib sources and deduplicate
-func dfsrExpandEnabledSources(enabled string) []string {
-	separated := strings.Split(enabled, ",")
-	unique := map[string]bool{}
-	for _, s := range separated {
-		if s != "" {
-			unique[s] = true
-		}
-	}
-	result := make([]string, 0, len(unique))
-	for s := range unique {
-		result = append(result, s)
-	}
-	return result
-}
-
 // Map Perflib sources to DFSR collector names
 // E.G. volume -> DFS Replication Service Volumes
 func dfsrGetPerfObjectName(collector string) string {
@@ -124,7 +107,7 @@ func dfsrGetPerfObjectName(collector string) string {
 func NewDFSRCollector() (Collector, error) {
 	const subsystem = "dfsr"
 
-	enabled := dfsrExpandEnabledSources(*dfsrEnabledCollectors)
+	enabled := expandEnabledChildCollectors(*dfsrEnabledCollectors)
 	perfCounters := make([]string, 0, len(enabled))
 	for _, c := range enabled {
 		perfCounters = append(perfCounters, dfsrGetPerfObjectName(c))
diff --git a/collector/mssql.go b/collector/mssql.go
index 477dc26b..4f746146 100644
--- a/collector/mssql.go
+++ b/collector/mssql.go
@@ -90,21 +90,6 @@ func (c *MSSQLCollector) getMSSQLCollectors() mssqlCollectorsMap {
 	return mssqlCollectors
 }
 
-func mssqlExpandEnabledCollectors(enabled string) []string {
-	separated := strings.Split(enabled, ",")
-	unique := map[string]bool{}
-	for _, s := range separated {
-		if s != "" {
-			unique[s] = true
-		}
-	}
-	result := make([]string, 0, len(unique))
-	for s := range unique {
-		result = append(result, s)
-	}
-	return result
-}
-
 // mssqlGetPerfObjectName - Returns the name of the Windows Performance
 // Counter object for the given SQL instance and collector.
 func mssqlGetPerfObjectName(sqlInstance string, collector string) string {
@@ -407,7 +392,7 @@ func NewMSSQLCollector() (Collector, error) {
 
 	const subsystem = "mssql"
 
-	enabled := mssqlExpandEnabledCollectors(*mssqlEnabledCollectors)
+	enabled := expandEnabledChildCollectors(*mssqlEnabledCollectors)
 	mssqlInstances := getMSSQLInstances()
 	perfCounters := make([]string, 0, len(mssqlInstances)*len(enabled))
 	for instance := range mssqlInstances {
@@ -1857,7 +1842,7 @@ func (c *MSSQLCollector) execute(ctx *ScrapeContext, name string, fn mssqlCollec
 func (c *MSSQLCollector) Collect(ctx *ScrapeContext, ch chan<- prometheus.Metric) error {
 	wg := sync.WaitGroup{}
 
-	enabled := mssqlExpandEnabledCollectors(*mssqlEnabledCollectors)
+	enabled := expandEnabledChildCollectors(*mssqlEnabledCollectors)
 	for sqlInstance := range c.mssqlInstances {
 		for _, name := range enabled {
 			function := c.mssqlCollectors[name]