diff --git a/cmd/labs/project/command_test.go b/cmd/labs/project/command_test.go index 453329e1d1..ec6476c4aa 100644 --- a/cmd/labs/project/command_test.go +++ b/cmd/labs/project/command_test.go @@ -2,12 +2,14 @@ package project_test import ( "context" + "os" "path/filepath" "testing" "time" "github.com/databricks/cli/internal/testcli" "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/python" "github.com/databricks/databricks-sdk-go" "github.com/stretchr/testify/assert" @@ -67,3 +69,100 @@ func TestRenderingTable(t *testing.T) { Third Fourth `) } + +func TestLogLevelHandoff(t *testing.T) { + if _, ok := os.LookupEnv("DATABRICKS_LOG_LEVEL"); ok { + t.Fatal("DATABRICKS_LOG_LEVEL must not be set when running this test") + } + + testCases := []struct { + name string + envVar string + args []string + expectedLevel string + }{ + { + // Historical handoff value when the user does not set an explicit log level. + name: "not set by default", + expectedLevel: "disabled", + }, + { + name: "set explicitly with --log-level", + args: []string{"--log-level", "iNFo"}, + expectedLevel: "info", + }, + { + name: "set by --debug flag", + args: []string{"--debug"}, + expectedLevel: "debug", + }, + { + name: "set by env var", + envVar: "tRaCe", + expectedLevel: "trace", + }, + { + name: "set to default", + envVar: "warn", + expectedLevel: "warn", + }, + { + name: "invalid env var ignored", + envVar: "invalid-level", + expectedLevel: "disabled", + }, + { + name: "conflict: --debug trumps --log-level and env var", + envVar: "error", + args: []string{"--debug", "--log-level", "trace"}, + expectedLevel: "debug", + }, + { + name: "conflict: --log-level trumps env var", + envVar: "error", + args: []string{"--log-level", "info"}, + expectedLevel: "info", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx := devEnvContext(t) + if tc.envVar != "" { + ctx = env.Set(ctx, "DATABRICKS_LOG_LEVEL", tc.envVar) + } + args := append([]string{"labs", "blueprint", "echo"}, tc.args...) + r := testcli.NewRunner(t, ctx, args...) + var out echoOut + r.RunAndParseJSON(&out) + usedLevel := out.Flags["log_level"] + assert.Equal(t, tc.expectedLevel, usedLevel) + + // Verify that our expectation matches what the logger has actually been configured to use. + // This should catch drift between the logic in cmd/root/logger.go and cmd/labs/project/proxy.go. + if tc.expectedLevel != "disabled" { + actualLoggerLevel := getLoggerLevel(ctx) + assert.Equal(t, tc.expectedLevel, actualLoggerLevel) + } + }) + } +} + +func getLoggerLevel(ctx context.Context) string { + logger := log.GetLogger(ctx) + var level string + if logger.Enabled(ctx, log.LevelTrace) { + level = "trace" + } else if logger.Enabled(ctx, log.LevelDebug) { + level = "debug" + } else if logger.Enabled(ctx, log.LevelInfo) { + level = "info" + } else if logger.Enabled(ctx, log.LevelWarn) { + level = "warn" + } else if logger.Enabled(ctx, log.LevelError) { + level = "error" + } else { + level = "disabled" + } + return level +} diff --git a/cmd/labs/project/proxy.go b/cmd/labs/project/proxy.go index 82c7fb90ca..54da6de5a0 100644 --- a/cmd/labs/project/proxy.go +++ b/cmd/labs/project/proxy.go @@ -10,12 +10,15 @@ import ( "strings" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/process" "github.com/spf13/cobra" "github.com/spf13/pflag" ) +const envLogLevel = "DATABRICKS_LOG_LEVEL" + type proxy struct { Entrypoint `yaml:",inline"` Name string `yaml:"name"` @@ -107,12 +110,33 @@ func (cp *proxy) commandInput(cmd *cobra.Command) ([]string, error) { } commandInput.Flags[f.Name] = v } + ctx := cmd.Context() + /* + * Although we _could_ get the log-level from the logger in context, that would not tell us + * whether the user explicitly set it or whether it's just the default. So instead here we + * check the same places that the root command does when initializing logging: + * DATABRICKS_LOG_LEVEL (env), --log-level and --debug. + * Note: we rely on tests to catch any drift between here and the root log-level. + */ logLevelFlag := flags.Lookup("log-level") if logLevelFlag != nil { - commandInput.Flags["log_level"] = logLevelFlag.Value.String() + debugFlag := flags.Lookup("debug") + envValue, hasEnvLogLevel := env.Lookup(ctx, envLogLevel) + logLevelInUse := logLevelFlag.Value.String() + // Quirk: env var is ignored if invalid, so only treat as user-supplied if it matches + // the value in use. + userSupplied := logLevelFlag.Changed || (debugFlag != nil && debugFlag.Changed) || + hasEnvLogLevel && strings.EqualFold(envValue, logLevelInUse) + var logLevel string + if userSupplied { + logLevel = logLevelInUse + } else { + // Historical value to indicate "not set by user". + logLevel = "disabled" + } + commandInput.Flags["log_level"] = logLevel } var args []string - ctx := cmd.Context() if cp.IsPythonProject() { args = append(args, cp.virtualEnvPython(ctx)) libDir := cp.EffectiveLibDir()