From 29c896b13b98decc22a23ff70ec770c3b67d7822 Mon Sep 17 00:00:00 2001
From: Masashi SHIBATA <c-bata@users.noreply.github.com>
Date: Mon, 13 Apr 2020 20:37:47 +0900
Subject: [PATCH] Refactor file-metricscollector (#1137)

---
 .../v1alpha3/file-metricscollector/main.go    | 29 +++++++++----------
 .../file-metricscollector.go                  | 21 ++++----------
 2 files changed, 18 insertions(+), 32 deletions(-)

diff --git a/cmd/metricscollector/v1alpha3/file-metricscollector/main.go b/cmd/metricscollector/v1alpha3/file-metricscollector/main.go
index 438e60db41c..7c6b758ae0d 100644
--- a/cmd/metricscollector/v1alpha3/file-metricscollector/main.go
+++ b/cmd/metricscollector/v1alpha3/file-metricscollector/main.go
@@ -53,14 +53,16 @@ import (
 	filemc "github.com/kubeflow/katib/pkg/metricscollector/v1alpha3/file-metricscollector"
 )
 
-var metricsFileName = flag.String("path", "", "Metrics File Path")
-var trialName = flag.String("t", "", "Trial Name")
-var managerService = flag.String("s", "", "Katib Manager service")
-var metricNames = flag.String("m", "", "Metric names")
-var filters = flag.String("f", "", "Metric filters")
-var pollInterval = flag.Duration("p", common.DefaultPollInterval, "Poll interval to check if main process of worker container exit")
-var timeout = flag.Duration("timeout", common.DefaultTimeout, "Timeout to check if main process of worker container exit")
-var waitAll = flag.Bool("w", common.DefaultWaitAll, "Whether wait for all other main process of container exiting")
+var (
+	metricsFileName = flag.String("path", "", "Metrics File Path")
+	trialName       = flag.String("t", "", "Trial Name")
+	managerService  = flag.String("s", "", "Katib Manager service")
+	metricNames     = flag.String("m", "", "Metric names")
+	filters         = flag.String("f", "", "Metric filters")
+	pollInterval    = flag.Duration("p", common.DefaultPollInterval, "Poll interval to check if main process of worker container exit")
+	timeout         = flag.Duration("timeout", common.DefaultTimeout, "Timeout to check if main process of worker container exit")
+	waitAll         = flag.Bool("w", common.DefaultWaitAll, "Whether wait for all other main process of container exiting")
+)
 
 func printMetricsFile(mFile string) {
 	for {
@@ -101,20 +103,16 @@ func main() {
 	}
 	defer conn.Close()
 	c := api.NewManagerClient(conn)
-	mc, err := filemc.NewFileMetricsCollector()
-	if err != nil {
-		klog.Fatalf("Failed to create MetricsCollector: %v", err)
-	}
 	ctx := context.Background()
-	metricList := []string{}
+	var metricList []string
 	if len(*metricNames) != 0 {
 		metricList = strings.Split(*metricNames, ";")
 	}
-	filterList := []string{}
+	var filterList []string
 	if len(*filters) != 0 {
 		filterList = strings.Split(*filters, ";")
 	}
-	olog, err := mc.CollectObservationLog(*metricsFileName, metricList, filterList)
+	olog, err := filemc.CollectObservationLog(*metricsFileName, metricList, filterList)
 	if err != nil {
 		klog.Fatalf("Failed to collect logs: %v", err)
 	}
@@ -127,5 +125,4 @@ func main() {
 		klog.Fatalf("Failed to Report logs: %v", err)
 	}
 	klog.Infof("Metrics reported. :\n%v", olog)
-	return
 }
diff --git a/pkg/metricscollector/v1alpha3/file-metricscollector/file-metricscollector.go b/pkg/metricscollector/v1alpha3/file-metricscollector/file-metricscollector.go
index 897f37524f0..e6d86cff0b6 100644
--- a/pkg/metricscollector/v1alpha3/file-metricscollector/file-metricscollector.go
+++ b/pkg/metricscollector/v1alpha3/file-metricscollector/file-metricscollector.go
@@ -12,14 +12,7 @@ import (
 	"k8s.io/klog"
 )
 
-type FileMetricsCollector struct {
-}
-
-func NewFileMetricsCollector() (*FileMetricsCollector, error) {
-	return &FileMetricsCollector{}, nil
-}
-
-func (d *FileMetricsCollector) CollectObservationLog(fileName string, metrics []string, filters []string) (*v1alpha3.ObservationLog, error) {
+func CollectObservationLog(fileName string, metrics []string, filters []string) (*v1alpha3.ObservationLog, error) {
 	file, err := os.Open(fileName)
 	if err != nil {
 		return nil, err
@@ -27,15 +20,14 @@ func (d *FileMetricsCollector) CollectObservationLog(fileName string, metrics []
 	defer file.Close()
 	content, err := ioutil.ReadAll(file)
 	logs := string(content)
-	olog, err := d.parseLogs(strings.Split(logs, "\n"), metrics, filters)
+	olog, err := parseLogs(strings.Split(logs, "\n"), metrics, filters)
 	return olog, err
 }
 
-func (d *FileMetricsCollector) parseLogs(logs []string, metrics []string, filters []string) (*v1alpha3.ObservationLog, error) {
-	var lasterr error
+func parseLogs(logs []string, metrics []string, filters []string) (*v1alpha3.ObservationLog, error) {
 	olog := &v1alpha3.ObservationLog{}
-	mlogs := []*v1alpha3.MetricLog{}
 	metricRegList := getFilterRegexpList(filters)
+	mlogs := make([]*v1alpha3.MetricLog, 0, len(logs))
 
 	for _, logline := range logs {
 		// skip line which doesn't contain any metrics keywords, avoiding unnecessary pattern match
@@ -87,14 +79,11 @@ func (d *FileMetricsCollector) parseLogs(logs []string, metrics []string, filter
 		}
 	}
 	olog.MetricLogs = mlogs
-	if lasterr != nil {
-		return olog, lasterr
-	}
 	return olog, nil
 }
 
 func getFilterRegexpList(filters []string) []*regexp.Regexp {
-	regexpList := []*regexp.Regexp{}
+	regexpList := make([]*regexp.Regexp, 0, len(filters))
 	if len(filters) == 0 {
 		filters = append(filters, common.DefaultFilter)
 	}