upload: allow caller to control logging of errors

Add a Logging io.Writer to be used to log errors, if set.
Otherwise errors are not logged.

Change-Id: I76609645c0fc178f7820dc4e51f4bac10efe8e5e
Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/518777
Reviewed-by: Jamal Carvalho <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Run-TryBot: Peter Weinberger <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

internal/upload/date.go

@@ -6,7 +6,6 @@ package upload
 
 import (
 	"fmt"
-	"log"
 	"os"
 	"sync"
 	"time"
@@ -24,7 +23,7 @@ var distantPast = 21 * 24 * time.Hour
 func tooOld(date string) bool {
 	t, err := time.Parse("2006-01-02", date)
 	if err != nil {
-		log.Printf("tooOld: %v", err)
+		logger.Printf("tooOld: %v", err)
 		return false
 	}
 	return now().Sub(t) > distantPast
@@ -50,12 +49,12 @@ var farFuture = time.UnixMilli(1 << 62)
 func expiry(fname string) time.Time {
 	parsed, err := parse(fname)
 	if err != nil {
-		log.Printf("expiry Parse: %v for %s", err, fname)
+		logger.Printf("expiry Parse: %v for %s", err, fname)
 		return farFuture // don't process it, whatever it is
 	}
 	expiry, err := time.Parse(time.RFC3339, parsed.Meta["TimeEnd"])
 	if err != nil {
-		log.Printf("time.Parse: %v for %s", err, fname)
+		logger.Printf("time.Parse: %v for %s", err, fname)
 		return farFuture // don't process it, whatever it is
 	}
 	// TODO(pjw): check for off-by-one-day?

internal/upload/findwork.go

@@ -5,7 +5,6 @@
 package upload
 
 import (
-	"log"
 	"os"
 	"path/filepath"
 	"strings"
@@ -27,8 +26,8 @@ func findWork(localdir, uploaddir string) work {
 	var ans work
 	fis, err := os.ReadDir(localdir)
 	if err != nil {
-		// This occurs in a separate upload process, not in gopls or go.
-		log.Fatal(err)
+		logger.Printf("could not read %s, progress impossible (%v)", localdir, err)
+		return ans
 	}
 	// count files end in .v1.count
 	// reports end in .json. If they are not to be uploaded they

internal/upload/reports.go

@@ -9,7 +9,6 @@ import (
 	"encoding/binary"
 	"encoding/json"
 	"fmt"
-	"log"
 	"math"
 	"os"
 	"path/filepath"
@@ -95,7 +94,7 @@ func createReport(date string, files []string, lastWeek string) (string, error)
 	if uploadConfig == nil {
 		a, err := configstore.Download("latest", nil)
 		if err != nil {
-			log.Print(err) // or something (e.g., panic(err))
+			logger.Print(err) // or something (e.g., panic(err))
 		}
 		uploadConfig = &a
 	}
@@ -120,7 +119,7 @@ func createReport(date string, files []string, lastWeek string) (string, error)
 	for _, f := range files {
 		x, err := parse(string(f))
 		if err != nil {
-			log.Printf("unparseable (%v) %s", err, f)
+			logger.Printf("unparseable (%v) %s", err, f)
 			continue
 		}
 		prog := findProgReport(x.Meta, report)
@@ -250,7 +249,7 @@ func computeRandom() float64 {
 		b := make([]byte, 8)
 		_, err := rand.Read(b)
 		if err != nil {
-			log.Fatalf("rand.Read: %v", err)
+			logger.Fatalf("rand.Read: %v", err)
 		}
 		// and turn it into a float64
 		x := math.Float64frombits(binary.LittleEndian.Uint64(b))

internal/upload/run.go

@@ -5,20 +5,31 @@
 package upload
 
 import (
+	"io"
 	"log"
 
 	"golang.org/x/telemetry"
 )
 
+var logger *log.Logger
+
+func init() {
+	logger = log.New(io.Discard, "", 0)
+}
+
 // Run generates and uploads reports
-// TODO(pjw): decide what to do about error reporting throughout the package
-func Run(c *telemetry.Configuration) {
-	if c != nil && c.UploadConfig != nil {
-		uploadConfig = c.UploadConfig()
+func Run(c *telemetry.Control) {
+	if c != nil {
+		if c.UploadConfig != nil {
+			uploadConfig = c.UploadConfig()
+		}
+		if c.Logging != nil {
+			logger.SetOutput(c.Logging)
+		}
 	}
 	todo := findWork(telemetry.LocalDir, telemetry.UploadDir)
 	if err := reports(todo); err != nil {
-		log.Printf("reports: %v", err)
+		logger.Printf("reports: %v", err)
 	}
 	for _, f := range todo.readyfiles {
 		uploadReport(f)

internal/upload/upload.go

@@ -7,7 +7,6 @@ package upload
 import (
 	"bytes"
 	"crypto/tls"
-	"log"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -24,7 +23,7 @@ var uploadURL = "https://telemetry.go.dev/upload"
 func uploadReport(fname string) {
 	buf, err := os.ReadFile(fname)
 	if err != nil {
-		log.Printf("%v reading %s", err, fname)
+		logger.Printf("%v reading %s", err, fname)
 		return
 	}
 	if uploadReportContents(fname, buf) {
@@ -45,11 +44,11 @@ func uploadReportContents(fname string, buf []byte) bool {
 
 	resp, err := client.Post(server, "application/json", b)
 	if err != nil {
-		log.Printf("error on Post: %v %q for %q", err, server, fname)
+		logger.Printf("error on Post: %v %q for %q", err, server, fname)
 		return false
 	}
 	if resp.StatusCode != 200 {
-		log.Printf("resp error on upload %q: %v for %q %q", server, resp.Status, fname, fdate)
+		logger.Printf("resp error on upload %q: %v for %q %q", server, resp.Status, fname, fdate)
 		return false
 	}
 	// put a copy in the uploaded directory

internal/upload/utils_test.go

@@ -28,7 +28,8 @@ func setup(t *testing.T) {
 		addr := <-serverChan
 		uploadURL = addr.path
 	}
-	log.SetFlags(log.Lshortfile)
+	logger = log.Default()
+	logger.SetFlags(log.Lshortfile)
 	dir := t.TempDir()
 	it.LocalDir = dir + "/local"
 	it.UploadDir = dir + "/upload"

types.go

@@ -5,6 +5,8 @@
 package telemetry
 
 import (
+	"io"
+
 	"golang.org/x/telemetry/internal/telemetry"
 )
 
@@ -54,15 +56,18 @@ type ProgramReport struct {
 	Stacks    map[string]int64
 }
 
-// A Configuration allows the user to override various default
+// A Control allows the user to override various default
 // reporting and uploading choices.
 // Future versions may also allow the user to set the upload URL.
-type Configuration struct {
+type Control struct {
 	// UploadConfig provides the telemetry UploadConfig used to
 	// decide which counters get uploaded. nil is legal, and
 	// means the code will use the latest version of the module
 	// golang.org/x/telemetry/config.
 	UploadConfig func() *UploadConfig
+	// Logging provides a io.Writer for error messages during uploading
+	// nil is legal and no log messages get generated
+	Logging io.Writer
 }
 
 var (

upload/upload.go

@@ -12,10 +12,10 @@ import (
 )
 
 // Run generates and uploads reports, as allowed by the mode file.
-// A nil Config is legal and uses the latest version of the module
+// A nil Control is legal and uses the latest version of the module
 // golang.org/x/telemetry/config and the derault upload URL
 // (presently https://telemetry.go.dev/upload).
-func Run(c *telemetry.Configuration) {
+func Run(c *telemetry.Control) {
 	defer func() {
 		if err := recover(); err != nil {
 			log.Printf("upload recover: %v", err)