Skip to content

Cvd logs subcommand #2498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ser-io wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Cvd logs subcommand #2498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4ee4cc2
Boilerplate for new subcommand: "logs"
ser-io Apr 29, 2026
d7a8efb
Fail if using invalid selector.
ser-io Apr 29, 2026
d288ca4
List available log names.
ser-io Apr 29, 2026
1decfea
Print given log name.
ser-io Apr 30, 2026
905829d
Split output between stdout and stderr
ser-io Apr 30, 2026
1177b9c
Add cvd logs e2e test.
ser-io Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions base/cvd/cuttlefish/host/commands/cvd/cli/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ cf_cc_library(
"//cuttlefish/host/commands/cvd/cli/commands:host_tool_target",
"//cuttlefish/host/commands/cvd/cli/commands:lint",
"//cuttlefish/host/commands/cvd/cli/commands:login",
"//cuttlefish/host/commands/cvd/cli/commands:logs",
"//cuttlefish/host/commands/cvd/cli/commands:power_btn",
"//cuttlefish/host/commands/cvd/cli/commands:powerwash",
"//cuttlefish/host/commands/cvd/cli/commands:remove",
Expand Down
18 changes: 18 additions & 0 deletions base/cvd/cuttlefish/host/commands/cvd/cli/commands/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,3 +450,21 @@ cf_cc_library(
"@jsoncpp",
],
)

cf_cc_library(
name = "logs",
srcs = ["logs.cpp"],
hdrs = ["logs.h"],
deps = [
"//cuttlefish/common/libs/utils:files",
"//cuttlefish/common/libs/utils:flag_parser",
"//cuttlefish/common/libs/utils:subprocess",
"//cuttlefish/host/commands/cvd/cli:command_request",
"//cuttlefish/host/commands/cvd/cli:types",
"//cuttlefish/host/commands/cvd/cli/commands:command_handler",
"//cuttlefish/host/commands/cvd/cli/selector",
"//cuttlefish/host/commands/cvd/instances:instance_manager",
"//cuttlefish/result",
"//libbase",
],
)
117 changes: 117 additions & 0 deletions base/cvd/cuttlefish/host/commands/cvd/cli/commands/logs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
#include "cuttlefish/host/commands/cvd/cli/commands/logs.h"

#include <algorithm>
#include <iostream>
#include <memory>
#include <string>
#include <vector>

#include <android-base/file.h>

#include "cuttlefish/common/libs/utils/files.h"
#include "cuttlefish/common/libs/utils/flag_parser.h"
#include "cuttlefish/common/libs/utils/subprocess.h"
#include "cuttlefish/host/commands/cvd/cli/command_request.h"
#include "cuttlefish/host/commands/cvd/cli/commands/command_handler.h"
#include "cuttlefish/host/commands/cvd/cli/selector/selector.h"
#include "cuttlefish/host/commands/cvd/cli/types.h"
#include "cuttlefish/host/commands/cvd/instances/instance_manager.h"
#include "cuttlefish/result/result.h"

