From e3ba9ffff22e081a7f503abd5f8d5f5bad444dc4 Mon Sep 17 00:00:00 2001
From: Michael Li <alimy@gility.net>
Date: Wed, 6 Feb 2019 01:33:52 +0800
Subject: [PATCH] telemetry: Improve parsing of disabled-metrics flag (#2389)

* optimized parse cli's disabledMetrics flag string to initTelemetry

* add splitTrim to obtain string slice that not contain empty string

* change TestSplitTrim error output

* gofmt for run_test.go

* restore name of disabledMetrics made more sense

* optimized TestSplitTrim case

* just update splitTrim comment to force CI restart
---
 caddy/caddymain/run.go      | 28 +++++++++++++++++++++++-----
 caddy/caddymain/run_test.go | 26 ++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/caddy/caddymain/run.go b/caddy/caddymain/run.go
index 8de692bda..446dc3297 100644
--- a/caddy/caddymain/run.go
+++ b/caddy/caddymain/run.go
@@ -412,14 +412,11 @@ func initTelemetry() error {
 			// mitigate disk space exhaustion at the collection endpoint
 			return fmt.Errorf("too many metrics to disable")
 		}
-		disabledMetricsSlice = strings.Split(disabledMetrics, ",")
-		for i, metric := range disabledMetricsSlice {
+		disabledMetricsSlice = splitTrim(disabledMetrics, ",")
+		for _, metric := range disabledMetricsSlice {
 			if metric == "instance_id" || metric == "timestamp" || metric == "disabled_metrics" {
 				return fmt.Errorf("instance_id, timestamp, and disabled_metrics cannot be disabled")
 			}
-			if metric == "" {
-				disabledMetricsSlice = append(disabledMetricsSlice[:i], disabledMetricsSlice[i+1:]...)
-			}
 		}
 	}
 
@@ -435,6 +432,27 @@ func initTelemetry() error {
 	return nil
 }
 
+// Split string s into all substrings separated by sep and returns a slice of
+// the substrings between those separators.
+//
+// If s does not contain sep and sep is not empty, Split returns a
+// slice of length 1 whose only element is s.
+//
+// If sep is empty, Split splits after each UTF-8 sequence. If both s
+// and sep are empty, Split returns an empty slice.
+//
+// Each item that in result is trim space and not empty string
+func splitTrim(s string, sep string) []string {
+	splitItems := strings.Split(s, sep)
+	trimItems := make([]string, 0, len(splitItems))
+	for _, item := range splitItems {
+		if item = strings.TrimSpace(item); item != "" {
+			trimItems = append(trimItems, item)
+		}
+	}
+	return trimItems
+}
+
 // LoadEnvFromFile loads additional envs if file provided and exists
 // Envs in file should be in KEY=VALUE format
 func LoadEnvFromFile(envFile string) error {
diff --git a/caddy/caddymain/run_test.go b/caddy/caddymain/run_test.go
index 6daa3e26f..f774688af 100644
--- a/caddy/caddymain/run_test.go
+++ b/caddy/caddymain/run_test.go
@@ -60,6 +60,32 @@ func TestSetCPU(t *testing.T) {
 	}
 }
 
+func TestSplitTrim(t *testing.T) {
+	for i, test := range []struct {
+		input  string
+		output []string
+		sep    string
+	}{
+		{"os,arch,cpu,caddy_version", []string{"os", "arch", "cpu", "caddy_version"}, ","},
+		{"os,arch,cpu,caddy_version,", []string{"os", "arch", "cpu", "caddy_version"}, ","},
+		{"os,,, arch, cpu, caddy_version,", []string{"os", "arch", "cpu", "caddy_version"}, ","},
+		{", , os, arch, cpu , caddy_version,, ,", []string{"os", "arch", "cpu", "caddy_version"}, ","},
+		{"os, ,, arch, cpu , caddy_version,, ,", []string{"os", "arch", "cpu", "caddy_version"}, ","},
+	} {
+		got := splitTrim(test.input, test.sep)
+		if len(got) != len(test.output) {
+			t.Errorf("Test %d: spliteTrim() = %v, want %v", i, got, test.output)
+			continue
+		}
+		for j, item := range test.output {
+			if item != got[j] {
+				t.Errorf("Test %d: spliteTrim() = %v, want %v", i, got, test.output)
+				break
+			}
+		}
+	}
+}
+
 func TestParseEnvFile(t *testing.T) {
 	tests := []struct {
 		name    string