Custom SDK implementation about opentelemetry-go-instrumentation HOT 10 OPEN

@MrAlias I did a POC for the second option we talked about. It is in main...RonFed:opentelemetry-go-instrumentation_fork:flag_POC

Here are some logs from the app being instrumented and the agent:

rolldice-1 | 2024-08-09T15:42:14.291Z INFO app/main.go:64 starting http server {"port": ":8080"}
rolldice-1 | 2024-08-09T15:42:14.291Z INFO app/main.go:52 probed function called with flag unset
go-auto-1 | {"level":"info","ts":1723218134.4202611,"logger":"go.opentelemetry.io/auto","caller":"cli/main.go:86","msg":"building OpenTelemetry Go instrumentation ...","globalImpl":false}
rolldice-1 | 2024-08-09T15:42:15.295Z INFO app/main.go:52 probed function called with flag unset
rolldice-1 | 2024-08-09T15:42:16.297Z INFO app/main.go:52 probed function called with flag unset
go-auto-1 | {"level":"info","ts":1723218136.4233193,"logger":"Instrumentation.Analyzer","caller":"process/discover.go:67","msg":"found process","pid":12030}
go-auto-1 | {"level":"info","ts":1723218136.4287593,"logger":"Instrumentation.Allocate","caller":"process/allocate.go:73","msg":"Detaching from process","pid":12030}
go-auto-1 | {"level":"info","ts":1723218136.4289303,"logger":"Instrumentation","caller":"app/instrumentation.go:144","msg":"target process analysis completed","pid":12030,"go_version":"1.22.5","dependencies":{"go.uber.org/multierr":"1.10.0","go.uber.org/zap":"1.27.0","std":"1.22.5"},"total_functions_found":3}
go-auto-1 | {"level":"info","ts":1723218136.428993,"logger":"go.opentelemetry.io/auto","caller":"cli/main.go:119","msg":"starting instrumentation..."}
go-auto-1 | {"level":"info","ts":1723218136.4319375,"logger":"Instrumentation.Manager","caller":"instrumentation/manager.go:198","msg":"loading probe","name":"net/http/server"}
go-auto-1 | {"level":"info","ts":1723218136.4832294,"logger":"Instrumentation.Manager","caller":"instrumentation/manager.go:198","msg":"loading probe","name":"main/client"}
go-auto-1 | {"level":"info","ts":1723218136.4842162,"logger":"go.opentelemetry.io/auto","caller":"cli/main.go:115","msg":"instrumentation loaded successfully"}
rolldice-1 | 2024-08-09T15:42:17.299Z INFO app/main.go:50 probed function called with flag set
rolldice-1 | 2024-08-09T15:42:18.300Z INFO app/main.go:50 probed function called with flag set
rolldice-1 | 2024-08-09T15:42:19.301Z INFO app/main.go:50 probed function called with flag set
rolldice-1 | 2024-08-09T15:42:20.302Z INFO app/main.go:50 probed function called with flag set
rolldice-1 | 2024-08-09T15:42:21.303Z INFO app/main.go:50 probed function called with flag set
rolldice-1 | 2024-08-09T15:42:22.304Z INFO app/main.go:50 probed function called with flag set
rolldice-1 | 2024-08-09T15:42:23.305Z INFO app/main.go:50 probed function called with flag set
rolldice-1 | 2024-08-09T15:42:24.307Z INFO app/main.go:50 probed function called with flag set
rolldice-1 | 2024-08-09T15:42:25.308Z INFO app/main.go:50 probed function called with flag set
rolldice-1 | 2024-08-09T15:42:26.309Z INFO app/main.go:50 probed function called with flag set

from opentelemetry-go-instrumentation.

@MrAlias What do you think should be included in the prototype?

from opentelemetry-go-instrumentation.

Ideally, something like this:

package main

import (
	"fmt"
	"time"
)

var Flag bool

func main() {
	for {
		fmt.Printf("Flag: %t", Flag)
		time.Sleep(time.Second)
	}
	// Output: Flag: false
	// Auto-instrumentation added
	// Output: Flag: true
}

from opentelemetry-go-instrumentation.

We may also need to look into having the global package call something like:

func (t *tracer) start(eBPFFlag *bool, ctx context.Context, name string, opts ...trace.SpanStartOption) (context.Context, trace.Span) {
/* ... */
}

and instrument that instead of Start so we can find the flag location.

from opentelemetry-go-instrumentation.

Awesome! Let me see about getting an issue created in https://github.com/open-telemetry/opentelemetry-go to make the proposal based on these findings.

from opentelemetry-go-instrumentation.

opentelemetry-go proposal: open-telemetry/opentelemetry-go#5702

@RonFed PTAL

from opentelemetry-go-instrumentation.

@MrAlias LGTM,
In the suggested change, do you think we should probe the tracer.newSpan (which has the flag pointer) or the interop.newSpan (which has the configuration ready)?
Ideally, we should build it in a way that we can handle both in a single eBPF program if it is possible.

from opentelemetry-go-instrumentation.

@MrAlias LGTM, In the suggested change, do you think we should probe the tracer.newSpan (which has the flag pointer) or the interop.newSpan (which has the configuration ready)? Ideally, we should build it in a way that we can handle both in a single eBPF program if it is possible.

I was thinking tracer.newSpan as that would allow us to set the bool and trace in one probe. Right?

from opentelemetry-go-instrumentation.

I was thinking tracer.newSpan as that would allow us to set the bool and trace in one probe. Right?

Yea, that sounds good, we'll need to have a return probe for that function as well simillar to what we have today - in order to save the returned span pointer in a map.

from opentelemetry-go-instrumentation.

I was thinking tracer.newSpan as that would allow us to set the bool and trace in one probe. Right?

Yea, that sounds good, we'll need to have a return probe for that function as well simillar to what we have today - in order to save the returned span pointer in a map.

Correct.

from opentelemetry-go-instrumentation.