⚠️ 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 <[email protected]> * log: Replace instances of `zap.Logger` with `log.Logger` Signed-off-by: Stephen Augustus <[email protected]> * log: Add logic to parse `zapcore.Level`s as strings Signed-off-by: Stephen Augustus <[email protected]> * log: Express log levels Signed-off-by: Stephen Augustus <[email protected]> * log: Replace instances of `zapcore.Level` with `log.Level` Signed-off-by: Stephen Augustus <[email protected]> * log: Fixup comments for exported functions Signed-off-by: Stephen Augustus <[email protected]>
ossf · Jan 20, 2022 · 13b78ab · 13b78ab
1 parent f4e9dfd
commit 13b78ab
Show file tree

Hide file tree

Showing 19 changed files with 204 additions and 109 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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