diff --git a/checks/binary_artifact_test.go b/checks/binary_artifact_test.go index bb8b9e3d249..e4bb9ffda24 100644 --- a/checks/binary_artifact_test.go +++ b/checks/binary_artifact_test.go @@ -22,7 +22,6 @@ import ( "github.com/golang/mock/gomock" "github.com/ossf/scorecard/v4/checker" - "github.com/ossf/scorecard/v4/clients/githubrepo" "github.com/ossf/scorecard/v4/clients/localdir" "github.com/ossf/scorecard/v4/log" scut "github.com/ossf/scorecard/v4/utests" @@ -60,13 +59,7 @@ func TestBinaryArtifacts(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - logger, err := githubrepo.NewLogger(log.DebugLevel) - if err != nil { - t.Errorf("githubrepo.NewLogger: %v", err) - } - - // nolint - defer logger.Zap.Sync() + logger := log.NewLogger(log.DebugLevel) ctrl := gomock.NewController(t) repo, err := localdir.MakeLocalDirRepo(tt.inputFolder) diff --git a/checks/license_test.go b/checks/license_test.go index dd91ac01987..c126f5a73a2 100644 --- a/checks/license_test.go +++ b/checks/license_test.go @@ -22,7 +22,6 @@ import ( "github.com/golang/mock/gomock" "github.com/ossf/scorecard/v4/checker" - "github.com/ossf/scorecard/v4/clients/githubrepo" "github.com/ossf/scorecard/v4/clients/localdir" "github.com/ossf/scorecard/v4/log" scut "github.com/ossf/scorecard/v4/utests" @@ -142,13 +141,7 @@ func TestLicenseFileSubdirectory(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - logger, err := githubrepo.NewLogger(log.DebugLevel) - if err != nil { - t.Errorf("githubrepo.NewLogger: %v", err) - } - - // nolint - defer logger.Zap.Sync() + logger := log.NewLogger(log.DebugLevel) ctrl := gomock.NewController(t) repo, err := localdir.MakeLocalDirRepo(tt.inputFolder) diff --git a/checks/raw/security_policy.go b/checks/raw/security_policy.go index 36350637461..fbcdf9ecdae 100644 --- a/checks/raw/security_policy.go +++ b/checks/raw/security_policy.go @@ -16,7 +16,6 @@ package raw import ( "errors" - "fmt" "strings" "github.com/ossf/scorecard/v4/checker" @@ -70,10 +69,7 @@ func SecurityPolicy(c *checker.CheckRequest) (checker.SecurityPolicyData, error) } // https://docs.github.com/en/github/building-a-strong-community/creating-a-default-community-health-file. - logger, err := githubrepo.NewLogger(log.InfoLevel) - if err != nil { - return checker.SecurityPolicyData{}, fmt.Errorf("%w", err) - } + logger := log.NewLogger(log.InfoLevel) dotGitHub := &checker.CheckRequest{ Ctx: c.Ctx, Dlogger: c.Dlogger, diff --git a/clients/githubrepo/client.go b/clients/githubrepo/client.go index 60ea264f797..e21f3ca85f8 100644 --- a/clients/githubrepo/client.go +++ b/clients/githubrepo/client.go @@ -181,7 +181,7 @@ func (client *Client) Close() error { // CreateGithubRepoClient returns a Client which implements RepoClient interface. func CreateGithubRepoClient(ctx context.Context, logger *log.Logger) clients.RepoClient { // Use our custom roundtripper - rt := roundtripper.NewTransport(ctx, logger.Zap.Sugar()) + rt := roundtripper.NewTransport(ctx, logger) httpClient := &http.Client{ Transport: rt, } @@ -219,18 +219,6 @@ func CreateGithubRepoClient(ctx context.Context, logger *log.Logger) clients.Rep } } -// NewLogger creates an instance of *log.Logger. -// TODO(log): Consider removing this function, as it only serves to wrap -// `log.NewLogger` for convenience. -func NewLogger(logLevel log.Level) (*log.Logger, error) { - logger, err := log.NewLogger(logLevel) - if err != nil { - return nil, fmt.Errorf("creating GitHub repo client logger: %w", err) - } - - return logger, nil -} - // CreateOssFuzzRepoClient returns a RepoClient implementation // intialized to `google/oss-fuzz` GitHub repository. func CreateOssFuzzRepoClient(ctx context.Context, logger *log.Logger) (clients.RepoClient, error) { diff --git a/clients/githubrepo/roundtripper/rate_limit.go b/clients/githubrepo/roundtripper/rate_limit.go index b682d70b19c..8fb55af3f4b 100644 --- a/clients/githubrepo/roundtripper/rate_limit.go +++ b/clients/githubrepo/roundtripper/rate_limit.go @@ -20,13 +20,12 @@ import ( "strconv" "time" - "go.uber.org/zap" - sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/log" ) // MakeRateLimitedTransport returns a RoundTripper which rate limits GitHub requests. -func MakeRateLimitedTransport(innerTransport http.RoundTripper, logger *zap.SugaredLogger) http.RoundTripper { +func MakeRateLimitedTransport(innerTransport http.RoundTripper, logger *log.Logger) http.RoundTripper { return &rateLimitTransport{ logger: logger, innerTransport: innerTransport, @@ -35,7 +34,7 @@ func MakeRateLimitedTransport(innerTransport http.RoundTripper, logger *zap.Suga // rateLimitTransport is a rate-limit aware http.Transport for Github. type rateLimitTransport struct { - logger *zap.SugaredLogger + logger *log.Logger innerTransport http.RoundTripper } @@ -58,11 +57,13 @@ func (gh *rateLimitTransport) RoundTrip(r *http.Request) (*http.Response, error) } duration := time.Until(time.Unix(int64(reset), 0)) - gh.logger.Warnf("Rate limit exceeded. Waiting %s to retry...", duration) + // TODO(log): Previously Warn. Consider logging an error here. + gh.logger.Info(fmt.Sprintf("Rate limit exceeded. Waiting %s to retry...", duration)) // Retry time.Sleep(duration) - gh.logger.Warnf("Rate limit exceeded. Retrying...") + // TODO(log): Previously Warn. Consider logging an error here. + gh.logger.Info("Rate limit exceeded. Retrying...") return gh.RoundTrip(r) } diff --git a/clients/githubrepo/roundtripper/roundtripper.go b/clients/githubrepo/roundtripper/roundtripper.go index d8fc3374f43..bdead649b5d 100644 --- a/clients/githubrepo/roundtripper/roundtripper.go +++ b/clients/githubrepo/roundtripper/roundtripper.go @@ -17,15 +17,15 @@ package roundtripper import ( "context" - "log" + "fmt" "net/http" "os" "strconv" "github.com/bradleyfalzon/ghinstallation/v2" - "go.uber.org/zap" "github.com/ossf/scorecard/v4/clients/githubrepo/roundtripper/tokens" + "github.com/ossf/scorecard/v4/log" ) const ( @@ -38,7 +38,7 @@ const ( ) // NewTransport returns a configured http.Transport for use with GitHub. -func NewTransport(ctx context.Context, logger *zap.SugaredLogger) http.RoundTripper { +func NewTransport(ctx context.Context, logger *log.Logger) http.RoundTripper { transport := http.DefaultTransport // nolint @@ -48,19 +48,19 @@ func NewTransport(ctx context.Context, logger *zap.SugaredLogger) http.RoundTrip } else if keyPath := os.Getenv(githubAppKeyPath); keyPath != "" { // Also try a GITHUB_APP appID, err := strconv.Atoi(os.Getenv(githubAppID)) if err != nil { - log.Panic(err) + logger.Error(err, "getting GitHub application ID from environment") } installationID, err := strconv.Atoi(os.Getenv(githubAppInstallationID)) if err != nil { - log.Panic(err) + logger.Error(err, "getting GitHub application installation ID") } transport, err = ghinstallation.NewKeyFromFile(transport, int64(appID), int64(installationID), keyPath) if err != nil { - log.Panic(err) + logger.Error(err, "getting a private key from file") } } else { - log.Fatalf("GitHub token env var is not set. " + - "Please read https://github.com/ossf/scorecard#authentication") + // TODO(log): Improve error message + logger.Error(fmt.Errorf("an error occurred while getting GitHub credentials"), "GitHub token env var is not set. Please read https://github.com/ossf/scorecard#authentication") } return MakeCensusTransport(MakeRateLimitedTransport(transport, logger)) diff --git a/clients/localdir/client_test.go b/clients/localdir/client_test.go index d0f9914ddb2..f4fbb64c6bc 100644 --- a/clients/localdir/client_test.go +++ b/clients/localdir/client_test.go @@ -24,7 +24,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/ossf/scorecard/v4/clients/githubrepo" "github.com/ossf/scorecard/v4/log" ) @@ -63,12 +62,7 @@ func TestClient_CreationAndCaching(t *testing.T) { t.Parallel() ctx := context.Background() - logger, err := githubrepo.NewLogger(log.DebugLevel) - if err != nil { - t.Errorf("githubrepo.NewLogger: %v", err) - } - // nolint - defer logger.Zap.Sync() // Flushes buffer, if any. + logger := log.NewLogger(log.DebugLevel) // Create repo. repo, err := MakeLocalDirRepo(tt.inputFolder) diff --git a/cmd/root.go b/cmd/root.go index 4a481c22375..c38ebee4d7f 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -188,13 +188,7 @@ func scorecardCmd(cmd *cobra.Command, args []string) { } ctx := context.Background() - logger, err := githubrepo.NewLogger(sclog.Level(logLevel)) - if err != nil { - log.Panic(err) - } - // nolint: errcheck - defer logger.Zap.Sync() // Flushes buffer, if any. - + logger := sclog.NewLogger(sclog.Level(logLevel)) repoURI, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, repoType, err := getRepoAccessors(ctx, uri, logger) if err != nil { log.Panic(err) diff --git a/cmd/serve.go b/cmd/serve.go index 63cfdc52671..9d7e9c57dfa 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -17,7 +17,6 @@ package cmd import ( "fmt" "html/template" - "log" "net/http" "os" "strings" @@ -27,7 +26,7 @@ import ( "github.com/ossf/scorecard/v4/checks" "github.com/ossf/scorecard/v4/clients" "github.com/ossf/scorecard/v4/clients/githubrepo" - sclog "github.com/ossf/scorecard/v4/log" + "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" ) @@ -41,16 +40,13 @@ var serveCmd = &cobra.Command{ Short: "Serve the scorecard program over http", Long: ``, Run: func(cmd *cobra.Command, args []string) { - logger, err := githubrepo.NewLogger(sclog.Level(logLevel)) - if err != nil { - log.Fatalf("unable to construct logger: %v", err) - } - //nolint - defer logger.Zap.Sync() // flushes buffer, if any - sugar := logger.Zap.Sugar() + logger := log.NewLogger(log.Level(logLevel)) + t, err := template.New("webpage").Parse(tpl) if err != nil { - sugar.Panic(err) + // TODO(log): Should this actually panic? + logger.Error(err, "parsing webpage template") + panic(err) } http.HandleFunc("/", func(rw http.ResponseWriter, r *http.Request) { @@ -69,7 +65,7 @@ var serveCmd = &cobra.Command{ ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(ctx, logger) vulnsClient := clients.DefaultVulnerabilitiesClient() if err != nil { - sugar.Error(err) + logger.Error(err, "initializing clients") rw.WriteHeader(http.StatusInternalServerError) } defer ossFuzzRepoClient.Close() @@ -77,19 +73,21 @@ var serveCmd = &cobra.Command{ repoResult, err := pkg.RunScorecards(ctx, repo, false, checks.AllChecks, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient) if err != nil { - sugar.Error(err) + logger.Error(err, "running enabled scorecard checks on repo") rw.WriteHeader(http.StatusInternalServerError) } if r.Header.Get("Content-Type") == "application/json" { - if err := repoResult.AsJSON(showDetails, sclog.Level(logLevel), rw); err != nil { - sugar.Error(err) + if err := repoResult.AsJSON(showDetails, log.Level(logLevel), rw); err != nil { + // TODO(log): Improve error message + logger.Error(err, "") rw.WriteHeader(http.StatusInternalServerError) } return } if err := t.Execute(rw, repoResult); err != nil { - sugar.Warn(err) + // TODO(log): Improve error message + logger.Error(err, "") } }) port := os.Getenv("PORT") @@ -99,7 +97,9 @@ var serveCmd = &cobra.Command{ fmt.Printf("Listening on localhost:%s\n", port) err = http.ListenAndServe(fmt.Sprintf("0.0.0.0:%s", port), nil) if err != nil { - log.Fatal("ListenAndServe ", err) + // TODO(log): Should this actually panic? + logger.Error(err, "listening and serving") + panic(err) } }, } diff --git a/cron/worker/main.go b/cron/worker/main.go index 8aeb9f46639..6c025d551a8 100644 --- a/cron/worker/main.go +++ b/cron/worker/main.go @@ -68,7 +68,7 @@ func processRequest(ctx context.Context, return fmt.Errorf("error during BlobExists: %w", err) } if exists1 && exists2 { - logger.Zap.Info(fmt.Sprintf("Already processed shard %s. Nothing to do.", filename)) + logger.Info(fmt.Sprintf("Already processed shard %s. Nothing to do.", filename)) // We have already processed this request, nothing to do. return nil } @@ -77,10 +77,11 @@ func processRequest(ctx context.Context, var buffer2 bytes.Buffer // TODO: run Scorecard for each repo in a separate thread. for _, repo := range batchRequest.GetRepos() { - logger.Zap.Info(fmt.Sprintf("Running Scorecard for repo: %s", *repo.Url)) + logger.Info(fmt.Sprintf("Running Scorecard for repo: %s", *repo.Url)) repo, err := githubrepo.MakeGithubRepo(*repo.Url) if err != nil { - logger.Zap.Warn(fmt.Sprintf("invalid GitHub URL: %v", err)) + // TODO(log): Previously Warn. Consider logging an error here. + logger.Info(fmt.Sprintf("invalid GitHub URL: %v", err)) continue } repo.AppendMetadata(repo.Metadata()...) @@ -103,7 +104,8 @@ func processRequest(ctx context.Context, // nolint: goerr113 return errors.New(errorMsg) } - logger.Zap.Warn(errorMsg) + // TODO(log): Previously Warn. Consider logging an error here. + logger.Info(errorMsg) } result.Date = batchRequest.GetJobTime().AsTime() if err := format.AsJSON(&result, true /*showDetails*/, log.InfoLevel, &buffer); err != nil { @@ -122,7 +124,7 @@ func processRequest(ctx context.Context, return fmt.Errorf("error during WriteToBlobStore2: %w", err) } - logger.Zap.Info(fmt.Sprintf("Write to shard file successful: %s", filename)) + logger.Info(fmt.Sprintf("Write to shard file successful: %s", filename)) return nil } @@ -185,10 +187,7 @@ func main() { panic(err) } - logger, err := githubrepo.NewLogger(log.InfoLevel) - if err != nil { - panic(err) - } + logger := log.NewLogger(log.InfoLevel) repoClient := githubrepo.CreateGithubRepoClient(ctx, logger) ciiClient := clients.BlobCIIBestPracticesClient(ciiDataBucketURL) ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(ctx, logger) @@ -206,7 +205,8 @@ func main() { // Exposed for monitoring runtime profiles go func() { - logger.Zap.Fatal(fmt.Sprintf("%v", http.ListenAndServe(":8080", nil))) + // TODO(log): Previously Fatal. Need to handle the error here. + logger.Info(fmt.Sprintf("%v", http.ListenAndServe(":8080", nil))) }() checksToRun := checks.AllChecks @@ -218,21 +218,23 @@ func main() { if err != nil { panic(err) } - logger.Zap.Info("Received message from subscription") + + logger.Info("Received message from subscription") if req == nil { - logger.Zap.Warn("subscription returned nil message during Receive, exiting") + // TODO(log): Previously Warn. Consider logging an error here. + logger.Info("subscription returned nil message during Receive, exiting") break } if err := processRequest(ctx, req, checksToRun, bucketURL, bucketURL2, checkDocs, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, logger); err != nil { - logger.Zap.Warn(fmt.Sprintf("error processing request: %v", err)) + // TODO(log): Previously Warn. Consider logging an error here. + logger.Info(fmt.Sprintf("error processing request: %v", err)) // Nack the message so that another worker can retry. subscriber.Nack() continue } - // nolint: errcheck // flushes buffer - logger.Zap.Sync() + exporter.Flush() subscriber.Ack() } diff --git a/e2e/e2e_suite_test.go b/e2e/e2e_suite_test.go index 820d02e4337..e1956afb7f5 100644 --- a/e2e/e2e_suite_test.go +++ b/e2e/e2e_suite_test.go @@ -21,7 +21,6 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/ossf/scorecard/v4/clients/githubrepo" "github.com/ossf/scorecard/v4/log" ) @@ -41,8 +40,7 @@ var _ = BeforeSuite(func() { "GITHUB_AUTH_TOKEN env variable is not set.The GITHUB_AUTH_TOKEN env variable has to be set to run e2e test.") Expect(len(token)).ShouldNot(BeZero(), "Length of the GITHUB_AUTH_TOKEN env variable is zero.") - l, err := githubrepo.NewLogger(log.InfoLevel) - Expect(err).Should(BeNil()) + l := log.NewLogger(log.InfoLevel) logger = l }) diff --git a/go.mod b/go.mod index def2959f34c..8819ba15d85 100644 --- a/go.mod +++ b/go.mod @@ -2,14 +2,23 @@ module github.com/ossf/scorecard/v4 go 1.17 +// TODO(go.mod): Is there a reason these deps are kept separately from the +// other `require`s? +require ( + github.com/rhysd/actionlint v1.6.8 + gotest.tools v2.2.0+incompatible +) + require ( cloud.google.com/go/bigquery v1.27.0 cloud.google.com/go/monitoring v0.1.0 // indirect cloud.google.com/go/pubsub v1.17.0 cloud.google.com/go/trace v0.1.0 // indirect contrib.go.opencensus.io/exporter/stackdriver v0.13.8 + github.com/bombsimon/logrusr/v2 v2.0.1 github.com/bradleyfalzon/ghinstallation/v2 v2.0.4 github.com/go-git/go-git/v5 v5.4.2 + github.com/go-logr/logr v1.2.2 github.com/golang/mock v1.6.0 github.com/google/go-cmp v0.5.7 github.com/google/go-containerregistry v0.8.0 @@ -22,10 +31,10 @@ require ( github.com/onsi/gomega v1.18.0 github.com/shurcooL/githubv4 v0.0.0-20201206200315-234843c633fa github.com/shurcooL/graphql v0.0.0-20200928012149-18c5c3165e3a // indirect + github.com/sirupsen/logrus v1.8.1 github.com/spf13/cobra v1.3.0 github.com/xeipuuv/gojsonschema v0.0.0-20180618132009-1d523034197f go.opencensus.io v0.23.0 - go.uber.org/zap v1.20.0 gocloud.dev v0.24.0 golang.org/x/tools v0.1.8 google.golang.org/genproto v0.0.0-20220111164026-67b88f271998 @@ -35,11 +44,6 @@ require ( mvdan.cc/sh/v3 v3.4.2 ) -require ( - github.com/rhysd/actionlint v1.6.8 - gotest.tools v2.2.0+incompatible -) - require ( cloud.google.com/go v0.100.2 // indirect cloud.google.com/go/compute v0.1.0 // indirect @@ -86,14 +90,11 @@ require ( github.com/rivo/uniseg v0.2.0 // indirect github.com/robfig/cron v1.2.0 // indirect github.com/sergi/go-diff v1.1.0 // indirect - github.com/sirupsen/logrus v1.8.1 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/vbatts/tar-split v0.11.2 // indirect github.com/xanzy/ssh-agent v0.3.0 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect - go.uber.org/atomic v1.9.0 // indirect - go.uber.org/multierr v1.7.0 // indirect golang.org/x/crypto v0.0.0-20210817164053-32db794688a5 // indirect golang.org/x/net v0.0.0-20211216030914-fe4d6282115f // indirect golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8 // indirect diff --git a/go.sum b/go.sum index 28e3e489911..fed2206b289 100644 --- a/go.sum +++ b/go.sum @@ -283,7 +283,6 @@ github.com/aws/aws-sdk-go-v2/service/sts v1.7.0/go.mod h1:0qcSMCyASQPN2sk/1KQLQ2 github.com/aws/smithy-go v1.8.0 h1:AEwwwXQZtUwP5Mz506FeXXrKBe0jA8gVM+1gEcSRooc= github.com/aws/smithy-go v1.8.0/go.mod h1:SObp3lf9smib00L/v3U2eAKG8FyQ7iLrJnQiAmR5n+E= github.com/aybabtme/rgbterm v0.0.0-20170906152045-cc83f3b3ce59/go.mod h1:q/89r3U2H7sSsE2t6Kca0lfwTK8JdoNGS/yzM/4iH5I= -github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8= github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v0.0.0-20160804104726-4c0e84591b9a/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= @@ -298,6 +297,8 @@ github.com/blang/semver v3.1.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnweb github.com/blang/semver v3.5.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4= +github.com/bombsimon/logrusr/v2 v2.0.1 h1:1VgxVNQMCvjirZIYaT9JYn6sAVGVEcNtRE0y4mvaOAM= +github.com/bombsimon/logrusr/v2 v2.0.1/go.mod h1:ByVAX+vHdLGAfdroiMg6q0zgq2FODY2lc5YJvzmOJio= github.com/bombsimon/wsl/v2 v2.0.0/go.mod h1:mf25kr/SqFEPhhcxW1+7pxzGlW+hIl/hYTKY95VwV8U= github.com/bombsimon/wsl/v2 v2.2.0/go.mod h1:Azh8c3XGEJl9LyX0/sFC+CKMc7Ssgua0g+6abzXN4Pg= github.com/bombsimon/wsl/v3 v3.0.0/go.mod h1:st10JtZYLE4D5sC7b8xV4zTKZwAQjCH/Hy2Pm1FNZIc= @@ -584,6 +585,9 @@ github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9 github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= github.com/go-logr/logr v0.1.0/go.mod h1:ixOQHD9gLJUVQQ2ZOR7zLEifBX6tGkNJF4QyIY7sIas= github.com/go-logr/logr v0.2.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU= +github.com/go-logr/logr v1.0.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/logr v1.2.2 h1:ahHml/yUpnlb96Rp8HCvtYVPY8ZYpxq3g7UYchIYwbs= +github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-ole/go-ole v1.2.1/go.mod h1:7FAglXiTm7HKlQRDeOQ6ZNUHidzCWXuZWq/1dTyBNF8= github.com/go-openapi/jsonpointer v0.0.0-20160704185906-46af16f9f7b1/go.mod h1:+35s3my2LFTysnkMfxsJBAMHj/DoqoB9knIWoYG/Vk0= github.com/go-openapi/jsonpointer v0.19.2/go.mod h1:3akKfEdA7DF1sugOqz1dVQHBcuDBPKZGEoHC/NkiQRg= @@ -1414,21 +1418,15 @@ go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqe go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= -go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE= go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= -go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI= -go.uber.org/goleak v1.1.11/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= -go.uber.org/multierr v1.7.0 h1:zaiO/rmgFjbmCXdSYJWQcdvOCsthmdaHfr3Gm2Kx4Ec= go.uber.org/multierr v1.7.0/go.mod h1:7EAYxJLBy9rStEaz58O2t4Uvip6FSURkq8/ppBp95ak= go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= go.uber.org/zap v1.17.0/go.mod h1:MXVU+bhUf/A7Xi2HNOnopQOrmycQ5Ih87HtOu4q5SSo= go.uber.org/zap v1.18.1/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI= go.uber.org/zap v1.19.0/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI= -go.uber.org/zap v1.20.0 h1:N4oPlghZwYG55MlU6LXk/Zp00FVNE9X9wrYO8CEs4lc= -go.uber.org/zap v1.20.0/go.mod h1:wjWOCqI0f2ZZrJF/UufIOkiC8ii6tm1iqIsLo76RfJw= go4.org v0.0.0-20180809161055-417644f6feb5/go.mod h1:MkTOUMDaeVYJUOUsaDXIhWPZYa1yOyC1qaOBpL57BhE= gocloud.dev v0.19.0/go.mod h1:SmKwiR8YwIMMJvQBKLsC3fHNyMwXLw3PMDO+VVteJMI= gocloud.dev v0.24.0 h1:cNtHD07zQQiv02OiwwDyVMuHmR7iQt2RLkzoAgz7wBs= @@ -1714,6 +1712,7 @@ golang.org/x/sys v0.0.0-20210503080704-8803ae5d1324/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210514084401-e8d321eab015/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210603125802-9665404d3644/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210608053332-aa57babbf139/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/log/log.go b/log/log.go index 545fd8fa5b8..b217ca4c911 100644 --- a/log/log.go +++ b/log/log.go @@ -15,54 +15,76 @@ package log import ( - "fmt" + "log" + "strings" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" + "github.com/bombsimon/logrusr/v2" + "github.com/go-logr/logr" + "github.com/sirupsen/logrus" ) -// Logger exposes logging capabilities. -// The struct name was chosen to closely mimic other logging facilities within -// to project to make them easier to search/replace. -// Initial implementation was designed to encapsulate calls to `zap`, but -// future iterations will seek to directly expose logging methods. +// Logger exposes logging capabilities using +// https://pkg.go.dev/github.com/go-logr/logr. type Logger struct { - Zap *zap.Logger + *logr.Logger } -// NewLogger creates an instance of *zap.Logger. -// Copied from clients/githubrepo/client.go. -func NewLogger(logLevel Level) (*Logger, error) { - zapCfg := zap.NewProductionConfig() - zapLevel := parseLogLevelZap(string(logLevel)) - zapCfg.Level.SetLevel(zapLevel) +// NewLogger creates an instance of *Logger. +// TODO(log): Consider adopting production config from zap. +func NewLogger(logLevel Level) *Logger { + logrusLog := logrus.New() - zapLogger, err := zapCfg.Build() - if err != nil { - return nil, fmt.Errorf("configuring zap logger: %w", err) - } + // Set log level from logrus + logrusLevel := parseLogrusLevel(logLevel) + logrusLog.SetLevel(logrusLevel) + logrLogger := logrusr.New(logrusLog) logger := &Logger{ - Zap: zapLogger, + &logrLogger, + } + + return logger +} + +// ParseLevel takes a string level and returns the Logrus log level constant. +// If the level is not recognized, it defaults to `logrus.InfoLevel` to swallow +// potential configuration errors/typos when specifying log levels. +// https://pkg.go.dev/github.com/sirupsen/logrus#ParseLevel +func ParseLevel(lvl string) logrus.Level { + logLevel := DefaultLevel + + switch strings.ToLower(lvl) { + case "panic": + logLevel = PanicLevel + case "fatal": + logLevel = FatalLevel + case "error": + logLevel = ErrorLevel + case "warn": + logLevel = WarnLevel + case "info": + logLevel = InfoLevel + case "debug": + logLevel = DebugLevel + case "trace": + logLevel = TraceLevel } - return logger, nil + return parseLogrusLevel(logLevel) } // Level is a string representation of log level, which can easily be passed as // a parameter, in lieu of defined types in upstream logging packages. type Level string -// Log levels -// TODO(log): Revisit if all levels are required. The current list mimics zap -// log levels. +// Log levels. const ( DefaultLevel = InfoLevel + TraceLevel Level = "trace" DebugLevel Level = "debug" InfoLevel Level = "info" WarnLevel Level = "warn" ErrorLevel Level = "error" - DPanicLevel Level = "dpanic" PanicLevel Level = "panic" FatalLevel Level = "fatal" ) @@ -71,30 +93,17 @@ func (l Level) String() string { return string(l) } -// parseLogLevelZap parses a log level string and returning a zapcore.Level, -// which defaults to `zapcore.InfoLevel` when the provided string is not -// recognized. -// It is an inversion of go.uber.org/zap/zapcore.Level.String(). -// TODO(log): Should we include a strict option here, which fails if the -// provided log level is not recognized or is it fine to default to -// InfoLevel? -func parseLogLevelZap(level string) zapcore.Level { - switch level { - case "debug": - return zapcore.DebugLevel - case "info": - return zapcore.InfoLevel - case "warn": - return zapcore.WarnLevel - case "error": - return zapcore.ErrorLevel - case "dpanic": - return zapcore.DPanicLevel - case "panic": - return zapcore.PanicLevel - case "fatal": - return zapcore.FatalLevel - default: - return zapcore.InfoLevel +func parseLogrusLevel(lvl Level) logrus.Level { + logrusLevel, err := logrus.ParseLevel(lvl.String()) + if err != nil { + log.Printf( + "defaulting to INFO log level, as %s is not a valid log level: %+v", + lvl, + err, + ) + + logrusLevel = logrus.InfoLevel } + + return logrusLevel }