nikolaydubina / go-instrument Goto Github PK

Please leave your feedback, vote, comments, anything if you want to keep it or switch to something else.

I am ok to switch!

Lets collect feedback here!

//go:instrument

• ????

something-else

• sean from golang-nuts

Hi,

I'm wondering, what's the rationale behind returning an error, in case the file is generated?
https://github.com/nikolaydubina/go-instrument/blob/master/main.go#L60

Shouldn't a log and return nil be a better choice here?

Hi,

First of all thanks for your awesome work ❤️

I'm trying to use your project on our code base, but I'm getting some errors.
The following perfectly valid code fails to instrument:

package foo

import "context"

type SomeCommand struct{}

func (cmd SomeCommand) Validate() error {
	return nil
}

func SomeFunction(ctx context.Context, cmd SomeCommand) error {
	if err := cmd.Validate(); err != nil {
		return err
	}

	if err := Exec(ctx, func(ctx context.Context) error {
		return nil
	}); err != nil {
		return err
	}
	return nil
}

func Exec(ctx context.Context, fn func(ctx context.Context) error) error {
	return fn(ctx)
}

The error is rather cryptic missing ',' before newline in argument list (and 10 more errors).
My guess is that it doesn't like the invocation of Exec function.

If a file begins with

//go:build ignore
// +build ignore

it should be ignored by this tool. Otherwise these comments get stripped, and obviously that causes issues as now a file which should be ignored during build is not.

We end up with an error like

#17 0.634 somepackage/somefile.go:7:2: found packages somepackage (fileA.go) and main (fileB.go) in /app

I ran this tool against some of my code bases using the command provided in the README:

find . -name "*.go" | xargs -I{} go-instrument -app my-service -w -filename {}

It seems to change the indentation of various files, even though they are already properly formatted using go fmt. A git diff of one of those files:

index 6da35ba..8c5f832 100644
--- a/defaults.go
+++ b/defaults.go
@@ -10,58 +10,58 @@ import (
 )

 const (
-    logFormatJSON = "json"
+    logFormatJSON   = "json"

-    logTargetStdout = "stdout"
-    logTargetStderr = "stderr"
+    logTargetStdout = "stdout"
+    logTargetStderr = "stderr"

-    logLevelPanic = "panic"
-    logLevelFatal = "fatal"
-    logLevelError = "error"
-    logLevelWarn  = "warn"
-    logLevelInfo  = "info"
-    logLevelDebug = "debug"
-    logLevelTrace = "trace"
+    logLevelPanic   = "panic"
+    logLevelFatal   = "fatal"
+    logLevelError   = "error"
+    logLevelWarn    = "warn"
+    logLevelInfo    = "info"
+    logLevelDebug   = "debug"
+    logLevelTrace   = "trace"
 )

// ...

I ran this tool against some of my code bases using the command provided in the README:

find . -name "*.go" | xargs -I{} go-instrument -app my-service -w -filename {}

It seems to remove all comments. A git diff of one of those files:

diff --git a/pprof.go b/pprof.go
index 352adf6..975e75f 100644
--- a/pprof.go
+++ b/pprof.go
// ...

     go func() {
         <-ctx.Done()
-        defer cpuf.Close() //nolint:errcheck,gosec // error ignored on purpose
-        defer memf.Close() //nolint:errcheck,gosec // error ignored on purpose
+        defer cpuf.Close()
+        defer memf.Close()

         pprof.StopCPUProfile()
         if err := pprof.WriteHeapProfile(memf); err != nil {

suggested:

issues:

• how to combine?
• case 1: exclude all, but select some
• case 2: include all, but select some