datadog / orchestrion Goto Github PK

When a file uses two packages that have different published names but the same package path name, the decorator errors out. An example error is Error: Failed to scan: error scanning file model/customer.go: error decorating file model/customer.go: goast.DecoratorResolver found multiple packages using name v3: github.com/chargebee/chargebee-go/v3 and github.com/gocardless/gocardless-pro-go/v3

Version of orchestrion
v0.4.1

Describe what happened:
Running orchestrion errored out when it found two packages with the same package path name.

Describe what you expected:
Orchestrion should ideally be smart enough to know what the underlying package name is. It is very common for third-party clients to version their packages with /client/v1, etc.. so these sorts of clashes are going to happen a bit without orchestrion knowing the published package name.

Steps to reproduce the issue:
create a file that uses the two packages in the workaround below and attempt to run orchestrion

Workaround
If you give the package an alias name on import (even the one that's actually being used under the hood) it fixes the problem for orchestrion. For example,

import (
	chargebee "github.com/chargebee/chargebee-go/v3"
	gocardless "github.com/gocardless/gocardless-pro-go/v3"
)

Request to add auto tracing support for Fiber for automatic tracing.

Describe what happened:

got an error like below

Failed to scan: error scanning file <file.go>: error decorating file <file.go>: unsupported dot-import found for github.com/foo

Describe what you expected:

expected orchestrion to handle inserts, etc, however it failed on imports with dots. i had to rename the dot to bypass it, and then revert the rename. it would be good if dot would not break orchestrion

Steps to reproduce the issue:

import . github.com/foo

Version of orchestrion

v0.4.1

Describe what happened:

Orchestrion fails to remove instrumentation from grpc.Dial if it's contained within an if block and assigns the return value to something (even underscores). If you remove the if condition before adding instrumentation, orchestrion can successfully remove the instrumentation.

I first noticed this in a more complex file and was able to wittle it down to file below.

As a side note, if you remove the assignment it doesn't get instrumented in the first place.

Describe what you expected:

Instrumentation should be completely removed with the syntax in question.

Steps to reproduce the issue:

With the following in main.go:

package main

import "google.golang.org/grpc"

func main() {
	if true {
		_, _ = grpc.Dial("")
	}
}

Execute orchestrion -w . && orchestrion -w -rm . and you'll be left with:

package main

import (
	"github.com/datadog/orchestrion/instrument"
	"google.golang.org/grpc"
)

func main() {
	if true {
		//dd:startwrap
		_, _ = grpc.Dial("", instrument.GRPCStreamClientInterceptor(), instrument.GRPCUnaryClientInterceptor())
		//dd:endwrap
	}
}

Additional environment details (Version of Go, Operating System, etc.):

go version go1.21.3 darwin/arm64 (macOS Sonoma 14.0, M1 Max)

Our team utilizes the Fiber framework (https://github.com/gofiber/fiber) for developing web apps. It would be awesome if Orchestrion could support it.

Version of orchestrion

v0.4.1

Describe what happened:
I would like to instrument a project that I regularly use. But I've a panic.

panic: ast.Walk: unexpected node type <nil>

goroutine 1 [running]:
github.com/dave/dst.Walk({0x727d38?, 0xc0005a7800?}, {0x0?, 0x0?})
        /home/bagl3y/go/pkg/mod/github.com/dave/[email protected]/walk.go:332 +0x1894
github.com/dave/dst.walkStmtList(...)
        /home/bagl3y/go/pkg/mod/github.com/dave/[email protected]/walk.go:32
github.com/dave/dst.Walk({0x727d38?, 0xc0005a7800?}, {0x727ed8?, 0xc00017be00?})
        /home/bagl3y/go/pkg/mod/github.com/dave/[email protected]/walk.go:216 +0x1ca5
github.com/dave/dst.Walk({0x727d38?, 0xc0005a7800?}, {0x728398?, 0xc0001103c0?})
        /home/bagl3y/go/pkg/mod/github.com/dave/[email protected]/walk.go:315 +0x1725
github.com/dave/dst.walkDeclList(...)
        /home/bagl3y/go/pkg/mod/github.com/dave/[email protected]/walk.go:38
github.com/dave/dst.Walk({0x727d38?, 0xc0005a7800?}, {0x7283b8?, 0xc0000f0410?})
        /home/bagl3y/go/pkg/mod/github.com/dave/[email protected]/walk.go:321 +0x18e5
github.com/dave/dst.Inspect(...)
        /home/bagl3y/go/pkg/mod/github.com/dave/[email protected]/walk.go:353
github.com/dave/dst/decorator.(*FileRestorer).updateImports(0xc00045c2d0)
        /home/bagl3y/go/pkg/mod/github.com/dave/[email protected]/decorator/restorer.go:190 +0x176
github.com/dave/dst/decorator.(*FileRestorer).RestoreFile(0xc00045c2d0, 0x0?)
        /home/bagl3y/go/pkg/mod/github.com/dave/[email protected]/decorator/restorer.go:135 +0x23e
github.com/dave/dst/decorator.(*Restorer).RestoreFile(...)
        /home/bagl3y/go/pkg/mod/github.com/dave/[email protected]/decorator/restorer.go:65
github.com/dave/dst/decorator.(*Restorer).Fprint(0xc0005a8480, {0x727c58, 0xc000003890}, 0x6c18c0?)
        /home/bagl3y/go/pkg/mod/github.com/dave/[email protected]/decorator/restorer.go:56 +0x93
github.com/datadog/orchestrion/internal/instrument.InstrumentFile({0xc000164828, 0x16}, {0x727ab8?, 0xc000324788}, {{0x6c8f9b?, 0xc000164828?}})
        /home/bagl3y/go/pkg/mod/github.com/datadog/[email protected]/internal/instrument/instrument.go:103 +0x777
github.com/datadog/orchestrion/internal/instrument.ProcessPackage.func1({0xc000164828, 0x16}, {0x72af90?, 0xc0001b3440?}, {0x0?, 0x0?})
        /home/bagl3y/go/pkg/mod/github.com/datadog/[email protected]/internal/instrument/instrument.go:48 +0x20c
io/fs.walkDir({0x727bb8, 0xc0000421a0}, {0xc000164828, 0x16}, {0x72af90, 0xc0001b3440}, 0xc0004fbdc8)
        /home/linuxbrew/.linuxbrew/Cellar/go/1.21.3/libexec/src/io/fs/walk.go:73 +0x6c
io/fs.walkDir({0x727bb8, 0xc0000421a0}, {0xc000128d10, 0xf}, {0x72af90, 0xc0001b3180}, 0xc0004fbdc8)
        /home/linuxbrew/.linuxbrew/Cellar/go/1.21.3/libexec/src/io/fs/walk.go:95 +0x2c5
io/fs.walkDir({0x727bb8, 0xc0000421a0}, {0xc000128ca0, 0xb}, {0x72af90, 0xc0001bd0c0}, 0xc0004fbdc8)
        /home/linuxbrew/.linuxbrew/Cellar/go/1.21.3/libexec/src/io/fs/walk.go:95 +0x2c5
io/fs.walkDir({0x727bb8, 0xc0000421a0}, {0xc00026fb10, 0x3}, {0x72af90, 0xc0000c66c0}, 0xc0004fbdc8)
        /home/linuxbrew/.linuxbrew/Cellar/go/1.21.3/libexec/src/io/fs/walk.go:95 +0x2c5
io/fs.walkDir({0x727bb8, 0xc0000421a0}, {0x726af4, 0x1}, {0x72afc8, 0xc0000421b0}, 0xc0004fbdc8)
        /home/linuxbrew/.linuxbrew/Cellar/go/1.21.3/libexec/src/io/fs/walk.go:95 +0x2c5
io/fs.WalkDir({0x727bb8, 0xc0000421a0}, {0x726af4, 0x1}, 0xc000185dc8)
        /home/linuxbrew/.linuxbrew/Cellar/go/1.21.3/libexec/src/io/fs/walk.go:122 +0xd2
github.com/datadog/orchestrion/internal/instrument.ProcessPackage({0xc0000305c0, 0x32}, 0x6dd7d8, 0x6dd7f8, {{0x6c8f9b?, 0x1?}})
        /home/bagl3y/go/pkg/mod/github.com/datadog/[email protected]/internal/instrument/instrument.go:36 +0xd6
main.main()
        /home/bagl3y/go/pkg/mod/github.com/datadog/[email protected]/main.go:77 +0x4e9

Describe what you expected:
Instrument my project

Steps to reproduce the issue:
Clone https://github.com/gotenberg/gotenberg and try to instrument it (orchestrion -w ./)

Additional environment details (Version of Go, Operating System, etc.):

Golang 1.21.3 linux/amd64 (Same issue in a Docker Build)

In search of being able to set the service name on the tracer of my database/sql client, it came to my attention that there is no current way to set the options through orchestrion. After digging a little deeper into the internals, I noticed the underlying tracer function (coming from gopkg.in/DataDog/dd-trace-go.v1/contrib/database/sql) is variadic, with the ability to pass 0 or many options, so it is pretty straightforward to add, if only there is a way to tell orchestrion what to add as options.

Proposal

I would like to propose adding another "magic comment" in orchestrion called //dd:options that can be used directly above an instantiation of one of the supported auto-wire libs. It would be used like so,

package main

import "database/sql"

func main() {
	//dd:options service:test-name tag:my_key:my_val
	db, err := sql.Open("driver", "connectionString")
}

Doing so would generate the following code,

package main

import "github.com/datadog/orchestrion/instrument"

func register() {
	//dd:options service:test-name tag:my_key:my_val
	//dd:startwrap
	db, err := instrument.Open("driver", "connString", instrument.SqlWithServiceName("test-name"), instrument.SqlWithCustomTag("my_key", "my_value"))
	//dd:endwrap
}

This proposal requires the instrument.Open function that currently accepts 2 params and is non-variadic to accept 2+ params with the last param being variadic and accept 0 or many options. This function is for database/sql, but the same general need applies to the other supported libs.

Some other questions that should be discussed and answered,

• one vs many entries on the comment line: Some options support passing many (such as custom tags... I think anyways), while others should only be passed once (like service name). Should orchestrion be smart enough to know which ones support one vs. many and throw an error if invalid?
• invalid options: if an invalid/unsupported option is passed in the magic comment, should orchestrion throw an error, inform the user, or do nothing?

I got a bit ahead of myself and put together a PR, but can change as necessary if the broader proposal is accepted.

The PR: #60