datadog / orchestrion Goto Github PK

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

But present on the main branch last commit: 5130436

Describe what happened:

Describe what you expected:

I expected both commands to work but the one with -C to displace the call to all go commands was not properly intepreted by orchestrion while the second one works fine.

Steps to reproduce the issue:

git clone https://github.com/DataDog/orchestrion
go -C orchestrion build -o="../orchestrion.exe" .
./orchestrion.exe go -C orchestrion/_integration-tests mod tidy

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

go env:

❯ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/eliott/.cache/go-build'
GOENV='/home/eliott/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/eliott/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/eliott/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.5'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/eliott/git/github.com/DataDog/orchestrion/_integration-tests/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1348386482=/tmp/go-build -gno-record-gcc-switches'

Version of orchestrion
main

Describe what happened:

I built the following program with Orchestrion:

package main

import "context"

type event struct {}

//dd:span
func DoThing(ctx context.Context, e event) {}

func main() {
	DoThing(context.Background(), event{})
}

I saw the following error:

# testapp
./main.go:5: event already declared through import of package event ("github.com/datadog/orchestrion/instrument/event")
	<generated>:2[/var/folders/f3/g91d13pd6kd3vdxts_gsgd1r0000gn/T/go-build1208255770/b001/orchestrion/src/main/main.go:8:2]: other declaration of event
./main.go:8: use of package event not in selector
./main.go:11: use of package event not in selector

Describe what you expected:

The program to build successfully, with span instrumentation added to DoThing.

There is name mangling to handle conflicting imports (I haven't found where, but I know it happens :) looks like this package does it?). But we can still have clashes between package names and package-scope type/variable/const names. I noticed this when working on #173, where adding event imports was clashing with event types in other packages. See this failure, for example.

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)

Version of orchestrion: v0.7.0-dev.1

For example a method with the prototype as follows:

func test(x string, y ...string)

and a call done like this:

y := []string{"y1", "y2"}
test("x", y...)

will be instrumented like this:

y := []string{"y1", "y2"}
test("x", y, interceptor...)

Which fails the compilation. Something like this should work:

y := []string{"y1", "y2"}
test("x", append(interceptor, y...)...)

@tonyredondo has been toying with a fork/copy of the -toolexec machinery and ran it with go test -toolexec '<blah>' ./..., which resulted in problems due to some properties of PackageInjector:

• It only edits importcfg files for the b001 stage, whereas it should do it for any stage in which an import is being added;
• It only edits importcfg.link files in the b001 stage, but go test actually has multiple stages producing binaries that need their linker config edited.

I've been able to get this to work via the following changes: https://github.com/tonyredondo/rd-toolexec/pull/1/files... However while it works on my machine with go1.22, it fails on a checksum mismatch at link time on @tonyredondo's with go1.21... This is likely because builds of the injected package aren't correctly de-duplicated; which I have not investigated at this point.

Some constructor instrumentation functions return different types than the original calls, resulting in compilation errors when the result of the constructor function is assigned to an explicitly typed store, or if the implicitly typed variable is shared with another assignment of a value from the original type.

This is for example the case with gorilla/mux where mux.NewRouter is *mux.Router, but muxtrace.NewRouter returns *muxtrace.Router.

In order to fix this, the best strategy would be to migrate from call-site instrumentation to callee instrumentation, effectively modifying the original type & constructor as appropriate.

Version of orchestrion

Running main version

Describe what happened:

The compilation of the folder _integration_tests/tests fails when running with some debugger flags.

Command: go test -toolexec "orchestrion toolexec" -tags integration -gcflags all='' ./_integration_tests/tests

Output:

# orchestrion/integration/tests [orchestrion/integration/tests.test]
./suite_test.go:22: undefined: testing
./suite_test.go:23: undefined: require
./suite_test.go:24: undefined: require
./suite_test.go:26: undefined: agent
./suite_test.go:27: undefined: require
./suite_test.go:32: undefined: testing
./suite_test.go:42: undefined: require
./suite_test.go:45: undefined: require
./suite_test.go:47: undefined: trace
./suite_test.go:48: undefined: require
./suite_test.go:48: too many errors
exit status 2
FAIL	orchestrion/integration/tests [build failed]
FAIL

Running with -work created the following file, without the right imports:

// Unless explicitly stated otherwise all files in this repository are licensed
// under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2023-present Datadog, Inc.

//go:build integration

package tests

//dd:orchestrion-enabled
const orchestrionEnabled =
//line <generated>:1
true

//line ./suite_test.go:22
func Test(t *testing.T) {
	require.True(t, orchestrionEnabled, "this test suite must be run with orchestrion enabled")
	require.NotEmpty(t, suite, "no test case registered")

	mockAgent, err := agent.New(t)
	require.NoError(t, err)
	defer mockAgent.Close()

	for name, tc := range suite {
		tc := tc
		t.Run(name, func(t *testing.T) {
			t.Log("Running setup")
			tc.Setup(t)

			defer func() {
				t.Log("Running teardown")
				tc.Teardown(t)
			}()

			sess, err := mockAgent.NewSession(t)
			require.NoError(t, err)
			defer func() {
				jsonTraces, err := sess.Close(t)
				require.NoError(t, err)

				var traces trace.Spans
				require.NoError(t, trace.ParseRaw(jsonTraces, &traces))
				t.Logf("Received %d traces", len(traces))

				for _, expected := range tc.ExpectedTraces() {
					expected.RequireAnyMatch(t, traces)
				}
			}()

			t.Log("Running test")
			tc.Run(t)
		})
	}
}

