Do not use logging (#956)

Fixes #567

## What was changed
- When executing short-lived transactional commands (e.g. `start`,
`list` etc, as opposed to the long-running `server start-dev` process)
report errors/warnings as unstructured plain text instead of structured
logging-formatted messages.
- The logger is now used only by the server and SDK

## Why?
CLIs should report errors/warnings by printing to stderr. They should
not use structured logging messages for this.

## How was this tested
- New in-codebase tests
- Manually.

Author: dandavison

cliext/flags.gen.go

@@ -38,8 +38,8 @@ func (v *CommonOptions) BuildFlags(f *pflag.FlagSet) {
 	f.StringVar(&v.Profile, "profile", "", "Profile to use for config file. EXPERIMENTAL.")
 	f.BoolVar(&v.DisableConfigFile, "disable-config-file", false, "If set, disables loading environment config from config file. EXPERIMENTAL.")
 	f.BoolVar(&v.DisableConfigEnv, "disable-config-env", false, "If set, disables loading environment config from environment variables. EXPERIMENTAL.")
-	v.LogLevel = NewFlagStringEnum([]string{"debug", "info", "warn", "error", "never"}, "info")
-	f.Var(&v.LogLevel, "log-level", "Log level. Default is \"info\" for most commands and \"warn\" for \"server start-dev\". Accepted values: debug, info, warn, error, never.")
+	v.LogLevel = NewFlagStringEnum([]string{"debug", "info", "warn", "error", "never"}, "never")
+	f.Var(&v.LogLevel, "log-level", "Log level. Default is \"never\" for most commands and \"warn\" for \"server start-dev\". Accepted values: debug, info, warn, error, never.")
 	v.LogFormat = NewFlagStringEnum([]string{"text", "json", "pretty"}, "text")
 	f.Var(&v.LogFormat, "log-format", "Log format. Accepted values: text, json.")
 	v.Output = NewFlagStringEnum([]string{"text", "json", "jsonl", "none"}, "text")

cliext/option-sets.yaml

@@ -51,8 +51,8 @@ option-sets:
           - never
         description: |
           Log level.
-          Default is "info" for most commands and "warn" for "server start-dev".
-        default: info
+          Default is "never" for most commands and "warn" for "server start-dev".
+        default: never
       - name: log-format
         type: string-enum
         description: Log format.

internal/temporalcli/commands.config.go

@@ -325,7 +325,6 @@ func writeEnvConfigFile(cctx *CommandContext, conf *envconfig.ClientConfig) erro
 	}
 
 	// Write to file, making dirs as needed
-	cctx.Logger.Info("Writing config file", "file", configFile)
 	if err := os.MkdirAll(filepath.Dir(configFile), 0700); err != nil {
 		return fmt.Errorf("failed making config file parent dirs: %w", err)
 	} else if err := os.WriteFile(configFile, b, 0600); err != nil {

internal/temporalcli/commands.env.go

@@ -13,7 +13,7 @@ import (
 
 func (c *TemporalEnvCommand) envNameAndKey(cctx *CommandContext, args []string, keyFlag string) (string, string, error) {
 	if len(args) > 0 {
-		cctx.Logger.Warn("Arguments to env commands are deprecated; please use --env and --key (or -k) instead")
+		fmt.Fprintln(cctx.Options.Stderr, "Warning: Arguments to env commands are deprecated; please use --env and --key (or -k) instead")
 
 		if c.Parent.Env != "default" || keyFlag != "" {
 			return "", "", fmt.Errorf("cannot specify both an argument and flags; please use flags instead")
@@ -49,10 +49,8 @@ func (c *TemporalEnvDeleteCommand) run(cctx *CommandContext, args []string) erro
 	env, _ := cctx.DeprecatedEnvConfigValues[envName]
 	// User can remove single flag or all in env
 	if key != "" {
-		cctx.Logger.Info("Deleting env property", "env", envName, "property", key)
 		delete(env, key)
 	} else {
-		cctx.Logger.Info("Deleting env", "env", env)
 		delete(cctx.DeprecatedEnvConfigValues, envName)
 	}
 	return writeDeprecatedEnvConfigToFile(cctx)
@@ -129,7 +127,6 @@ func (c *TemporalEnvSetCommand) run(cctx *CommandContext, args []string) error {
 	if cctx.DeprecatedEnvConfigValues[envName] == nil {
 		cctx.DeprecatedEnvConfigValues[envName] = map[string]string{}
 	}
-	cctx.Logger.Info("Setting env property", "env", envName, "property", key, "value", value)
 	cctx.DeprecatedEnvConfigValues[envName][key] = value
 	return writeDeprecatedEnvConfigToFile(cctx)
 }
@@ -138,7 +135,6 @@ func writeDeprecatedEnvConfigToFile(cctx *CommandContext) error {
 	if cctx.Options.DeprecatedEnvConfig.EnvConfigFile == "" {
 		return fmt.Errorf("unable to find place for env file (unknown HOME dir)")
 	}
-	cctx.Logger.Info("Writing env file", "file", cctx.Options.DeprecatedEnvConfig.EnvConfigFile)
 	return writeDeprecatedEnvConfigFile(cctx.Options.DeprecatedEnvConfig.EnvConfigFile, cctx.DeprecatedEnvConfigValues)
 }
 

internal/temporalcli/commands.env_test.go

@@ -2,6 +2,7 @@ package temporalcli_test
 
 import (
 	"os"
+	"strings"
 	"testing"
 
 	"gopkg.in/yaml.v3"
@@ -104,3 +105,32 @@ func TestEnv_InputValidation(t *testing.T) {
 	res = h.Execute("env", "set", "myenv1.foo")
 	h.ErrorContains(res.Err, `no value provided`)
 }
+
+func TestEnv_DeprecationWarningBypassesLogger(t *testing.T) {
+	h := NewCommandHarness(t)
+	defer h.Close()
+
+	tmpFile, err := os.CreateTemp("", "")
+	h.NoError(err)
+	h.Options.DeprecatedEnvConfig.EnvConfigFile = tmpFile.Name()
+	defer os.Remove(h.Options.DeprecatedEnvConfig.EnvConfigFile)
+	res := h.Execute("env", "set", "--env", "myenv1", "-k", "foo", "-v", "bar")
+	h.NoError(res.Err)
+
+	// Using deprecated argument syntax should produce a warning on stderr.
+	// The warning must bypass the logger so it appears regardless of log level.
+	for _, logLevel := range []string{"never", "error", "info"} {
+		t.Run("log-level="+logLevel, func(t *testing.T) {
+			res = h.Execute("env", "get", "--log-level", logLevel, "myenv1")
+			h.NoError(res.Err)
+
+			stderr := res.Stderr.String()
+			h.Contains(stderr, "Warning:")
+			h.Contains(stderr, "deprecated")
+
+			// Must be a plain-text warning, not a structured log message
+			h.False(strings.Contains(stderr, "time="), "warning should not be a structured log message")
+			h.False(strings.Contains(stderr, "level="), "warning should not be a structured log message")
+		})
+	}
+}

internal/temporalcli/commands.gen.go

@@ -2117,7 +2117,7 @@ func NewTemporalServerStartDevCommand(cctx *CommandContext, parent *TemporalServ
 	s.Command.Flags().StringVar(&s.UiCodecEndpoint, "ui-codec-endpoint", "", "UI remote codec HTTP endpoint.")
 	s.Command.Flags().StringArrayVar(&s.SqlitePragma, "sqlite-pragma", nil, "SQLite pragma statements in \"PRAGMA=VALUE\" format.")
 	s.Command.Flags().StringArrayVar(&s.DynamicConfigValue, "dynamic-config-value", nil, "Dynamic configuration value using `KEY=VALUE` pairs. Keys must be identifiers, and values must be JSON values. For example: `YourKey=\"YourString\"` Can be passed multiple times.")
-	s.Command.Flags().BoolVar(&s.LogConfig, "log-config", false, "Log the server config to stderr.")
+	s.Command.Flags().BoolVar(&s.LogConfig, "log-config", false, "Print the server config to stderr.")
 	s.Command.Flags().StringArrayVar(&s.SearchAttribute, "search-attribute", nil, "Search attributes to register using `KEY=VALUE` pairs. Keys must be identifiers, and values must be the search attribute type, which is one of the following: Text, Keyword, Int, Double, Bool, Datetime, KeywordList.")
 	s.Command.Run = func(c *cobra.Command, args []string) {
 		if err := s.run(cctx, args); err != nil {

internal/temporalcli/commands.go

@@ -48,7 +48,9 @@ type CommandContext struct {
 	FlagsWithEnvVars          []*pflag.Flag
 
 	// These values may not be available until after pre-run of main command
-	Printer               *printer.Printer
+	Printer *printer.Printer
+	// The logger is for the SDK and server. The CLI itself should print warnings or errors to
+	// stderr but should not use logging.
 	Logger                *slog.Logger
 	JSONOutput            bool
 	JSONShorthandPayloads bool
@@ -143,11 +145,7 @@ func (c *CommandContext) preprocessOptions() error {
 			if c.Err() != nil {
 				err = fmt.Errorf("program interrupted")
 			}
-			if c.Logger != nil {
-				c.Logger.Error(err.Error())
-			} else {
-				fmt.Fprintln(os.Stderr, err)
-			}
+			fmt.Fprintf(c.Options.Stderr, "Error: %v\n", err)
 			os.Exit(1)
 		}
 	}
@@ -260,13 +258,11 @@ func UnmarshalProtoJSONWithOptions(b []byte, m proto.Message, jsonShorthandPaylo
 	return opts.Unmarshal(b, m)
 }
 
-// Set flag values from environment file & variables. Returns a callback to log anything interesting
-// since logging will not yet be initialized when this runs.
-func (c *CommandContext) populateFlagsFromEnv(flags *pflag.FlagSet) (func(*slog.Logger), error) {
+// Set flag values from environment file & variables.
+func (c *CommandContext) populateFlagsFromEnv(flags *pflag.FlagSet) error {
 	if flags == nil {
-		return func(logger *slog.Logger) {}, nil
+		return nil
 	}
-	var logCalls []func(*slog.Logger)
 	var flagErr error
 	flags.VisitAll(func(flag *pflag.Flag) {
 		// If the flag was already changed by the user, we don't overwrite
@@ -289,20 +285,14 @@ func (c *CommandContext) populateFlagsFromEnv(flags *pflag.FlagSet) (func(*slog.
 					return
 				}
 				if flag.Changed {
-					logCalls = append(logCalls, func(l *slog.Logger) {
-						l.Info("Env var overrode --env setting", "env_var", anns[0], "flag", flag.Name)
-					})
+					fmt.Fprintf(c.Options.Stderr,
+						"Warning: env var %s overrode --env setting for flag --%s\n", anns[0], flag.Name)
 				}
 				flag.Changed = true
 			}
 		}
 	})
-	logFn := func(logger *slog.Logger) {
-		for _, call := range logCalls {
-			call(logger)
-		}
-	}
-	return logFn, flagErr
+	return flagErr
 }
 
 // Returns error if JSON output enabled
@@ -459,8 +449,7 @@ func (c *TemporalCommand) initCommand(cctx *CommandContext) {
 		cctx.CurrentCommand = cmd
 		// Populate environ. We will make the error return here which will cause
 		// usage to be printed.
-		logCalls, err := cctx.populateFlagsFromEnv(cmd.Flags())
-		if err != nil {
+		if err := cctx.populateFlagsFromEnv(cmd.Flags()); err != nil {
 			return err
 		}
 
@@ -472,8 +461,6 @@ func (c *TemporalCommand) initCommand(cctx *CommandContext) {
 
 		res := c.preRun(cctx)
 
-		logCalls(cctx.Logger)
-
 		// Always disable color if JSON output is on (must be run after preRun so JSONOutput is set)
 		if cctx.JSONOutput {
 			color.NoColor = true

internal/temporalcli/commands.operator_namespace.go

@@ -19,7 +19,7 @@ func (c *TemporalOperatorCommand) getNSFromFlagOrArg0(cctx *CommandContext, args
 	}
 
 	if len(args) > 0 {
-		cctx.Logger.Warn("Passing the namespace as an argument is now deprecated; please switch to using -n instead")
+		fmt.Fprintln(cctx.Options.Stderr, "Warning: Passing the namespace as an argument is now deprecated; please switch to using -n instead")
 		return args[0], nil
 	}
 	return c.Namespace, nil

internal/temporalcli/commands.server.go

@@ -134,7 +134,6 @@ func (t *TemporalServerStartDevCommand) run(cctx *CommandContext, args []string)
 	// Log config if requested
 	if t.LogConfig {
 		opts.LogConfig = func(b []byte) {
-			cctx.Logger.Info("Logging config")
 			_, _ = cctx.Options.Stderr.Write(b)
 		}
 	}

internal/temporalcli/commands.workflow_exec.go

@@ -94,7 +94,7 @@ func (c *TemporalWorkflowExecuteCommand) run(cctx *CommandContext, args []string
 	// Log print failure and return workflow failure if workflow failed
 	if closeEvent.EventType != enums.EVENT_TYPE_WORKFLOW_EXECUTION_COMPLETED {
 		if err != nil {
-			cctx.Logger.Error("Workflow failed, and printing the output also failed", "error", err)
+			fmt.Fprintf(cctx.Options.Stderr, "Warning: printing workflow output failed: %v\n", err)
 		}
 		err = fmt.Errorf("workflow failed")
 	}