diff --git a/tool/internal/instrument/instrument_test.go b/tool/internal/instrument/instrument_test.go index d057da1f..1e17db19 100644 --- a/tool/internal/instrument/instrument_test.go +++ b/tool/internal/instrument/instrument_test.go @@ -78,7 +78,7 @@ func runTest(t *testing.T, testName string) { ruleSet := loadRulesYAML(t, testName, sourceFile) writeMatchedJSON(ruleSet) - args := compileArgs(tempDir, sourceFile) + args := compileArgs(t, tempDir, sourceFile, ruleSet) err := Toolexec(ctx, args) if testName == invalidReceiver { @@ -157,12 +157,16 @@ func writeMatchedJSON(ruleSet *rule.InstRuleSet) { util.WriteFile(matchedFile, string(matchedJSON)) } -func compileArgs(tempDir, sourceFile string) []string { +func compileArgs(t *testing.T, tempDir, sourceFile string, ruleSet *rule.InstRuleSet) []string { + t.Helper() output, _ := exec.Command("go", "env", "GOTOOLDIR").Output() + // Collect imports declared by rules so createImportCfg can resolve them + extraImports := collectRuleImports(ruleSet) + // Create importcfg file for the test importCfgPath := filepath.Join(tempDir, "importcfg") - createImportCfg(importCfgPath) + createImportCfg(t, importCfgPath, extraImports) return []string{ filepath.Join(strings.TrimSpace(string(output)), "compile"), @@ -176,46 +180,100 @@ func compileArgs(tempDir, sourceFile string) []string { } } -// createImportCfg creates a basic importcfg file with standard library packages. -func createImportCfg(path string) { - // Get standard library package locations - // We'll use go list to populate common packages - ctx := context.Background() +// collectRuleImports gathers all import paths declared in rule imports fields. +// This allows createImportCfg to pre-resolve non-stdlib packages that rules +// reference, fixing the gap where golden tests could only use standard library +// imports in call rules. +func collectRuleImports(ruleSet *rule.InstRuleSet) []string { + seen := make(map[string]bool) + for _, rules := range ruleSet.CallRules { + for _, r := range rules { + for _, importPath := range r.Imports { + if !seen[importPath] { + seen[importPath] = true + } + } + } + } + for _, rules := range ruleSet.RawRules { + for _, r := range rules { + for _, importPath := range r.Imports { + if !seen[importPath] { + seen[importPath] = true + } + } + } + } + for _, rules := range ruleSet.DeclRules { + for _, r := range rules { + for _, importPath := range r.Imports { + if !seen[importPath] { + seen[importPath] = true + } + } + } + } - // Start with an empty config - cfg := struct { - PackageFile map[string]string - }{ - PackageFile: make(map[string]string), + importPaths := make([]string, 0, len(seen)) + for p := range seen { + importPaths = append(importPaths, p) } + return importPaths +} + +// createImportCfg creates an importcfg file with standard library packages and +// any additional packages required by rule imports. The extraImports parameter +// allows test cases to pre-resolve non-stdlib packages (e.g. helper packages +// from pkg/instrumentation/) so that the compile command can find them. +func createImportCfg(t *testing.T, path string, extraImports []string) { + t.Helper() + ctx := context.Background() + + packageFiles := make(map[string]string) // Resolve common standard library packages that might be needed commonPkgs := []string{"fmt", "unsafe", "runtime", "strings", "io"} for _, pkg := range commonPkgs { - cmd := exec.CommandContext(ctx, "go", "list", "-export", "-json", pkg) - output, err := cmd.Output() - if err != nil { - continue // Skip if package not found - } + resolvePackageExport(ctx, pkg, packageFiles) + } - var info struct { - ImportPath string `json:"ImportPath"` - Export string `json:"Export"` - } - if err2 := json.Unmarshal(output, &info); err2 == nil && info.Export != "" { - cfg.PackageFile[info.ImportPath] = info.Export + // Resolve additional imports declared by rules. This is the key + // infrastructure fix: without this, call rules that reference helper + // packages through the imports field would fail to compile in golden tests + // because the importcfg would not include the helper package's archive. + for _, pkg := range extraImports { + if _, exists := packageFiles[pkg]; exists { + continue } + resolvePackageExport(ctx, pkg, packageFiles) } // Write the importcfg file f, err := os.Create(path) + require.NoError(t, err, "creating importcfg at %s", path) + defer f.Close() + + for importPath, archive := range packageFiles { + fmt.Fprintf(f, "packagefile %s=%s\n", importPath, archive) + } +} + +// resolvePackageExport uses go list to find a package's export file and adds +// it to the packageFiles map. Packages that cannot be resolved (e.g. builtin +// pseudo-packages like "unsafe") are silently skipped. +func resolvePackageExport(ctx context.Context, pkg string, packageFiles map[string]string) { + cmd := exec.CommandContext(ctx, "go", "list", "-export", "-json", pkg) + output, err := cmd.Output() if err != nil { return } - defer f.Close() - for importPath, archive := range cfg.PackageFile { - fmt.Fprintf(f, "packagefile %s=%s\n", importPath, archive) + var info struct { + ImportPath string `json:"ImportPath"` + Export string `json:"Export"` + } + if err2 := json.Unmarshal(output, &info); err2 == nil && info.Export != "" { + packageFiles[info.ImportPath] = info.Export } } diff --git a/tool/internal/instrument/testdata/golden/call-rule-local-import/call_rule_local_import.main.go.golden b/tool/internal/instrument/testdata/golden/call-rule-local-import/call_rule_local_import.main.go.golden new file mode 100644 index 00000000..e17387e4 --- /dev/null +++ b/tool/internal/instrument/testdata/golden/call-rule-local-import/call_rule_local_import.main.go.golden @@ -0,0 +1,21 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "unsafe" + + "github.com/open-telemetry/opentelemetry-go-compile-instrumentation/tool/internal/instrument/testhelper/wrapper" +) + +func CallSizeof() { + x := 42 + size := wrapper.Sizeof(unsafe.Sizeof(x)) + _ = size +} + +func main() { + y := "hello" + _ = wrapper.Sizeof(unsafe.Sizeof(y)) +} diff --git a/tool/internal/instrument/testdata/golden/call-rule-local-import/rules.yml b/tool/internal/instrument/testdata/golden/call-rule-local-import/rules.yml new file mode 100644 index 00000000..05fbf135 --- /dev/null +++ b/tool/internal/instrument/testdata/golden/call-rule-local-import/rules.yml @@ -0,0 +1,6 @@ +wrap_sizeof_imported: + target: main + function_call: unsafe.Sizeof + replace: "wrapper.Sizeof({{ . }})" + imports: + wrapper: "github.com/open-telemetry/opentelemetry-go-compile-instrumentation/tool/internal/instrument/testhelper/wrapper" diff --git a/tool/internal/instrument/testdata/golden/call-rule-local-import/source.go b/tool/internal/instrument/testdata/golden/call-rule-local-import/source.go new file mode 100644 index 00000000..376d26b7 --- /dev/null +++ b/tool/internal/instrument/testdata/golden/call-rule-local-import/source.go @@ -0,0 +1,17 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package main + +import "unsafe" + +func CallSizeof() { + x := 42 + size := unsafe.Sizeof(x) + _ = size +} + +func main() { + y := "hello" + _ = unsafe.Sizeof(y) +} diff --git a/tool/internal/instrument/testhelper/wrapper/wrapper.go b/tool/internal/instrument/testhelper/wrapper/wrapper.go new file mode 100644 index 00000000..f4f01455 --- /dev/null +++ b/tool/internal/instrument/testhelper/wrapper/wrapper.go @@ -0,0 +1,14 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +// Package wrapper provides helper functions used by golden tests to validate +// that call rules can correctly reference imported helper packages via the +// imports field. This package is intentionally outside testdata/ so that the +// Go build system can resolve it during test compilation. +package wrapper + +// Sizeof wraps a uintptr value, demonstrating how an imported helper function +// can be used in a call rule's replace template (e.g. "wrapper.Sizeof({{ . }})"). +func Sizeof(size uintptr) uintptr { + return size +}