Merge "Run the metrics uploader in the background." (c9d83d25) · Commits · e / os / android_build_soong

cmd/soong_ui/main.go

+1 −1

Original line number	Diff line number	Diff line
		@@ -171,7 +171,7 @@ func main() {
		buildErrorFile := filepath.Join(logsDir, c.logsPrefix+"build_error")
		rbeMetricsFile := filepath.Join(logsDir, c.logsPrefix+"rbe_metrics.pb")
		soongMetricsFile := filepath.Join(logsDir, c.logsPrefix+"soong_metrics")
		defer build.UploadMetrics(buildCtx, config, buildStartedMilli, buildErrorFile, rbeMetricsFile, soongMetricsFile)
		defer build.UploadMetrics(buildCtx, config, c.forceDumbOutput, buildStartedMilli, buildErrorFile, rbeMetricsFile, soongMetricsFile)

		os.MkdirAll(logsDir, 0777)
		log.SetOutput(filepath.Join(logsDir, c.logsPrefix+"soong.log"))

ui/build/upload.go

+39 −6

Original line number	Diff line number	Diff line
		@@ -23,6 +23,7 @@ import (
		"path/filepath"
		"time"

		"android/soong/ui/metrics"
		"github.com/golang/protobuf/proto"

		upload_proto "android/soong/ui/metrics/upload_proto"
		@@ -32,11 +33,21 @@ const (
		uploadPbFilename = ".uploader.pb"
		)

		var (
		// For testing purpose
		getTmpDir = ioutil.TempDir
		)

		// UploadMetrics uploads a set of metrics files to a server for analysis. An
		// uploader full path is required to be specified in order to upload the set
		// of metrics files. This is accomplished by defining the ANDROID_ENABLE_METRICS_UPLOAD
		// environment variable.
		func UploadMetrics(ctx Context, config Config, buildStartedMilli int64, files ...string) {
		// environment variable. The metrics files are copied to a temporary directory
		// and the uploader is then executed in the background to allow the user to continue
		// working.
		func UploadMetrics(ctx Context, config Config, forceDumbOutput bool, buildStartedMilli int64, files ...string) {
		ctx.BeginTrace(metrics.RunSetupTool, "upload_metrics")
		defer ctx.EndTrace()

		uploader := config.MetricsUploaderApp()
		// No metrics to upload if the path to the uploader was not specified.
		if uploader == "" {
		@@ -56,6 +67,22 @@ func UploadMetrics(ctx Context, config Config, buildStartedMilli int64, files ..
		return
		}

		// The temporary directory cannot be deleted as the metrics uploader is started
		// in the background and requires to exist until the operation is done. The
		// uploader can delete the directory as it is specified in the upload proto.
		tmpDir, err := getTmpDir("", "upload_metrics")
		if err != nil {
		ctx.Fatalf("failed to create a temporary directory to store the list of metrics files: %v\n", err)
		}

		for i, src := range metricsFiles {
		dst := filepath.Join(tmpDir, filepath.Base(src))
		if _, err := copyFile(src, dst); err != nil {
		ctx.Fatalf("failed to copy %q to %q: %v\n", src, dst, err)
		}
		metricsFiles[i] = dst
		}

		// For platform builds, the branch and target name is hardcoded to specific
		// values for later extraction of the metrics in the data metrics pipeline.
		data, err := proto.Marshal(&upload_proto.Upload{
		@@ -64,17 +91,23 @@ func UploadMetrics(ctx Context, config Config, buildStartedMilli int64, files ..
		BranchName: proto.String("developer-metrics"),
		TargetName: proto.String("platform-build-systems-metrics"),
		MetricsFiles: metricsFiles,
		DirectoriesToDelete: []string{tmpDir},
		})
		if err != nil {
		ctx.Fatalf("failed to marshal metrics upload proto buffer message: %v\n", err)
		}

		pbFile := filepath.Join(config.OutDir(), uploadPbFilename)
		pbFile := filepath.Join(tmpDir, uploadPbFilename)
		if err := ioutil.WriteFile(pbFile, data, 0644); err != nil {
		ctx.Fatalf("failed to write the marshaled metrics upload protobuf to %q: %v\n", pbFile, err)
		}
		// Remove the upload file as it's not longer needed after it has been processed by the uploader.
		defer os.Remove(pbFile)

		Command(ctx, config, "upload metrics", uploader, "--upload-metrics", pbFile).RunAndStreamOrFatal()
		// Start the uploader in the background as it takes several milliseconds to start the uploader
		// and prepare the metrics for upload. This affects small commands like "lunch".
		cmd := Command(ctx, config, "upload metrics", uploader, "--upload-metrics", pbFile)
		if forceDumbOutput {
		cmd.RunOrFatal()
		} else {
		cmd.RunAndStreamOrFatal()
		}
		}

ui/build/upload_test.go

+67 −29

Original line number	Diff line number	Diff line
		@@ -15,6 +15,7 @@
		package build

		import (
		"errors"
		"io/ioutil"
		"os"
		"path/filepath"
		@@ -61,6 +62,22 @@ func TestUploadMetrics(t *testing.T) {
		}
		defer os.RemoveAll(outDir)

		// Supply our own getTmpDir to delete the temp dir once the test is done.
		orgGetTmpDir := getTmpDir
		getTmpDir = func(string, string) (string, error) {
		retDir := filepath.Join(outDir, "tmp_upload_dir")
		if err := os.Mkdir(retDir, 0755); err != nil {
		t.Fatalf("failed to create temporary directory %q: %v", retDir, err)
		}
		return retDir, nil
		}
		defer func() { getTmpDir = orgGetTmpDir }()

		metricsUploadDir := filepath.Join(outDir, ".metrics_uploader")
		if err := os.Mkdir(metricsUploadDir, 0755); err != nil {
		t.Fatalf("failed to create %q directory for oauth valid check: %v", metricsUploadDir, err)
		}

		var metricsFiles []string
		if tt.createFiles {
		for _, f := range tt.files {
		@@ -80,21 +97,34 @@ func TestUploadMetrics(t *testing.T) {
		buildDateTime: strconv.FormatInt(time.Now().UnixNano()/int64(time.Millisecond), 10),
		}}

		UploadMetrics(ctx, config, 1591031903, metricsFiles...)

		if _, err := os.Stat(filepath.Join(outDir, uploadPbFilename)); err == nil {
		t.Error("got true, want false for upload protobuf file to exist")
		}
		UploadMetrics(ctx, config, false, 1591031903, metricsFiles...)
		})
		}
		}

		func TestUploadMetricsErrors(t *testing.T) {
		expectedErr := "failed to write the marshaled"
		ctx := testContext()
		tests := []struct {
		description string
		tmpDir string
		tmpDirErr error
		expectedErr string
		}{{
		description: "getTmpDir returned error",
		tmpDirErr: errors.New("getTmpDir failed"),
		expectedErr: "getTmpDir failed",
		}, {
		description: "copyFile operation error",
		tmpDir: "/fake_dir",
		expectedErr: "failed to copy",
		}}

		for _, tt := range tests {
		t.Run(tt.description, func(t *testing.T) {
		defer logger.Recover(func(err error) {
		got := err.Error()
		if !strings.Contains(got, expectedErr) {
		t.Errorf("got %q, want %q to be contained in error", got, expectedErr)
		if !strings.Contains(got, tt.expectedErr) {
		t.Errorf("got %q, want %q to be contained in error", got, tt.expectedErr)
		}
		})

		@@ -104,6 +134,12 @@ func TestUploadMetricsErrors(t *testing.T) {
		}
		defer os.RemoveAll(outDir)

		orgGetTmpDir := getTmpDir
		getTmpDir = func(string, string) (string, error) {
		return tt.tmpDir, tt.tmpDirErr
		}
		defer func() { getTmpDir = orgGetTmpDir }()

		metricsFile := filepath.Join(outDir, "metrics_file_1")
		if err := ioutil.WriteFile(metricsFile, []byte("test file"), 0644); err != nil {
		t.Fatalf("failed to create a fake metrics file %q for uploading: %v", metricsFile, err)
		@@ -115,6 +151,8 @@ func TestUploadMetricsErrors(t *testing.T) {
		"OUT_DIR=/bad",
		}}}

		UploadMetrics(testContext(), config, 1591031903, metricsFile)
		t.Errorf("got nil, expecting %q as a failure", expectedErr)
		UploadMetrics(ctx, config, true, 1591031903, metricsFile)
		t.Errorf("got nil, expecting %q as a failure", tt.expectedErr)
		})
		}
		}

ui/build/util.go

+18 −0

Original line number	Diff line number	Diff line
		@@ -15,6 +15,7 @@
		package build

		import (
		"io"
		"os"
		"path/filepath"
		"strings"
		@@ -124,3 +125,20 @@ func decodeKeyValue(str string) (string, string, bool) {
		}
		return str[:idx], str[idx+1:], true
		}

		// copyFile copies a file from src to dst. filepath.Dir(dst) must exist.
		func copyFile(src, dst string) (int64, error) {
		source, err := os.Open(src)
		if err != nil {
		return 0, err
		}
		defer source.Close()

		destination, err := os.Create(dst)
		if err != nil {
		return 0, err
		}
		defer destination.Close()

		return io.Copy(destination, source)
		}

ui/build/util_test.go

+70 −0

Original line number	Diff line number	Diff line
		@@ -15,6 +15,7 @@
		package build

		import (
		"bytes"
		"io/ioutil"
		"os"
		"path/filepath"
		@@ -49,3 +50,72 @@ func TestEnsureEmptyDirs(t *testing.T) {

		ensureEmptyDirectoriesExist(ctx, filepath.Join(tmpDir, "a"))
		}

		func TestCopyFile(t *testing.T) {
		tmpDir, err := ioutil.TempDir("", "test_copy_file")
		if err != nil {
		t.Fatalf("failed to create temporary directory to hold test text files: %v", err)
		}
		defer os.Remove(tmpDir)

		data := []byte("fake data")
		src := filepath.Join(tmpDir, "src.txt")
		if err := ioutil.WriteFile(src, data, 0755); err != nil {
		t.Fatalf("failed to create a src file %q for copying: %v", src, err)
		}

		dst := filepath.Join(tmpDir, "dst.txt")

		l, err := copyFile(src, dst)
		if err != nil {
		t.Fatalf("got %v, expecting nil error on copyFile operation", err)
		}

		if l != int64(len(data)) {
		t.Errorf("got %d, expecting %d for copied bytes", l, len(data))
		}

		dstData, err := ioutil.ReadFile(dst)
		if err != nil {
		t.Fatalf("got %v, expecting nil error reading dst %q file", err, dst)
		}

		if bytes.Compare(data, dstData) != 0 {
		t.Errorf("got %q, expecting data %q from dst %q text file", string(data), string(dstData), dst)
		}
		}

		func TestCopyFileErrors(t *testing.T) {
		tmpDir, err := ioutil.TempDir("", "test_copy_file_errors")
		if err != nil {
		t.Fatalf("failed to create temporary directory to hold test text files: %v", err)
		}
		defer os.Remove(tmpDir)

		srcExists := filepath.Join(tmpDir, "src_exist.txt")
		if err := ioutil.WriteFile(srcExists, []byte("fake data"), 0755); err != nil {
		t.Fatalf("failed to create a src file %q for copying: %v", srcExists, err)
		}

		tests := []struct {
		description string
		src string
		dst string
		}{{
		description: "src file does not exist",
		src: "/src/not/exist",
		dst: "/dst/not/exist",
		}, {
		description: "dst directory does not exist",
		src: srcExists,
		dst: "/dst/not/exist",
		}}

		for _, tt := range tests {
		t.Run(tt.description, func(t *testing.T) {
		if _, err := copyFile(tt.src, tt.dst); err == nil {
		t.Errorf("got nil, expecting error")
		}
		})
		}
		}