namespace cuttlefish {
namespace {

constexpr char kSummaryHelpText[] = "Print device logs";

constexpr char kDetailedHelpText[] = R"(
usage: cvd [--group_name name [--instance_name name]] logs [-p|--print name]
)";

struct LogsCmdOptions {
std::string print_target;

std::vector<Flag> Flags() {
return {
GflagsCompatFlag("print", print_target),
GflagsCompatFlag("p", print_target),
Comment on lines +35 to +36
Copy link
Copy Markdown
Member

@jemoreira jemoreira Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This creates two different flags, instead of a single flag with two aliases. Do this instead:

GflagsCompatFlag("print", print_target).Alias({FlagAliasMode::kFlagConsumesFollowing, "-p"}),

};
}
};

Result<void> PrintLogsList(const std::string& dir) {
auto callback = [](const std::string& filename) -> Result<void> {
constexpr int kMaxPadding = 30;
std::string basename = android::base::Basename(filename);
std::cout << basename;
std::cout << std::string(std::max(int(kMaxPadding - basename.length()), 1), ' ');
Copy link
Copy Markdown
Member

@jemoreira jemoreira Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: While the padding might make it look nice for a human it will complicate using this in a script. For example, cvd logs | cut -d" " -f2 can't be used to get the file paths because of the unpredictable number of spaces. You could use padding only if stdout is a terminal, like ls does to determine whether to print one file per line or not.

Copy link
Copy Markdown
Member

@jemoreira jemoreira Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Assign that expression to a variable to make it more clear what it is.

std::cout << filename;
std::cout << std::endl;
return {};
};
return WalkDirectory(dir, callback);
Copy link
Copy Markdown
Member

@jemoreira jemoreira Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use CF_EXPECT on WalkDirectory so this function gets added to the stack trace.

}

Result<void> PrintLog(const std::string& filename) {
Command cat("cat");
Copy link
Copy Markdown
Member

@jemoreira jemoreira Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds an implicit dependency between cvd and cat. While it's unlikely cvd will run in a system that doesn't have cat it's still better to not depend on it. The simplest way to implement this in-process is something like:

std::ifstream file("/path/to/file");                                              
CF_EXPECT(file.is_open());
std::cout << file.rdbuf();

If you still think calling cat is preferable for some reason then do a plain exec without fork to at least avoid creating a new process unnecessarily.

cat.AddParameter(filename);
int exit_code = cat.Start().Wait();
CF_EXPECTF(exit_code == 0, "{} {} failed: exited with status {}",
cat.Executable(), filename, exit_code);
return {};
Comment on lines +55 to +60
Copy link
Copy Markdown
Member

@Databean Databean Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the suggestions in https://clig.dev/#output is

Use a pager (e.g. less) if you are outputting a lot of text. For example, git diff does this by default. Using a pager can be error-prone, so be careful with your implementation such that you don’t make the experience worse for the user. Use a pager only if stdin or stdout is an interactive terminal.

A good sensible set of options to use for less is less -FIRX. This does not page if the content fills one screen, ignores case when you search, enables color and formatting, and leaves the contents on the screen when less quits.

}

class CvdLogsHandler : public CvdCommandHandler {
public:
CvdLogsHandler(InstanceManager& instance_manager)
: instance_manager_(instance_manager) {}

Result<void> Handle(const CommandRequest& request) override {
CF_EXPECT(CanHandle(request));

auto [instance, _] =
CF_EXPECT(selector::SelectInstance(instance_manager_, request),
"Unable to select an instance");

LogsCmdOptions opts;
std::vector<std::string> args = request.SubcommandArguments();
CF_EXPECT(ConsumeFlags(opts.Flags(), args));

std::string dir = instance.instance_dir();
std::string logs_dir = dir + "/logs";
Copy link
Copy Markdown
Member

@jemoreira jemoreira Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some logs in the instance dir are symlinks to the assembly dir that may not be created if the instance fails to boot. This code should avoid those symlinks and go for the actual files instead.

There is also fetch.log in the artifacts dir.

Copy link
Copy Markdown
Member Author

@ser-io ser-io May 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I noticed the following log names: assemble_cvd.log and cuttlefish_config.json and fetch.log are symlinks in the instance directory. Created b/509586405 to handle this in a different PR.

CF_EXPECT(FileExists(logs_dir));
Copy link
Copy Markdown
Member

@jemoreira jemoreira Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cvd create --nostart && cvd logs is a valid command sequence but the second command will fail with an obscure error message about some directory not existing.

It should print a line informing the user that there are no logs available when no log files are found. If --verbose was given it could list the directories it searched too.

CF_EXPECT(IsDirectory(logs_dir));

if (opts.print_target.empty()) {
return PrintLogsList(logs_dir);
Copy link
Copy Markdown
Member

@Databean Databean Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: return CF_EXPECT(PrintLogsList(logs_dir));

otherwise Handle() will not appear in the stack trace.

}

std::string print_target = logs_dir + "/" + opts.print_target;
CF_EXPECT(FileExists(print_target));
return PrintLog(print_target);
}

cvd_common::Args CmdList() const override { return {"logs"}; }

Result<std::string> SummaryHelp() const override { return kSummaryHelpText; }

bool ShouldInterceptHelp() const override { return true; }

bool RequiresDeviceExists() const override { return true; }

Result<std::string> DetailedHelp(std::vector<std::string>&) const override {
return kDetailedHelpText;
}

private:
InstanceManager& instance_manager_;
};

} // namespace

std::unique_ptr<CvdCommandHandler> NewCvdLogsHandler(
InstanceManager& instance_manager) {
return std::unique_ptr<CvdCommandHandler>(
new CvdLogsHandler(instance_manager));
}

} // namespace cuttlefish
13 changes: 13 additions & 0 deletions base/cvd/cuttlefish/host/commands/cvd/cli/commands/logs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#pragma once

