fix: capture exit code (#39)

Capture exit code, so that the user can use that info to debug why the
spawned command is failing.

Author: eh-am

cmd/exec.go

@@ -33,14 +33,20 @@ func execCmd() *ffcli.Command {
 				return fmt.Errorf("at least one argument is required")
 			}
 
-			cmdError, err := exec.Exec(args, exec.ExecCfg{
+			l, err := logrus.ParseLevel(*logLevel)
+			if err != nil {
+				return fmt.Errorf("parsing log level: %w", err)
+			}
+			logger := logrus.New()
+			logger.SetLevel(l)
+
+			cmdError, err := exec.Exec(logger, args, exec.ExecCfg{
 				OutputDir:         *outputDir,
 				APIKey:            *apiKey,
 				ServerAddress:     *cloudServerAddress,
 				CommitSHA:         *commitSHA,
 				UploadToCloud:     *uploadToCloud,
 				Branch:            *branch,
-				LogLevel:          *logLevel,
 				Export:            *exportLocally,
 				UploadToPublicAPI: *uploadToPublicAPI,
 			})
@@ -50,11 +56,12 @@ func execCmd() *ffcli.Command {
 				return err
 			}
 
-			// Add additional context, since it may be weird to just see "exit eror code X"
 			if cmdError != nil {
-				logrus.Error("error in spawned command")
+				// Add additional context, since it may be weird to just see "exit eror code X"
+				return fmt.Errorf("error in spawned command: %w", cmdError)
 			}
-			return cmdError
+
+			return nil
 		},
 	}
 }

examples/go/testscript.txtar

@@ -1,5 +1,5 @@
 pyroscope-ci go install --applicationName=myapp .
-pyroscope-ci exec --uploadToCloud=false --exportLocally -- docker run -v $WORK:/app --link $PYROSCOPE_PROXY_ADDRESS --env PYROSCOPE_ADHOC_SERVER_ADDRESS $IMAGE_NAME sh -c 'go test ./... -v'
+pyroscope-ci exec --logLevel=debug --uploadToCloud=false --exportLocally -- docker run -v $WORK:/app --link $PYROSCOPE_PROXY_ADDRESS --env PYROSCOPE_ADHOC_SERVER_ADDRESS $IMAGE_NAME sh -c 'go test ./... -v'
 
 exists pyroscope-ci-output/myapp.cpu.json
 exists pyroscope-ci-output/myapp.goroutines.json

examples/nodejs/jest/testscript.txtar

@@ -1,2 +1,2 @@
-pyroscope-ci exec --uploadToCloud=false --exportLocally -- docker run --link $PYROSCOPE_PROXY_ADDRESS --env PYROSCOPE_ADHOC_SERVER_ADDRESS $IMAGE_NAME yarn test
+pyroscope-ci exec --logLevel=debug --uploadToCloud=false --exportLocally -- docker run --link $PYROSCOPE_PROXY_ADDRESS --env PYROSCOPE_ADHOC_SERVER_ADDRESS $IMAGE_NAME yarn test
 exists pyroscope-ci-output/pyroscope.tests.cpu.json

examples/nodejs/mocha/testscripts.txtar

@@ -1,3 +1,3 @@
-pyroscope-ci exec --uploadToCloud=false --exportLocally -- docker run --link $PYROSCOPE_PROXY_ADDRESS --env PYROSCOPE_ADHOC_SERVER_ADDRESS $IMAGE_NAME yarn test
+pyroscope-ci exec --logLevel=debug --uploadToCloud=false --exportLocally -- docker run --link $PYROSCOPE_PROXY_ADDRESS --env PYROSCOPE_ADHOC_SERVER_ADDRESS $IMAGE_NAME yarn test
 exists pyroscope-ci-output/example-mocha.cpu.json
 

examples/ruby/rspec/testscripts.txtar

