Skip to content

Commit

Permalink
Add debug logging to runner (#7324)
Browse files Browse the repository at this point in the history
  • Loading branch information
maggie-lou authored Aug 28, 2024
1 parent b6ffdc3 commit b22e5f1
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 7 deletions.
10 changes: 7 additions & 3 deletions enterprise/server/remote_execution/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,11 +437,15 @@ type VM interface {
// PullImageIfNecessary pulls the image configured for the container if it
// is not cached locally.
func PullImageIfNecessary(ctx context.Context, env environment.Env, ctr CommandContainer, creds oci.Credentials, imageRef string) error {
ctx, span := tracing.StartSpan(ctx)
defer span.End()
if *debugUseLocalImagesOnly {
if *debugUseLocalImagesOnly || imageRef == "" {
return nil
}

ctx, span := tracing.StartSpan(ctx)
defer span.End()
log.CtxDebugf(ctx, "Preparing to pull image")
defer log.CtxDebugf(ctx, "Finished pulling image")

cacheAuth := env.GetImageCacheAuthenticator()
if cacheAuth == nil || env.GetAuthenticator() == nil {
// If we don't have an authenticator available, fall back to
Expand Down
1 change: 1 addition & 0 deletions enterprise/server/remote_execution/containers/bare/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ go_library(
"//enterprise/server/util/procstats",
"//proto:remote_execution_go_proto",
"//server/interfaces",
"//server/util/log",
"//server/util/status",
],
)
Expand Down
4 changes: 4 additions & 0 deletions enterprise/server/remote_execution/containers/bare/bare.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/buildbuddy-io/buildbuddy/enterprise/server/util/oci"
"github.com/buildbuddy-io/buildbuddy/enterprise/server/util/procstats"
"github.com/buildbuddy-io/buildbuddy/server/interfaces"
"github.com/buildbuddy-io/buildbuddy/server/util/log"
"github.com/buildbuddy-io/buildbuddy/server/util/status"

repb "github.com/buildbuddy-io/buildbuddy/proto/remote_execution"
Expand Down Expand Up @@ -81,6 +82,7 @@ func (c *bareCommandContainer) Signal(ctx context.Context, sig syscall.Signal) e
}

func (c *bareCommandContainer) exec(ctx context.Context, cmd *repb.Command, workDir string, stdio *interfaces.Stdio) (result *interfaces.CommandResult) {
log.CtxDebugf(ctx, "Beginning exec in container")
var statsListener procstats.Listener
if c.opts.EnableStats {
defer container.Metrics.Unregister(c)
Expand All @@ -90,6 +92,7 @@ func (c *bareCommandContainer) exec(ctx context.Context, cmd *repb.Command, work
}

if *enableLogFiles {
log.CtxDebugf(ctx, "Creating log files")
stdoutPath := workDir + ".stdout"
stdoutFile, err := os.Create(stdoutPath)
if err != nil {
Expand Down Expand Up @@ -127,6 +130,7 @@ func (c *bareCommandContainer) exec(ctx context.Context, cmd *repb.Command, work
stdio.Stderr = io.MultiWriter(stdio.Stderr, stderrFile)
}

log.CtxDebugf(ctx, "Preparing to run command")
return commandutil.RunWithOpts(ctx, cmd, &commandutil.RunOpts{
Dir: workDir,
StatsListener: statsListener,
Expand Down
12 changes: 8 additions & 4 deletions enterprise/server/remote_execution/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ func (r *taskRunner) String() string {
truncate(r.key.InstanceName, 8, "..."), truncate(ph, 8, ""))
}

func (r *taskRunner) pullCredentials() (oci.Credentials, error) {
func (r *taskRunner) pullCredentials(ctx context.Context) (oci.Credentials, error) {
log.CtxDebugf(ctx, "Pulling credentials")
return oci.CredentialsFromProperties(r.PlatformProperties)
}

Expand All @@ -232,7 +233,7 @@ func (r *taskRunner) PrepareForTask(ctx context.Context) error {

// Pull the container image before Run() is called, so that we don't
// use up the whole exec ctx timeout with a slow container pull.
creds, err := r.pullCredentials()
creds, err := r.pullCredentials(ctx)
if err != nil {
return err
}
Expand Down Expand Up @@ -298,6 +299,7 @@ func (r *taskRunner) DownloadInputs(ctx context.Context, ioStats *repb.IOStats)

// Run runs the task that is currently bound to the command runner.
func (r *taskRunner) Run(ctx context.Context) (res *interfaces.CommandResult) {
log.CtxDebugf(ctx, "Starting run")
start := time.Now()
defer func() {
// Discard nonsensical PSI full-stall durations which are greater
Expand Down Expand Up @@ -346,10 +348,11 @@ func (r *taskRunner) Run(ctx context.Context) (res *interfaces.CommandResult) {
command := r.task.GetCommand()

if !r.PlatformProperties.RecycleRunner {
log.CtxDebugf(ctx, "Runner is not recyclable - running full container lifecycle")
// If the container is not recyclable, then use `Run` to walk through
// the entire container lifecycle in a single step.
// TODO: Remove this `Run` method and call lifecycle methods directly.
creds, err := r.pullCredentials()
creds, err := r.pullCredentials(ctx)
if err != nil {
return commandutil.ErrorResult(err)
}
Expand All @@ -367,7 +370,7 @@ func (r *taskRunner) Run(ctx context.Context) (res *interfaces.CommandResult) {
r.p.mu.RUnlock()
switch s {
case initial:
creds, err := r.pullCredentials()
creds, err := r.pullCredentials(ctx)
if err != nil {
return commandutil.ErrorResult(err)
}
Expand All @@ -392,6 +395,7 @@ func (r *taskRunner) Run(ctx context.Context) (res *interfaces.CommandResult) {
}

if _, ok := persistentworker.Key(r.PlatformProperties, command.GetArguments()); ok {
log.CtxDebugf(ctx, "Sending persistent work request")
return r.sendPersistentWorkRequest(ctx, command)
}

Expand Down

0 comments on commit b22e5f1

Please sign in to comment.