Change the approach to decide whether to upload metrics.

1. Don't upload if the uploader binary is not present.

Bug: b/193703183
Test: Run the following tests locally:
1. run "go test ." in build/soong/ui/build
2. create vendor/google/misc/metrics_uploader_prebuilt/metrics_uploader.sh
and run m nothing to ensure metrics_uploader.sh was called
3. remove the file created in step 2 then run m nothing and ensure it
doesn't attempt uploading

Change-Id: I081a5510b3f30480720c3e7dd235623c805fa7a4

ui/build/config.go

@@ -83,6 +83,8 @@ type configImpl struct {
 
 	// Set by multiproduct_kati
 	emptyNinjaFile bool
+
+	metricsUploader string
 }
 
 const srcDirFileCheck = "build/soong/root.bp"
@@ -237,7 +239,8 @@ func NewConfig(ctx Context, args ...string) Config {
 	// Precondition: the current directory is the top of the source tree
 	checkTopDir(ctx)
 
-	if srcDir := absPath(ctx, "."); strings.ContainsRune(srcDir, ' ') {
+	srcDir := absPath(ctx, ".")
+	if strings.ContainsRune(srcDir, ' ') {
 		ctx.Println("You are building in a directory whose absolute path contains a space character:")
 		ctx.Println()
 		ctx.Printf("%q\n", srcDir)
@@ -245,6 +248,8 @@ func NewConfig(ctx Context, args ...string) Config {
 		ctx.Fatalln("Directory names containing spaces are not supported")
 	}
 
+	ret.metricsUploader = GetMetricsUploader(srcDir, ret.environ)
+
 	if outDir := ret.OutDir(); strings.ContainsRune(outDir, ' ') {
 		ctx.Println("The absolute path of your output directory ($OUT_DIR) contains a space character:")
 		ctx.Println()
@@ -1246,10 +1251,7 @@ func (c *configImpl) BuildDateTime() string {
 }
 
 func (c *configImpl) MetricsUploaderApp() string {
-	if p, ok := c.environ.Get("ANDROID_ENABLE_METRICS_UPLOAD"); ok {
-		return p
-	}
-	return ""
+	return c.metricsUploader
 }
 
 // LogsDir returns the logs directory where build log and metrics
@@ -1277,3 +1279,14 @@ func (c *configImpl) SetEmptyNinjaFile(v bool) {
 func (c *configImpl) EmptyNinjaFile() bool {
 	return c.emptyNinjaFile
 }
+
+func GetMetricsUploader(topDir string, env *Environment) string {
+	if p, ok := env.Get("METRICS_UPLOADER"); ok {
+		metricsUploader := filepath.Join(topDir, p)
+		if _, err := os.Stat(metricsUploader); err == nil {
+			return metricsUploader
+		}
+	}
+
+	return ""
+}

ui/build/config_test.go

@@ -1122,3 +1122,65 @@ func TestBuildConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestGetMetricsUploaderApp(t *testing.T) {
+
+	metricsUploaderDir := "metrics_uploader_dir"
+	metricsUploaderBinary := "metrics_uploader_binary"
+	metricsUploaderPath := filepath.Join(metricsUploaderDir, metricsUploaderBinary)
+	tests := []struct {
+		description string
+		environ     Environment
+		createFiles bool
+		expected    string
+	}{{
+		description: "Uploader binary exist",
+		environ:     Environment{"METRICS_UPLOADER=" + metricsUploaderPath},
+		createFiles: true,
+		expected:    metricsUploaderPath,
+	}, {
+		description: "Uploader binary not exist",
+		environ:     Environment{"METRICS_UPLOADER=" + metricsUploaderPath},
+		createFiles: false,
+		expected:    "",
+	}, {
+		description: "Uploader binary variable not set",
+		createFiles: true,
+		expected:    "",
+	}}
+
+	for _, tt := range tests {
+		t.Run(tt.description, func(t *testing.T) {
+			defer logger.Recover(func(err error) {
+				t.Fatalf("got unexpected error: %v", err)
+			})
+
+			// Create the root source tree.
+			topDir, err := ioutil.TempDir("", "")
+			if err != nil {
+				t.Fatalf("failed to create temp dir: %v", err)
+			}
+			defer os.RemoveAll(topDir)
+
+			expected := tt.expected
+			if len(expected) > 0 {
+				expected = filepath.Join(topDir, expected)
+			}
+
+			if tt.createFiles {
+				if err := os.MkdirAll(filepath.Join(topDir, metricsUploaderDir), 0755); err != nil {
+					t.Errorf("failed to create %s directory: %v", metricsUploaderDir, err)
+				}
+				if err := ioutil.WriteFile(filepath.Join(topDir, metricsUploaderPath), []byte{}, 0644); err != nil {
+					t.Errorf("failed to create file %s: %v", expected, err)
+				}
+			}
+
+			actual := GetMetricsUploader(topDir, &tt.environ)
+
+			if actual != expected {
+				t.Errorf("expecting: %s, actual: %s", expected, actual)
+			}
+		})
+	}
+}

ui/build/upload.go

@@ -70,12 +70,11 @@ func pruneMetricsFiles(paths []string) []string {
 	return metricsFiles
 }
 
-// UploadMetrics uploads a set of metrics files to a server for analysis. An
-// uploader full path is specified in ANDROID_ENABLE_METRICS_UPLOAD environment
-// variable in order to upload the set of metrics files. The metrics files are
-// first copied to a temporary directory and the uploader is then executed in
-// the background to allow the user/system to continue working. Soong communicates
-// to the uploader through the upload_proto raw protobuf file.
+// UploadMetrics uploads a set of metrics files to a server for analysis.
+// The metrics files are first copied to a temporary directory
+// and the uploader is then executed in the background to allow the user/system
+// to continue working. Soong communicates to the uploader through the
+// upload_proto raw protobuf file.
 func UploadMetrics(ctx Context, config Config, simpleOutput bool, buildStarted time.Time, paths ...string) {
 	ctx.BeginTrace(metrics.RunSetupTool, "upload_metrics")
 	defer ctx.EndTrace()

ui/build/upload_test.go

@@ -80,13 +80,10 @@ func TestUploadMetrics(t *testing.T) {
 		createFiles bool
 		files       []string
 	}{{
-		description: "ANDROID_ENABLE_METRICS_UPLOAD not set",
-	}, {
-		description: "no metrics files to upload",
-		uploader:    "fake",
+		description: "no metrics uploader",
 	}, {
 		description: "non-existent metrics files no upload",
-		uploader:    "fake",
+		uploader:    "echo",
 		files:       []string{"metrics_file_1", "metrics_file_2", "metrics_file_3"},
 	}, {
 		description: "trigger upload",
@@ -137,9 +134,9 @@ func TestUploadMetrics(t *testing.T) {
 			config := Config{&configImpl{
 				environ: &Environment{
 					"OUT_DIR=" + outDir,
-					"ANDROID_ENABLE_METRICS_UPLOAD=" + tt.uploader,
 				},
-				buildDateTime: strconv.FormatInt(time.Now().UnixNano()/int64(time.Millisecond), 10),
+				buildDateTime:   strconv.FormatInt(time.Now().UnixNano()/int64(time.Millisecond), 10),
+				metricsUploader: tt.uploader,
 			}}
 
 			UploadMetrics(ctx, config, false, time.Now(), metricsFiles...)
@@ -192,9 +189,10 @@ func TestUploadMetricsErrors(t *testing.T) {
 
 			config := Config{&configImpl{
 				environ: &Environment{
-					"ANDROID_ENABLE_METRICS_UPLOAD=fake",
 					"OUT_DIR=/bad",
-				}}}
+				},
+				metricsUploader: "echo",
+			}}
 
 			UploadMetrics(ctx, config, true, time.Now(), metricsFile)
 			t.Errorf("got nil, expecting %q as a failure", tt.expectedErr)

ui/metrics/metrics.go

@@ -25,16 +25,11 @@ package metrics
 // that captures the metrics and is them added as a perfInfo into the set
 // of the collected metrics. Finally, when soong_ui has finished the build,
 // the defer Dump function is invoked to store the collected metrics to the
-// raw protobuf file in the $OUT directory.
-//
-// There is one additional step that occurs after the raw protobuf file is written.
-// If the configuration environment variable ANDROID_ENABLE_METRICS_UPLOAD is
-// set with the path, the raw protobuf file is uploaded to the destination. See
-// ui/build/upload.go for more details. The filename of the raw protobuf file
-// and the list of files to be uploaded is defined in cmd/soong_ui/main.go.
-//
-// See ui/metrics/event.go for the explanation of what an event is and how
-// the metrics system is a stack based system.
+// raw protobuf file in the $OUT directory and this raw protobuf file will be
+// uploaded to the destination. See ui/build/upload.go for more details. The
+// filename of the raw protobuf file and the list of files to be uploaded is
+// defined in cmd/soong_ui/main.go. See ui/metrics/event.go for the explanation
+// of what an event is and how the metrics system is a stack based system.
 
 import (
 	"io/ioutil"