diff --git a/internal/configstore/download.go b/internal/configstore/download.go index a38f371..e60ab7e 100644 --- a/internal/configstore/download.go +++ b/internal/configstore/download.go @@ -16,6 +16,7 @@ import ( "os" "os/exec" "path/filepath" + "sync/atomic" "golang.org/x/telemetry/internal/telemetry" ) @@ -29,12 +30,22 @@ const ( // creation flag. var needNoConsole = func(cmd *exec.Cmd) {} +var downloads int64 + +// Downloads reports, for testing purposes, the number of times [Download] has +// been called. +func Downloads() int64 { + return atomic.LoadInt64(&downloads) +} + // Download fetches the requested telemetry UploadConfig using "go mod // download". If envOverlay is provided, it is appended to the environment used // for invoking the go command. // // The second result is the canonical version of the requested configuration. func Download(version string, envOverlay []string) (*telemetry.UploadConfig, string, error) { + atomic.AddInt64(&downloads, 1) + if version == "" { version = "latest" } diff --git a/internal/upload/run.go b/internal/upload/run.go index eba13b1..e9c8dc2 100644 --- a/internal/upload/run.go +++ b/internal/upload/run.go @@ -112,9 +112,24 @@ func newUploader(rcfg RunConfig) (*uploader, error) { logger := log.New(logWriter, "", log.Ltime|log.Lmicroseconds|log.Lshortfile) // Fetch the upload config, if it is not provided. - config, configVersion, err := configstore.Download("latest", rcfg.Env) - if err != nil { - return nil, err + var ( + config *telemetry.UploadConfig + configVersion string + ) + + if mode, _ := dir.Mode(); mode == "on" { + // golang/go#68946: only download the upload config if it will be used. + // + // TODO(rfindley): This is a narrow change aimed at minimally fixing the + // associated bug. In the future, we should read the mode only once during + // the upload process. + config, configVersion, err = configstore.Download("latest", rcfg.Env) + if err != nil { + return nil, err + } + } else { + config = &telemetry.UploadConfig{} + configVersion = "v0.0.0-0" } // Set the start time, if it is not provided. diff --git a/internal/upload/run_test.go b/internal/upload/run_test.go index c2cb08e..4231985 100644 --- a/internal/upload/run_test.go +++ b/internal/upload/run_test.go @@ -20,6 +20,7 @@ import ( "time" "golang.org/x/telemetry/counter" + "golang.org/x/telemetry/internal/configstore" "golang.org/x/telemetry/internal/configtest" "golang.org/x/telemetry/internal/regtest" "golang.org/x/telemetry/internal/telemetry" @@ -430,12 +431,13 @@ func TestRun_ModeHandling(t *testing.T) { prog := regtest.NewIncProgram(t, "prog1", "counter") tests := []struct { - mode string - wantUploads int + mode string + wantConfigDownloads int64 + wantUploads int }{ - {"off", 0}, - {"local", 0}, - {"on", 1}, // only the second week is uploadable + {"off", 0, 0}, + {"local", 0, 0}, + {"on", 1, 1}, // only the second week is uploadable } for _, test := range tests { t.Run(test.mode, func(t *testing.T) { @@ -459,10 +461,15 @@ func TestRun_ModeHandling(t *testing.T) { t.Fatal(err) } + downloadsBefore := configstore.Downloads() if err := upload.Run(cfg); err != nil { t.Fatal(err) } + if got := configstore.Downloads() - downloadsBefore; got != test.wantConfigDownloads { + t.Errorf("configstore.Download called: %v, want %v", got, test.wantConfigDownloads) + } + uploads := getUploads() if gotUploads := len(uploads); gotUploads != test.wantUploads { t.Fatalf("got %d uploads, want %d", gotUploads, test.wantUploads)