Skip to content

Commit

Permalink
cli/demo: simplify the logging initialization
Browse files Browse the repository at this point in the history
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
  • Loading branch information
knz committed Aug 24, 2020
1 parent c457f1f commit 120169c
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 16 deletions.
21 changes: 6 additions & 15 deletions pkg/cli/demo_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion pkg/util/log/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 120169c

Please sign in to comment.