Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 13 additions & 21 deletions tool/internal/setup/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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.
Expand All @@ -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))
}
Expand Down Expand Up @@ -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
}
Expand Down
126 changes: 126 additions & 0 deletions tool/internal/setup/find_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}
})
}
}
31 changes: 21 additions & 10 deletions tool/internal/setup/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
}

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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() {
Comment thread
txabman42 marked this conversation as resolved.
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)
}
Expand Down
12 changes: 6 additions & 6 deletions tool/internal/setup/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
Expand Down
Loading