diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/BUILD.bazel b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/BUILD.bazel index bb49785758c..0f2f57c3cd4 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/BUILD.bazel +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/BUILD.bazel @@ -232,7 +232,6 @@ cf_cc_library( srcs = ["logs.cpp"], hdrs = ["logs.h"], deps = [ - "//cuttlefish/common/libs/utils:files", "//cuttlefish/common/libs/utils:flag_parser", "//cuttlefish/host/commands/cvd/cli:command_request", "//cuttlefish/host/commands/cvd/cli:types", diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/logs.cpp b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/logs.cpp index 488f4ec564d..1c36260aad0 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/commands/logs.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/commands/logs.cpp @@ -12,7 +12,6 @@ #include #include "absl/log/log.h" -#include "cuttlefish/common/libs/utils/files.h" #include "cuttlefish/common/libs/utils/flag_parser.h" #include "cuttlefish/host/commands/cvd/cli/command_request.h" #include "cuttlefish/host/commands/cvd/cli/commands/command_handler.h" @@ -42,8 +41,8 @@ struct LogsCmdOptions { } }; -Result PrintLogsList(const std::string& dir) { - auto callback = [](const std::string& filename) -> Result { +void PrintLogsList(const std::vector& filenames) { + for (auto& filename : filenames) { std::string basename = android::base::Basename(filename); std::cout << basename; std::cout << " "; @@ -55,10 +54,20 @@ Result PrintLogsList(const std::string& dir) { } std::cout << filename; std::cout << std::endl; - return {}; }; - CF_EXPECT(WalkDirectory(dir, callback)); - return {}; +} + +std::vector RemoveInaccessibleFilenames( + std::vector filenames) { + std::erase_if(filenames, [](const std::string& v) { + bool accessible = access(v.c_str(), F_OK) == 0; + if (!accessible) { + std::string basename = android::base::Basename(v); + VLOG(0) << "Logs file `" << basename << "` not found at \"" << v << "\""; + } + return !accessible; + }); + return filenames; } Result PrintLog(const std::string& filename) { @@ -73,7 +82,7 @@ CvdLogsHandler::CvdLogsHandler(InstanceManager& instance_manager) : instance_manager_(instance_manager) {} Result CvdLogsHandler::Handle(const CommandRequest& request) { - auto [instance, _] = + auto [instance, group] = CF_EXPECT(selector::SelectInstance(instance_manager_, request), "Unable to select an instance"); @@ -81,23 +90,43 @@ Result CvdLogsHandler::Handle(const CommandRequest& request) { std::vector args = request.SubcommandArguments(); CF_EXPECT(ConsumeFlags(opts.Flags(), args)); - std::string dir = instance.instance_dir(); - std::string logs_dir = dir + "/logs"; - if (!FileExists(logs_dir)) { - VLOG(0) << "Logs directory `" << logs_dir << "` does not exist."; - LOG(INFO) << "There are no logs files available"; - return {}; + std::vector logs_filenames; + + auto group_names = RemoveInaccessibleFilenames(group.LogsFilenames()); + logs_filenames = group_names; + auto ins_names = + RemoveInaccessibleFilenames(CF_EXPECT(instance.LogsFilenames())); + // Avoid inserting instance log names that are already fetched at group level. + for (auto ins_filename : ins_names) { + std::string base = android::base::Basename(ins_filename); + auto p = [base](const auto& v) { + return base == android::base::Basename(v); + }; + auto it = std::find_if(group_names.begin(), group_names.end(), p); + if (it == group_names.end()) { + logs_filenames.push_back(ins_filename); + } } - CF_EXPECT(IsDirectory(logs_dir)); if (opts.print_target.empty()) { - CF_EXPECT(PrintLogsList(logs_dir)); + if (logs_filenames.empty()) { + LOG(INFO) << "There are no log files available"; + } else { + PrintLogsList(logs_filenames); + } return {}; } - std::string print_target = logs_dir + "/" + opts.print_target; - CF_EXPECT(FileExists(print_target)); - CF_EXPECT(PrintLog(print_target)); + for (auto& filename : logs_filenames) { + std::string basename = android::base::Basename(filename); + if (basename == opts.print_target) { + CF_EXPECT(PrintLog(filename)); + return {}; + } + }; + + CF_EXPECTF(false, "Not found `{}` logs", opts.print_target); + return {}; } diff --git a/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance.cpp b/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance.cpp index b5671618a17..8ab0d1dcda9 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance.cpp @@ -16,14 +16,16 @@ #include "cuttlefish/host/commands/cvd/instances/local_instance.h" +#include +#include + #include #include #include #include -#include #include "absl/log/log.h" - +#include "cuttlefish/common/libs/utils/contains.h" #include "cuttlefish/common/libs/utils/files.h" #include "cuttlefish/common/libs/utils/subprocess.h" #include "cuttlefish/common/libs/utils/subprocess_managed_stdio.h" @@ -62,7 +64,7 @@ void LocalInstance::set_state(cvd::InstanceState state) { std::string LocalInstance::instance_dir() const { std::string to_ret = fmt::format("{}/cuttlefish/instances/cvd-{}", group_proto_->home_directory(), id()); - if(!FileExists(to_ret)) { + if (!FileExists(to_ret)) { // Legacy launchers create cuttlefish_runtime.{$ID} to_ret = fmt::format("{}/cuttlefish_runtime.{}", group_proto_->home_directory(), id()); @@ -272,4 +274,14 @@ Result LocalInstance::GetLauncherMonitor( return monitor; } +Result> LocalInstance::LogsFilenames() const { + std::vector result = {}; + auto callback = [&result](const std::string& filename) -> Result { + result.push_back(filename); + return {}; + }; + CF_EXPECT(WalkDirectory(instance_dir() + "/logs", callback)); + return result; +} + } // namespace cuttlefish diff --git a/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance.h b/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance.h index 6db8601c616..384f5f90c7a 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance.h +++ b/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance.h @@ -16,14 +16,14 @@ #pragma once +#include + #include #include #include #include #include -#include - #include "cuttlefish/common/libs/fs/shared_fd.h" #include "cuttlefish/host/commands/cvd/instances/cvd_persistent_data.pb.h" #include "cuttlefish/host/libs/config/cuttlefish_config.h" @@ -74,6 +74,9 @@ class LocalInstance { Result StartRecording(std::chrono::seconds launcher_timeout); Result StopRecording(std::chrono::seconds launcher_timeout); + // Return list of filenames of instance-level log files. + Result> LogsFilenames() const; + private: LocalInstance(std::shared_ptr group_proto, cvd::Instance* instance_proto); diff --git a/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.cpp b/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.cpp index e171adfa559..3c4001b38ce 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.cpp @@ -16,6 +16,9 @@ #include "cuttlefish/host/commands/cvd/instances/local_instance_group.h" +#include +#include + #include #include #include @@ -24,10 +27,7 @@ #include #include -#include #include "absl/strings/str_join.h" -#include - #include "cuttlefish/host/commands/cvd/instances/instance_database_types.h" #include "cuttlefish/host/commands/cvd/instances/local_instance.h" #include "cuttlefish/host/commands/cvd/utils/common.h" @@ -131,8 +131,7 @@ Result LocalInstanceGroup::Builder::Build() { product_out_paths.emplace_back(ProductDirFromBase(base_dir_, i)); } - group_proto_.set_product_out_path( - absl::StrJoin(product_out_paths, ",")); + group_proto_.set_product_out_path(absl::StrJoin(product_out_paths, ",")); return CF_EXPECT(LocalInstanceGroup::Create(group_proto_)); } @@ -204,6 +203,15 @@ std::string LocalInstanceGroup::BaseDir() const { return android::base::Dirname(HomeDir()); } +std::vector LocalInstanceGroup::LogsFilenames() const { + return { + AssemblyDir() + "/assemble_cvd.log", + AssemblyDir() + "/cuttlefish_config.json", + ArtifactsDir() + "/fetch.log", + MetricsDir() + "/metrics.log", + }; +} + Result LocalInstanceGroup::FetchStatus( std::chrono::seconds timeout) { Json::Value instances_json(Json::arrayValue); diff --git a/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.h b/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.h index b130abff8e8..954eec437a9 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.h +++ b/base/cvd/cuttlefish/host/commands/cvd/instances/local_instance_group.h @@ -16,12 +16,12 @@ #pragma once +#include + #include #include #include -#include - #include "cuttlefish/host/commands/cvd/instances/cvd_persistent_data.pb.h" #include "cuttlefish/host/commands/cvd/instances/instance_database_types.h" #include "cuttlefish/host/commands/cvd/instances/local_instance.h" @@ -74,6 +74,9 @@ class LocalInstanceGroup { std::string ArtifactsDir() const; std::string ProductDir(int instance_index) const; + // Return list of filenames of group-level log files. + std::vector LogsFilenames() const; + Result FindInstanceById(unsigned id) const; /** * Find by per-instance name. diff --git a/e2etests/cvd/logs_tests/main_test.go b/e2etests/cvd/logs_tests/main_test.go index bf0d22ce7dd..c16a59108f1 100644 --- a/e2etests/cvd/logs_tests/main_test.go +++ b/e2etests/cvd/logs_tests/main_test.go @@ -23,67 +23,91 @@ import ( ) func TestPrintLogs(t *testing.T) { - testcases := []struct { - branch string - target string - }{ - { - branch: "git_main", - target: "aosp_cf_x86_64_only_phone-trunk_staging-userdebug", - }, - } c := e2etests.TestContext{} - for _, tc := range testcases { - t.Run("foo", func(t *testing.T) { - c.SetUp(t) - defer c.TearDown() - - if err := c.CVDFetch(e2etests.FetchArgs{ - DefaultBuildBranch: tc.branch, - DefaultBuildTarget: tc.target, - }); err != nil { - t.Fatal(err) - } + c.SetUp(t) + defer c.TearDown() - if err := c.CVDCreate(e2etests.CreateArgs{}); err != nil { - t.Fatal(err) - } + env_config := ` +{ + "instances": [ + { + "name": "ins-1", + "disk": { + "default_build": "@ab\/git_main\/aosp_cf_x86_64_only_phone-trunk_staging-userdebug" + }, + "vm": { + "cpus": 4, + "memory_mb": 4096, + "setupwizard_mode": "REQUIRED" + }, + "graphics": { + "displays": [ + { + "width": 720, + "height": 1280, + "dpi": 140, + "refresh_rate_hertz": 60 + } + ], + "record_screen": false + } + } + ], + "netsim_bt": false, + "metrics": { + "enable": true + }, + "common": { + "host_package": "@ab\/git_main\/aosp_cf_x86_64_only_phone-trunk_staging-userdebug" + } +} +` - res, err := c.RunCmd(c.TargetBin(), "logs") - if err != nil { - t.Fatal(err) - } - stdOut := res.Stdout + if err := c.CVDLoad(e2etests.LoadArgs{LoadConfig: env_config}); err != nil { + t.Fatal(err) + } - listedNames := []string{} - lines := strings.Split(stdOut, "\n") - for _, line := range lines[:len(lines)-1] { - fields := strings.Fields(line) - if len(fields) != 2 { - t.Errorf("line %q does not match format ' '", line) - } - res, err := c.RunCmd("stat", fields[1]) - if err != nil { - t.Fatal(res.Stderr) - } - listedNames = append(listedNames, fields[0]) - } - keyExpectedNames := []string{"logcat", "launcher.log", "kernel.log"} - for _, name := range keyExpectedNames { - if !slices.Contains(listedNames, name) { - t.Fatalf("missing log name: %q", name) - } - } + res, err := c.RunCmd(c.TargetBin(), "logs") + if err != nil { + t.Fatal(err) + } + stdOut := res.Stdout - res, err = c.RunCmd(c.TargetBin(), "logs", "--print", "launcher.log") - if err != nil { - t.Fatal(err) - } - stdOut = res.Stdout - if len(stdOut) == 0 { - t.Fatalf("empty launcher.log") - } + listedNames := []string{} + lines := strings.Split(stdOut, "\n") + for _, line := range lines[:len(lines)-1] { + fields := strings.Fields(line) + if len(fields) != 2 { + t.Errorf("line %q does not match format ' '", line) + } + res, err := c.RunCmd("stat", fields[1]) + if err != nil { + t.Fatal(res.Stderr) + } + listedNames = append(listedNames, fields[0]) + } + keyExpectedNames := []string{ + // Group-level logs + "assemble_cvd.log", + "fetch.log", + // Instance-level logs + "kernel.log", + "launcher.log", + "logcat", + } + for _, name := range keyExpectedNames { + if !slices.Contains(listedNames, name) { + t.Fatalf("missing log name: %q", name) + } + } - }) + res, err = c.RunCmd(c.TargetBin(), "logs", "--print", "launcher.log") + if err != nil { + t.Fatal(err) } + stdOut = res.Stdout + if len(stdOut) == 0 { + t.Fatalf("empty launcher.log") + } + }