From dcb6846ad494e1fda35482f948294577df47964a Mon Sep 17 00:00:00 2001
From: Pavlo Strokov <pstrokov@gitlab.com>
Date: Mon, 17 Feb 2020 19:58:42 +0000
Subject: Praefect sub-commands: avoid garbage in logs

The output of the sub-commands needs to be related only to
execution of the command.
Tracing, Sentry and Prometheus systems do not needed for
sql-ping, sql-migrate and dial-nodes sub-commands.
Validation of configuration required only by dial-nodes
sub-command.

Closes: https://gitlab.com/gitlab-org/gitaly/issues/2381
---
 .../unreleased/ps-garbage-out-of-sub-cmd.yml       |  5 +++++
 cmd/praefect/main.go                               | 24 ++++++++++++++--------
 cmd/praefect/main_test.go                          |  2 +-
 cmd/praefect/subcommand.go                         |  2 +-
 4 files changed, 22 insertions(+), 11 deletions(-)
 create mode 100644 changelogs/unreleased/ps-garbage-out-of-sub-cmd.yml

diff --git a/changelogs/unreleased/ps-garbage-out-of-sub-cmd.yml b/changelogs/unreleased/ps-garbage-out-of-sub-cmd.yml
new file mode 100644
index 000000000..6259dc3d2
--- /dev/null
+++ b/changelogs/unreleased/ps-garbage-out-of-sub-cmd.yml
@@ -0,0 +1,5 @@
+---
+title: 'Praefect sub-commands: avoid garbage in logs'
+merge_request: 1819
+author:
+type: other
diff --git a/cmd/praefect/main.go b/cmd/praefect/main.go
index e74e09c05..c4a263276 100644
--- a/cmd/praefect/main.go
+++ b/cmd/praefect/main.go
@@ -70,16 +70,19 @@ func main() {
 		os.Exit(0)
 	}
 
-	conf, err := configure()
+	conf, err := initConfig()
 	if err != nil {
-		logger.Fatal(err)
+		printfErr("%s: configuration error: %v\n", progname, err)
+		os.Exit(1)
 	}
 
 	if args := flag.Args(); len(args) > 0 {
 		os.Exit(subCommand(conf, args[0], args[1:]))
 	}
 
-	logger.WithField("version", praefect.GetVersionString()).Info("Starting Praefect")
+	configure(conf)
+
+	logger.WithField("version", praefect.GetVersionString()).Info("Starting " + progname)
 
 	starterConfigs, err := getStarterConfigs(conf.SocketPath, conf.ListenAddr)
 	if err != nil {
@@ -91,7 +94,7 @@ func main() {
 	}
 }
 
-func configure() (config.Config, error) {
+func initConfig() (config.Config, error) {
 	var conf config.Config
 
 	if *flagConfig == "" {
@@ -104,11 +107,16 @@ func configure() (config.Config, error) {
 	}
 
 	if err := conf.Validate(); err != nil {
-		return conf, err
+		return config.Config{}, err
 	}
 
-	logger = conf.ConfigureLogger()
-	tracing.Initialize(tracing.WithServiceName("praefect"))
+	return conf, nil
+}
+
+func configure(conf config.Config) {
+	conf.ConfigureLogger()
+
+	tracing.Initialize(tracing.WithServiceName(progname))
 
 	if conf.PrometheusListenAddr != "" {
 		logger.WithField("address", conf.PrometheusListenAddr).Info("Starting prometheus listener")
@@ -124,8 +132,6 @@ func configure() (config.Config, error) {
 	}
 
 	sentry.ConfigureSentry(version.GetVersion(), conf.Sentry)
-
-	return conf, nil
 }
 
 func run(cfgs []starter.Config, conf config.Config) error {
diff --git a/cmd/praefect/main_test.go b/cmd/praefect/main_test.go
index 62ffe7786..f73cdd816 100644
--- a/cmd/praefect/main_test.go
+++ b/cmd/praefect/main_test.go
@@ -10,7 +10,7 @@ import (
 )
 
 func TestNoConfigFlag(t *testing.T) {
-	_, err := configure()
+	_, err := initConfig()
 
 	assert.Equal(t, err, errNoConfigFile)
 }
diff --git a/cmd/praefect/subcommand.go b/cmd/praefect/subcommand.go
index 74e9d7989..064dd5167 100644
--- a/cmd/praefect/subcommand.go
+++ b/cmd/praefect/subcommand.go
@@ -27,7 +27,7 @@ func subCommand(conf config.Config, arg0 string, argRest []string) int {
 	case "dial-nodes":
 		return dialNodes(conf)
 	default:
-		fmt.Printf("%s: unknown subcommand: %q\n", progname, arg0)
+		printfErr("%s: unknown subcommand: %q\n", progname, arg0)
 		return 1
 	}
 }
-- 
cgit v1.2.3