From 947fdbfdeee21cbb6490b01f3c2175bb039995c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20M=C3=A1rquez=20P=C3=A9rez=20Mu=C3=B1=C3=ADz=20D?= =?UTF-8?q?=C3=ADaz=20P=C3=BAras=20Thaureaux?= Date: Wed, 10 Nov 2021 09:55:20 -0500 Subject: [PATCH] Log bp2build_metrics .pb Also share `Save(pb proto.Message, filepath string)` Bug: 201539536 Test: bp2build_metrics.pb has expected content & path Test: m nothing Test: {bp2build,mixed_{libc,droid}}.sh Test: CI Change-Id: I7d8ad87fca6a4b0355010090a527f5ae67b27c88 --- bp2build/Android.bp | 2 ++ bp2build/build_conversion.go | 2 +- bp2build/metrics.go | 70 ++++++++++++++++++++++++++++++++---- cmd/soong_build/main.go | 11 ++++++ shared/Android.bp | 2 ++ shared/proto.go | 41 +++++++++++++++++++++ ui/build/config.go | 14 +++++--- ui/build/soong.go | 1 + ui/metrics/Android.bp | 1 + ui/metrics/metrics.go | 26 ++------------ 10 files changed, 136 insertions(+), 34 deletions(-) create mode 100644 shared/proto.go diff --git a/bp2build/Android.bp b/bp2build/Android.bp index 9b66354b1..337fe86c1 100644 --- a/bp2build/Android.bp +++ b/bp2build/Android.bp @@ -19,6 +19,7 @@ bootstrap_go_package { deps: [ "soong-android", "soong-android-soongconfig", + "soong-shared", "soong-apex", "soong-bazel", "soong-cc", @@ -27,6 +28,7 @@ bootstrap_go_package { "soong-genrule", "soong-python", "soong-sh", + "soong-ui-metrics", ], testSrcs: [ "android_app_certificate_conversion_test.go", diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index eb60cd16c..54b59af3a 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -259,7 +259,7 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers // Simple metrics tracking for bp2build metrics := CodegenMetrics{ - ruleClassCount: make(map[string]int), + ruleClassCount: make(map[string]uint64), } dirs := make(map[string]bool) diff --git a/bp2build/metrics.go b/bp2build/metrics.go index b3d5afb74..68ac54435 100644 --- a/bp2build/metrics.go +++ b/bp2build/metrics.go @@ -2,34 +2,52 @@ package bp2build import ( "fmt" + "os" + "path/filepath" "strings" "android/soong/android" + "android/soong/shared" + "android/soong/ui/metrics/bp2build_metrics_proto" ) // Simple metrics struct to collect information about a Blueprint to BUILD // conversion process. type CodegenMetrics struct { // Total number of Soong modules converted to generated targets - generatedModuleCount int + generatedModuleCount uint64 // Total number of Soong modules converted to handcrafted targets - handCraftedModuleCount int + handCraftedModuleCount uint64 // Total number of unconverted Soong modules - unconvertedModuleCount int + unconvertedModuleCount uint64 // Counts of generated Bazel targets per Bazel rule class - ruleClassCount map[string]int + ruleClassCount map[string]uint64 + // List of modules with unconverted deps + // NOTE: NOT in the .proto moduleWithUnconvertedDepsMsgs []string + // List of converted modules convertedModules []string } +// Serialize returns the protoized version of CodegenMetrics: bp2build_metrics_proto.Bp2BuildMetrics +func (metrics *CodegenMetrics) Serialize() bp2build_metrics_proto.Bp2BuildMetrics { + return bp2build_metrics_proto.Bp2BuildMetrics{ + GeneratedModuleCount: metrics.generatedModuleCount, + HandCraftedModuleCount: metrics.handCraftedModuleCount, + UnconvertedModuleCount: metrics.unconvertedModuleCount, + RuleClassCount: metrics.ruleClassCount, + ConvertedModules: metrics.convertedModules, + } +} + // Print the codegen metrics to stdout. func (metrics *CodegenMetrics) Print() { - generatedTargetCount := 0 + generatedTargetCount := uint64(0) for _, ruleClass := range android.SortedStringKeys(metrics.ruleClassCount) { count := metrics.ruleClassCount[ruleClass] fmt.Printf("[bp2build] %s: %d targets\n", ruleClass, count) @@ -45,6 +63,40 @@ func (metrics *CodegenMetrics) Print() { strings.Join(metrics.moduleWithUnconvertedDepsMsgs, "\n\t")) } +const bp2buildMetricsFilename = "bp2build_metrics.pb" + +// fail prints $PWD to stderr, followed by the given printf string and args (vals), +// then the given alert, and then exits with 1 for failure +func fail(err error, alertFmt string, vals ...interface{}) { + cwd, wderr := os.Getwd() + if wderr != nil { + cwd = "FAILED TO GET $PWD: " + wderr.Error() + } + fmt.Fprintf(os.Stderr, "\nIn "+cwd+":\n"+alertFmt+"\n"+err.Error()+"\n", vals...) + os.Exit(1) +} + +// Write the bp2build-protoized codegen metrics into the given directory +func (metrics *CodegenMetrics) Write(dir string) { + if _, err := os.Stat(dir); os.IsNotExist(err) { + // The metrics dir doesn't already exist, so create it (and parents) + if err := os.MkdirAll(dir, 0755); err != nil { // rx for all; w for user + fail(err, "Failed to `mkdir -p` %s", dir) + } + } else if err != nil { + fail(err, "Failed to `stat` %s", dir) + } + metricsFile := filepath.Join(dir, bp2buildMetricsFilename) + if err := metrics.dump(metricsFile); err != nil { + fail(err, "Error outputting %s", metricsFile) + } + if _, err := os.Stat(metricsFile); err != nil { + fail(err, "MISSING BP2BUILD METRICS OUTPUT: Failed to `stat` %s", metricsFile) + } else { + fmt.Printf("\nWrote bp2build metrics to: %s\n", metricsFile) + } +} + func (metrics *CodegenMetrics) IncrementRuleClassCount(ruleClass string) { metrics.ruleClassCount[ruleClass] += 1 } @@ -53,12 +105,18 @@ func (metrics *CodegenMetrics) IncrementUnconvertedCount() { metrics.unconvertedModuleCount += 1 } -func (metrics *CodegenMetrics) TotalModuleCount() int { +func (metrics *CodegenMetrics) TotalModuleCount() uint64 { return metrics.handCraftedModuleCount + metrics.generatedModuleCount + metrics.unconvertedModuleCount } +// Dump serializes the metrics to the given filename +func (metrics *CodegenMetrics) dump(filename string) (err error) { + ser := metrics.Serialize() + return shared.Save(&ser, filename) +} + type ConversionType int const ( diff --git a/cmd/soong_build/main.go b/cmd/soong_build/main.go index c81d4bc6a..f07eafabe 100644 --- a/cmd/soong_build/main.go +++ b/cmd/soong_build/main.go @@ -537,6 +537,7 @@ func runBp2Build(configuration android.Config, extraNinjaDeps []string) { // for queryview, since that's a total repo-wide conversion and there's a // 1:1 mapping for each module. metrics.Print() + writeBp2BuildMetrics(&metrics, configuration) ninjaDeps = append(ninjaDeps, codegenContext.AdditionalNinjaDeps()...) ninjaDeps = append(ninjaDeps, symlinkForestDeps...) @@ -546,3 +547,13 @@ func runBp2Build(configuration android.Config, extraNinjaDeps []string) { // Create an empty bp2build marker file. touch(shared.JoinPath(topDir, bp2buildMarker)) } + +// Write Bp2Build metrics into $LOG_DIR +func writeBp2BuildMetrics(metrics *bp2build.CodegenMetrics, configuration android.Config) { + metricsDir := configuration.Getenv("LOG_DIR") + if len(metricsDir) < 1 { + fmt.Fprintf(os.Stderr, "\nMissing required env var for generating bp2build metrics: LOG_DIR\n") + os.Exit(1) + } + metrics.Write(metricsDir) +} diff --git a/shared/Android.bp b/shared/Android.bp index deb17f8f6..3c84f5532 100644 --- a/shared/Android.bp +++ b/shared/Android.bp @@ -9,11 +9,13 @@ bootstrap_go_package { "env.go", "paths.go", "debug.go", + "proto.go", ], testSrcs: [ "paths_test.go", ], deps: [ "soong-bazel", + "golang-protobuf-proto", ], } diff --git a/shared/proto.go b/shared/proto.go new file mode 100644 index 000000000..232656ba4 --- /dev/null +++ b/shared/proto.go @@ -0,0 +1,41 @@ +// Copyright 2021 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 shared + +import ( + "io/ioutil" + "os" + + "google.golang.org/protobuf/proto" +) + +// Save takes a protobuf message, marshals to an array of bytes +// and is then saved to a file. +func Save(pb proto.Message, filepath string) (err error) { + data, err := proto.Marshal(pb) + if err != nil { + return err + } + tempFilepath := filepath + ".tmp" + if err := ioutil.WriteFile(tempFilepath, []byte(data), 0644 /* rw-r--r-- */); err != nil { + return err + } + + if err := os.Rename(tempFilepath, filepath); err != nil { + return err + } + + return nil +} diff --git a/ui/build/config.go b/ui/build/config.go index c30663349..4c26d3ea5 100644 --- a/ui/build/config.go +++ b/ui/build/config.go @@ -1255,16 +1255,22 @@ func (c *configImpl) MetricsUploaderApp() string { return c.metricsUploader } -// LogsDir returns the logs directory where build log and metrics -// files are located. By default, the logs directory is the out +// LogsDir returns the absolute path to the logs directory where build log and +// metrics files are located. By default, the logs directory is the out // directory. If the argument dist is specified, the logs directory // is /logs. func (c *configImpl) LogsDir() string { + dir := c.OutDir() if c.Dist() { // Always write logs to the real dist dir, even if Bazel is using a rigged dist dir for other files - return filepath.Join(c.RealDistDir(), "logs") + dir = filepath.Join(c.RealDistDir(), "logs") } - return c.OutDir() + absDir, err := filepath.Abs(dir) + if err != nil { + fmt.Fprintf(os.Stderr, "\nError making log dir '%s' absolute: %s\n", dir, err.Error()) + os.Exit(1) + } + return absDir } // BazelMetricsDir returns the /bazel_metrics directory diff --git a/ui/build/soong.go b/ui/build/soong.go index ae9a2ce25..5360342eb 100644 --- a/ui/build/soong.go +++ b/ui/build/soong.go @@ -373,6 +373,7 @@ func runSoong(ctx Context, config Config) { soongBuildEnv.Set("BAZEL_OUTPUT_BASE", filepath.Join(config.BazelOutDir(), "output")) soongBuildEnv.Set("BAZEL_WORKSPACE", absPath(ctx, ".")) soongBuildEnv.Set("BAZEL_METRICS_DIR", config.BazelMetricsDir()) + soongBuildEnv.Set("LOG_DIR", config.LogsDir()) // For Soong bootstrapping tests if os.Getenv("ALLOW_MISSING_DEPENDENCIES") == "true" { diff --git a/ui/metrics/Android.bp b/ui/metrics/Android.bp index 96f6389e8..3ba3907a6 100644 --- a/ui/metrics/Android.bp +++ b/ui/metrics/Android.bp @@ -25,6 +25,7 @@ bootstrap_go_package { "soong-ui-metrics_proto", "soong-ui-bp2build_metrics_proto", "soong-ui-tracer", + "soong-shared", ], srcs: [ "metrics.go", diff --git a/ui/metrics/metrics.go b/ui/metrics/metrics.go index f1bb862bd..80f8c1ad0 100644 --- a/ui/metrics/metrics.go +++ b/ui/metrics/metrics.go @@ -32,12 +32,12 @@ package metrics // of what an event is and how the metrics system is a stack based system. import ( - "io/ioutil" "os" "runtime" "strings" "time" + "android/soong/shared" "google.golang.org/protobuf/proto" soong_metrics_proto "android/soong/ui/metrics/metrics_proto" @@ -196,7 +196,7 @@ func (m *Metrics) Dump(out string) error { } m.metrics.HostOs = proto.String(runtime.GOOS) - return save(&m.metrics, out) + return shared.Save(&m.metrics, out) } // SetSoongBuildMetrics sets the metrics collected from the soong_build @@ -228,25 +228,5 @@ func (c *CriticalUserJourneysMetrics) Add(name string, metrics *Metrics) { // Dump saves the collected CUJs metrics to the raw protobuf file. func (c *CriticalUserJourneysMetrics) Dump(filename string) (err error) { - return save(&c.cujs, filename) -} - -// save takes a protobuf message, marshals to an array of bytes -// and is then saved to a file. -func save(pb proto.Message, filename string) (err error) { - data, err := proto.Marshal(pb) - if err != nil { - return err - } - - tempFilename := filename + ".tmp" - if err := ioutil.WriteFile(tempFilename, []byte(data), 0644 /* rw-r--r-- */); err != nil { - return err - } - - if err := os.Rename(tempFilename, filename); err != nil { - return err - } - - return nil + return shared.Save(&c.cujs, filename) }