From 3cdb927eb0a0107b63206741aa66c8d66f281e59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Wed, 21 Feb 2018 17:06:30 +0200 Subject: [PATCH] Implement handlers to normalise how logging is handled --- Gopkg.lock | 11 +++- cmd/ooni/main.go | 5 +- internal/cli/app/app.go | 2 - internal/cli/info/info.go | 4 +- internal/cli/list/list.go | 4 +- internal/cli/nettest/nettest.go | 4 +- internal/cli/root/root.go | 15 +++-- internal/cli/run/run.go | 5 +- internal/cli/show/show.go | 4 +- internal/cli/upload/upload.go | 4 +- internal/database/models.go | 4 +- internal/log/handlers/batch/batch.go | 33 +++++++++++ internal/log/handlers/cli/cli.go | 84 ++++++++++++++++++++++++++++ nettests/nettests.go | 27 +++++++-- 14 files changed, 176 insertions(+), 30 deletions(-) create mode 100644 internal/log/handlers/batch/batch.go create mode 100644 internal/log/handlers/cli/cli.go diff --git a/Gopkg.lock b/Gopkg.lock index ef58d31..5e758c1 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -26,7 +26,8 @@ name = "github.com/apex/log" packages = [ ".", - "handlers/cli" + "handlers/cli", + "handlers/json" ] revision = "0296d6eb16bb28f8a0c55668affcf4876dc269be" version = "v1.0.0" @@ -43,6 +44,12 @@ packages = ["quantile"] revision = "4c0e84591b9aa9e6dcfdf3e020114cd81f89d5f9" +[[projects]] + name = "github.com/fatih/color" + packages = ["."] + revision = "507f6050b8568533fb3f5504de8e5205fa62a114" + version = "v1.6.0" + [[projects]] name = "github.com/golang/protobuf" packages = ["proto"] @@ -174,6 +181,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "5405ce55dbd69df4847de599c4e30d7540f208db389b67f2c9789ef6c17351d0" + inputs-digest = "6b04c53f65567785f5fc4ea3563d11c7744c450d1a0c45c0b8047ad10bf766ea" solver-name = "gps-cdcl" solver-version = 1 diff --git a/cmd/ooni/main.go b/cmd/ooni/main.go index 2068be0..9925661 100644 --- a/cmd/ooni/main.go +++ b/cmd/ooni/main.go @@ -2,6 +2,8 @@ package main import ( // commands + "github.com/apex/log" + _ "github.com/openobservatory/gooni/internal/cli/info" _ "github.com/openobservatory/gooni/internal/cli/list" _ "github.com/openobservatory/gooni/internal/cli/nettest" @@ -9,7 +11,6 @@ import ( _ "github.com/openobservatory/gooni/internal/cli/show" _ "github.com/openobservatory/gooni/internal/cli/upload" _ "github.com/openobservatory/gooni/internal/cli/version" - "github.com/openobservatory/gooni/internal/util" "github.com/openobservatory/gooni/internal/cli/app" ) @@ -19,5 +20,5 @@ func main() { if err == nil { return } - util.Fatal(err) + log.WithError(err).Fatal("main exit") } diff --git a/internal/cli/app/app.go b/internal/cli/app/app.go index db83d1c..aed0a5b 100644 --- a/internal/cli/app/app.go +++ b/internal/cli/app/app.go @@ -5,12 +5,10 @@ import ( "github.com/openobservatory/gooni/internal/cli/root" "github.com/openobservatory/gooni/internal/cli/version" - "github.com/openobservatory/gooni/internal/util" ) // Run the app. This is the main app entry point func Run() error { - util.Log("Running") root.Cmd.Version(version.Version) _, err := root.Cmd.Parse(os.Args[1:]) return err diff --git a/internal/cli/info/info.go b/internal/cli/info/info.go index ae0dd7a..ac86bcd 100644 --- a/internal/cli/info/info.go +++ b/internal/cli/info/info.go @@ -2,15 +2,15 @@ package info import ( "github.com/alecthomas/kingpin" + "github.com/apex/log" "github.com/openobservatory/gooni/internal/cli/root" - "github.com/openobservatory/gooni/internal/util" ) func init() { cmd := root.Command("info", "Display information about OONI Probe") cmd.Action(func(_ *kingpin.ParseContext) error { - util.Log("Info") + log.Info("Info") return nil }) } diff --git a/internal/cli/list/list.go b/internal/cli/list/list.go index 2e7462f..c0230a6 100644 --- a/internal/cli/list/list.go +++ b/internal/cli/list/list.go @@ -2,15 +2,15 @@ package list import ( "github.com/alecthomas/kingpin" + "github.com/apex/log" "github.com/openobservatory/gooni/internal/cli/root" - "github.com/openobservatory/gooni/internal/util" ) func init() { cmd := root.Command("list", "List measurements") cmd.Action(func(_ *kingpin.ParseContext) error { - util.Log("Listing") + log.Info("Listing") return nil }) } diff --git a/internal/cli/nettest/nettest.go b/internal/cli/nettest/nettest.go index 8210ccf..e10964c 100644 --- a/internal/cli/nettest/nettest.go +++ b/internal/cli/nettest/nettest.go @@ -2,15 +2,15 @@ package nettest import ( "github.com/alecthomas/kingpin" + "github.com/apex/log" "github.com/openobservatory/gooni/internal/cli/root" - "github.com/openobservatory/gooni/internal/util" ) func init() { cmd := root.Command("nettest", "Run a specific nettest") cmd.Action(func(_ *kingpin.ParseContext) error { - util.Log("Nettest") + log.Info("Nettest") return nil }) } diff --git a/internal/cli/root/root.go b/internal/cli/root/root.go index 6f288af..8da75cc 100644 --- a/internal/cli/root/root.go +++ b/internal/cli/root/root.go @@ -3,9 +3,10 @@ package root import ( "github.com/alecthomas/kingpin" "github.com/apex/log" - "github.com/apex/log/handlers/cli" ooni "github.com/openobservatory/gooni" "github.com/openobservatory/gooni/internal/database" + "github.com/openobservatory/gooni/internal/log/handlers/batch" + "github.com/openobservatory/gooni/internal/log/handlers/cli" "github.com/prometheus/common/version" ) @@ -20,11 +21,17 @@ var Init func() (*ooni.Config, *ooni.Context, error) func init() { configPath := Cmd.Flag("config", "Set a custom config file path").Short('c').String() - verbose := Cmd.Flag("verbose", "Enable verbose log output.").Short('v').Bool() + + isVerbose := Cmd.Flag("verbose", "Enable verbose log output.").Short('v').Bool() + isBatch := Cmd.Flag("batch", "Enable batch command line usage.").Bool() Cmd.PreAction(func(ctx *kingpin.ParseContext) error { - log.SetHandler(cli.Default) - if *verbose { + if *isBatch { + log.SetHandler(batch.Default) + } else { + log.SetHandler(cli.Default) + } + if *isVerbose { log.SetLevel(log.DebugLevel) log.Debugf("ooni version %s", version.Version) } diff --git a/internal/cli/run/run.go b/internal/cli/run/run.go index b18196b..1fe3b1f 100644 --- a/internal/cli/run/run.go +++ b/internal/cli/run/run.go @@ -7,7 +7,6 @@ import ( "github.com/apex/log" "github.com/openobservatory/gooni/internal/cli/root" "github.com/openobservatory/gooni/internal/database" - "github.com/openobservatory/gooni/internal/util" "github.com/openobservatory/gooni/nettests" "github.com/openobservatory/gooni/nettests/groups" ) @@ -18,7 +17,7 @@ func init() { nettestGroup := cmd.Arg("name", "the nettest group to run").Required().String() cmd.Action(func(_ *kingpin.ParseContext) error { - util.Log("Starting %s", *nettestGroup) + log.Infof("Starting %s", *nettestGroup) _, ctx, err := root.Init() if err != nil { log.Errorf("%s", err) @@ -37,7 +36,7 @@ func init() { } for _, nt := range group.Nettests { - ctl := nettests.NewController(ctx) + ctl := nettests.NewController(ctx, result) nt.Run(ctl) // XXX // 1. Generate the summary diff --git a/internal/cli/show/show.go b/internal/cli/show/show.go index 51848fa..78199c6 100644 --- a/internal/cli/show/show.go +++ b/internal/cli/show/show.go @@ -2,15 +2,15 @@ package nettest import ( "github.com/alecthomas/kingpin" + "github.com/apex/log" "github.com/openobservatory/gooni/internal/cli/root" - "github.com/openobservatory/gooni/internal/util" ) func init() { cmd := root.Command("show", "Show a specific measurement") cmd.Action(func(_ *kingpin.ParseContext) error { - util.Log("Show") + log.Info("Show") return nil }) } diff --git a/internal/cli/upload/upload.go b/internal/cli/upload/upload.go index 4a9d75f..d6d8606 100644 --- a/internal/cli/upload/upload.go +++ b/internal/cli/upload/upload.go @@ -2,15 +2,15 @@ package upload import ( "github.com/alecthomas/kingpin" + "github.com/apex/log" "github.com/openobservatory/gooni/internal/cli/root" - "github.com/openobservatory/gooni/internal/util" ) func init() { cmd := root.Command("upload", "Upload a specific measurement") cmd.Action(func(_ *kingpin.ParseContext) error { - util.Log("Uploading") + log.Info("Uploading") return nil }) } diff --git a/internal/database/models.go b/internal/database/models.go index e9afcd2..c0a2787 100644 --- a/internal/database/models.go +++ b/internal/database/models.go @@ -37,9 +37,9 @@ func CreateMeasurement(db *sqlx.DB, m Measurement) (*Measurement, error) { report_id, input, measurement_id, result_id) VALUES (:name,:start_time, - :summary,:asn,:ip,:country, + :asn,:ip,:country, :state,:failure,:report_file, - :report_id,:input,:measurement_id, + :report_id,:input, :result_id)`, m) if err != nil { diff --git a/internal/log/handlers/batch/batch.go b/internal/log/handlers/batch/batch.go new file mode 100644 index 0000000..2b11371 --- /dev/null +++ b/internal/log/handlers/batch/batch.go @@ -0,0 +1,33 @@ +package batch + +import ( + j "encoding/json" + "io" + "os" + "sync" + + "github.com/apex/log" +) + +// Default handler outputting to stderr. +var Default = New(os.Stderr) + +// Handler implementation. +type Handler struct { + *j.Encoder + mu sync.Mutex +} + +// New handler. +func New(w io.Writer) *Handler { + return &Handler{ + Encoder: j.NewEncoder(w), + } +} + +// HandleLog implements log.Handler. +func (h *Handler) HandleLog(e *log.Entry) error { + h.mu.Lock() + defer h.mu.Unlock() + return h.Encoder.Encode(e) +} diff --git a/internal/log/handlers/cli/cli.go b/internal/log/handlers/cli/cli.go new file mode 100644 index 0000000..10d7fba --- /dev/null +++ b/internal/log/handlers/cli/cli.go @@ -0,0 +1,84 @@ +package cli + +import ( + "fmt" + "io" + "os" + "sync" + "time" + + "github.com/apex/log" + "github.com/fatih/color" + colorable "github.com/mattn/go-colorable" +) + +// Default handler outputting to stderr. +var Default = New(os.Stderr) + +// start time. +var start = time.Now() + +var bold = color.New(color.Bold) + +// Colors mapping. +var Colors = [...]*color.Color{ + log.DebugLevel: color.New(color.FgWhite), + log.InfoLevel: color.New(color.FgBlue), + log.WarnLevel: color.New(color.FgYellow), + log.ErrorLevel: color.New(color.FgRed), + log.FatalLevel: color.New(color.FgRed), +} + +// Strings mapping. +var Strings = [...]string{ + log.DebugLevel: "•", + log.InfoLevel: "•", + log.WarnLevel: "•", + log.ErrorLevel: "⨯", + log.FatalLevel: "⨯", +} + +// Handler implementation. +type Handler struct { + mu sync.Mutex + Writer io.Writer + Padding int +} + +// New handler. +func New(w io.Writer) *Handler { + if f, ok := w.(*os.File); ok { + return &Handler{ + Writer: colorable.NewColorable(f), + Padding: 3, + } + } + + return &Handler{ + Writer: w, + Padding: 3, + } +} + +// HandleLog implements log.Handler. +func (h *Handler) HandleLog(e *log.Entry) error { + color := Colors[e.Level] + level := Strings[e.Level] + names := e.Fields.Names() + + h.mu.Lock() + defer h.mu.Unlock() + + color.Fprintf(h.Writer, "%s %-25s", bold.Sprintf("%*s", h.Padding+1, level), e.Message) + + for _, name := range names { + if name == "source" { + continue + } + fmt.Fprintf(h.Writer, " %s=%s", color.Sprint(name), e.Fields.Get(name)) + } + + fmt.Fprintln(h.Writer) + + return nil +} diff --git a/nettests/nettests.go b/nettests/nettests.go index 9efbcda..0d7e208 100644 --- a/nettests/nettests.go +++ b/nettests/nettests.go @@ -1,12 +1,11 @@ package nettests import ( - "fmt" - "github.com/apex/log" "github.com/measurement-kit/go-measurement-kit" ooni "github.com/openobservatory/gooni" "github.com/openobservatory/gooni/internal/cli/version" + "github.com/openobservatory/gooni/internal/database" ) // Nettest interface. Every Nettest should implement this. @@ -24,15 +23,17 @@ type NettestGroup struct { } // NewController creates a nettest controller -func NewController(ctx *ooni.Context) *Controller { +func NewController(ctx *ooni.Context, res *database.Result) *Controller { return &Controller{ ctx, + res, } } // Controller is passed to the run method of every Nettest type Controller struct { ctx *ooni.Context + res *database.Result } // Init should be called once to initialise the nettest @@ -53,7 +54,23 @@ func (c *Controller) Init(nt *mk.Nettest) { CaBundlePath: "", } nt.RegisterEventHandler(func(event interface{}) { - fmt.Println("Got event", event) + e := event.(map[string]interface{}) + if e["type"].(string) == "LOG" { + msg := e["message"].(string) + switch level := e["verbosity"].(string); level { + case "ERROR": + log.Error(msg) + case "INFO": + log.Info(msg) + default: + log.Debug(msg) + } + } else { + log.WithFields(log.Fields{ + "key": "event", + "value": e, + }).Info("got event") + } }) } @@ -70,7 +87,7 @@ func (c *Controller) OnEntry(entry string) { // MKStart is the interface for the mk.Nettest Start() function type MKStart func(name string) (chan bool, error) -// Start should be called every time there is a new entry +// Start should be called to start the test func (c *Controller) Start(f MKStart) { log.Debugf("MKStart: %s", f) }