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
This commit is contained in:
Patrice Arruda 2020-06-10 18:48:01 +00:00
parent 000aef69b5
commit 7cc2074885
7 changed files with 227 additions and 52 deletions

View File

@ -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"))

View File

@ -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()
}
}

View File

@ -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,41 +97,62 @@ 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"
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)
}
})
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",
}}
outDir, err := ioutil.TempDir("", "")
if err != nil {
t.Fatalf("failed to create out directory: %v", outDir)
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, tt.expectedErr) {
t.Errorf("got %q, want %q to be contained in error", got, tt.expectedErr)
}
})
outDir, err := ioutil.TempDir("", "")
if err != nil {
t.Fatalf("failed to create out directory: %v", outDir)
}
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)
}
config := Config{&configImpl{
environ: &Environment{
"ANDROID_ENABLE_METRICS_UPLOAD=fake",
"OUT_DIR=/bad",
}}}
UploadMetrics(ctx, config, true, 1591031903, metricsFile)
t.Errorf("got nil, expecting %q as a failure", tt.expectedErr)
})
}
defer os.RemoveAll(outDir)
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)
}
config := Config{&configImpl{
environ: &Environment{
"ANDROID_ENABLE_METRICS_UPLOAD=fake",
"OUT_DIR=/bad",
}}}
UploadMetrics(testContext(), config, 1591031903, metricsFile)
t.Errorf("got nil, expecting %q as a failure", expectedErr)
}

View File

@ -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)
}

View File

@ -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")
}
})
}
}

View File

