From 120169ca940b758e7a99d4c326370a677ec02c01 Mon Sep 17 00:00:00 2001
From: Raphael 'kena' Poss <knz@thaumogen.net>
Date: Mon, 24 Aug 2020 12:44:15 +0200
Subject: [PATCH] cli/demo: simplify the logging initialization

The demo code was trying a complicated dance with the flag structs to
disable logging. This was not necessary - a simple adjustment of the
advertised store config is sufficient to reach the desired effect.

Additionally, this commit provides a hint to the user if they want
to enable redaction markers but there is no logging dir yet.

Release note: None
---
 pkg/cli/demo_cluster.go | 21 ++++++---------------
 pkg/util/log/flags.go   |  5 ++++-
 2 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/pkg/cli/demo_cluster.go b/pkg/cli/demo_cluster.go
index 6e22c4a90c57..f623f448102b 100644
--- a/pkg/cli/demo_cluster.go
+++ b/pkg/cli/demo_cluster.go
@@ -35,7 +35,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
-	"github.com/cockroachdb/cockroach/pkg/util/log/logflags"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/workload"
@@ -75,20 +74,12 @@ func (c *transientCluster) checkConfigAndSetupLogging(
 		}
 	}
 
-	// Set up logging. For demo/transient server we use non-standard
-	// behavior where we avoid file creation if possible.
-	fl := flagSetForCmd(cmd)
-	df := fl.Lookup(cliflags.LogDir.Name)
-	sf := fl.Lookup(logflags.LogToStderrName)
-	if !df.Changed && !sf.Changed {
-		// User did not request logging flags; shut down all logging.
-		// Otherwise, the demo command would cause a cockroach-data
-		// directory to appear in the current directory just for logs.
-		_ = df.Value.Set("")
-		df.Changed = true
-		_ = sf.Value.Set(log.Severity_NONE.String())
-		sf.Changed = true
-	}
+	// Override the default server store spec.
+	//
+	// This is needed because the logging setup code peeks into this to
+	// decide how to enable logging.
+	serverCfg.Stores.Specs = nil
+
 	c.stopper, err = setupAndInitializeLoggingAndProfiling(ctx, cmd)
 	if err != nil {
 		return err
diff --git a/pkg/util/log/flags.go b/pkg/util/log/flags.go
index 0b5a71c97c2f..9f9bec07981e 100644
--- a/pkg/util/log/flags.go
+++ b/pkg/util/log/flags.go
@@ -14,6 +14,7 @@ import (
 	"context"
 	"flag"
 
+	"github.com/cockroachdb/cockroach/pkg/cli/cliflags"
 	"github.com/cockroachdb/cockroach/pkg/util/log/logflags"
 	"github.com/cockroachdb/errors"
 )
@@ -152,7 +153,9 @@ func SetupRedactionAndStderrRedirects() (cleanupForTestingOnly func(), err error
 	// log entries on stderr, that's a configuration we cannot support
 	// safely. Reject it.
 	if redactableLogsRequested && mainLog.stderrThreshold.get() != Severity_NONE {
-		return nil, errors.New("cannot enable redactable logging without a logging directory")
+		return nil, errors.WithHintf(
+			errors.New("cannot enable redactable logging without a logging directory"),
+			"You can pass --%s to set up a logging directory explicitly.", cliflags.LogDir.Name)
 	}
 
 	// Configuration valid. Assign it.