diff --git a/tool/internal/setup/find.go b/tool/internal/setup/find.go index 12400cf6..2a5af62e 100644 --- a/tool/internal/setup/find.go +++ b/tool/internal/setup/find.go @@ -18,6 +18,9 @@ import ( const maxBuildPlanBufferSize = 10 * 1024 * 1024 // 10MB +//nolint:gochecknoglobals // allows us to mock exec.CommandContext in tests +var execCommandContext = exec.CommandContext + type Dependency struct { ImportPath string Version string @@ -69,17 +72,10 @@ func findCommands(buildPlanLog *os.File) ([]string, error) { return commands, nil } -// listBuildPlan lists the build plan by running `go build/install -a -x -n` +// listBuildPlan lists the build plan by running `go build -a -x -n` // and then filtering the commands (cd, cgo, compile) from the build plan log. -func (sp *SetupPhase) listBuildPlan(ctx context.Context, goBuildCmd []string) ([]string, error) { - const goBuildMinArgs = 2 // go build +func (sp *SetupPhase) listBuildPlan(ctx context.Context, cmdArgs []string) ([]string, error) { const buildPlanLogName = "build-plan.log" - if len(goBuildCmd) < goBuildMinArgs { - return nil, ex.Newf("at least %d arguments are required", goBuildMinArgs) - } - if goBuildCmd[1] != "build" && goBuildCmd[1] != "install" { - return nil, ex.Newf("must be go build/install, got %s", goBuildCmd[1]) - } // Create a build plan log file in the temporary directory buildPlanLog, err := os.Create(util.GetBuildTemp(buildPlanLogName)) @@ -88,16 +84,13 @@ func (sp *SetupPhase) listBuildPlan(ctx context.Context, goBuildCmd []string) ([ } defer buildPlanLog.Close() // The full build command is: "go build/install -a -x -n {...}" - args := []string{} - args = append(args, goBuildCmd[:goBuildMinArgs]...) // go build/install - args = append(args, []string{"-a", "-x", "-n"}...) // -a -x -n - if len(goBuildCmd) > goBuildMinArgs { // {...} remaining - args = append(args, goBuildCmd[goBuildMinArgs:]...) - } - sp.Info("New build command", "new", args, "old", goBuildCmd) + prefix := []string{"build", "-a", "-x", "-n"} + args := make([]string, 0, len(prefix)+len(cmdArgs)) + args = append(args, prefix...) + args = append(args, cmdArgs...) // args from original build/install or setup command + sp.Info("go build command", "args", args) - //nolint:gosec // Command arguments are validated with above assertions - cmd := exec.CommandContext(ctx, args[0], args[1:]...) + cmd := execCommandContext(ctx, "go", args...) // This is a little anti-intuitive as the error message is not printed to // the stderr, instead it is printed to the stdout, only the build tool // knows the reason why. @@ -109,7 +102,6 @@ func (sp *SetupPhase) listBuildPlan(ctx context.Context, goBuildCmd []string) ([ err = cmd.Run() if err != nil { // Read the build plan log to see what went wrong - _, _ = buildPlanLog.Seek(0, 0) logContent, _ := os.ReadFile(util.GetBuildTemp(buildPlanLogName)) return nil, ex.Wrapf(err, "failed to run build plan: \n%s", string(logContent)) } @@ -204,8 +196,8 @@ func findGoSources(sp *SetupPhase, args []string, cgoObjDirs map[string]string) } // findDeps finds dependencies by listing the build plan. -func (sp *SetupPhase) findDeps(ctx context.Context, goBuildCmd []string) ([]*Dependency, error) { - buildPlan, err := sp.listBuildPlan(ctx, goBuildCmd) +func (sp *SetupPhase) findDeps(ctx context.Context, cmdArgs []string) ([]*Dependency, error) { + buildPlan, err := sp.listBuildPlan(ctx, cmdArgs) if err != nil { return nil, err } diff --git a/tool/internal/setup/find_test.go b/tool/internal/setup/find_test.go index 3441d88e..68d94bca 100644 --- a/tool/internal/setup/find_test.go +++ b/tool/internal/setup/find_test.go @@ -4,10 +4,14 @@ package setup import ( + "context" "os" + "os/exec" "path/filepath" + "strings" "testing" + "github.com/open-telemetry/opentelemetry-go-compile-instrumentation/tool/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -268,3 +272,125 @@ C:/Go/pkg/tool/windows_amd64/compile.exe -o C:/tmp/out.a -p main -buildid abc ma }) } } + +func TestListBuildPlan(t *testing.T) { + oldExec := execCommandContext + defer func() { + execCommandContext = oldExec + }() + + tests := []struct { + name string + buildPlan string + args []string + expected []string + wantErr bool + buildFails bool + expectedGoCmd []string + }{ + { + name: "filters compile and cgo commands", + buildPlan: ` +cd /project/pkg +.../cgo -objdir /tmp/b001 -importpath pkg/cgo +.../compile -o /tmp/out.a -buildid abc -p main main.go +echo ignored +`, + args: []string{"./..."}, + expected: []string{ + "cd /project/pkg", + ".../cgo -objdir /tmp/b001 -importpath pkg/cgo", + ".../compile -o /tmp/out.a -buildid abc -p main main.go", + }, + expectedGoCmd: []string{ + "build", "-a", "-x", "-n", "./...", + }, + }, + { + name: "passes additional build args", + buildPlan: ` +.../compile -o /tmp/out.a -buildid abc -p main main.go +`, + args: []string{"-tags=integration", "./cmd"}, + expected: []string{ + ".../compile -o /tmp/out.a -buildid abc -p main main.go", + }, + expectedGoCmd: []string{ + "build", "-a", "-x", "-n", + "-tags=integration", + "./cmd", + }, + }, + { + name: "returns build failure", + buildPlan: ` +go: module example.com missing +`, + args: []string{"./bad"}, + buildFails: true, + wantErr: true, + expectedGoCmd: []string{ + "build", "-a", "-x", "-n", "./bad", + }, + }, + { + name: "empty build plan", + buildPlan: ` +echo nothing useful +`, + args: []string{"./..."}, + expectedGoCmd: []string{ + "build", "-a", "-x", "-n", "./...", + }, + }, + { + name: "ignores malformed compile lines", + buildPlan: ` +.../compile foo +.../cgo blah +`, + args: []string{"./..."}, + expected: nil, + expectedGoCmd: []string{"build", "-a", "-x", "-n", "./..."}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir := t.TempDir() + err := os.Mkdir(filepath.Join(tempDir, util.BuildTempDir), 0o755) + require.NoError(t, err) + + t.Setenv(util.EnvOtelcWorkDir, tempDir) + + execCommandContext = func( + _ context.Context, + name string, + args ...string, + ) *exec.Cmd { + assert.Equal(t, "go", name) + assert.Equal(t, tt.expectedGoCmd, args) + + script := "cat <<'EOF' >&2\n" + tt.buildPlan + "\nEOF\n" + if tt.buildFails { + script += "\nexit 1\n" + } + + return exec.Command("sh", "-c", script) + } + + sp := newTestSetupPhase() + buildPlan, err := sp.listBuildPlan(t.Context(), tt.args) + + if tt.wantErr { + require.Error(t, err) + if tt.buildPlan != "" { + assert.Contains(t, err.Error(), strings.TrimSpace(tt.buildPlan)) + } + } else { + require.NoError(t, err) + assert.Equal(t, tt.expected, buildPlan) + } + }) + } +} diff --git a/tool/internal/setup/setup.go b/tool/internal/setup/setup.go index 79aa88db..e3e39a03 100644 --- a/tool/internal/setup/setup.go +++ b/tool/internal/setup/setup.go @@ -75,16 +75,16 @@ var flagsWithPathValues = map[string]bool{ "-toolexec": true, } -// GetBuildPackages loads all packages from the go build command arguments. +// GetBuildPackages loads all packages from the otelc go build/install or otelc setup command arguments. // Returns a list of loaded packages. If no package patterns are found in args, // defaults to loading the current directory package. -// The args parameter should be the go build command arguments (e.g., ["build", "-a", "./cmd"]). +// The args parameter should be the go build/install command arguments (e.g., ["-a", "./cmd"]). // Returns an error if package loading fails or if invalid patterns are provided. // For example: -// - args ["build", "-a", "./cmd"] returns packages for "./cmd" -// - args ["build", "-a", "cmd"] returns packages for the "cmd" package in the module -// - args ["build", "-a", ".", "./cmd"] returns packages for both "." and "./cmd" -// - args ["build"] returns packages for "." +// - args ["-a", "./cmd"] returns packages for "./cmd" +// - args ["-a", "cmd"] returns packages for the "cmd" package in the module +// - args ["-a", ".", "./cmd"] returns packages for both "." and "./cmd" +// - args [] returns packages for "." func getBuildPackages(ctx context.Context, args []string) ([]*packages.Package, error) { logger := util.LoggerFromContext(ctx) @@ -103,7 +103,7 @@ func getBuildPackages(ctx context.Context, args []string) ([]*packages.Package, // If we hit a flag, stop. Packages come after all flags // go build [-o output] [build flags] [packages] - if strings.HasPrefix(arg, "-") || arg == "go" || arg == "build" || arg == "install" { + if strings.HasPrefix(arg, "-") { break } @@ -140,9 +140,12 @@ func getPackageDir(pkg *packages.Package) string { // Setup prepares the environment for further instrumentation. func Setup(ctx context.Context, cmd *cli.Command) error { - // The args are "go build ..." + // Since Setup can be invoked in different contexts (i.e, via `otelc setup` or as part of `otelc go build`), + // we need to handle the arguments accordingly. If the command is `go build` or `go install`, we should trim the first argument args := cmd.Args().Slice() - args = append([]string{"go"}, args...) + if cmd.Name == "go" { + args = cmd.Args().Tail() // trim build/install + } logger := util.LoggerFromContext(ctx) @@ -386,9 +389,17 @@ func GoBuild(ctx context.Context, cmd *cli.Command) error { // to prevent stale data from affecting this build. instrument.CleanupImportTrackingFiles() + if !cmd.Args().Present() { + return ex.Newf("no command provided. Only 'go build' and 'go install' are supported") + } + + if cmd.Args().First() != "build" && cmd.Args().First() != "install" { + return ex.Newf("unsupported command: %s. Only 'go build' and 'go install' are supported", cmd.Args().First()) + } + defer func() { // Remove otelc.runtime.go from each instrumented package directory. - pkgs, pkgErr := getBuildPackages(ctx, cmd.Args().Slice()) + pkgs, pkgErr := getBuildPackages(ctx, cmd.Args().Tail()) // pass args without build/install if pkgErr != nil { logger.DebugContext(ctx, "failed to get build packages", "error", pkgErr) } diff --git a/tool/internal/setup/setup_test.go b/tool/internal/setup/setup_test.go index ab7aac34..ecc99916 100644 --- a/tool/internal/setup/setup_test.go +++ b/tool/internal/setup/setup_test.go @@ -25,37 +25,37 @@ func TestGetPackages(t *testing.T) { }{ { name: "single package", - args: []string{"build", "-a", "-o", "tmp", "./cmd"}, + args: []string{"-a", "-o", "tmp", "./cmd"}, expectedCount: 1, expectedPackages: []string{"testmodule/cmd"}, }, { name: "multiple packages", - args: []string{"build", "./cmd", "./foo/demo"}, + args: []string{"./cmd", "./foo/demo"}, expectedCount: 2, expectedPackages: []string{"testmodule/cmd", "testmodule/foo/demo"}, }, { name: "wildcard pattern", - args: []string{"build", "./cmd/..."}, + args: []string{"./cmd/..."}, expectedCount: 1, expectedPackages: []string{"testmodule/cmd"}, }, { name: "default to current directory", - args: []string{"build"}, + args: []string{}, expectedCount: 1, expectedPackages: []string{"."}, }, { name: "current directory explicit", - args: []string{"build", "."}, + args: []string{"."}, expectedCount: 1, expectedPackages: []string{"."}, }, { name: "nonexistent package mixed with valid", - args: []string{"build", "./cmd", "./nonexistent"}, + args: []string{"./cmd", "./nonexistent"}, expectedCount: 1, expectedPackages: []string{"testmodule/cmd"}, },