#include <memory>

#include "cuttlefish/host/commands/cvd/cli/commands/command_handler.h"
#include "cuttlefish/host/commands/cvd/instances/instance_manager.h"

namespace cuttlefish {

std::unique_ptr<CvdCommandHandler> NewCvdLogsHandler(
InstanceManager& instance_manager);

} // namespace cuttlefish
2 changes: 2 additions & 0 deletions base/cvd/cuttlefish/host/commands/cvd/cli/request_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "cuttlefish/host/commands/cvd/cli/commands/lint.h"
#include "cuttlefish/host/commands/cvd/cli/commands/load_configs.h"
#include "cuttlefish/host/commands/cvd/cli/commands/login.h"
#include "cuttlefish/host/commands/cvd/cli/commands/logs.h"

#include "cuttlefish/host/commands/cvd/cli/commands/power_btn.h"
#include "cuttlefish/host/commands/cvd/cli/commands/powerwash.h"
Expand Down Expand Up @@ -115,6 +116,7 @@ RequestContext::RequestContext(InstanceManager& instance_manager,
request_handlers_.emplace_back(NewCvdStartCommandHandler(instance_manager));
request_handlers_.emplace_back(NewCvdStatusCommandHandler(instance_manager));
request_handlers_.emplace_back(NewCvdVersionHandler());
request_handlers_.emplace_back(NewCvdLogsHandler(instance_manager));
}

