⚠️ log: Initial logr/logrusr implementation

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)

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)

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,

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) {

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)
 	}
 

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))

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)

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)

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,27 +65,29 @@ 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()
 			ciiClient := clients.DefaultCIIBestPracticesClient()
 			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)
 		}
 	},
 }