From 13b78ab0108ff6be6728f38b674199bc62c39589 Mon Sep 17 00:00:00 2001 From: "Stephen Augustus (he/him)" Date: Thu, 20 Jan 2022 18:57:39 -0500 Subject: [PATCH] :warning: Create a dedicated logging package to encapsulate calls to `zap` (#1502) * log: Init log package Creates a wrapper around existing `zap.Logger` to make it easier to replace/extend with scorecard logging. Signed-off-by: Stephen Augustus * log: Replace instances of `zap.Logger` with `log.Logger` Signed-off-by: Stephen Augustus * log: Add logic to parse `zapcore.Level`s as strings Signed-off-by: Stephen Augustus * log: Express log levels Signed-off-by: Stephen Augustus * log: Replace instances of `zapcore.Level` with `log.Level` Signed-off-by: Stephen Augustus * log: Fixup comments for exported functions Signed-off-by: Stephen Augustus --- checks/binary_artifact_test.go | 6 +- checks/license_test.go | 6 +- checks/raw/security_policy.go | 5 +- clients/githubrepo/client.go | 22 +++---- clients/localdir/client.go | 7 +-- clients/localdir/client_test.go | 6 +- cmd/root.go | 26 +++++---- cmd/serve.go | 9 +-- cron/format/json.go | 7 +-- cron/format/json_test.go | 16 ++--- cron/worker/main.go | 31 +++++----- e2e/e2e_suite_test.go | 6 +- log/log.go | 100 ++++++++++++++++++++++++++++++++ pkg/common.go | 11 ++-- pkg/json.go | 7 +-- pkg/json_test.go | 16 ++--- pkg/sarif.go | 7 +-- pkg/sarif_test.go | 21 ++++--- pkg/scorecard_result.go | 4 +- 19 files changed, 204 insertions(+), 109 deletions(-) create mode 100644 log/log.go diff --git a/checks/binary_artifact_test.go b/checks/binary_artifact_test.go index 52db9129ae5..bb8b9e3d249 100644 --- a/checks/binary_artifact_test.go +++ b/checks/binary_artifact_test.go @@ -20,11 +20,11 @@ import ( "testing" "github.com/golang/mock/gomock" - "go.uber.org/zap/zapcore" "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 +60,13 @@ func TestBinaryArtifacts(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - logger, err := githubrepo.NewLogger(zapcore.DebugLevel) + logger, err := githubrepo.NewLogger(log.DebugLevel) if err != nil { t.Errorf("githubrepo.NewLogger: %v", err) } // nolint - defer logger.Sync() + defer logger.Zap.Sync() ctrl := gomock.NewController(t) repo, err := localdir.MakeLocalDirRepo(tt.inputFolder) diff --git a/checks/license_test.go b/checks/license_test.go index 25a5045d2b9..dd91ac01987 100644 --- a/checks/license_test.go +++ b/checks/license_test.go @@ -20,11 +20,11 @@ import ( "testing" "github.com/golang/mock/gomock" - "go.uber.org/zap/zapcore" "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 +142,13 @@ func TestLicenseFileSubdirectory(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - logger, err := githubrepo.NewLogger(zapcore.DebugLevel) + logger, err := githubrepo.NewLogger(log.DebugLevel) if err != nil { t.Errorf("githubrepo.NewLogger: %v", err) } // nolint - defer logger.Sync() + defer logger.Zap.Sync() 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 19aa684d975..36350637461 100644 --- a/checks/raw/security_policy.go +++ b/checks/raw/security_policy.go @@ -19,12 +19,11 @@ import ( "fmt" "strings" - "go.uber.org/zap" - "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/checks/fileparser" "github.com/ossf/scorecard/v4/clients/githubrepo" sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/log" ) // SecurityPolicy checks for presence of security policy. @@ -71,7 +70,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(zap.InfoLevel) + logger, err := githubrepo.NewLogger(log.InfoLevel) if err != nil { return checker.SecurityPolicyData{}, fmt.Errorf("%w", err) } diff --git a/clients/githubrepo/client.go b/clients/githubrepo/client.go index 5713dcfa3b1..60ea264f797 100644 --- a/clients/githubrepo/client.go +++ b/clients/githubrepo/client.go @@ -23,12 +23,11 @@ import ( "github.com/google/go-github/v38/github" "github.com/shurcooL/githubv4" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" "github.com/ossf/scorecard/v4/clients" "github.com/ossf/scorecard/v4/clients/githubrepo/roundtripper" sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/log" ) var errInputRepoType = errors.New("input repo should be of type repoURL") @@ -180,9 +179,9 @@ func (client *Client) Close() error { } // CreateGithubRepoClient returns a Client which implements RepoClient interface. -func CreateGithubRepoClient(ctx context.Context, logger *zap.Logger) clients.RepoClient { +func CreateGithubRepoClient(ctx context.Context, logger *log.Logger) clients.RepoClient { // Use our custom roundtripper - rt := roundtripper.NewTransport(ctx, logger.Sugar()) + rt := roundtripper.NewTransport(ctx, logger.Zap.Sugar()) httpClient := &http.Client{ Transport: rt, } @@ -220,20 +219,21 @@ func CreateGithubRepoClient(ctx context.Context, logger *zap.Logger) clients.Rep } } -// NewLogger creates an instance of *zap.Logger. -func NewLogger(logLevel zapcore.Level) (*zap.Logger, error) { - cfg := zap.NewProductionConfig() - cfg.Level.SetLevel(logLevel) - logger, err := cfg.Build() +// 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("cfg.Build: %w", err) + 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 *zap.Logger) (clients.RepoClient, error) { +func CreateOssFuzzRepoClient(ctx context.Context, logger *log.Logger) (clients.RepoClient, error) { ossFuzzRepo, err := MakeGithubRepo("google/oss-fuzz") if err != nil { return nil, fmt.Errorf("error during githubrepo.MakeGithubRepo: %w", err) diff --git a/clients/localdir/client.go b/clients/localdir/client.go index 5634ec425ae..27e5c0f30e8 100644 --- a/clients/localdir/client.go +++ b/clients/localdir/client.go @@ -27,16 +27,15 @@ import ( "strings" "sync" - "go.uber.org/zap" - clients "github.com/ossf/scorecard/v4/clients" + "github.com/ossf/scorecard/v4/log" ) var errInputRepoType = errors.New("input repo should be of type repoLocal") //nolint:govet type localDirClient struct { - logger *zap.Logger + logger *log.Logger ctx context.Context path string once sync.Once @@ -214,7 +213,7 @@ func (client *localDirClient) Close() error { } // CreateLocalDirClient returns a client which implements RepoClient interface. -func CreateLocalDirClient(ctx context.Context, logger *zap.Logger) clients.RepoClient { +func CreateLocalDirClient(ctx context.Context, logger *log.Logger) clients.RepoClient { return &localDirClient{ ctx: ctx, logger: logger, diff --git a/clients/localdir/client_test.go b/clients/localdir/client_test.go index 9fb0509488b..d0f9914ddb2 100644 --- a/clients/localdir/client_test.go +++ b/clients/localdir/client_test.go @@ -23,9 +23,9 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "go.uber.org/zap/zapcore" "github.com/ossf/scorecard/v4/clients/githubrepo" + "github.com/ossf/scorecard/v4/log" ) func TestClient_CreationAndCaching(t *testing.T) { @@ -63,12 +63,12 @@ func TestClient_CreationAndCaching(t *testing.T) { t.Parallel() ctx := context.Background() - logger, err := githubrepo.NewLogger(zapcore.DebugLevel) + logger, err := githubrepo.NewLogger(log.DebugLevel) if err != nil { t.Errorf("githubrepo.NewLogger: %v", err) } // nolint - defer logger.Sync() // Flushes buffer, if any. + defer logger.Zap.Sync() // Flushes buffer, if any. // Create repo. repo, err := MakeLocalDirRepo(tt.inputFolder) diff --git a/cmd/root.go b/cmd/root.go index e5cc748e344..4a481c22375 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -18,7 +18,6 @@ package cmd import ( "context" "encoding/json" - goflag "flag" "fmt" "log" "net/http" @@ -28,7 +27,6 @@ import ( "time" "github.com/spf13/cobra" - "go.uber.org/zap" "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/checks" @@ -37,6 +35,7 @@ import ( "github.com/ossf/scorecard/v4/clients/localdir" docs "github.com/ossf/scorecard/v4/docs/checks" sce "github.com/ossf/scorecard/v4/errors" + sclog "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" spol "github.com/ossf/scorecard/v4/policy" ) @@ -47,8 +46,7 @@ var ( local string checksToRun []string metaData []string - // This one has to use goflag instead of pflag because it's defined by zap. - logLevel = zap.LevelFlag("verbosity", zap.InfoLevel, "override the default log level") + logLevel string format string npm string pypi string @@ -84,10 +82,14 @@ const cliEnableSarif = "ENABLE_SARIF" //nolint:gochecknoinits func init() { - // Add the zap flag manually - rootCmd.PersistentFlags().AddGoFlagSet(goflag.CommandLine) rootCmd.Flags().StringVar(&repo, "repo", "", "repository to check") rootCmd.Flags().StringVar(&local, "local", "", "local folder to check") + rootCmd.Flags().StringVar( + &logLevel, + "verbosity", + sclog.DefaultLevel.String(), + "set the log level", + ) rootCmd.Flags().StringVar( &npm, "npm", "", "npm package to check, given that the npm package has a GitHub repository") @@ -186,12 +188,12 @@ func scorecardCmd(cmd *cobra.Command, args []string) { } ctx := context.Background() - logger, err := githubrepo.NewLogger(*logLevel) + logger, err := githubrepo.NewLogger(sclog.Level(logLevel)) if err != nil { log.Panic(err) } // nolint: errcheck - defer logger.Sync() // Flushes buffer, if any. + defer logger.Zap.Sync() // Flushes buffer, if any. repoURI, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, repoType, err := getRepoAccessors(ctx, uri, logger) if err != nil { @@ -249,15 +251,15 @@ func scorecardCmd(cmd *cobra.Command, args []string) { switch format { case formatDefault: - err = repoResult.AsString(showDetails, *logLevel, checkDocs, os.Stdout) + err = repoResult.AsString(showDetails, sclog.Level(logLevel), checkDocs, os.Stdout) case formatSarif: // TODO: support config files and update checker.MaxResultScore. - err = repoResult.AsSARIF(showDetails, *logLevel, os.Stdout, checkDocs, policy) + err = repoResult.AsSARIF(showDetails, sclog.Level(logLevel), os.Stdout, checkDocs, policy) case formatJSON: if raw { err = repoResult.AsRawJSON(os.Stdout) } else { - err = repoResult.AsJSON2(showDetails, *logLevel, checkDocs, os.Stdout) + err = repoResult.AsJSON2(showDetails, sclog.Level(logLevel), checkDocs, os.Stdout) } default: @@ -413,7 +415,7 @@ func validateFormat(format string) bool { } } -func getRepoAccessors(ctx context.Context, uri string, logger *zap.Logger) ( +func getRepoAccessors(ctx context.Context, uri string, logger *sclog.Logger) ( repo clients.Repo, repoClient clients.RepoClient, ossFuzzRepoClient clients.RepoClient, diff --git a/cmd/serve.go b/cmd/serve.go index 39540d223fe..63cfdc52671 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -27,6 +27,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/pkg" ) @@ -40,13 +41,13 @@ var serveCmd = &cobra.Command{ Short: "Serve the scorecard program over http", Long: ``, Run: func(cmd *cobra.Command, args []string) { - logger, err := githubrepo.NewLogger(*logLevel) + logger, err := githubrepo.NewLogger(sclog.Level(logLevel)) if err != nil { log.Fatalf("unable to construct logger: %v", err) } //nolint - defer logger.Sync() // flushes buffer, if any - sugar := logger.Sugar() + defer logger.Zap.Sync() // flushes buffer, if any + sugar := logger.Zap.Sugar() t, err := template.New("webpage").Parse(tpl) if err != nil { sugar.Panic(err) @@ -81,7 +82,7 @@ var serveCmd = &cobra.Command{ } if r.Header.Get("Content-Type") == "application/json" { - if err := repoResult.AsJSON(showDetails, *logLevel, rw); err != nil { + if err := repoResult.AsJSON(showDetails, sclog.Level(logLevel), rw); err != nil { sugar.Error(err) rw.WriteHeader(http.StatusInternalServerError) } diff --git a/cron/format/json.go b/cron/format/json.go index 8c54030ec31..bf05d8690b0 100644 --- a/cron/format/json.go +++ b/cron/format/json.go @@ -22,10 +22,9 @@ import ( // nolint:gosec _ "net/http/pprof" - "go.uber.org/zap/zapcore" - docs "github.com/ossf/scorecard/v4/docs/checks" sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" ) @@ -87,7 +86,7 @@ type jsonScorecardResultV2 struct { } // AsJSON exports results as JSON for new detail format. -func AsJSON(r *pkg.ScorecardResult, showDetails bool, logLevel zapcore.Level, writer io.Writer) error { +func AsJSON(r *pkg.ScorecardResult, showDetails bool, logLevel log.Level, writer io.Writer) error { encoder := json.NewEncoder(writer) out := jsonScorecardResult{ @@ -123,7 +122,7 @@ func AsJSON(r *pkg.ScorecardResult, showDetails bool, logLevel zapcore.Level, wr // AsJSON2 exports results as JSON for the cron job and in the new detail format. func AsJSON2(r *pkg.ScorecardResult, showDetails bool, - logLevel zapcore.Level, checkDocs docs.Doc, writer io.Writer) error { + logLevel log.Level, checkDocs docs.Doc, writer io.Writer) error { score, err := r.GetAggregateScore(checkDocs) if err != nil { //nolint:wrapcheck diff --git a/cron/format/json_test.go b/cron/format/json_test.go index c0a87f32949..6f6f05a7936 100644 --- a/cron/format/json_test.go +++ b/cron/format/json_test.go @@ -24,9 +24,9 @@ import ( "time" "github.com/xeipuuv/gojsonschema" - "go.uber.org/zap/zapcore" "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" ) @@ -84,14 +84,14 @@ func TestJSONOutput(t *testing.T) { name string expected string showDetails bool - logLevel zapcore.Level + logLevel log.Level result pkg.ScorecardResult }{ { name: "check-1", showDetails: true, expected: "./testdata/check1.json", - logLevel: zapcore.DebugLevel, + logLevel: log.DebugLevel, result: pkg.ScorecardResult{ Repo: pkg.RepoInfo{ Name: repoName, @@ -130,7 +130,7 @@ func TestJSONOutput(t *testing.T) { name: "check-2", showDetails: true, expected: "./testdata/check2.json", - logLevel: zapcore.DebugLevel, + logLevel: log.DebugLevel, result: pkg.ScorecardResult{ Repo: pkg.RepoInfo{ Name: repoName, @@ -168,7 +168,7 @@ func TestJSONOutput(t *testing.T) { name: "check-3", showDetails: true, expected: "./testdata/check3.json", - logLevel: zapcore.InfoLevel, + logLevel: log.InfoLevel, result: pkg.ScorecardResult{ Repo: pkg.RepoInfo{ Name: repoName, @@ -268,7 +268,7 @@ func TestJSONOutput(t *testing.T) { name: "check-4", showDetails: true, expected: "./testdata/check4.json", - logLevel: zapcore.DebugLevel, + logLevel: log.DebugLevel, result: pkg.ScorecardResult{ Repo: pkg.RepoInfo{ Name: repoName, @@ -368,7 +368,7 @@ func TestJSONOutput(t *testing.T) { name: "check-5", showDetails: true, expected: "./testdata/check5.json", - logLevel: zapcore.WarnLevel, + logLevel: log.WarnLevel, result: pkg.ScorecardResult{ Repo: pkg.RepoInfo{ Name: repoName, @@ -407,7 +407,7 @@ func TestJSONOutput(t *testing.T) { name: "check-6", showDetails: true, expected: "./testdata/check6.json", - logLevel: zapcore.WarnLevel, + logLevel: log.WarnLevel, result: pkg.ScorecardResult{ Repo: pkg.RepoInfo{ Name: repoName, diff --git a/cron/worker/main.go b/cron/worker/main.go index e3c93bc8db4..8aeb9f46639 100644 --- a/cron/worker/main.go +++ b/cron/worker/main.go @@ -27,8 +27,6 @@ import ( _ "net/http/pprof" "go.opencensus.io/stats/view" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/checks" @@ -42,6 +40,7 @@ import ( "github.com/ossf/scorecard/v4/cron/pubsub" docs "github.com/ossf/scorecard/v4/docs/checks" sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" "github.com/ossf/scorecard/v4/stats" ) @@ -54,7 +53,7 @@ func processRequest(ctx context.Context, repoClient clients.RepoClient, ossFuzzRepoClient clients.RepoClient, ciiClient clients.CIIBestPracticesClient, vulnsClient clients.VulnerabilitiesClient, - logger *zap.Logger) error { + logger *log.Logger) error { filename := data.GetBlobFilename( fmt.Sprintf("shard-%07d", batchRequest.GetShardNum()), batchRequest.GetJobTime().AsTime()) @@ -69,7 +68,7 @@ func processRequest(ctx context.Context, return fmt.Errorf("error during BlobExists: %w", err) } if exists1 && exists2 { - logger.Info(fmt.Sprintf("Already processed shard %s. Nothing to do.", filename)) + logger.Zap.Info(fmt.Sprintf("Already processed shard %s. Nothing to do.", filename)) // We have already processed this request, nothing to do. return nil } @@ -78,10 +77,10 @@ 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.Info(fmt.Sprintf("Running Scorecard for repo: %s", *repo.Url)) + logger.Zap.Info(fmt.Sprintf("Running Scorecard for repo: %s", *repo.Url)) repo, err := githubrepo.MakeGithubRepo(*repo.Url) if err != nil { - logger.Warn(fmt.Sprintf("invalid GitHub URL: %v", err)) + logger.Zap.Warn(fmt.Sprintf("invalid GitHub URL: %v", err)) continue } repo.AppendMetadata(repo.Metadata()...) @@ -104,14 +103,14 @@ func processRequest(ctx context.Context, // nolint: goerr113 return errors.New(errorMsg) } - logger.Warn(errorMsg) + logger.Zap.Warn(errorMsg) } result.Date = batchRequest.GetJobTime().AsTime() - if err := format.AsJSON(&result, true /*showDetails*/, zapcore.InfoLevel, &buffer); err != nil { + if err := format.AsJSON(&result, true /*showDetails*/, log.InfoLevel, &buffer); err != nil { return fmt.Errorf("error during result.AsJSON: %w", err) } - if err := format.AsJSON2(&result, true /*showDetails*/, zapcore.InfoLevel, checkDocs, &buffer2); err != nil { + if err := format.AsJSON2(&result, true /*showDetails*/, log.InfoLevel, checkDocs, &buffer2); err != nil { return fmt.Errorf("error during result.AsJSON2: %w", err) } } @@ -123,7 +122,7 @@ func processRequest(ctx context.Context, return fmt.Errorf("error during WriteToBlobStore2: %w", err) } - logger.Info(fmt.Sprintf("Write to shard file successful: %s", filename)) + logger.Zap.Info(fmt.Sprintf("Write to shard file successful: %s", filename)) return nil } @@ -186,7 +185,7 @@ func main() { panic(err) } - logger, err := githubrepo.NewLogger(zap.InfoLevel) + logger, err := githubrepo.NewLogger(log.InfoLevel) if err != nil { panic(err) } @@ -207,7 +206,7 @@ func main() { // Exposed for monitoring runtime profiles go func() { - logger.Fatal(fmt.Sprintf("%v", http.ListenAndServe(":8080", nil))) + logger.Zap.Fatal(fmt.Sprintf("%v", http.ListenAndServe(":8080", nil))) }() checksToRun := checks.AllChecks @@ -219,21 +218,21 @@ func main() { if err != nil { panic(err) } - logger.Info("Received message from subscription") + logger.Zap.Info("Received message from subscription") if req == nil { - logger.Warn("subscription returned nil message during Receive, exiting") + logger.Zap.Warn("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.Warn(fmt.Sprintf("error processing request: %v", err)) + logger.Zap.Warn(fmt.Sprintf("error processing request: %v", err)) // Nack the message so that another worker can retry. subscriber.Nack() continue } // nolint: errcheck // flushes buffer - logger.Sync() + logger.Zap.Sync() exporter.Flush() subscriber.Ack() } diff --git a/e2e/e2e_suite_test.go b/e2e/e2e_suite_test.go index c1c7fcdd843..820d02e4337 100644 --- a/e2e/e2e_suite_test.go +++ b/e2e/e2e_suite_test.go @@ -20,12 +20,12 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "go.uber.org/zap" "github.com/ossf/scorecard/v4/clients/githubrepo" + "github.com/ossf/scorecard/v4/log" ) -var logger *zap.Logger +var logger *log.Logger func TestE2e(t *testing.T) { t.Parallel() @@ -41,7 +41,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(zap.InfoLevel) + l, err := githubrepo.NewLogger(log.InfoLevel) Expect(err).Should(BeNil()) logger = l }) diff --git a/log/log.go b/log/log.go new file mode 100644 index 00000000000..545fd8fa5b8 --- /dev/null +++ b/log/log.go @@ -0,0 +1,100 @@ +// Copyright 2022 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package log + +import ( + "fmt" + + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +// 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. +type Logger struct { + Zap *zap.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) + + zapLogger, err := zapCfg.Build() + if err != nil { + return nil, fmt.Errorf("configuring zap logger: %w", err) + } + + logger := &Logger{ + Zap: zapLogger, + } + + return logger, nil +} + +// 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. +const ( + DefaultLevel = InfoLevel + DebugLevel Level = "debug" + InfoLevel Level = "info" + WarnLevel Level = "warn" + ErrorLevel Level = "error" + DPanicLevel Level = "dpanic" + PanicLevel Level = "panic" + FatalLevel Level = "fatal" +) + +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 + } +} diff --git a/pkg/common.go b/pkg/common.go index c94a1ac1987..ec72ddec111 100644 --- a/pkg/common.go +++ b/pkg/common.go @@ -18,9 +18,8 @@ import ( "fmt" "strings" - "go.uber.org/zap/zapcore" - "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/log" ) func textToMarkdown(s string) string { @@ -28,11 +27,11 @@ func textToMarkdown(s string) string { } // DetailToString turns a detail information into a string. -func DetailToString(d *checker.CheckDetail, logLevel zapcore.Level) string { +func DetailToString(d *checker.CheckDetail, logLevel log.Level) string { // UPGRADEv3: remove switch statement. switch d.Msg.Version { case 3: - if d.Type == checker.DetailDebug && logLevel != zapcore.DebugLevel { + if d.Type == checker.DetailDebug && logLevel != log.DebugLevel { return "" } switch { @@ -46,14 +45,14 @@ func DetailToString(d *checker.CheckDetail, logLevel zapcore.Level) string { return fmt.Sprintf("%s: %s", typeToString(d.Type), d.Msg.Text) } default: - if d.Type == checker.DetailDebug && logLevel != zapcore.DebugLevel { + if d.Type == checker.DetailDebug && logLevel != log.DebugLevel { return "" } return fmt.Sprintf("%s: %s", typeToString(d.Type), d.Msg.Text) } } -func detailsToString(details []checker.CheckDetail, logLevel zapcore.Level) (string, bool) { +func detailsToString(details []checker.CheckDetail, logLevel log.Level) (string, bool) { // UPGRADEv2: change to make([]string, len(details)). var sa []string for i := range details { diff --git a/pkg/json.go b/pkg/json.go index 1a860d90f24..a59a66ccf38 100644 --- a/pkg/json.go +++ b/pkg/json.go @@ -19,10 +19,9 @@ import ( "fmt" "io" - "go.uber.org/zap/zapcore" - docs "github.com/ossf/scorecard/v4/docs/checks" sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/log" ) //nolint @@ -83,7 +82,7 @@ type jsonScorecardResultV2 struct { } // AsJSON exports results as JSON for new detail format. -func (r *ScorecardResult) AsJSON(showDetails bool, logLevel zapcore.Level, writer io.Writer) error { +func (r *ScorecardResult) AsJSON(showDetails bool, logLevel log.Level, writer io.Writer) error { encoder := json.NewEncoder(writer) out := jsonScorecardResult{ @@ -119,7 +118,7 @@ func (r *ScorecardResult) AsJSON(showDetails bool, logLevel zapcore.Level, write // AsJSON2 exports results as JSON for new detail format. func (r *ScorecardResult) AsJSON2(showDetails bool, - logLevel zapcore.Level, checkDocs docs.Doc, writer io.Writer) error { + logLevel log.Level, checkDocs docs.Doc, writer io.Writer) error { score, err := r.GetAggregateScore(checkDocs) if err != nil { return err diff --git a/pkg/json_test.go b/pkg/json_test.go index 305869bdb37..5c3b01c5800 100644 --- a/pkg/json_test.go +++ b/pkg/json_test.go @@ -24,9 +24,9 @@ import ( "time" "github.com/xeipuuv/gojsonschema" - "go.uber.org/zap/zapcore" "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/log" ) func jsonMockDocRead() *mockDoc { @@ -83,14 +83,14 @@ func TestJSONOutput(t *testing.T) { name string expected string showDetails bool - logLevel zapcore.Level + logLevel log.Level result ScorecardResult }{ { name: "check-1", showDetails: true, expected: "./testdata/check1.json", - logLevel: zapcore.DebugLevel, + logLevel: log.DebugLevel, result: ScorecardResult{ Repo: RepoInfo{ Name: repoName, @@ -129,7 +129,7 @@ func TestJSONOutput(t *testing.T) { name: "check-2", showDetails: true, expected: "./testdata/check2.json", - logLevel: zapcore.DebugLevel, + logLevel: log.DebugLevel, result: ScorecardResult{ Repo: RepoInfo{ Name: repoName, @@ -167,7 +167,7 @@ func TestJSONOutput(t *testing.T) { name: "check-3", showDetails: true, expected: "./testdata/check3.json", - logLevel: zapcore.InfoLevel, + logLevel: log.InfoLevel, result: ScorecardResult{ Repo: RepoInfo{ Name: repoName, @@ -267,7 +267,7 @@ func TestJSONOutput(t *testing.T) { name: "check-4", showDetails: true, expected: "./testdata/check4.json", - logLevel: zapcore.DebugLevel, + logLevel: log.DebugLevel, result: ScorecardResult{ Repo: RepoInfo{ Name: repoName, @@ -367,7 +367,7 @@ func TestJSONOutput(t *testing.T) { name: "check-5", showDetails: true, expected: "./testdata/check5.json", - logLevel: zapcore.WarnLevel, + logLevel: log.WarnLevel, result: ScorecardResult{ Repo: RepoInfo{ Name: repoName, @@ -406,7 +406,7 @@ func TestJSONOutput(t *testing.T) { name: "check-6", showDetails: true, expected: "./testdata/check6.json", - logLevel: zapcore.WarnLevel, + logLevel: log.WarnLevel, result: ScorecardResult{ Repo: RepoInfo{ Name: repoName, diff --git a/pkg/sarif.go b/pkg/sarif.go index eb805f4a343..ad79ad41a00 100644 --- a/pkg/sarif.go +++ b/pkg/sarif.go @@ -22,12 +22,11 @@ import ( "strings" "time" - "go.uber.org/zap/zapcore" - "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/checks" docs "github.com/ossf/scorecard/v4/docs/checks" sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/log" spol "github.com/ossf/scorecard/v4/policy" ) @@ -508,7 +507,7 @@ func filterOutDetailType(details []checker.CheckDetail, t checker.DetailType) [] func createDefaultLocationMessage(check *checker.CheckResult) string { details := filterOutDetailType(check.Details2, checker.DetailInfo) - s, b := detailsToString(details, zapcore.WarnLevel) + s, b := detailsToString(details, log.WarnLevel) if b { // Warning: GitHub UX needs a single `\n` to turn it into a `
`. return fmt.Sprintf("%s:\n%s", check.Reason, s) @@ -517,7 +516,7 @@ func createDefaultLocationMessage(check *checker.CheckResult) string { } // AsSARIF outputs ScorecardResult in SARIF 2.1.0 format. -func (r *ScorecardResult) AsSARIF(showDetails bool, logLevel zapcore.Level, +func (r *ScorecardResult) AsSARIF(showDetails bool, logLevel log.Level, writer io.Writer, checkDocs docs.Doc, policy *spol.ScorecardPolicy) error { //nolint // https://docs.oasis-open.org/sarif/sarif/v2.1.0/cs01/sarif-v2.1.0-cs01.html. diff --git a/pkg/sarif_test.go b/pkg/sarif_test.go index 30fbb3ffe36..73f7ec8cea7 100644 --- a/pkg/sarif_test.go +++ b/pkg/sarif_test.go @@ -21,9 +21,8 @@ import ( "testing" "time" - "go.uber.org/zap/zapcore" - "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/log" spol "github.com/ossf/scorecard/v4/policy" ) @@ -122,7 +121,7 @@ func TestSARIFOutput(t *testing.T) { name string expected string showDetails bool - logLevel zapcore.Level + logLevel log.Level result ScorecardResult policy spol.ScorecardPolicy }{ @@ -130,7 +129,7 @@ func TestSARIFOutput(t *testing.T) { name: "check-1", showDetails: true, expected: "./testdata/check1.sarif", - logLevel: zapcore.DebugLevel, + logLevel: log.DebugLevel, policy: spol.ScorecardPolicy{ Version: 1, Policies: map[string]*spol.CheckPolicy{ @@ -180,7 +179,7 @@ func TestSARIFOutput(t *testing.T) { name: "check-2", showDetails: true, expected: "./testdata/check2.sarif", - logLevel: zapcore.DebugLevel, + logLevel: log.DebugLevel, policy: spol.ScorecardPolicy{ Version: 1, Policies: map[string]*spol.CheckPolicy{ @@ -229,7 +228,7 @@ func TestSARIFOutput(t *testing.T) { name: "check-3", showDetails: true, expected: "./testdata/check3.sarif", - logLevel: zapcore.InfoLevel, + logLevel: log.InfoLevel, policy: spol.ScorecardPolicy{ Version: 1, Policies: map[string]*spol.CheckPolicy{ @@ -336,7 +335,7 @@ func TestSARIFOutput(t *testing.T) { name: "check-4", showDetails: true, expected: "./testdata/check4.sarif", - logLevel: zapcore.DebugLevel, + logLevel: log.DebugLevel, policy: spol.ScorecardPolicy{ Version: 1, Policies: map[string]*spol.CheckPolicy{ @@ -443,7 +442,7 @@ func TestSARIFOutput(t *testing.T) { name: "check-5", showDetails: true, expected: "./testdata/check5.sarif", - logLevel: zapcore.WarnLevel, + logLevel: log.WarnLevel, policy: spol.ScorecardPolicy{ Version: 1, Policies: map[string]*spol.CheckPolicy{ @@ -491,7 +490,7 @@ func TestSARIFOutput(t *testing.T) { // https://github.com/github/codeql-action/issues/754 // Disabled related locations. expected: "./testdata/check6.sarif", - logLevel: zapcore.WarnLevel, + logLevel: log.WarnLevel, policy: spol.ScorecardPolicy{ Version: 1, Policies: map[string]*spol.CheckPolicy{ @@ -535,7 +534,7 @@ func TestSARIFOutput(t *testing.T) { name: "check-7", showDetails: true, expected: "./testdata/check7.sarif", - logLevel: zapcore.DebugLevel, + logLevel: log.DebugLevel, policy: spol.ScorecardPolicy{ Version: 1, Policies: map[string]*spol.CheckPolicy{ @@ -642,7 +641,7 @@ func TestSARIFOutput(t *testing.T) { name: "check-8", showDetails: true, expected: "./testdata/check8.sarif", - logLevel: zapcore.DebugLevel, + logLevel: log.DebugLevel, policy: spol.ScorecardPolicy{ Version: 1, Policies: map[string]*spol.CheckPolicy{ diff --git a/pkg/scorecard_result.go b/pkg/scorecard_result.go index ceb3b68d606..6c3150651d5 100644 --- a/pkg/scorecard_result.go +++ b/pkg/scorecard_result.go @@ -21,11 +21,11 @@ import ( "time" "github.com/olekukonko/tablewriter" - "go.uber.org/zap/zapcore" "github.com/ossf/scorecard/v4/checker" docs "github.com/ossf/scorecard/v4/docs/checks" sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/log" ) // ScorecardInfo contains information about the scorecard code that was run. @@ -98,7 +98,7 @@ func (r *ScorecardResult) GetAggregateScore(checkDocs docs.Doc) (float64, error) } // AsString returns ScorecardResult in string format. -func (r *ScorecardResult) AsString(showDetails bool, logLevel zapcore.Level, +func (r *ScorecardResult) AsString(showDetails bool, logLevel log.Level, checkDocs docs.Doc, writer io.Writer) error { data := make([][]string, len(r.Checks)) //nolint