From 8809b5d1c35e761e1214330c7cab52bfddf906bf Mon Sep 17 00:00:00 2001 From: Tom Pierantoni Date: Sun, 27 Oct 2024 14:09:54 +0100 Subject: [PATCH 1/5] add logrus dep --- go.mod | 2 ++ go.sum | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/go.mod b/go.mod index a14f6b3..2838d39 100644 --- a/go.mod +++ b/go.mod @@ -18,9 +18,11 @@ require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/kr/pretty v0.3.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/sirupsen/logrus v1.9.3 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect + golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 // indirect gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index aa67312..229b555 100644 --- a/go.sum +++ b/go.sum @@ -23,12 +23,15 @@ github.com/radovskyb/watcher v1.0.7/go.mod h1:78okwvY5wPdzcb1UYnip1pvrZNIVEIh/Cm github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= +github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= @@ -38,10 +41,13 @@ github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHo github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ= github.com/xeipuuv/gojsonschema v1.2.0 h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17UxZ74= github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y= +golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 h1:0A+M6Uqn+Eje4kHMK80dtF3JCXC4ykBgQG4Fe06QRhQ= +golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 6995fce76f14001b6b2a4a447e733be77a51fda9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Pierantoni?= Date: Mon, 28 Oct 2024 22:24:16 +0100 Subject: [PATCH 2/5] structured logging, 1st iteration * add log level option and logging for non-matching requests * format * replace all log imports for logrus * structured logs for watcher.go * add log fields helper and log for route_matchers * add exits on fatal errors, log level flag/config, logging field methods for requests and imposters * replace printf for leveled logs * add logs for handlers * logging on configurations * add os.Exit(1) after fatal errors * replace all calls to log.Fatal to log.Error+os.Exit * write body for missing requests * undo makefile changes --- Makefile | 3 +- cmd/killgrave/main.go | 7 ++-- internal/app/cmd/cmd.go | 46 ++++++++++++++++++++++---- internal/config.go | 11 ++++++ internal/config_test.go | 3 ++ internal/loghelper.go | 19 +++++++++++ internal/server/http/handler.go | 8 +++-- internal/server/http/imposter.go | 9 +++++ internal/server/http/route_matchers.go | 7 ++-- internal/server/http/server.go | 33 +++++++++++++----- internal/server/http/server_test.go | 2 +- internal/test/testdata/config.yml | 2 ++ internal/watcher.go | 11 +++--- internal/watcher_test.go | 2 +- 14 files changed, 133 insertions(+), 30 deletions(-) create mode 100644 internal/loghelper.go diff --git a/Makefile b/Makefile index b773558..4e4695b 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,4 @@ .PHONY: build build: - go build -ldflags "-s -w -X 'github.com/friendsofgo/killgrave/internal/app/cmd._version=`git rev-parse --abbrev-ref HEAD`-`git rev-parse --short HEAD`'" -o bin/killgrave cmd/killgrave/main.go \ No newline at end of file + go build -ldflags "-s -w -X 'github.com/friendsofgo/killgrave/internal/app/cmd._version=`git rev-parse --abbrev-ref HEAD`-`git rev-parse --short HEAD`'" -o bin/killgrave cmd/killgrave/main.go + diff --git a/cmd/killgrave/main.go b/cmd/killgrave/main.go index 3327976..afef4a2 100644 --- a/cmd/killgrave/main.go +++ b/cmd/killgrave/main.go @@ -1,13 +1,16 @@ package main import ( - "log" + "os" + + log "github.com/sirupsen/logrus" "github.com/friendsofgo/killgrave/internal/app" ) func main() { if err := app.Run(); err != nil { - log.Fatal(err) + log.Error(err) + os.Exit(1) } } diff --git a/internal/app/cmd/cmd.go b/internal/app/cmd/cmd.go index 4ddbe0b..20f9d81 100644 --- a/internal/app/cmd/cmd.go +++ b/internal/app/cmd/cmd.go @@ -3,7 +3,6 @@ package cmd import ( "errors" "fmt" - "log" "net/http" "os" "os/signal" @@ -14,6 +13,7 @@ import ( "github.com/gorilla/handlers" "github.com/gorilla/mux" "github.com/radovskyb/watcher" + log "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -26,6 +26,7 @@ const ( _defaultPort = 3000 _defaultProxyMode = killgrave.ProxyNone _defaultStrictSlash = true + _defaultLogLevel = "info" _impostersFlag = "imposters" _configFlag = "config" @@ -35,6 +36,7 @@ const ( _secureFlag = "secure" _proxyModeFlag = "proxy-mode" _proxyURLFlag = "proxy-url" + _loglevelFlag = "log-level" ) var ( @@ -77,6 +79,7 @@ func NewKillgraveCmd() *cobra.Command { rootCmd.Flags().BoolP(_secureFlag, "s", false, "Run mock server using TLS (https)") rootCmd.Flags().StringP(_proxyModeFlag, "m", _defaultProxyMode.String(), "Proxy mode, the options are all, missing or none") rootCmd.Flags().StringP(_proxyURLFlag, "u", "", "The url where the proxy will redirect to") + rootCmd.Flags().String(_loglevelFlag, _defaultLogLevel, "The log level to output") rootCmd.SetVersionTemplate("Killgrave version: {{.Version}}\n") @@ -87,6 +90,12 @@ func runHTTP(cmd *cobra.Command, cfg killgrave.Config) error { done := make(chan os.Signal, 1) defer close(done) + logLevel, err := log.ParseLevel(cfg.Log.Level) + if err != nil { + return fmt.Errorf("Could not parse log level: %v", err) + } + log.SetLevel(logLevel) + signal.Notify(done, syscall.SIGINT, syscall.SIGTERM) srv := runServer(cfg) @@ -103,7 +112,8 @@ func runHTTP(cmd *cobra.Command, cfg killgrave.Config) error { <-done if err := srv.Shutdown(); err != nil { - log.Fatal(err) + log.Error(err) + os.Exit(1) } return nil @@ -121,12 +131,14 @@ func runServer(cfg killgrave.Config) server.Server { proxyServer, err := server.NewProxy(cfg.Proxy.Url, cfg.Proxy.Mode) if err != nil { - log.Fatal(err) + log.Error(err) + os.Exit(1) } imposterFs, err := server.NewImposterFS(cfg.ImpostersPath) if err != nil { - log.Fatal(err) + log.Error(err) + os.Exit(1) } s := server.NewServer( @@ -137,7 +149,8 @@ func runServer(cfg killgrave.Config) server.Server { imposterFs, ) if err := s.Build(); err != nil { - log.Fatal(err) + log.Error(err) + os.Exit(1) } s.Run() @@ -152,7 +165,8 @@ func runWatcher(cfg killgrave.Config, currentSrv *server.Server) (*watcher.Watch killgrave.AttachWatcher(w, func() { if err := currentSrv.Shutdown(); err != nil { - log.Fatal(err) + log.Error(err) + os.Exit(1) } *currentSrv = runServer(cfg) }) @@ -190,7 +204,16 @@ func prepareConfig(cmd *cobra.Command) (killgrave.Config, error) { return killgrave.Config{}, err } - return cfg, configureProxyMode(cmd, &cfg) + if err := configureProxyMode(cmd, &cfg); err != nil { + return killgrave.Config{}, err + + } + + if err := configureLogging(cmd, &cfg); err != nil { + return killgrave.Config{}, err + } + + return cfg, nil } func configureProxyMode(cmd *cobra.Command, cfg *killgrave.Config) error { @@ -218,3 +241,12 @@ func configureProxyMode(cmd *cobra.Command, cfg *killgrave.Config) error { cfg.ConfigureProxy(pMode, url) return nil } + +func configureLogging(cmd *cobra.Command, cfg *killgrave.Config) error { + logLevel, err := cmd.Flags().GetString(_loglevelFlag) + if err != nil { + return err + } + cfg.Log.Level = logLevel + return nil +} diff --git a/internal/config.go b/internal/config.go index 648b722..06e7950 100644 --- a/internal/config.go +++ b/internal/config.go @@ -7,6 +7,7 @@ import ( "os" "path" + log "github.com/sirupsen/logrus" "gopkg.in/yaml.v2" ) @@ -19,6 +20,13 @@ type Config struct { Proxy ConfigProxy `yaml:"proxy"` Secure bool `yaml:"secure"` Watcher bool `yaml:"watcher"` + Log ConfigLog `yaml:"log"` +} + +// ConfigLog is a representation of the log section of the yaml +// This should include configurations for future features for logging (output format, dump file, etc) +type ConfigLog struct { + Level string `yaml:"level"` } // ConfigCORS representation of section CORS of the yaml @@ -150,6 +158,9 @@ func NewConfigFromFile(cfgPath string) (Config, error) { if err := yaml.Unmarshal(bytes, &cfg); err != nil { return Config{}, fmt.Errorf("%w: error while unmarshalling configFile file %s, using default configuration instead", err, cfgPath) } + if _, err := log.ParseLevel(cfg.Log.Level); err != nil { + return Config{}, err + } cfg.ImpostersPath = path.Join(path.Dir(cfgPath), cfg.ImpostersPath) diff --git a/internal/config_test.go b/internal/config_test.go index 38c0076..1188ade 100644 --- a/internal/config_test.go +++ b/internal/config_test.go @@ -107,6 +107,9 @@ func validConfig() Config { }, Watcher: true, Secure: true, + Log: ConfigLog{ + Level: "info", + }, } } diff --git a/internal/loghelper.go b/internal/loghelper.go new file mode 100644 index 0000000..aa40db4 --- /dev/null +++ b/internal/loghelper.go @@ -0,0 +1,19 @@ +package killgrave + +import ( + "net/http" + + log "github.com/sirupsen/logrus" +) + +const ( + methodField string = "Method" + urlField = "URL" +) + +func LogFieldsFromRequest(req *http.Request) log.Fields { + return log.Fields{ + methodField: req.Method, + urlField: req.URL, + } +} diff --git a/internal/server/http/handler.go b/internal/server/http/handler.go index cab0ae0..1ecd0c3 100644 --- a/internal/server/http/handler.go +++ b/internal/server/http/handler.go @@ -2,10 +2,11 @@ package http import ( "io" - "log" "net/http" "os" "time" + + log "github.com/sirupsen/logrus" ) // ImposterHandler create specific handler for the received imposter @@ -18,6 +19,7 @@ func ImposterHandler(i Imposter) http.HandlerFunc { writeHeaders(res, w) w.WriteHeader(res.Status) writeBody(i, res, w) + log.WithFields(i.LogFields()).Debugf("Request matched handler") } } @@ -43,7 +45,7 @@ func writeBody(i Imposter, r Response, w http.ResponseWriter) { func fetchBodyFromFile(bodyFile string) (bytes []byte) { if _, err := os.Stat(bodyFile); os.IsNotExist(err) { - log.Printf("the body file %s not found\n", bodyFile) + log.Warnf("the body file %s not found\n", bodyFile) return } @@ -51,7 +53,7 @@ func fetchBodyFromFile(bodyFile string) (bytes []byte) { defer f.Close() bytes, err := io.ReadAll(f) if err != nil { - log.Printf("imposible read the file %s: %v\n", bodyFile, err) + log.Warnf("imposible read the file %s: %v\n", bodyFile, err) } return } diff --git a/internal/server/http/imposter.go b/internal/server/http/imposter.go index e5ae638..938e545 100644 --- a/internal/server/http/imposter.go +++ b/internal/server/http/imposter.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strings" + log "github.com/sirupsen/logrus" "gopkg.in/yaml.v2" ) @@ -208,3 +209,11 @@ func (i ImposterFs) unmarshalImposters(imposterConfig ImposterConfig) ([]Imposte return imposters, nil } + +func (i Imposter) LogFields() log.Fields { + return log.Fields{ + "endpoint": i.Request.Endpoint, + "method": i.Request.Method, + "path": i.Path, + } +} diff --git a/internal/server/http/route_matchers.go b/internal/server/http/route_matchers.go index 0f6cbf2..69a763d 100644 --- a/internal/server/http/route_matchers.go +++ b/internal/server/http/route_matchers.go @@ -5,12 +5,14 @@ import ( "errors" "fmt" "io" - "log" "net/http" "os" "path/filepath" + killgrave "github.com/friendsofgo/killgrave/internal" + "github.com/gorilla/mux" + log "github.com/sirupsen/logrus" "github.com/xeipuuv/gojsonschema" ) @@ -19,9 +21,8 @@ func MatcherBySchema(imposter Imposter) mux.MatcherFunc { return func(req *http.Request, rm *mux.RouteMatch) bool { err := validateSchema(imposter, req) - // TODO: inject the logger if err != nil { - log.Println(err) + log.WithFields(killgrave.LogFieldsFromRequest(req)).Warn(err) return false } return true diff --git a/internal/server/http/server.go b/internal/server/http/server.go index d334e84..091120f 100644 --- a/internal/server/http/server.go +++ b/internal/server/http/server.go @@ -4,12 +4,14 @@ import ( "context" "crypto/tls" _ "embed" - "log" + "fmt" "net/http" + "os" killgrave "github.com/friendsofgo/killgrave/internal" "github.com/gorilla/handlers" "github.com/gorilla/mux" + log "github.com/sirupsen/logrus" ) //go:embed cert/server.key @@ -81,6 +83,7 @@ func PrepareAccessControl(config killgrave.ConfigCORS) (h []handlers.CORSOption) func (s *Server) Build() error { if s.proxy.mode == killgrave.ProxyAll { // not necessary load the imposters if you will use the tool as a proxy + log.Infoln("ProxyAll mode enabled, no imposter will be used") s.router.PathPrefix("/").HandlerFunc(s.proxy.Handler()) return nil } @@ -97,7 +100,6 @@ loop: select { case imposters := <-impostersCh: s.addImposterHandler(imposters) - log.Printf("imposter %s loaded\n", imposters[0].Path) case <-done: close(impostersCh) close(done) @@ -105,7 +107,11 @@ loop: } } if s.proxy.mode == killgrave.ProxyMissing { + log.Infof("Proxying missed requests to: %v", s.proxy.url) s.router.NotFoundHandler = s.proxy.Handler() + } else { + log.Infoln("No proxy has been configured for non-matching requests, defaulting to a 404 response") + s.router.NotFoundHandler = s.defaultNotFoundHandler() } return nil } @@ -118,10 +124,11 @@ func (s *Server) Run() { if s.secure { tlsString = "(TLS mode)" } - log.Printf("The fake server is on tap now: %s%s\n", s.httpServer.Addr, tlsString) + log.Infof("The fake server is on tap now: %s%s\n", s.httpServer.Addr, tlsString) err := s.run(s.secure) if err != http.ErrServerClosed { - log.Fatal(err) + log.Error(err) + os.Exit(1) } }() } @@ -133,21 +140,21 @@ func (s *Server) run(secure bool) error { cert, err := tls.X509KeyPair(serverCert, serverKey) if err != nil { - log.Fatal(err) + log.Error(err) + os.Exit(1) } s.httpServer.TLSConfig = &tls.Config{ Certificates: []tls.Certificate{cert}, } - return s.httpServer.ListenAndServeTLS("", "") } // Shutdown shutdown the current http server func (s *Server) Shutdown() error { - log.Println("stopping server...") + log.Info("stopping server...") if err := s.httpServer.Shutdown(context.TODO()); err != nil { - log.Fatalf("Server Shutdown Failed:%+v", err) + return fmt.Errorf("Server Shutdown Failed:%+v", err) } return nil @@ -170,9 +177,19 @@ func (s *Server) addImposterHandler(imposters []Imposter) { r.Queries(k, v) } } + log.WithFields(imposter.LogFields()).Debugln("imposter loaded") + } +} + +func (s *Server) defaultNotFoundHandler() http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + log.WithFields(killgrave.LogFieldsFromRequest(r)).Debugf("Request didn't match any imposter, and proxyMode is %v", s.proxy.mode) + w.WriteHeader(http.StatusNotFound) + w.Write([]byte("404 page not found\n")) } } +// not used? func (s *Server) handleAll(h http.HandlerFunc) { s.router.PathPrefix("/").HandlerFunc(h) } diff --git a/internal/server/http/server_test.go b/internal/server/http/server_test.go index 26ea559..9650d83 100644 --- a/internal/server/http/server_test.go +++ b/internal/server/http/server_test.go @@ -3,7 +3,6 @@ package http import ( "crypto/tls" "io" - "log" "net/http" "net/http/httptest" "os" @@ -12,6 +11,7 @@ import ( killgrave "github.com/friendsofgo/killgrave/internal" "github.com/gorilla/mux" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) diff --git a/internal/test/testdata/config.yml b/internal/test/testdata/config.yml index 3982a4c..14fc030 100644 --- a/internal/test/testdata/config.yml +++ b/internal/test/testdata/config.yml @@ -9,3 +9,5 @@ cors: allow_credentials: true watcher: true secure: true +log: + level: "info" diff --git a/internal/watcher.go b/internal/watcher.go index 397c365..713d5d6 100644 --- a/internal/watcher.go +++ b/internal/watcher.go @@ -2,10 +2,11 @@ package killgrave import ( "fmt" - "log" + "os" "time" "github.com/radovskyb/watcher" + log "github.com/sirupsen/logrus" ) // InitializeWatcher initialize a watcher to check for modification on all files @@ -18,6 +19,7 @@ func InitializeWatcher(pathToWatch string) (*watcher.Watcher, error) { if err := w.AddRecursive(pathToWatch); err != nil { return nil, fmt.Errorf("%w: error trying to watch change on %s directory", err, pathToWatch) } + log.Infof("Watching for changes in: %v\n", pathToWatch) return w, nil } @@ -27,7 +29,8 @@ func InitializeWatcher(pathToWatch string) (*watcher.Watcher, error) { func AttachWatcher(w *watcher.Watcher, fn func()) { go func() { if err := w.Start(time.Millisecond * 100); err != nil { - log.Fatalln(err) + log.Error(err) + os.Exit(1) } }() @@ -47,10 +50,10 @@ func readEventsFromWatcher(w *watcher.Watcher, fn func()) { for { select { case evt := <-w.Event: - log.Println("Modified file:", evt.Name()) + log.Info("Modified file:", evt.Name()) fn() case err := <-w.Error: - log.Printf("Error checking file change: %+v", err) + log.Warnf("Error checking file change: %+v", err) case <-w.Closed: return } diff --git a/internal/watcher_test.go b/internal/watcher_test.go index 4cdcb7e..7e02376 100644 --- a/internal/watcher_test.go +++ b/internal/watcher_test.go @@ -3,12 +3,12 @@ package killgrave import ( "errors" "io" - "log" "os" "testing" "time" "github.com/radovskyb/watcher" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" ) From 1df276138536b02a9e372b97e8e2c5767efa977e Mon Sep 17 00:00:00 2001 From: Tom Pierantoni Date: Tue, 29 Oct 2024 17:09:58 +0100 Subject: [PATCH 3/5] replace all calls to log.Error followed by os.Exit for log.Fatal --- cmd/killgrave/main.go | 5 +---- internal/app/cmd/cmd.go | 15 +++++---------- internal/server/http/server.go | 7 ++----- internal/watcher.go | 4 +--- 4 files changed, 9 insertions(+), 22 deletions(-) diff --git a/cmd/killgrave/main.go b/cmd/killgrave/main.go index afef4a2..b46f93c 100644 --- a/cmd/killgrave/main.go +++ b/cmd/killgrave/main.go @@ -1,8 +1,6 @@ package main import ( - "os" - log "github.com/sirupsen/logrus" "github.com/friendsofgo/killgrave/internal/app" @@ -10,7 +8,6 @@ import ( func main() { if err := app.Run(); err != nil { - log.Error(err) - os.Exit(1) + log.Fatal(err) } } diff --git a/internal/app/cmd/cmd.go b/internal/app/cmd/cmd.go index 20f9d81..bd094e9 100644 --- a/internal/app/cmd/cmd.go +++ b/internal/app/cmd/cmd.go @@ -112,8 +112,7 @@ func runHTTP(cmd *cobra.Command, cfg killgrave.Config) error { <-done if err := srv.Shutdown(); err != nil { - log.Error(err) - os.Exit(1) + log.Fatal(err) } return nil @@ -131,14 +130,12 @@ func runServer(cfg killgrave.Config) server.Server { proxyServer, err := server.NewProxy(cfg.Proxy.Url, cfg.Proxy.Mode) if err != nil { - log.Error(err) - os.Exit(1) + log.Fatal(err) } imposterFs, err := server.NewImposterFS(cfg.ImpostersPath) if err != nil { - log.Error(err) - os.Exit(1) + log.Fatal(err) } s := server.NewServer( @@ -149,8 +146,7 @@ func runServer(cfg killgrave.Config) server.Server { imposterFs, ) if err := s.Build(); err != nil { - log.Error(err) - os.Exit(1) + log.Fatal(err) } s.Run() @@ -165,8 +161,7 @@ func runWatcher(cfg killgrave.Config, currentSrv *server.Server) (*watcher.Watch killgrave.AttachWatcher(w, func() { if err := currentSrv.Shutdown(); err != nil { - log.Error(err) - os.Exit(1) + log.Fatal(err) } *currentSrv = runServer(cfg) }) diff --git a/internal/server/http/server.go b/internal/server/http/server.go index 091120f..3f6a368 100644 --- a/internal/server/http/server.go +++ b/internal/server/http/server.go @@ -6,7 +6,6 @@ import ( _ "embed" "fmt" "net/http" - "os" killgrave "github.com/friendsofgo/killgrave/internal" "github.com/gorilla/handlers" @@ -127,8 +126,7 @@ func (s *Server) Run() { log.Infof("The fake server is on tap now: %s%s\n", s.httpServer.Addr, tlsString) err := s.run(s.secure) if err != http.ErrServerClosed { - log.Error(err) - os.Exit(1) + log.Fatal(err) } }() } @@ -140,8 +138,7 @@ func (s *Server) run(secure bool) error { cert, err := tls.X509KeyPair(serverCert, serverKey) if err != nil { - log.Error(err) - os.Exit(1) + log.Fatal(err) } s.httpServer.TLSConfig = &tls.Config{ diff --git a/internal/watcher.go b/internal/watcher.go index 713d5d6..ee5d619 100644 --- a/internal/watcher.go +++ b/internal/watcher.go @@ -2,7 +2,6 @@ package killgrave import ( "fmt" - "os" "time" "github.com/radovskyb/watcher" @@ -29,8 +28,7 @@ func InitializeWatcher(pathToWatch string) (*watcher.Watcher, error) { func AttachWatcher(w *watcher.Watcher, fn func()) { go func() { if err := w.Start(time.Millisecond * 100); err != nil { - log.Error(err) - os.Exit(1) + log.Fatal(err) } }() From bd89f2f52ddb9954ecbd6a37e8d9c597491d8a11 Mon Sep 17 00:00:00 2001 From: Tom Pierantoni Date: Tue, 29 Oct 2024 17:32:30 +0100 Subject: [PATCH 4/5] move log level parsing to configureLogging function --- internal/app/cmd/cmd.go | 13 +++++++------ internal/config.go | 5 +---- internal/config_test.go | 3 ++- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/internal/app/cmd/cmd.go b/internal/app/cmd/cmd.go index bd094e9..8a14e77 100644 --- a/internal/app/cmd/cmd.go +++ b/internal/app/cmd/cmd.go @@ -90,11 +90,7 @@ func runHTTP(cmd *cobra.Command, cfg killgrave.Config) error { done := make(chan os.Signal, 1) defer close(done) - logLevel, err := log.ParseLevel(cfg.Log.Level) - if err != nil { - return fmt.Errorf("Could not parse log level: %v", err) - } - log.SetLevel(logLevel) + log.SetLevel(cfg.Log.Level) signal.Notify(done, syscall.SIGINT, syscall.SIGTERM) @@ -242,6 +238,11 @@ func configureLogging(cmd *cobra.Command, cfg *killgrave.Config) error { if err != nil { return err } - cfg.Log.Level = logLevel + logrusLogLevel, err := log.ParseLevel(logLevel) + if err != nil { + return err + } + + cfg.Log.Level = logrusLogLevel return nil } diff --git a/internal/config.go b/internal/config.go index 06e7950..4b4e984 100644 --- a/internal/config.go +++ b/internal/config.go @@ -26,7 +26,7 @@ type Config struct { // ConfigLog is a representation of the log section of the yaml // This should include configurations for future features for logging (output format, dump file, etc) type ConfigLog struct { - Level string `yaml:"level"` + Level log.Level `yaml:"level"` } // ConfigCORS representation of section CORS of the yaml @@ -158,9 +158,6 @@ func NewConfigFromFile(cfgPath string) (Config, error) { if err := yaml.Unmarshal(bytes, &cfg); err != nil { return Config{}, fmt.Errorf("%w: error while unmarshalling configFile file %s, using default configuration instead", err, cfgPath) } - if _, err := log.ParseLevel(cfg.Log.Level); err != nil { - return Config{}, err - } cfg.ImpostersPath = path.Join(path.Dir(cfgPath), cfg.ImpostersPath) diff --git a/internal/config_test.go b/internal/config_test.go index 1188ade..b13636e 100644 --- a/internal/config_test.go +++ b/internal/config_test.go @@ -4,6 +4,7 @@ import ( "errors" "testing" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" ) @@ -108,7 +109,7 @@ func validConfig() Config { Watcher: true, Secure: true, Log: ConfigLog{ - Level: "info", + Level: log.InfoLevel, }, } } From f1ed3bb5c1c86e6519d861877cc81c247a0e6dd7 Mon Sep 17 00:00:00 2001 From: Tom Pierantoni Date: Tue, 29 Oct 2024 17:34:27 +0100 Subject: [PATCH 5/5] remove unneeded constants --- internal/loghelper.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/internal/loghelper.go b/internal/loghelper.go index aa40db4..e7f6ec3 100644 --- a/internal/loghelper.go +++ b/internal/loghelper.go @@ -6,14 +6,9 @@ import ( log "github.com/sirupsen/logrus" ) -const ( - methodField string = "Method" - urlField = "URL" -) - func LogFieldsFromRequest(req *http.Request) log.Fields { return log.Fields{ - methodField: req.Method, - urlField: req.URL, + "method": req.Method, + "url": req.URL, } }