@@ -1,3 +1,3 @@
-pyroscope-ci exec --uploadToCloud=false --exportLocally -- docker run --link $PYROSCOPE_PROXY_ADDRESS --env PYROSCOPE_ADHOC_SERVER_ADDRESS $IMAGE_NAME ./bin/rspec
+pyroscope-ci exec --logLevel=debug --uploadToCloud=false --exportLocally -- docker run --link $PYROSCOPE_PROXY_ADDRESS --env PYROSCOPE_ADHOC_SERVER_ADDRESS $IMAGE_NAME ./bin/rspec
 exists pyroscope-ci-output/ruby.tests.cpu.json
 

internal/exec/exec.go

@@ -19,7 +19,6 @@ type ExecCfg struct {
 	UploadToCloud     bool
 	Export            bool
 	UploadToPublicAPI bool
-	LogLevel          string
 }
 
 // Exec executes a program and exports its captured profiles
@@ -34,11 +33,7 @@ type ExecCfg struct {
 // Notice that it returns 2 different errors:
 // cmdError refers to the error of the command exec'd
 // and err to any other error
-func Exec(args []string, cfg ExecCfg) (cmdError error, err error) {
-	logger := logrus.StandardLogger()
-	lvl, _ := logrus.ParseLevel(cfg.LogLevel)
-	logger.SetLevel(lvl)
-
+func Exec(logger *logrus.Logger, args []string, cfg ExecCfg) (cmdError error, err error) {
 	runner := NewRunner(logger)
 
 	if !cfg.Export && !cfg.UploadToCloud && !cfg.UploadToPublicAPI {
@@ -48,8 +43,8 @@ func Exec(args []string, cfg ExecCfg) (cmdError error, err error) {
 
 	logger.Debug("exec'ing command")
 	ingestedItems, duration, cmdError := runner.Run(args)
-	if err != nil {
-		logger.Errorf("process errored: %s. will still try to upload ingested data", err)
+	if cmdError != nil {
+		logger.Debug("process errored. will still try to upload ingested data", cmdError)
 	}
 
 	if len(ingestedItems) <= 0 {

internal/exec/exec_test.go

@@ -1,13 +1,17 @@
 package exec_test
 
 import (
+	"io"
 	"os"
 	"testing"
 
 	"github.com/pyroscope-io/ci/internal/exec"
+	"github.com/sirupsen/logrus"
 )
 
 func TestCapturingCmdError(t *testing.T) {
+	noopLogger := logrus.New()
+	noopLogger.SetOutput(io.Discard)
 	dir, err := os.MkdirTemp("", "")
 	if err != nil {
 		t.Fatal(err)
@@ -16,7 +20,7 @@ func TestCapturingCmdError(t *testing.T) {
 
 	// Running an unknown command, we should get back an error
 	cfg := exec.ExecCfg{UploadToCloud: true, Export: true, OutputDir: dir}
-	cmdError, err := exec.Exec([]string{"unknown command"}, cfg)
+	cmdError, err := exec.Exec(noopLogger, []string{"unknown-command"}, cfg)
 	if err != nil {
 		t.Error("did not expect error to happen")
 	}
@@ -27,13 +31,12 @@ func TestCapturingCmdError(t *testing.T) {
 
 	// Running a valid
 	// TODO: what command would be valid cross os?
-	cmdError, err = exec.Exec([]string{"echo"}, cfg)
+	cmdError, err = exec.Exec(noopLogger, []string{"echo"}, cfg)
 	if err != nil {
 		t.Error("did not expect error to happen")
 	}
 
 	if cmdError != nil {
 		t.Error("expected cmdError to NOT have happened")
 	}
-
 }

internal/exec/runner.go

@@ -96,7 +96,13 @@ func (p *Runner) Run(args []string) (map[string]flamebearer.FlamebearerProfile,
 		case <-ticker.C:
 			if !process.Exists(cmd.Process.Pid) {
 				logrus.Debug("child process exited")
-				return p.ingester.GetIngestedItems(), captureDuration(), cmd.Wait()
+				err := cmd.Wait()
+
+				if exiterr, ok := err.(*osExec.ExitError); ok {
+					err = fmt.Errorf("exit code %d: %v", exiterr.ExitCode(), err)
+				}
+
+				return p.ingester.GetIngestedItems(), captureDuration(), err
 			}
 		}
 	}