Result<CvdCommandHandler*> RequestContext::Handler(
Expand Down
2 changes: 1 addition & 1 deletion e2etests/cvd/bugreport_tests/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestTakeBugreport(t *testing.T) {
t.Fatal(err)
}

if _, err := c.RunCmd(c.TargetBin(), "host_bugreport", "--output=/tmp/host_bugreport.zip", "--include_adb_bugreport=true"); err != nil {
if _, _, err := c.RunCmd(c.TargetBin(), "host_bugreport", "--output=/tmp/host_bugreport.zip", "--include_adb_bugreport=true"); err != nil {
t.Fatal(err)
}

Expand Down
50 changes: 26 additions & 24 deletions e2etests/cvd/common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,15 @@ type TestContext struct {
usePodcvd bool
}

func runCmdWithContextEnv(ctx context.Context, command []string, envvars map[string]string) (string, error) {
func runCmdWithContextEnv(ctx context.Context, command []string, envvars map[string]string) (string, string, error) {
Copy link
Copy Markdown
Member

@Databean Databean Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe worth returning a structure with fields for stdout and stderr so code isn't reliant on the caller knowing what each string means? It could be reused for RunCmdWithEnv and RunCmd as well.

cmd := exec.CommandContext(ctx, command[0], command[1:]...)

cmdOutputBuf := bytes.Buffer{}
cmdWriter := io.MultiWriter(&cmdOutputBuf, log.Writer())
cmd.Stdout = cmdWriter
cmd.Stderr = cmdWriter
stdOutBuf := bytes.Buffer{}
stdErrBuf := bytes.Buffer{}
stdOutWriter := io.MultiWriter(&stdOutBuf, log.Writer())
stdErrWriter := io.MultiWriter(&stdErrBuf, log.Writer())
cmd.Stdout = stdOutWriter
cmd.Stderr = stdErrWriter

envvarPairs := []string{}
for k, v := range envvars {
Expand All @@ -97,14 +99,14 @@ func runCmdWithContextEnv(ctx context.Context, command []string, envvars map[str
log.Printf("Running `%s %s`\n", strings.Join(envvarPairs, " "), strings.Join(command, " "))
err := cmd.Run()
if err != nil {
return "", fmt.Errorf("`%s` failed: %w", strings.Join(command, " "), err)
return "", "", fmt.Errorf("`%s` failed: %w", strings.Join(command, " "), err)
}

return cmdOutputBuf.String(), nil
return stdOutBuf.String(), stdErrBuf.String(), nil
}

// Runs the given command with the given set of envvars overrided.
func (tc *TestContext) RunCmdWithEnv(command []string, envvars map[string]string) (string, error) {
func (tc *TestContext) RunCmdWithEnv(command []string, envvars map[string]string) (string, string, error) {
return runCmdWithContextEnv(tc.context, command, envvars)
}

Expand All @@ -117,14 +119,14 @@ func (tc *TestContext) RunAdbWaitForDevice() error {
"adb",
"wait-for-device",
}
if _, err := tc.RunCmd(adbCommand...); err != nil {
if _, _, err := tc.RunCmd(adbCommand...); err != nil {
return fmt.Errorf("timed out waiting for Cuttlefish device to connect to adb: %w", err)
}
return nil
}

// Runs the given command with the existing envvars.
func (tc *TestContext) RunCmd(args ...string) (string, error) {
func (tc *TestContext) RunCmd(args ...string) (string, string, error) {
command := []string{}
command = append(command, args...)
return tc.RunCmdWithEnv(command, map[string]string{})
Expand Down Expand Up @@ -174,7 +176,7 @@ func (tc *TestContext) CVDFetch(args FetchArgs) error {
if credentialArg != "" {
fetchCmd = append(fetchCmd, fmt.Sprintf("--credential_source=%s", credentialArg))
}
if _, err := tc.RunCmd(fetchCmd...); err != nil {
if _, _, err := tc.RunCmd(fetchCmd...); err != nil {
log.Printf("Failed to fetch: %w", err)
return err
}
Expand Down Expand Up @@ -202,7 +204,7 @@ func (tc *TestContext) CVDCreate(args CreateArgs) error {
if len(args.Args) > 0 {
createCmd = append(createCmd, args.Args...)
}
if _, err := tc.RunCmdWithEnv(createCmd, tempdirEnv); err != nil {
if _, _, err := tc.RunCmdWithEnv(createCmd, tempdirEnv); err != nil {
log.Printf("Failed to create instance(s): %w", err)
return err
}
Expand All @@ -218,7 +220,7 @@ func (tc *TestContext) CVDStop() error {
}

stopCmd := []string{tc.TargetBin(), "stop"}
if _, err := tc.RunCmdWithEnv(stopCmd, tempdirEnv); err != nil {
if _, _, err := tc.RunCmdWithEnv(stopCmd, tempdirEnv); err != nil {
log.Printf("Failed to stop instance(s): %w", err)
return err
}
Expand All @@ -238,7 +240,7 @@ func (tc *TestContext) LaunchCVD(args CreateArgs) error {
if len(args.Args) > 0 {
createCmd = append(createCmd, args.Args...)
}
if _, err := tc.RunCmdWithEnv(createCmd, tempdirEnv); err != nil {
if _, _, err := tc.RunCmdWithEnv(createCmd, tempdirEnv); err != nil {
log.Printf("Failed to create instance(s): %w", err)
return err
}
Expand All @@ -254,7 +256,7 @@ func (tc *TestContext) StopCVD() error {
}

stopCmd := []string{"bin/stop_cvd"}
if _, err := tc.RunCmdWithEnv(stopCmd, tempdirEnv); err != nil {
if _, _, err := tc.RunCmdWithEnv(stopCmd, tempdirEnv); err != nil {
log.Printf("Failed to stop instance(s): %w", err)
return err
}
Expand All @@ -269,7 +271,7 @@ func (tc *TestContext) CVDPowerwash() error {
}

createCmd := []string{tc.TargetBin(), "powerwash"}
if _, err := tc.RunCmdWithEnv(createCmd, tempdirEnv); err != nil {
if _, _, err := tc.RunCmdWithEnv(createCmd, tempdirEnv); err != nil {
log.Printf("Failed to powerwash instance(s): %w", err)
return err
}
Expand Down Expand Up @@ -304,7 +306,7 @@ func (tc *TestContext) CVDLoad(load LoadArgs) error {
if credentialArg != "" {
loadCmd = append(loadCmd, fmt.Sprintf("--credential_source=%s", credentialArg))
}
if _, err := tc.RunCmd(loadCmd...); err != nil {
if _, _, err := tc.RunCmd(loadCmd...); err != nil {
log.Printf("Failed to perform `cvd load`: %w", err)
return err
}
Expand All @@ -315,13 +317,13 @@ func (tc *TestContext) CVDLoad(load LoadArgs) error {
}

func (tc *TestContext) GetMetricsDir() (string, error) {
output, err := tc.RunCmd("cvd", "fleet")
stdOut, _, err := tc.RunCmd("cvd", "fleet")
if err != nil {
return "", fmt.Errorf("failed to run `cvd fleet`")
}

re := regexp.MustCompile(`"metrics_dir" : "(.*)",`)
matches := re.FindStringSubmatch(output)
matches := re.FindStringSubmatch(stdOut)
if len(matches) != 2 {
return "", fmt.Errorf("failed to find metrics directory")
}
Expand All @@ -344,7 +346,7 @@ func (tc *TestContext) SetUp(t *testing.T) {
log.Printf("Initializing %s test...", tc.t.Name())

log.Printf("Cleaning up any pre-existing instances...")
if _, err := tc.RunCmd(tc.TargetBin(), "reset", "-y"); err != nil {
if _, _, err := tc.RunCmd(tc.TargetBin(), "reset", "-y"); err != nil {
log.Printf("Failed to cleanup any pre-existing instances: %w", err)
}
log.Printf("Finished cleaning up any pre-existing instances!")
Expand Down Expand Up @@ -426,7 +428,7 @@ func (tc *TestContext) TearDown() {
matches, err := filepath.Glob(path.Join(tc.tempdir, pattern))
if err == nil {
for _, file := range matches {
_, err := tc.RunCmd("cp", "--dereference", file, testoutdir)
_, _, err := tc.RunCmd("cp", "--dereference", file, testoutdir)
if err != nil {
log.Printf("failed to copy %s to %s: %w", file, testoutdir, err)
}
Expand All @@ -445,7 +447,7 @@ func (tc *TestContext) TearDown() {
err := os.MkdirAll(outinstancedir, os.ModePerm)
if err == nil {
logdir := path.Join(instancedir, "logs")
_, err := runCmdWithContextEnv(context.TODO(), []string{"cp", "-r", "--dereference", logdir, outinstancedir}, map[string]string{})
_, _, err := runCmdWithContextEnv(context.TODO(), []string{"cp", "-r", "--dereference", logdir, outinstancedir}, map[string]string{})
if err != nil {
log.Printf("failed to copy %s to %s: %w", logdir, outinstancedir, err)
}
Expand All @@ -461,7 +463,7 @@ func (tc *TestContext) TearDown() {
outmetricsdir := path.Join(testoutdir, "metrics_files")
err := os.MkdirAll(outmetricsdir, os.ModePerm)
if err == nil {
_, err := runCmdWithContextEnv(context.TODO(), []string{"cp", "-r", "--dereference", metricsdir, outmetricsdir}, map[string]string{})
_, _, err := runCmdWithContextEnv(context.TODO(), []string{"cp", "-r", "--dereference", metricsdir, outmetricsdir}, map[string]string{})
if err != nil {
log.Printf("failed to copy %s to %s: %w", metricsdir, outmetricsdir, err)
}
Expand Down Expand Up @@ -586,7 +588,7 @@ func RunXts(t *testing.T, cuttlefishArgs FetchAndCreateArgs, xtsArgs XtsArgs) {
"--log-level-display=INFO",
}
xtsCommand = append(xtsCommand, xtsArgs.XtsArgs...)
if _, err := tc.RunCmd(xtsCommand...); err != nil {
if _, _, err := tc.RunCmd(xtsCommand...); err != nil {
t.Fatalf("failed to fully run XTS: %w", err)
}
log.Printf("Finished running XTS!")
Expand Down
6 changes: 3 additions & 3 deletions e2etests/cvd/cvd_powerwash_tests/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,19 @@ func TestCvdPowerwash(t *testing.T) {
}

const tmpFile = "/data/local/tmp/foo"
if _, err := c.RunCmd("adb", "shell", "touch", tmpFile); err != nil {
if _, _, err := c.RunCmd("adb", "shell", "touch", tmpFile); err != nil {
t.Fatalf("failed to create %s: %w", tmpFile, err)
}

if _, err := c.RunCmd("adb", "shell", "stat", tmpFile); err != nil {
if _, _, err := c.RunCmd("adb", "shell", "stat", tmpFile); err != nil {
t.Fatal("failed to verify %s created: %w", err)
}

if err := c.CVDPowerwash(); err != nil {
t.Fatal(err)
}

if _, err := c.RunCmd("adb", "shell", "stat", tmpFile); err == nil {
if _, _, err := c.RunCmd("adb", "shell", "stat", tmpFile); err == nil {
t.Fatal("failed to powerwash, %s still exists")
}
})
Expand Down
Loading
Loading