Merge "Refactor upload-only to process bazel metrics outside of upload.go"

cmd/soong_ui/main.go

@@ -99,7 +99,7 @@ var commands = []command{
 		// Upload-only mode mostly skips to the metrics-uploading phase of soong_ui.
 		// However, this invocation marks the true "end of the build", and thus we
 		// need to update the total runtime of the build to include this upload step.
-		run: updateTotalRealTime,
+		run: finalizeBazelMetrics,
 	},
 }
 
@@ -203,8 +203,6 @@ func main() {
 	bazelMetricsFile := filepath.Join(logsDir, c.logsPrefix+"bazel_metrics.pb")
 	soongBuildMetricsFile := filepath.Join(logsDir, c.logsPrefix+"soong_build_metrics.pb")
 
-	//the profile file generated by Bazel"
-	bazelProfileFile := filepath.Join(logsDir, c.logsPrefix+"analyzed_bazel_profile.txt")
 	metricsFiles := []string{
 		buildErrorFile,           // build error strings
 		rbeMetricsFile,           // high level metrics related to remote build execution.
@@ -226,7 +224,7 @@ func main() {
 		criticalPath.WriteToMetrics(met)
 		met.Dump(soongMetricsFile)
 		if !config.SkipMetricsUpload() {
-			build.UploadMetrics(buildCtx, config, c.simpleOutput, buildStarted, bazelProfileFile, bazelMetricsFile, metricsFiles...)
+			build.UploadMetrics(buildCtx, config, c.simpleOutput, buildStarted, metricsFiles...)
 		}
 	}()
 	c.run(buildCtx, config, args)
@@ -692,6 +690,15 @@ func setMaxFiles(ctx build.Context) {
 	}
 }
 
+func finalizeBazelMetrics(ctx build.Context, config build.Config, args []string) {
+	updateTotalRealTime(ctx, config, args)
+
+	logsDir := config.LogsDir()
+	logsPrefix := config.GetLogsPrefix()
+	bazelMetricsFile := filepath.Join(logsDir, logsPrefix+"bazel_metrics.pb")
+	bazelProfileFile := filepath.Join(logsDir, logsPrefix+"analyzed_bazel_profile.txt")
+	build.ProcessBazelMetrics(bazelProfileFile, bazelMetricsFile, ctx, config)
+}
 func updateTotalRealTime(ctx build.Context, config build.Config, args []string) {
 	soongMetricsFile := filepath.Join(config.LogsDir(), "soong_metrics")
 

ui/build/Android.bp

@@ -46,6 +46,7 @@ bootstrap_go_package {
         "soong-ui-tracer",
     ],
     srcs: [
+        "bazel_metrics.go",
         "build.go",
         "cleanbuild.go",
         "config.go",

ui/build/bazel_metrics.go

@@ -0,0 +1,135 @@
+// Copyright 2023 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package build
+
+// This file contains functionality to parse bazel profile data into
+// a bazel_metrics proto, defined in build/soong/ui/metrics/bazel_metrics_proto
+// These metrics are later uploaded in upload.go
+
+import (
+	"bufio"
+	"os"
+	"strconv"
+	"strings"
+
+	"android/soong/shared"
+	"google.golang.org/protobuf/proto"
+
+	bazel_metrics_proto "android/soong/ui/metrics/bazel_metrics_proto"
+)
+
+func parseTimingToNanos(str string) int64 {
+	millisString := removeDecimalPoint(str)
+	timingMillis, _ := strconv.ParseInt(millisString, 10, 64)
+	return timingMillis * 1000000
+}
+
+func parsePercentageToTenThousandths(str string) int32 {
+	percentageString := removeDecimalPoint(str)
+	//remove the % at the end of the string
+	percentage := strings.ReplaceAll(percentageString, "%", "")
+	percentagePortion, _ := strconv.ParseInt(percentage, 10, 32)
+	return int32(percentagePortion)
+}
+
+func removeDecimalPoint(numString string) string {
+	// The format is always 0.425 or 10.425
+	return strings.ReplaceAll(numString, ".", "")
+}
+
+func parseTotal(line string) int64 {
+	words := strings.Fields(line)
+	timing := words[3]
+	return parseTimingToNanos(timing)
+}
+
+func parsePhaseTiming(line string) bazel_metrics_proto.PhaseTiming {
+	words := strings.Fields(line)
+	getPhaseNameAndTimingAndPercentage := func([]string) (string, int64, int32) {
+		// Sample lines include:
+		// Total launch phase time   0.011 s    2.59%
+		// Total target pattern evaluation phase time  0.011 s    2.59%
+		var beginning int
+		var end int
+		for ind, word := range words {
+			if word == "Total" {
+				beginning = ind + 1
+			} else if beginning > 0 && word == "phase" {
+				end = ind
+				break
+			}
+		}
+		phaseName := strings.Join(words[beginning:end], " ")
+
+		// end is now "phase" - advance by 2 for timing and 4 for percentage
+		percentageString := words[end+4]
+		timingString := words[end+2]
+		timing := parseTimingToNanos(timingString)
+		percentagePortion := parsePercentageToTenThousandths(percentageString)
+		return phaseName, timing, percentagePortion
+	}
+
+	phaseName, timing, portion := getPhaseNameAndTimingAndPercentage(words)
+	phaseTiming := bazel_metrics_proto.PhaseTiming{}
+	phaseTiming.DurationNanos = &timing
+	phaseTiming.PortionOfBuildTime = &portion
+
+	phaseTiming.PhaseName = &phaseName
+	return phaseTiming
+}
+
+// This method takes a file created by bazel's --analyze-profile mode and
+// writes bazel metrics data to the provided filepath.
+func ProcessBazelMetrics(bazelProfileFile string, bazelMetricsFile string, ctx Context, config Config) {
+	if bazelProfileFile == "" {
+		return
+	}
+
+	readBazelProto := func(filepath string) bazel_metrics_proto.BazelMetrics {
+		//serialize the proto, write it
+		bazelMetrics := bazel_metrics_proto.BazelMetrics{}
+
+		file, err := os.ReadFile(filepath)
+		if err != nil {
+			ctx.Fatalln("Error reading metrics file\n", err)
+		}
+
+		scanner := bufio.NewScanner(strings.NewReader(string(file)))
+		scanner.Split(bufio.ScanLines)
+
+		var phaseTimings []*bazel_metrics_proto.PhaseTiming
+		for scanner.Scan() {
+			line := scanner.Text()
+			if strings.HasPrefix(line, "Total run time") {
+				total := parseTotal(line)
+				bazelMetrics.Total = &total
+			} else if strings.HasPrefix(line, "Total") {
+				phaseTiming := parsePhaseTiming(line)
+				phaseTimings = append(phaseTimings, &phaseTiming)
+			}
+		}
+		bazelMetrics.PhaseTimings = phaseTimings
+
+		return bazelMetrics
+	}
+
+	if _, err := os.Stat(bazelProfileFile); err != nil {
+		// We can assume bazel didn't run if the profile doesn't exist
+		return
+	}
+	bazelProto := readBazelProto(bazelProfileFile)
+	bazelProto.ExitCode = proto.Int32(config.bazelExitCode)
+	shared.Save(&bazelProto, bazelMetricsFile)
+}

ui/build/upload.go

@@ -18,21 +18,16 @@ package build
 // another.
 
 import (
-	"bufio"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strconv"
-	"strings"
 	"time"
 
-	"android/soong/shared"
 	"android/soong/ui/metrics"
 
 	"google.golang.org/protobuf/proto"
 
-	bazel_metrics_proto "android/soong/ui/metrics/bazel_metrics_proto"
 	upload_proto "android/soong/ui/metrics/upload_proto"
 )
 
@@ -78,123 +73,16 @@ func pruneMetricsFiles(paths []string) []string {
 	return metricsFiles
 }
 
-func parseTimingToNanos(str string) int64 {
-	millisString := removeDecimalPoint(str)
-	timingMillis, _ := strconv.ParseInt(millisString, 10, 64)
-	return timingMillis * 1000000
-}
-
-func parsePercentageToTenThousandths(str string) int32 {
-	percentageString := removeDecimalPoint(str)
-	//remove the % at the end of the string
-	percentage := strings.ReplaceAll(percentageString, "%", "")
-	percentagePortion, _ := strconv.ParseInt(percentage, 10, 32)
-	return int32(percentagePortion)
-}
-
-func removeDecimalPoint(numString string) string {
-	// The format is always 0.425 or 10.425
-	return strings.ReplaceAll(numString, ".", "")
-}
-
-func parseTotal(line string) int64 {
-	words := strings.Fields(line)
-	timing := words[3]
-	return parseTimingToNanos(timing)
-}
-
-func parsePhaseTiming(line string) bazel_metrics_proto.PhaseTiming {
-	words := strings.Fields(line)
-	getPhaseNameAndTimingAndPercentage := func([]string) (string, int64, int32) {
-		// Sample lines include:
-		// Total launch phase time   0.011 s    2.59%
-		// Total target pattern evaluation phase time  0.011 s    2.59%
-		var beginning int
-		var end int
-		for ind, word := range words {
-			if word == "Total" {
-				beginning = ind + 1
-			} else if beginning > 0 && word == "phase" {
-				end = ind
-				break
-			}
-		}
-		phaseName := strings.Join(words[beginning:end], " ")
-
-		// end is now "phase" - advance by 2 for timing and 4 for percentage
-		percentageString := words[end+4]
-		timingString := words[end+2]
-		timing := parseTimingToNanos(timingString)
-		percentagePortion := parsePercentageToTenThousandths(percentageString)
-		return phaseName, timing, percentagePortion
-	}
-
-	phaseName, timing, portion := getPhaseNameAndTimingAndPercentage(words)
-	phaseTiming := bazel_metrics_proto.PhaseTiming{}
-	phaseTiming.DurationNanos = &timing
-	phaseTiming.PortionOfBuildTime = &portion
-
-	phaseTiming.PhaseName = &phaseName
-	return phaseTiming
-}
-
-// This method takes a file created by bazel's --analyze-profile mode and
-// writes bazel metrics data to the provided filepath.
-// TODO(b/279987768) - move this outside of upload.go
-func processBazelMetrics(bazelProfileFile string, bazelMetricsFile string, ctx Context, config Config) {
-	if bazelProfileFile == "" {
-		return
-	}
-
-	readBazelProto := func(filepath string) bazel_metrics_proto.BazelMetrics {
-		//serialize the proto, write it
-		bazelMetrics := bazel_metrics_proto.BazelMetrics{}
-
-		file, err := os.ReadFile(filepath)
-		if err != nil {
-			ctx.Fatalln("Error reading metrics file\n", err)
-		}
-
-		scanner := bufio.NewScanner(strings.NewReader(string(file)))
-		scanner.Split(bufio.ScanLines)
-
-		var phaseTimings []*bazel_metrics_proto.PhaseTiming
-		for scanner.Scan() {
-			line := scanner.Text()
-			if strings.HasPrefix(line, "Total run time") {
-				total := parseTotal(line)
-				bazelMetrics.Total = &total
-			} else if strings.HasPrefix(line, "Total") {
-				phaseTiming := parsePhaseTiming(line)
-				phaseTimings = append(phaseTimings, &phaseTiming)
-			}
-		}
-		bazelMetrics.PhaseTimings = phaseTimings
-
-		return bazelMetrics
-	}
-
-	if _, err := os.Stat(bazelProfileFile); err != nil {
-		// We can assume bazel didn't run if the profile doesn't exist
-		return
-	}
-	bazelProto := readBazelProto(bazelProfileFile)
-	bazelProto.ExitCode = proto.Int32(config.bazelExitCode)
-	shared.Save(&bazelProto, bazelMetricsFile)
-}
-
 // 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, bazelProfileFile string, bazelMetricsFile string, paths ...string) {
+func UploadMetrics(ctx Context, config Config, simpleOutput bool, buildStarted time.Time, paths ...string) {
 	ctx.BeginTrace(metrics.RunSetupTool, "upload_metrics")
 	defer ctx.EndTrace()
 
 	uploader := config.MetricsUploaderApp()
-	processBazelMetrics(bazelProfileFile, bazelMetricsFile, ctx, config)
-
 	if uploader == "" {
 		// If the uploader path was not specified, no metrics shall be uploaded.
 		return

ui/build/upload_test.go

@@ -166,7 +166,7 @@ func TestUploadMetrics(t *testing.T) {
 				metricsUploader: tt.uploader,
 			}}
 
-			UploadMetrics(ctx, config, false, time.Now(), "out/bazel_metrics.txt", "out/bazel_metrics.pb", metricsFiles...)
+			UploadMetrics(ctx, config, false, time.Now(), metricsFiles...)
 		})
 	}
 }
@@ -221,7 +221,7 @@ func TestUploadMetricsErrors(t *testing.T) {
 				metricsUploader: "echo",
 			}}
 
-			UploadMetrics(ctx, config, true, time.Now(), "", "", metricsFile)
+			UploadMetrics(ctx, config, true, time.Now(), metricsFile)
 			t.Errorf("got nil, expecting %q as a failure", tt.expectedErr)
 		})
 	}