From 7d4bb66f9eaad0dfd3adff752316373a8d913d90 Mon Sep 17 00:00:00 2001 From: Bilal Amarni Date: Sun, 8 Jul 2018 20:22:51 +0200 Subject: [PATCH 1/4] Add a custom log formatter It just keeps the log entry message and appends a newline to it. This makes them more user-friendly in the terminal. Before: INFO[0000] Log message. After: Log message. https://jira.mesosphere.com/browse/DCOS_OSS-3746 --- pkg/cli/context.go | 3 ++- pkg/log/formatter.go | 22 ++++++++++++++++++ pkg/log/formatter_test.go | 48 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 pkg/log/formatter.go create mode 100644 pkg/log/formatter_test.go diff --git a/pkg/cli/context.go b/pkg/cli/context.go index 5fd1b4bc3..3b6e94ad2 100644 --- a/pkg/cli/context.go +++ b/pkg/cli/context.go @@ -10,6 +10,7 @@ import ( "github.com/dcos/dcos-cli/pkg/config" "github.com/dcos/dcos-cli/pkg/httpclient" + "github.com/dcos/dcos-cli/pkg/log" "github.com/dcos/dcos-cli/pkg/login" "github.com/dcos/dcos-cli/pkg/open" "github.com/dcos/dcos-cli/pkg/plugin" @@ -70,7 +71,7 @@ func (ctx *Context) Logger() *logrus.Logger { if ctx.logger == nil { ctx.logger = &logrus.Logger{ Out: ctx.env.ErrOut, - Formatter: new(logrus.TextFormatter), + Formatter: &log.Formatter{}, Hooks: make(logrus.LevelHooks), } } diff --git a/pkg/log/formatter.go b/pkg/log/formatter.go new file mode 100644 index 000000000..769a0beef --- /dev/null +++ b/pkg/log/formatter.go @@ -0,0 +1,22 @@ +package log + +import ( + "bytes" + + "github.com/sirupsen/logrus" +) + +// Formatter is a simple formatter for logrus. +type Formatter struct{} + +// Format returns the log entry message with a trailing newline. +func (f *Formatter) Format(entry *logrus.Entry) ([]byte, error) { + var b *bytes.Buffer + if entry.Buffer != nil { + b = entry.Buffer + } else { + b = &bytes.Buffer{} + } + b.WriteString(entry.Message + "\n") + return b.Bytes(), nil +} diff --git a/pkg/log/formatter_test.go b/pkg/log/formatter_test.go new file mode 100644 index 000000000..0465a8eac --- /dev/null +++ b/pkg/log/formatter_test.go @@ -0,0 +1,48 @@ +package log + +import ( + "bytes" + "io/ioutil" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/sirupsen/logrus" +) + +func TestFormat(t *testing.T) { + formatter := &Formatter{} + logger := &logrus.Logger{ + Out: ioutil.Discard, + Formatter: formatter, + } + + fixtures := []struct { + entry *logrus.Entry + expOut string + }{ + { + // simple entry + &logrus.Entry{ + Logger: logger, + Message: "DEBUGME", + }, + "DEBUGME\n", + }, + { + // entry with buffer + &logrus.Entry{ + Logger: logger, + Message: "DEBUGMETOO", + Buffer: bytes.NewBuffer(nil), + }, + "DEBUGMETOO\n", + }, + } + + for _, fixture := range fixtures { + out, err := formatter.Format(fixture.entry) + require.NoError(t, err) + require.Equal(t, fixture.expOut, string(out)) + } +} From 1574786c82133cf65dca3386e2a317116e1db2bd Mon Sep 17 00:00:00 2001 From: Bilal Amarni Date: Sun, 8 Jul 2018 20:49:22 +0200 Subject: [PATCH 2/4] Improve httpclient logs Only dump requests/responses when the logging level is set to debug. --- pkg/httpclient/client.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/pkg/httpclient/client.go b/pkg/httpclient/client.go index c8700fb88..f85ca72e3 100644 --- a/pkg/httpclient/client.go +++ b/pkg/httpclient/client.go @@ -176,25 +176,27 @@ func (c *Client) NewRequest(method, path string, body io.Reader, opts ...Option) func (c *Client) Do(req *http.Request) (*http.Response, error) { logger := c.opts.Logger - if logger != nil { - dumpBody := (logger.Level >= logrus.DebugLevel) - reqDump, err := httputil.DumpRequestOut(req, dumpBody) + if logger != nil && logger.Level >= logrus.DebugLevel { + reqDump, err := httputil.DumpRequestOut(req, true) if err != nil { - logger.Warnf("Couldn't dump request: %s", err) + logger.Debug("Couldn't dump request: %s", err) } else { - logger.Infof("Outgoing request:\n%s", reqDump) + logger.Debug(string(reqDump)) } } resp, err := c.baseClient.Do(req) - if err == nil && logger != nil { - dumpBody := (logger.Level >= logrus.DebugLevel) - respDump, err := httputil.DumpResponse(resp, dumpBody) - if err != nil { - logger.Warnf("Couldn't dump response: %s", err) + if logger != nil && logger.Level >= logrus.DebugLevel { + if err == nil { + respDump, err := httputil.DumpResponse(resp, true) + if err != nil { + logger.Debug("Couldn't dump response: %s", err) + } else { + logger.Debug(string(respDump)) + } } else { - logger.Infof("Incoming response:\n%s", respDump) + logger.Debug(err) } } return resp, err From 67e8c88ac9a52d2bb13add49f7966e3a31b5b737 Mon Sep 17 00:00:00 2001 From: Bilal Amarni Date: Mon, 9 Jul 2018 12:12:53 +0200 Subject: [PATCH 3/4] Disable usage output on errors This causes command help messages to appear everytime an error is triggered. --- pkg/cmd/dcos.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cmd/dcos.go b/pkg/cmd/dcos.go index cf272d233..fa06915dc 100644 --- a/pkg/cmd/dcos.go +++ b/pkg/cmd/dcos.go @@ -21,6 +21,7 @@ func NewDCOSCommand(ctx api.Context) *cobra.Command { cmd := &cobra.Command{ Use: "dcos", PersistentPreRun: func(cmd *cobra.Command, args []string) { + cmd.SilenceUsage = true if verbose == 1 { // -v sets the logger level to info. ctx.Logger().SetLevel(logrus.InfoLevel) From 8e3b7041769ef728a0b052c55a2ec348bceafbe5 Mon Sep 17 00:00:00 2001 From: Bilal Amarni Date: Mon, 9 Jul 2018 19:01:49 +0200 Subject: [PATCH 4/4] Log a message when setting-up a cluster without scheme. --- pkg/cli/context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cli/context.go b/pkg/cli/context.go index 3b6e94ad2..f4cc3195d 100644 --- a/pkg/cli/context.go +++ b/pkg/cli/context.go @@ -173,8 +173,8 @@ func (ctx *Context) Login(flags *login.Flags, httpClient *httpclient.Client) (st // Setup configures a given cluster based on its URL and setup flags. func (ctx *Context) Setup(flags *setup.Flags, clusterURL string) (*config.Cluster, error) { - // This supports passing a cluster URL without scheme, it then defaults to HTTPS. if !strings.HasPrefix(clusterURL, "https://") && !strings.HasPrefix(clusterURL, "http://") { + ctx.Logger().Info("Missing scheme in cluster URL, assuming HTTPS.") clusterURL = "https://" + clusterURL }