From 84c192004a038d37101c1f574ce311329da27f14 Mon Sep 17 00:00:00 2001 From: Yi Yang Date: Sun, 28 Sep 2025 10:55:30 +0800 Subject: [PATCH 1/5] refactor: differentiate between Wrapf and Newf --- tool/cmd/cmd_go.go | 4 +- tool/cmd/cmd_setup.go | 2 +- tool/cmd/cmd_toolexec.go | 2 +- tool/cmd/cmd_version.go | 10 +- tool/cmd/main.go | 4 +- tool/data/export.go | 4 +- tool/ex/error.go | 138 +++++++++++++++++++++++++ tool/ex/ex_test.go | 26 +++++ tool/ex/wrap.go | 93 ----------------- tool/internal/ast/parser.go | 14 +-- tool/internal/instrument/apply_func.go | 2 +- tool/internal/instrument/match.go | 4 +- tool/internal/instrument/toolexec.go | 2 +- tool/internal/instrument/trampoline.go | 8 +- tool/internal/setup/extract.go | 16 +-- tool/internal/setup/find.go | 8 +- tool/internal/setup/match.go | 2 +- tool/internal/setup/setup.go | 2 +- tool/internal/setup/store.go | 6 +- tool/internal/setup/sync.go | 10 +- tool/util/sys.go | 18 ++-- 21 files changed, 223 insertions(+), 152 deletions(-) create mode 100644 tool/ex/error.go create mode 100644 tool/ex/ex_test.go delete mode 100644 tool/ex/wrap.go diff --git a/tool/cmd/cmd_go.go b/tool/cmd/cmd_go.go index b0d8e6c..70ecdc8 100644 --- a/tool/cmd/cmd_go.go +++ b/tool/cmd/cmd_go.go @@ -40,12 +40,12 @@ var commandGo = cli.Command{ err = setup.Setup(ctx) if err != nil { - return ex.Errorf(err, "failed to build with toolexec with exit code %d", exitCodeFailure) + return ex.Wrapf(err, "failed to build with toolexec with exit code %d", exitCodeFailure) } err = setup.BuildWithToolexec(ctx, cmd.Args().Slice()) if err != nil { - return ex.Errorf(err, "failed to build with toolexec with exit code %d", exitCodeFailure) + return ex.Wrapf(err, "failed to build with toolexec with exit code %d", exitCodeFailure) } return nil diff --git a/tool/cmd/cmd_setup.go b/tool/cmd/cmd_setup.go index 6cd71e8..6237e8b 100644 --- a/tool/cmd/cmd_setup.go +++ b/tool/cmd/cmd_setup.go @@ -19,7 +19,7 @@ var commandSetup = cli.Command{ Action: func(ctx context.Context, _ *cli.Command) error { err := setup.Setup(ctx) if err != nil { - return ex.Errorf(err, "failed to setup with exit code %d", exitCodeFailure) + return ex.Wrapf(err, "failed to setup with exit code %d", exitCodeFailure) } return nil }, diff --git a/tool/cmd/cmd_toolexec.go b/tool/cmd/cmd_toolexec.go index e879e55..fe2ff85 100644 --- a/tool/cmd/cmd_toolexec.go +++ b/tool/cmd/cmd_toolexec.go @@ -21,7 +21,7 @@ var commandToolexec = cli.Command{ Action: func(ctx context.Context, cmd *cli.Command) error { err := instrument.Toolexec(ctx, cmd.Args().Slice()) if err != nil { - return ex.Errorf(err, "failed to run toolexec with exit code %d", exitCodeFailure) + return ex.Wrapf(err, "failed to run toolexec with exit code %d", exitCodeFailure) } return nil }, diff --git a/tool/cmd/cmd_version.go b/tool/cmd/cmd_version.go index 424dbe0..7e33f52 100644 --- a/tool/cmd/cmd_version.go +++ b/tool/cmd/cmd_version.go @@ -26,32 +26,32 @@ var commandVersion = cli.Command{ Action: func(_ context.Context, cmd *cli.Command) error { _, err := fmt.Fprintf(cmd.Writer, "otel version %s", Version) if err != nil { - return ex.Errorf(err, "failed to print version with exit code %d", exitCodeFailure) + return ex.Wrapf(err, "failed to print version with exit code %d", exitCodeFailure) } if CommitHash != "unknown" { _, err = fmt.Fprintf(cmd.Writer, "+%s", CommitHash) if err != nil { - return ex.Errorf(err, "failed to print version with exit code %d", exitCodeFailure) + return ex.Wrapf(err, "failed to print version with exit code %d", exitCodeFailure) } } if BuildTime != "unknown" { _, err = fmt.Fprintf(cmd.Writer, " (%s)", BuildTime) if err != nil { - return ex.Errorf(err, "failed to print version with exit code %d", exitCodeFailure) + return ex.Wrapf(err, "failed to print version with exit code %d", exitCodeFailure) } } _, err = fmt.Fprint(cmd.Writer, "\n") if err != nil { - return ex.Errorf(err, "failed to print version with exit code %d", exitCodeFailure) + return ex.Wrapf(err, "failed to print version with exit code %d", exitCodeFailure) } if cmd.Bool("verbose") { _, err = fmt.Fprintf(cmd.Writer, "%s\n", runtime.Version()) if err != nil { - return ex.Errorf(err, "failed to print version with exit code %d", exitCodeFailure) + return ex.Wrapf(err, "failed to print version with exit code %d", exitCodeFailure) } } diff --git a/tool/cmd/main.go b/tool/cmd/main.go index e8a7fc2..1da4938 100644 --- a/tool/cmd/main.go +++ b/tool/cmd/main.go @@ -59,13 +59,13 @@ func initLogger(ctx context.Context, cmd *cli.Command) (context.Context, error) buildTempDir := cmd.String("work-dir") err := os.MkdirAll(buildTempDir, 0o755) if err != nil { - return ctx, ex.Errorf(err, "failed to create work directory %q", buildTempDir) + return ctx, ex.Wrapf(err, "failed to create work directory %q", buildTempDir) } logFilename := filepath.Join(buildTempDir, debugLogFilename) writer, err := os.OpenFile(logFilename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) if err != nil { - return ctx, ex.Errorf(err, "failed to open log file %q", buildTempDir) + return ctx, ex.Wrapf(err, "failed to open log file %q", buildTempDir) } // Create a custom handler with shorter time format diff --git a/tool/data/export.go b/tool/data/export.go index f8a1d32..47e4ffa 100644 --- a/tool/data/export.go +++ b/tool/data/export.go @@ -16,7 +16,7 @@ var dataFs embed.FS func ListEmbedFiles() ([]string, error) { rules, err := dataFs.ReadDir(".") if err != nil { - return nil, ex.Errorf(err, "failed to read directory") + return nil, ex.Wrapf(err, "failed to read directory") } var ruleFiles []string @@ -32,7 +32,7 @@ func ListEmbedFiles() ([]string, error) { func ReadEmbedFile(path string) ([]byte, error) { bs, err := dataFs.ReadFile(path) if err != nil { - return nil, ex.Errorf(err, "failed to read file") + return nil, ex.Wrapf(err, "failed to read file") } return bs, nil } diff --git a/tool/ex/error.go b/tool/ex/error.go new file mode 100644 index 0000000..ba9e508 --- /dev/null +++ b/tool/ex/error.go @@ -0,0 +1,138 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package ex + +import ( + "errors" + "fmt" + "os" + "runtime" + "strings" +) + +// ----------------------------------------------------------------------------- +// Extended Stackful Error Handling +// +// These APIs provide an error handling framework that allows errors to carry +// stack trace information. +// +// The core usage pattern is to create an error with a stack trace at its origin +// This allows the error to be propagated up the call stack by simply returning +// it (return err), without needing to be re-wrapped at each level. +// +// There are two primary ways to create the error, depending on the error's source: +// +// 1. To wrap an existing error from a standard or third-party library, use Wrap +// or Wrapf. This attaches a stack trace to the original error. +// +// Example: +// if err := some_lib.DoSomething(); err != nil { +// return ex.Wrap(err, "additional context for the error") +// } +// +// 2. To create a new error from scratch, use Newf. This generates a new +// error with a stack trace at the point of creation. +// +// Example: +// if id <= 0 { +// return ex.Newf("failed to do something") +// } +// +// Use Fatalf or Fatal to exit the program with an stackful error. It will print +// the error message and stack trace to the standard error output. + +const numSkipFrame = 4 // skip the Errorf/Fatalf caller + +// stackfulError represents an error with stack trace information +type stackfulError struct { + message []string + frame []string + wrapped error +} + +func (e *stackfulError) Error() string { return strings.Join(e.message, "\n") } +func (e *stackfulError) Unwrap() error { return e.wrapped } + +func getFrames() []string { + const initFrames = 30 + frameList := make([]string, 0) + pcs := make([]uintptr, initFrames) + n := runtime.Callers(numSkipFrame, pcs) + if n == 0 { + return frameList + } + pcs = pcs[:n] + frames := runtime.CallersFrames(pcs) + cnt := 0 + for { + frame, more := frames.Next() + if !more { + break + } + const prefix = "github.com/open-telemetry/opentelemetry-go-compile-instrumentation" + fnName := strings.TrimPrefix(frame.Function, prefix) + f := fmt.Sprintf("[%d]%s:%d %s", cnt, frame.File, frame.Line, fnName) + frameList = append(frameList, f) + cnt++ + } + return frameList +} + +// wrapOrCreate wraps an error with stack trace information and a formatted message +// If the error is already a stackfulError, it will be decorated with the new message. +// Otherwise, a new stackfulError will be created. +func wrapOrCreate(previousErr error, format string, args ...any) error { + se := &stackfulError{} + if errors.As(previousErr, &se) { + attach := fmt.Sprintf(format, args...) + if attach != "" { + se.message = append(se.message, attach) + } + return previousErr + } + // User defined error message + existing error message + errMsg := fmt.Sprintf(format, args...) + if previousErr != nil { + errMsg = fmt.Sprintf("%s: %s", errMsg, previousErr.Error()) + } + e := &stackfulError{ + message: []string{errMsg}, + frame: getFrames(), + wrapped: previousErr, + } + return e +} + +func Wrap(previousErr error) error { + return wrapOrCreate(previousErr, "") +} + +func Wrapf(previousErr error, format string, args ...any) error { + return wrapOrCreate(previousErr, format, args...) +} + +func Newf(format string, args ...any) error { + return wrapOrCreate(nil, format, args...) +} + +func Fatalf(format string, args ...any) { + Fatal(Newf(format, args...)) +} + +func Fatal(err error) { + if err == nil { + panic("Fatal error: unknown") + } + e := &stackfulError{} + if errors.As(err, &e) { + em := "" + for i, m := range e.message { + em += fmt.Sprintf("[%d] %s\n", i, m) + } + msg := fmt.Sprintf("Error:\n%s\nStack:\n%s", em, strings.Join(e.frame, "\n")) + _, _ = fmt.Fprint(os.Stderr, msg) + os.Exit(1) + } + panic(err) +} diff --git a/tool/ex/ex_test.go b/tool/ex/ex_test.go new file mode 100644 index 0000000..90a5b2a --- /dev/null +++ b/tool/ex/ex_test.go @@ -0,0 +1,26 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package ex + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestError(t *testing.T) { + err := Newf("a") + err = Wrapf(err, "b") + err = Wrap(Wrap(Wrap(err))) // make no sense + require.Contains(t, err.Error(), "a") + require.Contains(t, err.Error(), "b") + + err = fmt.Errorf("c") + err = Wrapf(err, "d") + err = Wrapf(err, "e") + err = Wrap(Wrap(Wrap(err))) // make no sense + require.Contains(t, err.Error(), "c") + require.Contains(t, err.Error(), "d") +} diff --git a/tool/ex/wrap.go b/tool/ex/wrap.go deleted file mode 100644 index ee2bd82..0000000 --- a/tool/ex/wrap.go +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package ex - -import ( - "errors" - "fmt" - "os" - "runtime" - "strings" -) - -const numSkipFrame = 3 // skip the Errorf/Fatalf caller - -// stackfulError represents an error with stack trace information -type stackfulError struct { - message []string - frame []string - wrapped error -} - -func (e *stackfulError) Error() string { return strings.Join(e.message, "\n") } -func (e *stackfulError) Unwrap() error { return e.wrapped } - -func getFrames() []string { - const initFrames = 30 - frameList := make([]string, 0) - pcs := make([]uintptr, initFrames) - n := runtime.Callers(numSkipFrame, pcs) - if n == 0 { - return frameList - } - pcs = pcs[:n] - frames := runtime.CallersFrames(pcs) - cnt := 0 - for { - frame, more := frames.Next() - if !more { - break - } - const prefix = "github.com/open-telemetry/opentelemetry-go-compile-instrumentation" - fnName := strings.TrimPrefix(frame.Function, prefix) - f := fmt.Sprintf("[%d]%s:%d %s", cnt, frame.File, frame.Line, fnName) - frameList = append(frameList, f) - cnt++ - } - return frameList -} - -func Error(previousErr error) error { - return Errorf(previousErr, "") -} - -// Errorf wraps an error with stack trace information and a formatted message -// If you want to decorate the existing error, use it. -func Errorf(previousErr error, format string, args ...any) error { - se := &stackfulError{} - if errors.As(previousErr, &se) { - se.message = append(se.message, fmt.Sprintf(format, args...)) - return previousErr - } - // User defined error message + existing error message - errMsg := fmt.Sprintf(format, args...) - if previousErr != nil { - errMsg = fmt.Sprintf("%s: %s", errMsg, previousErr.Error()) - } - e := &stackfulError{ - message: []string{errMsg}, - frame: getFrames(), - wrapped: previousErr, - } - return e -} - -func Fatalf(format string, args ...any) { Fatal(Errorf(nil, format, args...)) } - -func Fatal(err error) { - if err == nil { - panic("Fatal error: unknown") - } - e := &stackfulError{} - if errors.As(err, &e) { - em := "" - for i, m := range e.message { - em += fmt.Sprintf("[%d] %s\n", i, m) - } - msg := fmt.Sprintf("Error:\n%s\nStack:\n%s", em, strings.Join(e.frame, "\n")) - _, _ = fmt.Fprint(os.Stderr, msg) - os.Exit(1) - } - panic(err) -} diff --git a/tool/internal/ast/parser.go b/tool/internal/ast/parser.go index 3904023..ebc0e59 100644 --- a/tool/internal/ast/parser.go +++ b/tool/internal/ast/parser.go @@ -35,16 +35,16 @@ func (ap *AstParser) Parse(filePath string, mode parser.Mode) (*dst.File, error) name := filepath.Base(filePath) file, err := os.Open(filePath) if err != nil { - return nil, ex.Errorf(err, "failed to open file %s", filePath) + return nil, ex.Wrapf(err, "failed to open file %s", filePath) } defer file.Close() astFile, err := parser.ParseFile(ap.fset, name, file, mode) if err != nil { - return nil, ex.Errorf(err, "failed to parse file %s", filePath) + return nil, ex.Wrapf(err, "failed to parse file %s", filePath) } dstFile, err := ap.dec.DecorateFile(astFile) if err != nil { - return nil, ex.Errorf(err, "failed to decorate file %s", filePath) + return nil, ex.Wrapf(err, "failed to decorate file %s", filePath) } return dstFile, nil } @@ -52,11 +52,11 @@ func (ap *AstParser) Parse(filePath string, mode parser.Mode) (*dst.File, error) // ParseSource parses the AST from complete source code. func (ap *AstParser) ParseSource(source string) (*dst.File, error) { if source == "" { - return nil, ex.Errorf(nil, "empty source") + return nil, ex.Newf("empty source") } dstRoot, err := ap.dec.Parse(source) if err != nil { - return nil, ex.Error(err) + return nil, ex.Wrap(err) } return dstRoot, nil } @@ -74,13 +74,13 @@ func (ap *AstParser) FindPosition(node dst.Node) token.Position { func WriteFile(filePath string, root *dst.File) error { file, err := os.Create(filePath) if err != nil { - return ex.Errorf(err, "failed to create file %s", filePath) + return ex.Wrapf(err, "failed to create file %s", filePath) } defer file.Close() r := decorator.NewRestorer() err = r.Fprint(file, root) if err != nil { - return ex.Errorf(err, "failed to write to file %s", filePath) + return ex.Wrapf(err, "failed to write to file %s", filePath) } return nil } diff --git a/tool/internal/instrument/apply_func.go b/tool/internal/instrument/apply_func.go index 0817f53..5d51d70 100644 --- a/tool/internal/instrument/apply_func.go +++ b/tool/internal/instrument/apply_func.go @@ -275,7 +275,7 @@ func (ip *InstrumentPhase) writeInstrumented(root *dst.File, oldFile string) err } } if !replace { - return ex.Errorf(nil, "cannot apply the instrumented %s for %v", + return ex.Newf("cannot apply the instrumented %s for %v", oldFile, ip.compileArgs) } ip.Info("Write instrumented AST", "old", oldFile, "new", newFile) diff --git a/tool/internal/instrument/match.go b/tool/internal/instrument/match.go index bf655c9..0350833 100644 --- a/tool/internal/instrument/match.go +++ b/tool/internal/instrument/match.go @@ -19,7 +19,7 @@ func (ip *InstrumentPhase) load() ([]*rule.InstFuncRule, error) { f := util.GetMatchedRuleFile() content, err := os.ReadFile(f) if err != nil { - return nil, ex.Errorf(err, "failed to read file %s", f) + return nil, ex.Wrapf(err, "failed to read file %s", f) } if len(content) == 0 { return nil, nil @@ -28,7 +28,7 @@ func (ip *InstrumentPhase) load() ([]*rule.InstFuncRule, error) { rules := make([]*rule.InstFuncRule, 0) err = json.Unmarshal(content, &rules) if err != nil { - return nil, ex.Errorf(err, "failed to unmarshal rules from file %s", f) + return nil, ex.Wrapf(err, "failed to unmarshal rules from file %s", f) } ip.Debug("Loaded matched rules", "rules", rules) return rules, nil diff --git a/tool/internal/instrument/toolexec.go b/tool/internal/instrument/toolexec.go index 47e78d1..7d80403 100644 --- a/tool/internal/instrument/toolexec.go +++ b/tool/internal/instrument/toolexec.go @@ -82,7 +82,7 @@ func stripCompleteFlag(args []string) []string { func interceptCompile(ctx context.Context, args []string) ([]string, error) { // Read compilation output directory target := util.FindFlagValue(args, "-o") - util.Assert(target != "", "why not otherwise") + util.Assert(target != "", "missing -o flag value") ip := &InstrumentPhase{ logger: util.LoggerFromContext(ctx), workDir: filepath.Dir(target), diff --git a/tool/internal/instrument/trampoline.go b/tool/internal/instrument/trampoline.go index 2ca25a5..63d2178 100644 --- a/tool/internal/instrument/trampoline.go +++ b/tool/internal/instrument/trampoline.go @@ -189,7 +189,7 @@ func findHookFile(rule *rule.InstFuncRule) (string, error) { return file, nil } } - return "", ex.Errorf(nil, "no hook %s/%s found for %s from %v", + return "", ex.Newf("no hook %s/%s found for %s from %v", rule.GetBefore(), rule.GetAfter(), rule.GetFuncName(), files) } @@ -232,7 +232,7 @@ func getHookFunc(t *rule.InstFuncRule, before bool) (*dst.FuncDecl, error) { } } if target == nil { - return nil, ex.Errorf(nil, "hook %s or %s not found", + return nil, ex.Newf("hook %s or %s not found", t.GetBefore(), t.GetAfter()) } return target, nil @@ -332,7 +332,7 @@ func (ip *InstrumentPhase) callAfterHook(t *rule.InstFuncRule, traits []ParamTra func rectifyAnyType(paramList *dst.FieldList, traits []ParamTrait) error { if len(paramList.List) != len(traits) { - return ex.Errorf(nil, "hook func signature can not match with target function") + return ex.Newf("hook func signature can not match with target function") } for i, field := range paramList.List { trait := traits[i] @@ -731,7 +731,7 @@ func (ip *InstrumentPhase) callHookFunc(t *rule.InstFuncRule, before bool) error } // Fulfill the hook context before calling the real hook code. if !ip.replenishHookContext(before) { - return ex.Errorf(nil, "failed to replenish hook context") + return ex.Newf("failed to replenish hook context") } return nil } diff --git a/tool/internal/setup/extract.go b/tool/internal/setup/extract.go index d84f845..6f06390 100644 --- a/tool/internal/setup/extract.go +++ b/tool/internal/setup/extract.go @@ -38,26 +38,26 @@ func extract(tarReader *tar.Reader, header *tar.Header, targetPath string) error case tar.TypeDir: err := os.MkdirAll(targetPath, fileInfo.Mode()) if err != nil { - return ex.Error(err) + return ex.Wrap(err) } case tar.TypeReg: file, err := os.OpenFile(targetPath, os.O_CREATE|os.O_RDWR, fileInfo.Mode()) if err != nil { - return ex.Error(err) + return ex.Wrap(err) } _, err = io.CopyN(file, tarReader, header.Size) if err != nil { - return ex.Error(err) + return ex.Wrap(err) } err = file.Close() if err != nil { - return ex.Error(err) + return ex.Wrap(err) } default: - return ex.Errorf(nil, "unsupported file type: %c in %s", + return ex.Newf("unsupported file type: %c in %s", header.Typeflag, header.Name) } return nil @@ -66,12 +66,12 @@ func extract(tarReader *tar.Reader, header *tar.Header, targetPath string) error func extractGZip(data []byte, targetDir string) error { err0 := os.MkdirAll(targetDir, 0o755) if err0 != nil { - return ex.Error(err0) + return ex.Wrap(err0) } gzReader, err0 := gzip.NewReader(strings.NewReader(string(data))) if err0 != nil { - return ex.Error(err0) + return ex.Wrap(err0) } defer func() { err0 = gzReader.Close() @@ -87,7 +87,7 @@ func extractGZip(data []byte, targetDir string) error { break } if err != nil { - return ex.Error(err) + return ex.Wrap(err) } // Skip AppleDouble files (._filename) and other hidden files diff --git a/tool/internal/setup/find.go b/tool/internal/setup/find.go index 581d338..5dfd574 100644 --- a/tool/internal/setup/find.go +++ b/tool/internal/setup/find.go @@ -42,7 +42,7 @@ func findCompileCommands(buildPlanLog *os.File) ([]string, error) { // Seek to the beginning of the file before reading _, err := buildPlanLog.Seek(0, 0) if err != nil { - return nil, ex.Errorf(err, "failed to seek to beginning of build plan log") + return nil, ex.Wrapf(err, "failed to seek to beginning of build plan log") } // 10MB should be enough to accommodate most long line buffer := make([]byte, 0, buildPlanBufSize) @@ -56,7 +56,7 @@ func findCompileCommands(buildPlanLog *os.File) ([]string, error) { } err = scanner.Err() if err != nil { - return nil, ex.Errorf(err, "failed to parse build plan log") + return nil, ex.Wrapf(err, "failed to parse build plan log") } return compileCmds, nil } @@ -71,7 +71,7 @@ func (sp *SetupPhase) listBuildPlan(ctx context.Context, goBuildCmd []string) ([ // Create a build plan log file in the temporary directory buildPlanLog, err := os.Create(util.GetBuildTemp(BuildPlanLog)) if err != nil { - return nil, ex.Errorf(err, "failed to create build plan log file") + return nil, ex.Wrapf(err, "failed to create build plan log file") } defer buildPlanLog.Close() // The full build command is: "go build/install -a -x -n {...}" @@ -93,7 +93,7 @@ func (sp *SetupPhase) listBuildPlan(ctx context.Context, goBuildCmd []string) ([ cmd.Dir = "" err = cmd.Run() if err != nil { - return nil, ex.Errorf(err, "failed to run build plan") + return nil, ex.Wrapf(err, "failed to run build plan") } // Find compile commands from build plan log diff --git a/tool/internal/setup/match.go b/tool/internal/setup/match.go index d6b7040..d2d29ca 100644 --- a/tool/internal/setup/match.go +++ b/tool/internal/setup/match.go @@ -21,7 +21,7 @@ func parseEmbeddedRule(path string) ([]*rule.InstFuncRule, error) { rules := make(map[string]*rule.InstFuncRule) err = yaml.NewDecoder(bytes.NewReader(yamlFile)).Decode(&rules) if err != nil { - return nil, ex.Errorf(err, "failed to decode yaml file") + return nil, ex.Wrapf(err, "failed to decode yaml file") } arr := make([]*rule.InstFuncRule, 0) for name, r := range rules { diff --git a/tool/internal/setup/setup.go b/tool/internal/setup/setup.go index 8781fe4..bf6f4d6 100644 --- a/tool/internal/setup/setup.go +++ b/tool/internal/setup/setup.go @@ -92,7 +92,7 @@ func BuildWithToolexec(ctx context.Context, args []string) error { // Add -toolexec=otel to the original build command and run it execPath, err := os.Executable() if err != nil { - return ex.Errorf(err, "failed to get executable path") + return ex.Wrapf(err, "failed to get executable path") } insert := "-toolexec=" + execPath + " toolexec" const additionalCount = 2 diff --git a/tool/internal/setup/store.go b/tool/internal/setup/store.go index 348fdf6..27230da 100644 --- a/tool/internal/setup/store.go +++ b/tool/internal/setup/store.go @@ -18,18 +18,18 @@ func (sp *SetupPhase) store(matched []*rule.InstFuncRule) error { f := util.GetMatchedRuleFile() file, err := os.Create(f) if err != nil { - return ex.Errorf(err, "failed to create file %s", f) + return ex.Wrapf(err, "failed to create file %s", f) } defer file.Close() bs, err := json.Marshal(matched) if err != nil { - return ex.Errorf(err, "failed to marshal rules to JSON") + return ex.Wrapf(err, "failed to marshal rules to JSON") } _, err = file.Write(bs) if err != nil { - return ex.Errorf(err, "failed to write JSON to file %s", f) + return ex.Wrapf(err, "failed to write JSON to file %s", f) } sp.Info("Stored matched rules", "rules", matched) return nil diff --git a/tool/internal/setup/sync.go b/tool/internal/setup/sync.go index 854c5e7..8319636 100644 --- a/tool/internal/setup/sync.go +++ b/tool/internal/setup/sync.go @@ -18,11 +18,11 @@ import ( func parseGoMod(gomod string) (*modfile.File, error) { data, err := os.ReadFile(gomod) if err != nil { - return nil, ex.Errorf(err, "failed to read go.mod file") + return nil, ex.Wrapf(err, "failed to read go.mod file") } modFile, err := modfile.Parse(gomod, data, nil) if err != nil { - return nil, ex.Errorf(err, "failed to parse go.mod file") + return nil, ex.Wrapf(err, "failed to parse go.mod file") } return modFile, nil } @@ -30,11 +30,11 @@ func parseGoMod(gomod string) (*modfile.File, error) { func writeGoMod(gomod string, modfile *modfile.File) error { data, err := modfile.Format() if err != nil { - return ex.Errorf(err, "failed to format go.mod file") + return ex.Wrapf(err, "failed to format go.mod file") } err = os.WriteFile(gomod, data, 0o644) //nolint:gosec // 0644 is ok if err != nil { - return ex.Errorf(err, "failed to write go.mod file") + return ex.Wrapf(err, "failed to write go.mod file") } return nil } @@ -54,7 +54,7 @@ func addReplace(modfile *modfile.File, path, version, rpath, rversion string) (b if !hasReplace { err := modfile.AddReplace(path, version, rpath, rversion) if err != nil { - return false, ex.Errorf(err, "failed to add replace directive") + return false, ex.Wrapf(err, "failed to add replace directive") } return true, nil } diff --git a/tool/util/sys.go b/tool/util/sys.go index 89ccf46..c0d419d 100644 --- a/tool/util/sys.go +++ b/tool/util/sys.go @@ -27,7 +27,7 @@ func RunCmdWithEnv(ctx context.Context, env []string, args ...string) error { cmd.Env = env err := cmd.Run() if err != nil { - return ex.Errorf(err, "failed to run command %s", path) + return ex.Wrapf(err, "failed to run command %s", path) } return nil } @@ -49,25 +49,25 @@ func CopyFile(src, dst string) error { if os.IsNotExist(err) { err = os.MkdirAll(filepath.Dir(dst), 0o755) if err != nil { - return ex.Error(err) + return ex.Wrap(err) } } srcFile, err := os.Open(src) if err != nil { - return ex.Error(err) + return ex.Wrap(err) } defer srcFile.Close() dstFile, err := os.Create(dst) if err != nil { - return ex.Error(err) + return ex.Wrap(err) } defer dstFile.Close() _, err = io.Copy(dstFile, srcFile) if err != nil { - return ex.Error(err) + return ex.Wrap(err) } return nil } @@ -81,7 +81,7 @@ func ListFiles(dir string) ([]string, error) { var files []string walkFn := func(path string, info os.FileInfo, err error) error { if err != nil { - return ex.Error(err) + return ex.Wrap(err) } // Don't list files under hidden directories if strings.HasPrefix(info.Name(), ".") { @@ -94,7 +94,7 @@ func ListFiles(dir string) ([]string, error) { } err := filepath.Walk(dir, walkFn) if err != nil { - return nil, ex.Error(err) + return nil, ex.Wrap(err) } return files, nil } @@ -102,7 +102,7 @@ func ListFiles(dir string) ([]string, error) { func WriteFile(filePath string, content string) error { file, err := os.Create(filePath) if err != nil { - return ex.Error(err) + return ex.Wrap(err) } defer func(file *os.File) { err = file.Close() @@ -113,7 +113,7 @@ func WriteFile(filePath string, content string) error { _, err = file.WriteString(content) if err != nil { - return ex.Error(err) + return ex.Wrap(err) } return nil } From 48f1aa3b30ab7f1ac0b329b76d0fcacc3b80525e Mon Sep 17 00:00:00 2001 From: Yi Yang Date: Sun, 28 Sep 2025 10:58:43 +0800 Subject: [PATCH 2/5] remove developing.md --- docs/developing.md | 39 --------------------------------------- 1 file changed, 39 deletions(-) delete mode 100644 docs/developing.md diff --git a/docs/developing.md b/docs/developing.md deleted file mode 100644 index 66e180a..0000000 --- a/docs/developing.md +++ /dev/null @@ -1,39 +0,0 @@ -# Developing Details - -## Project Structure - -See the [project structure](./api-design-and-project-structure.md). - -## Error Handling - -We use the `ex` package to handle errors as we want to include the stack trace -in the error message for better debugging. - -Any error should be wrapped with `ex.Error` or `ex.Errorf` to include the stack trace. - -```go -// Wrap an existing error and return to the caller -return ex.Error(err) -``` - -```go -// Wrap the existing error with an additional message and return it to the caller. -return ex.Errorf(err, "additional context %s", "some value") -``` - -```go -// Create a new error with an additional message -return ex.Errorf(nil, "additional context %s", "some value") -``` - -Exit the program with `ex.Fatal` or `ex.Fatalf`. - -```go -// Exit the program with an error -ex.Fatal(err) -``` - -```go -// Exit the program with an error and an additional message -ex.Fatalf("additional context %s", "some value") -``` From 8cd1c6c4373e1b7a3b88b1bea12bd4e448410c3b Mon Sep 17 00:00:00 2001 From: Yi Yang Date: Sun, 28 Sep 2025 11:00:40 +0800 Subject: [PATCH 3/5] fix linter --- tool/ex/ex_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tool/ex/ex_test.go b/tool/ex/ex_test.go index 90a5b2a..0888d9a 100644 --- a/tool/ex/ex_test.go +++ b/tool/ex/ex_test.go @@ -4,7 +4,7 @@ package ex import ( - "fmt" + "errors" "testing" "github.com/stretchr/testify/require" @@ -17,7 +17,7 @@ func TestError(t *testing.T) { require.Contains(t, err.Error(), "a") require.Contains(t, err.Error(), "b") - err = fmt.Errorf("c") + err = errors.New("c") err = Wrapf(err, "d") err = Wrapf(err, "e") err = Wrap(Wrap(Wrap(err))) // make no sense From febde181d0616e64d9225f796b86210fb05078ca Mon Sep 17 00:00:00 2001 From: Yi Yang Date: Sun, 28 Sep 2025 14:49:11 +0800 Subject: [PATCH 4/5] more clear --- tool/ex/error.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tool/ex/error.go b/tool/ex/error.go index ba9e508..06e6504 100644 --- a/tool/ex/error.go +++ b/tool/ex/error.go @@ -12,7 +12,7 @@ import ( ) // ----------------------------------------------------------------------------- -// Extended Stackful Error Handling +// Extended Error Handling with Stack Traces // // These APIs provide an error handling framework that allows errors to carry // stack trace information. @@ -21,24 +21,34 @@ import ( // This allows the error to be propagated up the call stack by simply returning // it (return err), without needing to be re-wrapped at each level. // -// There are two primary ways to create the error, depending on the error's source: +// While the simplest pattern is to return the error directly, you can also use +// Wrap or Wrapf at any point in the call chain to add more contextual information. +// This is useful when an intermediate function has valuable context that is not +// available at the error's origin. +// +// Create and wrap an error: // // 1. To wrap an existing error from a standard or third-party library, use Wrap // or Wrapf. This attaches a stack trace to the original error. // // Example: // if err := some_lib.DoSomething(); err != nil { -// return ex.Wrap(err, "additional context for the error") +// return ex.Wrapf(err, "additional context for the error") +// } +// if err := some_lib.DoSomething(); err != nil { +// return ex.Wrap(err) // } // // 2. To create a new error from scratch, use Newf. This generates a new // error with a stack trace at the point of creation. // // Example: -// if id <= 0 { -// return ex.Newf("failed to do something") +// if unexpected { +// return ex.Newf("unexpected error") // } // +// Terminate the program: +// // Use Fatalf or Fatal to exit the program with an stackful error. It will print // the error message and stack trace to the standard error output. From ea23268e2b8e4ff0f31976846fea8f6cca37a61e Mon Sep 17 00:00:00 2001 From: Yi Yang Date: Tue, 30 Sep 2025 10:14:18 +0800 Subject: [PATCH 5/5] fix main branch linter error --- go.mod | 4 ++-- go.sum | 12 ++++++------ tool/internal/instrument/api.tmpl | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index bbb9b7a..87a2f5b 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/dave/dst v0.27.3 github.com/stretchr/testify v1.11.1 github.com/urfave/cli/v3 v3.4.1 - golang.org/x/mod v0.28.0 + golang.org/x/mod v0.25.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -17,5 +17,5 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect golang.org/x/sys v0.13.0 // indirect - golang.org/x/tools v0.1.12 // indirect + golang.org/x/tools v0.13.0 // indirect ) diff --git a/go.sum b/go.sum index 890d3dd..60d4fb0 100644 --- a/go.sum +++ b/go.sum @@ -8,18 +8,18 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ= github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= -github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= -github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/urfave/cli/v3 v3.4.1 h1:1M9UOCy5bLmGnuu1yn3t3CB4rG79Rtoxuv1sPhnm6qM= github.com/urfave/cli/v3 v3.4.1/go.mod h1:FJSKtM/9AiiTOJL4fJ6TbMUkxBXn7GO9guZqoZtpYpo= -golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s= -golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= +golang.org/x/mod v0.25.0 h1:n7a+ZbQKQA/Ysbyb0/6IbB1H/X41mKgbhfv7AfG/44w= +golang.org/x/mod v0.25.0/go.mod h1:IXM97Txy2VM4PJ3gI61r1YEk/gAj6zAHN3AdZt6S9Ww= +golang.org/x/sync v0.3.0 h1:ftCYgMx6zT/asHUrPw8BLLscYtGznsLAnjq5RH9P66E= +golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/tools v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU= -golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= +golang.org/x/tools v0.13.0 h1:Iey4qkscZuv0VvIt8E0neZjtPVQFSc870HQ448QgEmQ= +golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/tool/internal/instrument/api.tmpl b/tool/internal/instrument/api.tmpl index 7d0ee21..335ca0c 100644 --- a/tool/internal/instrument/api.tmpl +++ b/tool/internal/instrument/api.tmpl @@ -3,7 +3,7 @@ package inst -// !!! This file will auto-sync to tool/internal/instrument/api.tmpl +// !!! pkg/inst/context.go will auto-sync to tool/internal/instrument/api.tmpl type HookContext interface { // Set the skip call flag, can be used to skip the original function call SetSkipCall(bool)