@ -30,7 +30,10 @@ type Upload struct {
// The target name.
TargetName *string `protobuf:"bytes,4,opt,name=target_name,json=targetName" json:"target_name,omitempty"`
// A list of metrics filepaths to upload.
MetricsFiles []string `protobuf:"bytes,5,rep,name=metrics_files,json=metricsFiles" json:"metrics_files,omitempty"`
MetricsFiles []string `protobuf:"bytes,5,rep,name=metrics_files,json=metricsFiles" json:"metrics_files,omitempty"`
// A list of directories to delete after the copy of metrics files
// is completed for uploading.
DirectoriesToDelete []string `protobuf:"bytes,6,rep,name=directories_to_delete,json=directoriesToDelete" json:"directories_to_delete,omitempty"`
XXX_NoUnkeyedLiteral struct{} `json:"-"`
XXX_unrecognized []byte `json:"-"`
XXX_sizecache int32 `json:"-"`
@ -96,6 +99,13 @@ func (m *Upload) GetMetricsFiles() []string {
return nil
}
func (m *Upload) GetDirectoriesToDelete() []string {
if m != nil {
return m.DirectoriesToDelete
}
return nil
}
func init() {
proto.RegisterType((*Upload)(nil), "soong_metrics_upload.Upload")
}
@ -105,18 +115,20 @@ func init() {
}
var fileDescriptor_91b94b655bd2a7e5 = []byte{
// 201 bytes of a gzipped FileDescriptorProto
0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe2, 0xe2, 0x29, 0x2d, 0xc8, 0xc9,
0x4f, 0x4c, 0xd1, 0x2b, 0x28, 0xca, 0x2f, 0xc9, 0x17, 0x12, 0x29, 0xce, 0xcf, 0xcf, 0x4b, 0x8f,
0xcf, 0x4d, 0x2d, 0x29, 0xca, 0x4c, 0x2e, 0x8e, 0x87, 0xc8, 0x29, 0xdd, 0x66, 0xe4, 0x62, 0x0b,
0x05, 0x33, 0x85, 0x8c, 0xb8, 0x44, 0x93, 0x8b, 0x52, 0x13, 0x4b, 0x32, 0xf3, 0xf3, 0xe2, 0x4b,
0x32, 0x73, 0x53, 0x8b, 0x4b, 0x12, 0x73, 0x0b, 0xe2, 0x73, 0x8b, 0x25, 0x18, 0x15, 0x18, 0x35,
0x58, 0x82, 0x84, 0x61, 0x92, 0x21, 0x30, 0x39, 0xdf, 0x62, 0x21, 0x33, 0x2e, 0xf1, 0xe4, 0xfc,
0xdc, 0x82, 0x9c, 0x54, 0x4c, 0x5d, 0x4c, 0x60, 0x5d, 0xa2, 0x08, 0x69, 0x64, 0x7d, 0xf2, 0x5c,
0xdc, 0x49, 0x45, 0x89, 0x79, 0xc9, 0x19, 0xf1, 0x79, 0x89, 0xb9, 0xa9, 0x12, 0xcc, 0x0a, 0x8c,
0x1a, 0x9c, 0x41, 0x5c, 0x10, 0x21, 0xbf, 0xc4, 0xdc, 0x54, 0x90, 0x82, 0x92, 0xc4, 0xa2, 0xf4,
0xd4, 0x12, 0x88, 0x02, 0x16, 0x88, 0x02, 0x88, 0x10, 0x58, 0x81, 0x32, 0x17, 0x2f, 0xcc, 0x2b,
0x69, 0x99, 0x39, 0xa9, 0xc5, 0x12, 0xac, 0x0a, 0xcc, 0x1a, 0x9c, 0x41, 0x3c, 0x50, 0x41, 0x37,
0x90, 0x98, 0x93, 0x4c, 0x94, 0x14, 0x36, 0x5f, 0xc7, 0x83, 0x43, 0x04, 0x10, 0x00, 0x00, 0xff,
0xff, 0xe2, 0x01, 0x74, 0x65, 0x20, 0x01, 0x00, 0x00,
// 230 bytes of a gzipped FileDescriptorProto
0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x90, 0xb1, 0x4a, 0x04, 0x31,
0x10, 0x86, 0xd9, 0xbb, 0xf3, 0xe0, 0xe2, 0xd9, 0xec, 0x79, 0x18, 0x44, 0x70, 0xd1, 0x66, 0x2b,
0x0b, 0x0b, 0x1f, 0x40, 0xc4, 0x4e, 0x8b, 0xe5, 0x6c, 0x6c, 0x86, 0x98, 0x1d, 0xd7, 0x40, 0x92,
0x09, 0xc9, 0xf8, 0x1c, 0xbe, 0xb2, 0x6c, 0xe2, 0xe2, 0x82, 0x76, 0xc3, 0xff, 0x7d, 0x7f, 0x31,
0xbf, 0xd8, 0x7e, 0x06, 0x4b, 0xaa, 0xbf, 0x09, 0x91, 0x98, 0xea, 0xd3, 0x44, 0xe4, 0x07, 0x70,
0xc8, 0xd1, 0xe8, 0x04, 0x85, 0x5d, 0x7d, 0x2d, 0xc4, 0xfa, 0x25, 0x9f, 0xf5, 0xad, 0xd8, 0xeb,
0x88, 0x8a, 0x0d, 0x79, 0x60, 0xe3, 0x30, 0xb1, 0x72, 0x01, 0x5c, 0x92, 0x55, 0x53, 0xb5, 0xab,
0x6e, 0x37, 0xc1, 0xc3, 0xc4, 0x9e, 0x52, 0x7d, 0x27, 0xce, 0x34, 0xb9, 0x60, 0xf1, 0x6f, 0x6b,
0x91, 0x5b, 0xfb, 0x5f, 0x3c, 0xef, 0x5d, 0x8a, 0xe3, 0xb7, 0xa8, 0xbc, 0xfe, 0x00, 0xaf, 0x1c,
0xca, 0x65, 0x53, 0xb5, 0x9b, 0x4e, 0x94, 0xe8, 0x59, 0x39, 0x1c, 0x05, 0x56, 0x71, 0x40, 0x2e,
0xc2, 0xaa, 0x08, 0x25, 0xca, 0xc2, 0xb5, 0x38, 0x99, 0x5e, 0x79, 0x37, 0x16, 0x93, 0x3c, 0x6a,
0x96, 0xed, 0xa6, 0xdb, 0xfe, 0x84, 0x8f, 0x63, 0x36, 0xbe, 0xd4, 0x9b, 0x88, 0x9a, 0x29, 0x1a,
0x4c, 0xc0, 0x04, 0x3d, 0x5a, 0x64, 0x94, 0xeb, 0x2c, 0xef, 0x66, 0xf0, 0x40, 0x0f, 0x19, 0xdd,
0x5f, 0xbc, 0x9e, 0xff, 0xb7, 0x14, 0xe4, 0x15, 0xbf, 0x03, 0x00, 0x00, 0xff, 0xff, 0x64, 0x04,
0xa8, 0xf4, 0x54, 0x01, 0x00, 0x00,
}

View File

@ -32,4 +32,8 @@ message Upload {
// A list of metrics filepaths to upload.
repeated string metrics_files = 5;
// A list of directories to delete after the copy of metrics files
// is completed for uploading.
repeated string directories_to_delete = 6;
}