From f5578fcaa0f9fe301fb6bbd965a12abf0ed9d15f Mon Sep 17 00:00:00 2001 From: Mike Seplowitz Date: Fri, 27 Jan 2023 16:31:10 -0500 Subject: [PATCH] Set up and use logrus logger in main Also return errors more consistently from other functions. --- cmd/proxy/actions/app.go | 50 ++++++++++++++------------------------- cmd/proxy/actions/auth.go | 20 +++++++++------- cmd/proxy/main.go | 37 +++++++++++++++++++---------- 3 files changed, 53 insertions(+), 54 deletions(-) diff --git a/cmd/proxy/actions/app.go b/cmd/proxy/actions/app.go index 4b3d7c3fd..696b9f6b0 100644 --- a/cmd/proxy/actions/app.go +++ b/cmd/proxy/actions/app.go @@ -3,7 +3,6 @@ package actions import ( "fmt" "net/http" - "os" "github.com/gomods/athens/pkg/config" "github.com/gomods/athens/pkg/log" @@ -11,7 +10,6 @@ import ( "github.com/gomods/athens/pkg/module" "github.com/gomods/athens/pkg/observ" "github.com/gorilla/mux" - "github.com/sirupsen/logrus" "github.com/unrolled/secure" "go.opencensus.io/plugin/ochttp" ) @@ -22,38 +20,33 @@ const Service = "proxy" // App is where all routes and middleware for the proxy // should be defined. This is the nerve center of your // application. -func App(conf *config.Config) (http.Handler, error) { - // ENV is used to help switch settings based on where the - // application is being run. Default is "development". - ENV := conf.GoEnv - +func App(logger *log.Logger, conf *config.Config) (http.Handler, error) { if conf.GithubToken != "" { if conf.NETRCPath != "" { - fmt.Println("Cannot provide both GithubToken and NETRCPath. Only provide one.") - os.Exit(1) + return nil, fmt.Errorf("cannot provide both GithubToken and NETRCPath") } - netrcFromToken(conf.GithubToken) + if err := netrcFromToken(conf.GithubToken); err != nil { + return nil, fmt.Errorf("netrcFromToken error: %w", err) + } } // mount .netrc to home dir // to have access to private repos. - initializeAuthFile(conf.NETRCPath) + if err := initializeAuthFile(conf.NETRCPath); err != nil { + return nil, fmt.Errorf("initializeAuthFile(netrc) error: %w", err) + } // mount .hgrc to home dir // to have access to private repos. - initializeAuthFile(conf.HGRCPath) - - logLvl, err := logrus.ParseLevel(conf.LogLevel) - if err != nil { - return nil, err + if err := initializeAuthFile(conf.HGRCPath); err != nil { + return nil, fmt.Errorf("initializeAuthFile(hgrc) error: %w", err) } - lggr := log.New(conf.CloudRuntime, logLvl) r := mux.NewRouter() r.Use( mw.WithRequestID, - mw.LogEntryMiddleware(lggr), + mw.LogEntryMiddleware(logger), mw.RequestLogger, secure.New(secure.Options{ SSLRedirect: conf.ForceSSL, @@ -82,10 +75,10 @@ func App(conf *config.Config) (http.Handler, error) { conf.TraceExporter, conf.TraceExporterURL, Service, - ENV, + conf.GoEnv, ) if err != nil { - lggr.Infof("%s", err) + logger.Info(err) } else { defer flushTraces() } @@ -95,7 +88,7 @@ func App(conf *config.Config) (http.Handler, error) { // was specified by the user. flushStats, err := observ.RegisterStatsExporter(r, conf.StatsExporter, Service) if err != nil { - lggr.Infof("%s", err) + logger.Info(err) } else { defer flushStats() } @@ -108,7 +101,7 @@ func App(conf *config.Config) (http.Handler, error) { if !conf.FilterOff() { mf, err := module.NewFilter(conf.FilterFile) if err != nil { - lggr.Fatal(err) + return nil, fmt.Errorf("error from module.NewFilter: %w", err) } r.Use(mw.NewFilterMiddleware(mf, conf.GlobalEndpoint)) } @@ -126,22 +119,15 @@ func App(conf *config.Config) (http.Handler, error) { store, err := GetStorage(conf.StorageType, conf.Storage, conf.TimeoutDuration(), client) if err != nil { - err = fmt.Errorf("error getting storage configuration (%s)", err) - return nil, err + return nil, fmt.Errorf("error getting storage configuration: %w", err) } proxyRouter := r if subRouter != nil { proxyRouter = subRouter } - if err := addProxyRoutes( - proxyRouter, - store, - lggr, - conf, - ); err != nil { - err = fmt.Errorf("error adding proxy routes (%s)", err) - return nil, err + if err := addProxyRoutes(proxyRouter, store, logger, conf); err != nil { + return nil, fmt.Errorf("error adding proxy routes: %w", err) } h := &ochttp.Handler{ diff --git a/cmd/proxy/actions/auth.go b/cmd/proxy/actions/auth.go index e654da873..91ffc0ebb 100644 --- a/cmd/proxy/actions/auth.go +++ b/cmd/proxy/actions/auth.go @@ -2,7 +2,6 @@ package actions import ( "fmt" - "log" "os" "path/filepath" "runtime" @@ -14,40 +13,43 @@ import ( // initializeAuthFile checks if provided auth file is at a pre-configured path // and moves to home directory -- note that this will override whatever // .netrc/.hgrc file you have in your home directory. -func initializeAuthFile(path string) { +func initializeAuthFile(path string) error { if path == "" { - return + return nil } fileBts, err := os.ReadFile(path) if err != nil { - log.Fatalf("could not read %s: %v", path, err) + return fmt.Errorf("could not read %s: %w", path, err) } hdir, err := homedir.Dir() if err != nil { - log.Fatalf("could not get homedir: %v", err) + return fmt.Errorf("could not get homedir: %w", err) } fileName := transformAuthFileName(filepath.Base(path)) rcp := filepath.Join(hdir, fileName) if err := os.WriteFile(rcp, fileBts, 0600); err != nil { - log.Fatalf("could not write to file: %v", err) + return fmt.Errorf("could not write to file: %w", err) } + + return nil } // netrcFromToken takes a github token and creates a .netrc // file for you, overriding whatever might be already there. -func netrcFromToken(tok string) { +func netrcFromToken(tok string) error { fileContent := fmt.Sprintf("machine github.com login %s\n", tok) hdir, err := homedir.Dir() if err != nil { - log.Fatalf("netrcFromToken: could not get homedir: %v", err) + return fmt.Errorf("could not get homedir: %w", err) } rcp := filepath.Join(hdir, getNETRCFilename()) if err := os.WriteFile(rcp, []byte(fileContent), 0600); err != nil { - log.Fatalf("netrcFromToken: could not write to file: %v", err) + return fmt.Errorf("could not write to file: %w", err) } + return nil } func transformAuthFileName(authFileName string) string { diff --git a/cmd/proxy/main.go b/cmd/proxy/main.go index dafd68e25..50be6c340 100644 --- a/cmd/proxy/main.go +++ b/cmd/proxy/main.go @@ -2,9 +2,10 @@ package main import ( "context" + "errors" "flag" "fmt" - "log" + stdlog "log" "net/http" "os" "os/signal" @@ -16,6 +17,8 @@ import ( "github.com/gomods/athens/internal/shutdown" "github.com/gomods/athens/pkg/build" "github.com/gomods/athens/pkg/config" + athenslog "github.com/gomods/athens/pkg/log" + "github.com/sirupsen/logrus" ) var ( @@ -31,17 +34,24 @@ func main() { } conf, err := config.Load(*configFile) if err != nil { - log.Fatalf("could not load config file: %v", err) + stdlog.Fatalf("could not load config file: %v", err) } - handler, err := actions.App(conf) + logLvl, err := logrus.ParseLevel(conf.LogLevel) if err != nil { - log.Fatal(err) + stdlog.Fatalf("failed logrus.ParseLevel(%q): %v", conf.LogLevel, err) + } + + logger := athenslog.New(conf.CloudRuntime, logLvl) + + handler, err := actions.App(logger, conf) + if err != nil { + logger.WithError(err).Fatal("failed to create App") } cert, key, err := conf.TLSCertFiles() if err != nil { - log.Fatal(err) + logger.WithError(err).Fatal("failed conf.TLSCertFiles") } srv := &http.Server{ @@ -54,35 +64,36 @@ func main() { sigint := make(chan os.Signal, 1) signal.Notify(sigint, shutdown.GetSignals()...) s := <-sigint - log.Printf("Recived signal (%s): Shutting down server", s) + logger.WithField("signal", s).Infof("received signal, shutting down server") // We received an interrupt signal, shut down. ctx, cancel := context.WithTimeout(context.Background(), time.Second*time.Duration(conf.ShutdownTimeout)) defer cancel() if err := srv.Shutdown(ctx); err != nil { - log.Fatal(err) + logger.WithError(err).Fatal("failed srv.Shutdown") } close(idleConnsClosed) }() if conf.EnablePprof { go func() { - // pprof to be exposed on a different port than the application for security matters, not to expose profiling data and avoid DoS attacks (profiling slows down the service) + // pprof to be exposed on a different port than the application for security matters, + // not to expose profiling data and avoid DoS attacks (profiling slows down the service) // https://www.farsightsecurity.com/txt-record/2016/10/28/cmikk-go-remote-profiling/ - log.Printf("Starting `pprof` at port %v", conf.PprofPort) - log.Fatal(http.ListenAndServe(conf.PprofPort, nil)) + logger.WithField("port", conf.PprofPort).Infof("starting pprof") + logger.Fatal(http.ListenAndServe(conf.PprofPort, nil)) }() } - log.Printf("Starting application at port %v", conf.Port) + logger.WithField("port", conf.Port).Infof("starting application") if cert != "" && key != "" { err = srv.ListenAndServeTLS(conf.TLSCertFile, conf.TLSKeyFile) } else { err = srv.ListenAndServe() } - if err != http.ErrServerClosed { - log.Fatal(err) + if !errors.Is(err, http.ErrServerClosed) { + logger.WithError(err).Fatal("application error") } <-idleConnsClosed