Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c3cec872 authored by Patrice Arruda's avatar Patrice Arruda
Browse files

Run the metrics uploader in the background. 

The metrics uploader was currently running on foreground where it
would copy the metrics files in a separate directory and then forked
into the background for the upload process. As a result, running the
lunch command would take a second longer to run since each metrics
uploader run had an average of half a second.

Bug: 140638454
Test: * Wrote and updated unit test cases.
      * Set ANDROID_ENABLE_METRICS_UPLOAD to point to the latest
        metrics_uploader bash script. Executed the "lunch 1" command
	and measured the running time. Executed "m nothing" command
	and checked that the metrics were uploaded.
      * Ran "lunch 1" and "m nothing" with
        ANDROID_ENABLE_METRICS_UPLOAD=""
      * Removed oauth from metrics_uploader and ran "m nothing" and
        "lunch 1". The oauth Message appeared only to "m nothing"

Change-Id: I13c61e666c8f44613dee291a704cef6a27335188
Merged-In: I13c61e666c8f44613dee291a704cef6a27335188
parent d519a71f

cmd/soong_ui/main.go

+3 −1
Original line number Diff line number Diff line
@@ -50,6 +50,7 @@ func main() { 
	buildStartedMilli := time.Now().UnixNano() / int64(time.Millisecond)

	var stdio terminal.StdioInterface

	stdio = terminal.StdioImpl{}

	simpleOutput := false

	logsPrefix := ""



	// dumpvar uses stdout, everything else should be in stderr
@@ -58,6 +59,7 @@ func main() { 
		// collected to further aggregate the metrics. For dump-var mode, it is usually

		// related to the execution of lunch command.

		logsPrefix = "dumpvars-"

		simpleOutput = true

		stdio = terminal.NewCustomStdio(os.Stdin, os.Stderr, os.Stderr)

	}
@@ -118,7 +120,7 @@ func main() { 


	rbeMetricsFile := filepath.Join(logsDir, logsPrefix+"rbe_metrics.pb")

	soongMetricsFile := filepath.Join(logsDir, logsPrefix+"soong_metrics")

	defer build.UploadMetrics(buildCtx, config, buildStartedMilli, rbeMetricsFile, soongMetricsFile)

	defer build.UploadMetrics(buildCtx, config, simpleOutput, buildStartedMilli, rbeMetricsFile, soongMetricsFile)



	os.MkdirAll(logsDir, 0777)

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).RunAndPrintOrFatal()

	// 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.RunAndPrintOrFatal()

	}

}

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"
@@ -59,6 +60,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 {
@@ -77,21 +94,34 @@ func TestUploadMetrics(t *testing.T) { 
				},

			}}



			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)

				}

			})
@@ -101,6 +131,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)
@@ -112,6 +148,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"
@@ -113,3 +114,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")

			}

		})

	}

}