diff --git a/cmd/agent/agent.go b/cmd/agent/agent.go index 49faed5bb..45bae64f9 100644 --- a/cmd/agent/agent.go +++ b/cmd/agent/agent.go @@ -8,8 +8,7 @@ import ( "github.com/devsy-org/devsy/cmd/flags" "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/envfile" - "github.com/devsy-org/log" - "github.com/sirupsen/logrus" + "github.com/devsy-org/devsy/pkg/log" "github.com/spf13/cobra" ) @@ -52,18 +51,13 @@ func AgentPersistentPreRunE( } parent.Annotations[AgentExecutedAnnotation] = "true" - if globalFlags.LogOutput == "json" { - log.Default.SetFormat(log.JSONFormat) - } else { - log.Default.MakeRaw() - } - - switch { - case globalFlags.Quiet: - log.Default.SetLevel(logrus.FatalLevel) - case globalFlags.Debug: - log.Default.SetLevel(logrus.DebugLevel) - } + // Initialise the zap logger for the agent subprocess. + // stdout is the binary protocol channel, so all log output goes to stderr. + log.Init(log.Config{ + Quiet: globalFlags.Quiet, + Debug: globalFlags.Debug, + Format: "json", // Agent must use JSON: single-line output is captured by TunnelLogStreamer.lastLines + }) if globalFlags.DevsyHome != "" { _ = os.Setenv(config.EnvHome, globalFlags.DevsyHome) diff --git a/cmd/root.go b/cmd/root.go index 0bc8b4903..d0a46a260 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -18,8 +18,6 @@ import ( "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/telemetry" - log2 "github.com/devsy-org/log" - "github.com/sirupsen/logrus" "github.com/spf13/cobra" flag "github.com/spf13/pflag" "golang.org/x/crypto/ssh" @@ -42,20 +40,6 @@ func NewRootCmd() *cobra.Command { Format: globalFlags.LogOutput, }) - // Configure the old logger too, until the migration is complete. - if globalFlags.LogOutput == "json" { - log2.Default.SetFormat(log2.JSONFormat) - } else if globalFlags.LogOutput == "raw" { - log2.Default.SetFormat(log2.RawFormat) - } - - switch { - case globalFlags.Quiet: - log2.Default.SetLevel(logrus.FatalLevel) - case globalFlags.Debug || os.Getenv(config.EnvDebug) == config.BoolTrue: - log2.Default.SetLevel(logrus.DebugLevel) - } - if globalFlags.DevsyHome != "" { _ = os.Setenv(config.EnvHome, globalFlags.DevsyHome) } @@ -99,13 +83,13 @@ func Execute() { } if globalFlags.Debug { - log2.Default.Fatalf("%+v", err) + log.Fatalf("%+v", err) } else { if rootCmd.Annotations == nil || rootCmd.Annotations[agent.AgentExecutedAnnotation] != config.BoolTrue { - log2.Default.Error("Try using -v or --debug flag to see more verbose output") + log.Error("Try using -v or --debug flag to see more verbose output") } - log2.Default.Fatal(err) + log.Fatal(err) } } } @@ -174,7 +158,7 @@ func inheritFlagsFromEnvironment(flags *flag.FlagSet) { // set the variable holding the flag's value to the default supplied by the environment err := flag.Value.Set(value) if err != nil { - log2.Default.Fatalf( + log.Fatalf( "failed to set flag %s from the environment variable %s with value %s: %+v", flag.Name, environmentVariable, diff --git a/e2e/tests/up/helper.go b/e2e/tests/up/helper.go index c942ff98d..314e64b63 100644 --- a/e2e/tests/up/helper.go +++ b/e2e/tests/up/helper.go @@ -19,6 +19,7 @@ import ( // Only the fields we need for test assertions are included. type logLine struct { Message string `json:"message,omitempty"` + Msg string `json:"msg,omitempty"` } type baseTestContext struct { @@ -70,12 +71,32 @@ func findMessage(reader io.Reader, message string) error { for scan.Scan() { if line := scan.Bytes(); len(line) > 0 { lineObject := &logLine{} - if err := json.Unmarshal( - line, - lineObject, - ); err == nil && - strings.Contains(lineObject.Message, message) { - return nil + if err := json.Unmarshal(line, lineObject); err == nil { + msg := lineObject.Message + if msg == "" { + msg = lineObject.Msg + } + if strings.Contains(msg, message) { + return nil + } + // Agent JSON may be embedded in the parent's error chain. + // Parse any nested JSON lines within the msg to resolve + // double-escaped quotes. + for part := range strings.SplitSeq(msg, "\n") { + part = strings.TrimSpace(part) + if len(part) > 0 && part[0] == '{' { + inner := &logLine{} + if json.Unmarshal([]byte(part), inner) == nil { + innerMsg := inner.Message + if innerMsg == "" { + innerMsg = inner.Msg + } + if strings.Contains(innerMsg, message) { + return nil + } + } + } + } } } } diff --git a/go.mod b/go.mod index eb8e6b9ca..c7165f465 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,6 @@ require ( github.com/devsy-org/agentapi v1.0.1 github.com/devsy-org/api v1.1.0 github.com/devsy-org/apiserver v1.5.0 - github.com/devsy-org/log v1.1.0 github.com/devsy-org/ssh v1.1.0 github.com/distribution/reference v0.6.0 github.com/docker/cli v29.4.0+incompatible @@ -45,7 +44,6 @@ require ( github.com/onsi/ginkgo/v2 v2.28.1 github.com/onsi/gomega v1.39.1 github.com/pkg/sftp v1.13.10 - github.com/sirupsen/logrus v1.9.4 github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.10 @@ -92,7 +90,6 @@ require ( github.com/BurntSushi/toml v1.5.0 // indirect github.com/Masterminds/semver/v3 v3.4.0 // indirect github.com/NYTimes/gziphandler v1.1.1 // indirect - github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d // indirect github.com/agext/levenshtein v1.2.3 // indirect github.com/akutz/memconn v0.1.0 // indirect github.com/alexbrainman/sspi v0.0.0-20231016080023-1a75b4708caa // indirect @@ -196,7 +193,6 @@ require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/jsimonetti/rtnetlink v1.4.0 // indirect github.com/json-iterator/go v1.1.12 // indirect - github.com/k0kubun/go-ansi v0.0.0-20180517002512-3bf9e2903213 // indirect github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect github.com/klauspost/compress v1.18.5 // indirect github.com/kr/fs v0.1.0 // indirect @@ -245,6 +241,7 @@ require ( github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 // indirect github.com/secure-systems-lab/go-securesystemslib v0.10.0 // indirect github.com/shibumi/go-pathspec v1.3.0 // indirect + github.com/sirupsen/logrus v1.9.4 // indirect github.com/stoewer/go-strcase v1.3.1 // indirect github.com/tailscale/certstore v0.1.1-0.20231202035212-d3fa0460f47e // indirect github.com/tailscale/go-winio v0.0.0-20231025203758-c4f33415bf55 // indirect @@ -296,7 +293,6 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20260128011058-8636f8732409 // indirect gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect - gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect gvisor.dev/gvisor v0.0.0-20250205023644-9414b50a5633 // indirect k8s.io/apiextensions-apiserver v0.35.0 // indirect diff --git a/go.sum b/go.sum index 24a772a20..e424dd34f 100644 --- a/go.sum +++ b/go.sum @@ -58,8 +58,6 @@ github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2 h1:+vx7roKuyA63n github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2/go.mod h1:HBCaDeC1lPdgDeDbhX8XFpy1jqjK0IBG8W5K+xYqA0w= github.com/ProtonMail/go-crypto v1.3.0 h1:ILq8+Sf5If5DCpHQp4PbZdS1J7HDFRXz/+xKBiRGFrw= github.com/ProtonMail/go-crypto v1.3.0/go.mod h1:9whxjD8Rbs29b4XWbB8irEcE8KHMqaR2e7GWU1R+/PE= -github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d h1:licZJFw2RwpHMqeKTCYkitsPqHNxTmd4SNR5r94FGM8= -github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d/go.mod h1:asat636LX7Bqt5lYEZ27JNDcqxfjdBQuJ/MM4CN/Lzo= github.com/agext/levenshtein v1.2.3 h1:YB2fHEn0UJagG8T1rrWknE3ZQzWM06O8AMAatNn7lmo= github.com/agext/levenshtein v1.2.3/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= github.com/akutz/memconn v0.1.0 h1:NawI0TORU4hcOMsMr11g7vwlCdkYeLKXBcxWu2W/P8A= @@ -235,8 +233,6 @@ github.com/devsy-org/api v1.1.0 h1:l7T9k7RVwatwN4lxeDTF3iN6EYmfGZgR3ZTJMDGha1M= github.com/devsy-org/api v1.1.0/go.mod h1:mAZklKdnywJYiXDReBLte/H+3m69z6G7RHB3n1lI53Q= github.com/devsy-org/apiserver v1.5.0 h1:tAr2tH9QwtNEu8BTbBgHkJD6ae43S5LDdHGphLsELB8= github.com/devsy-org/apiserver v1.5.0/go.mod h1:2jKuqy8XdH/g4bIjmkj3efQgshN/NjMzm17SkvvSR88= -github.com/devsy-org/log v1.1.0 h1:LktC2pLnGwaduspO+euTzzj3Fbt4G1S1bDcb4f3xTSE= -github.com/devsy-org/log v1.1.0/go.mod h1:B/D8SOjJ34lQS5O1BupXwXvWsJRK+ul3JvYYahTkCE8= github.com/devsy-org/ssh v1.1.0 h1:LKwUPuyijX+x8L4zzEsObBptRjr8OTKugVCl3UPMTOs= github.com/devsy-org/ssh v1.1.0/go.mod h1:+/FJAVr49LxNS0E4IPOXQZOnwrvNqc6cNLT7unu3MD4= github.com/devsy-org/tailscale v1.92.2 h1:8Ew/cGVUaQa9N6Acab9SgXDWJp/Kzn3T0UejLyWPTfo= @@ -444,8 +440,6 @@ github.com/jsimonetti/rtnetlink v1.4.0 h1:Z1BF0fRgcETPEa0Kt0MRk3yV5+kF1FWTni6KUF github.com/jsimonetti/rtnetlink v1.4.0/go.mod h1:5W1jDvWdnthFJ7fxYX1GMK07BUpI4oskfOqvPteYS6E= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= -github.com/k0kubun/go-ansi v0.0.0-20180517002512-3bf9e2903213 h1:qGQQKEcAR99REcMpsXCp3lJ03zYT1PkRd3kQGPn9GVg= -github.com/k0kubun/go-ansi v0.0.0-20180517002512-3bf9e2903213/go.mod h1:vNUNkEQ1e29fT/6vq2aBdFsgNPmy8qMdSay1npru+Sw= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRtuthU= diff --git a/pkg/log/jsonstream.go b/pkg/log/jsonstream.go index 90e0445f1..47a707032 100644 --- a/pkg/log/jsonstream.go +++ b/pkg/log/jsonstream.go @@ -3,10 +3,9 @@ package log import ( "encoding/json" "io" + "strings" "github.com/devsy-org/devsy/pkg/scanner" - "github.com/devsy-org/log" - "github.com/sirupsen/logrus" ) func PipeJSONStream() (io.WriteCloser, chan struct{}) { @@ -20,34 +19,49 @@ func PipeJSONStream() (io.WriteCloser, chan struct{}) { return writer, done } -var levelFuncs = map[logrus.Level]func(...any){ - logrus.TraceLevel: Debug, - logrus.DebugLevel: Debug, - logrus.InfoLevel: Info, - logrus.WarnLevel: Warn, - logrus.ErrorLevel: Error, - logrus.PanicLevel: Error, - logrus.FatalLevel: Error, +// jsonLine is a self-contained representation of a JSON log line, +// replacing the old github.com/devsy-org/log.Line type. +type jsonLine struct { + Message string `json:"message,omitempty"` + Msg string `json:"msg,omitempty"` + Level string `json:"level,omitempty"` +} + +func (l *jsonLine) text() string { + if l.Message != "" { + return l.Message + } + return l.Msg +} + +var levelFuncs = map[string]func(...any){ + "trace": Debug, + "debug": Debug, + "info": Info, + "warning": Warn, + "warn": Warn, + "error": Error, + "panic": Error, + "fatal": Error, } func ReadJSONStream(reader io.Reader) { scan := scanner.NewScanner(reader) for scan.Scan() { - lineObject, err := Unmarshal(scan.Bytes()) - if err == nil && lineObject.Message != "" { - if fn, ok := levelFuncs[lineObject.Level]; ok { - fn(lineObject.Message) - } + line := scan.Bytes() + if len(line) == 0 { + continue + } + obj := &jsonLine{} + if err := json.Unmarshal(line, obj); err != nil { + continue + } + msg := obj.text() + if msg == "" { + continue + } + if fn, ok := levelFuncs[strings.ToLower(obj.Level)]; ok { + fn(msg) } } } - -func Unmarshal(line []byte) (*log.Line, error) { - lineObject := &log.Line{} - err := json.Unmarshal(line, lineObject) - if err != nil { - return nil, err - } - - return lineObject, nil -} diff --git a/pkg/log/log.go b/pkg/log/log.go deleted file mode 100644 index 6a4a6779a..000000000 --- a/pkg/log/log.go +++ /dev/null @@ -1,219 +0,0 @@ -package log - -import ( - "errors" - "io" - "sync" - - logLib "github.com/devsy-org/log" - oldsurvey "github.com/devsy-org/log/survey" - "github.com/go-logr/logr" - "github.com/sirupsen/logrus" -) - -// CombinedLogger implements the Logger interface and delegates logging to multiple loggers. -type CombinedLogger struct { - loggers []logLib.Logger - m sync.Mutex - level logrus.Level -} - -// NewCombinedLogger creates a new CombinedLogger. -func NewCombinedLogger(level logrus.Level, loggers ...logLib.Logger) *CombinedLogger { - return &CombinedLogger{ - loggers: loggers, - level: level, - } -} - -// log is a helper to execute a function for all loggers at the appropriate log level. -func (c *CombinedLogger) log(level logrus.Level, logFunc func(logLib.Logger)) { - c.m.Lock() - defer c.m.Unlock() - - if level > c.level { - return - } - - for _, logger := range c.loggers { - logFunc(logger) - } -} - -func (c *CombinedLogger) Debug(args ...any) { - c.log(logrus.DebugLevel, func(logger logLib.Logger) { - logger.Debug(args...) - }) -} - -func (c *CombinedLogger) Debugf(format string, args ...any) { - c.log(logrus.DebugLevel, func(logger logLib.Logger) { - logger.Debugf(format, args...) - }) -} - -func (c *CombinedLogger) Info(args ...any) { - c.log(logrus.InfoLevel, func(logger logLib.Logger) { - logger.Info(args...) - }) -} - -func (c *CombinedLogger) Infof(format string, args ...any) { - c.log(logrus.InfoLevel, func(logger logLib.Logger) { - logger.Infof(format, args...) - }) -} - -func (c *CombinedLogger) Warn(args ...any) { - c.log(logrus.WarnLevel, func(logger logLib.Logger) { - logger.Warn(args...) - }) -} - -func (c *CombinedLogger) Warnf(format string, args ...any) { - c.log(logrus.WarnLevel, func(logger logLib.Logger) { - logger.Warnf(format, args...) - }) -} - -func (c *CombinedLogger) Error(args ...any) { - c.log(logrus.ErrorLevel, func(logger logLib.Logger) { - logger.Error(args...) - }) -} - -func (c *CombinedLogger) Errorf(format string, args ...any) { - c.log(logrus.ErrorLevel, func(logger logLib.Logger) { - logger.Errorf(format, args...) - }) -} - -func (c *CombinedLogger) Fatal(args ...any) { - c.log(logrus.FatalLevel, func(logger logLib.Logger) { - logger.Fatal(args...) - }) -} - -func (c *CombinedLogger) Fatalf(format string, args ...any) { - c.log(logrus.FatalLevel, func(logger logLib.Logger) { - logger.Fatalf(format, args...) - }) -} - -func (c *CombinedLogger) Done(args ...any) { - c.log(logrus.InfoLevel, func(logger logLib.Logger) { - logger.Done(args...) - }) -} - -func (c *CombinedLogger) Donef(format string, args ...any) { - c.log(logrus.InfoLevel, func(logger logLib.Logger) { - logger.Donef(format, args...) - }) -} - -func (c *CombinedLogger) Print(level logrus.Level, args ...any) { - c.log(level, func(logger logLib.Logger) { - logger.Print(level, args...) - }) -} - -func (c *CombinedLogger) Printf(level logrus.Level, format string, args ...any) { - c.log(level, func(logger logLib.Logger) { - logger.Printf(level, format, args...) - }) -} - -func (c *CombinedLogger) SetLevel(level logrus.Level) { - c.m.Lock() - defer c.m.Unlock() - - c.level = level - for _, logger := range c.loggers { - logger.SetLevel(level) - } -} - -func (c *CombinedLogger) GetLevel() logrus.Level { - c.m.Lock() - defer c.m.Unlock() - - return c.level -} - -func (c *CombinedLogger) WriteString(level logrus.Level, message string) { - c.log(level, func(logger logLib.Logger) { - logger.WriteString(level, message) - }) -} - -func (c *CombinedLogger) WriteLevel(level logrus.Level, message []byte) (int, error) { - c.log(level, func(logger logLib.Logger) { - _, _ = logger.WriteLevel(level, message) - }) - - return len(message), nil -} - -func (c *CombinedLogger) Question(params *oldsurvey.QuestionOptions) (string, error) { - return "", errors.New("questions in combined logger not supported") -} - -func (c *CombinedLogger) ErrorStreamOnly() logLib.Logger { - return nil -} - -func (c *CombinedLogger) WithFields(fields logrus.Fields) logLib.Logger { - c.m.Lock() - defer c.m.Unlock() - - newLoggers := make([]logLib.Logger, len(c.loggers)) - for i, logger := range c.loggers { - newLoggers[i] = logger.WithFields(fields) - } - - return &CombinedLogger{ - loggers: newLoggers, - level: c.level, - } -} - -func (c *CombinedLogger) LogrLogSink() logr.LogSink { - return nil -} - -func (c *CombinedLogger) Writer(level logrus.Level, raw bool) io.WriteCloser { - c.m.Lock() - defer c.m.Unlock() - - var writers []io.WriteCloser - for _, logger := range c.loggers { - writer := logger.Writer(level, raw) - if writer != nil { - writers = append(writers, writer) - } - } - return &multiWriter{writers: writers} -} - -type multiWriter struct { - writers []io.WriteCloser -} - -func (m *multiWriter) Write(p []byte) (int, error) { - for _, w := range m.writers { - if _, err := w.Write(p); err != nil { - return 0, err - } - } - return len(p), nil -} - -func (m *multiWriter) Close() error { - for _, w := range m.writers { - if err := w.Close(); err != nil { - return err - } - } - return nil -} diff --git a/pkg/log/logger.go b/pkg/log/logger.go index 6c11c6c85..55eaf108f 100644 --- a/pkg/log/logger.go +++ b/pkg/log/logger.go @@ -30,7 +30,12 @@ func Init(cfg Config) { level := resolveLevel(cfg) encoder := resolveEncoder(cfg.Format) core := zapcore.NewCore(encoder, zapcore.Lock(os.Stderr), level) - logger := zap.New(core, zap.AddCaller(), zap.AddStacktrace(zapcore.FatalLevel)) + + opts := []zap.Option{ + zap.AddCaller(), + zap.AddStacktrace(zapcore.FatalLevel), + } + logger := zap.New(core, opts...) sugar = logger.Sugar() } diff --git a/pkg/log/question.go b/pkg/log/question.go index 923628f40..d4cefa4d9 100644 --- a/pkg/log/question.go +++ b/pkg/log/question.go @@ -2,23 +2,9 @@ package log import ( "github.com/devsy-org/devsy/pkg/survey" - oldlog "github.com/devsy-org/log" - oldsurvey "github.com/devsy-org/log/survey" ) -// QuestionDefault asks a question using the default logger, bridging the local -// survey type to the old log module's survey type during the migration. +// QuestionDefault asks a question using the default survey implementation. func QuestionDefault(opts *survey.QuestionOptions) (string, error) { - return oldlog.Default.Question(convertOpts(opts)) -} - -// QuestionWith asks a question using the given logger, bridging the local -// survey type to the old log module's survey type during the migration. -func QuestionWith(logger oldlog.Logger, opts *survey.QuestionOptions) (string, error) { - return logger.Question(convertOpts(opts)) -} - -func convertOpts(opts *survey.QuestionOptions) *oldsurvey.QuestionOptions { - converted := oldsurvey.QuestionOptions(*opts) - return &converted + return survey.NewSurvey().Question(opts) }