Describe what you expected:

I would expect the tests to compile as they do without the -gcflags

Steps to reproduce the issue:

go test -toolexec "orchestrion toolexec" -tags integration -gcflags all='' ./_integration_tests/tests

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

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"
)

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

Version of orchestrion
0.7.x

Describe what happened:

The implementation of the ServerHTTP method does not get instrumented, because its aspect is gated by httpmode=report, which cannot be enabled externally.

// You can edit this code!
// Click here and start typing.
package main

import "net/http"

type Handler struct{}

func (Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {}

func main() {
	// NB: http.Handle(...), using the default ServeMux, doesn't get instrumented either
	mu := http.NewServeMux()
	mu.Handle("/foo", Handler{})
	http.ListenAndServe(":8080", mu)
}

Version of orchestrion
0.7.0-dev.1

Describe what happened:
Running orchestrion version shows the version installed, not the version from go.mod as expected.

I was trying the new quick start mode with go install and it ends up in the following error:

~/d/appsec-go-test-app (main|✔) $ ./orchestrion go build -v .
github.com/gorilla/mux
# github.com/gorilla/mux
2024/04/24 22:34:07 resolving woven dependency on github.com/datadog/orchestrion/instrument: running ["go" "list" "-toolexec" "\"/Users/julio.guerra/dd/appsec-go-test-app/orchestrion\" \"toolexec\"" "-json" "-deps" "-export" "--" "github.com/datadog/orchestrion/instrument"]: exit status 1`

You can reproduce it on the appsec-go-test-app with:

$ GOBIN=$PWD go install github.com/datadog/orchestrion@main
$ ./orchestrion go build -v . 

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)

Version of orchestrion: v0.7.0-dev.1

Describe what happened:

Using panic compilation with all this logs:

❯ ORCHESTRION_LOG_FILE=orchestrion.log ORCHESTRION_LOG_LEVEL=TRACE orchestrion go build .
# runtime/internal/syscall
panic: unable to open log file "orchestrion.log": open orchestrion.log: permission denied

goroutine 1 [running]:
main.init.0()
        ../../../../go/pkg/mod/github.com/datadog/[email protected]/logger.go:36 +0x285
# sync/atomic
panic: unable to open log file "orchestrion.log": open orchestrion.log: permission denied

goroutine 1 [running]:
main.init.0()
        ../../../../go/pkg/mod/github.com/datadog/[email protected]/logger.go:36 +0x285
# internal/cpu
panic: unable to open log file "orchestrion.log": open orchestrion.log: permission denied

goroutine 1 [running]:
main.init.0()
        ../../../../go/pkg/mod/github.com/datadog/[email protected]/logger.go:36 +0x285
# crypto/subtle
panic: unable to open log file "orchestrion.log": open orchestrion.log: permission denied

goroutine 1 [running]:
main.init.0()
        ../../../../go/pkg/mod/github.com/datadog/[email protected]/logger.go:36 +0x285
# crypto/internal/boring/sig
panic: unable to open log file "orchestrion.log": open orchestrion.log: permission denied

goroutine 1 [running]:
main.init.0()
        ../../../../go/pkg/mod/github.com/datadog/[email protected]/logger.go:36 +0x285
# runtime/internal/atomic
panic: unable to open log file "orchestrion.log": open orchestrion.log: permission denied

goroutine 1 [running]:
main.init.0()
        ../../../../go/pkg/mod/github.com/datadog/[email protected]/logger.go:36 +0x285
# internal/abi
panic: unable to open log file "orchestrion.log": open orchestrion.log: permission denied

goroutine 1 [running]:
main.init.0()
        ../../../../go/pkg/mod/github.com/datadog/[email protected]/logger.go:36 +0x285
# internal/chacha8rand
panic: unable to open log file "orchestrion.log": open orchestrion.log: permission denied

goroutine 1 [running]:
main.init.0()
        ../../../../go/pkg/mod/github.com/datadog/[email protected]/logger.go:36 +0x285
# github.com/klauspost/compress/internal/cpuinfo
panic: unable to open log file "orchestrion.log": open orchestrion.log: permission denied

goroutine 1 [running]:
main.init.0()
        ../../../../go/pkg/mod/github.com/datadog/[email protected]/logger.go:36 +0x285
# maps
panic: unable to open log file "orchestrion.log": open orchestrion.log: permission denied

goroutine 1 [running]:
main.init.0()
        ../../../../go/pkg/mod/github.com/datadog/[email protected]/logger.go:36 +0x285
exit status 1

Describe what you expected:

A log file full with logs

Steps to reproduce the issue:

Command ran: ORCHESTRION_LOG_FILE=orchestrion.log ORCHESTRION_LOG_LEVEL=TRACE orchestrion go build .

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

Version of orchestrion

v0.7.x

Describe what happened:

When building with -cover / -covermode=atomic (possibly also with other cover modes), the link stage fails with a fingerprint mismatch.

This is because the build ID addendum does not get properly tainted with cache flags. But once this is fixed, the compilation fails on coverage-instrumented files because all imports appear to be removed from them.