Enable auto/sdk in otel global #1405
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
MrAlias
force-pushed
the
otel-global-autosdk
branch
2 times, most recently
from
bc3fd1e to
2cc75b0
Compare
MrAlias
force-pushed
the
otel-global-autosdk
branch
from
0fa0bd8 to
555a184
Compare
This comment was marked as outdated.
This comment was marked as outdated.
MrAlias
force-pushed
the
otel-global-autosdk
branch
from
555a184 to
dab479a
Compare
internal/pkg/instrumentation/bpf/go.opentelemetry.io/otel/traceglobal/bpf/probe.bpf.c
Show resolved
Hide resolved
MrAlias
force-pushed
the
otel-global-autosdk
branch
5 times, most recently
from
d5356ef to
89c0867
Compare
MrAlias
changed the title
MrAlias
force-pushed
the
otel-global-autosdk
branch
4 times, most recently
from
27dcc0f to
b25ff44
Compare
Set the auto/sdk flag in the global API to true when newSpan is called.
Verify the auto/sdk probe is being used.
MrAlias
force-pushed
the
otel-global-autosdk
branch
from
b25ff44 to
30e7015
Compare
internal/pkg/instrumentation/bpf/go.opentelemetry.io/otel/traceglobal/probe.go
Outdated
Show resolved
Hide resolved
| @@ -365,50 +371,76 @@ type Uprobe struct { | |||
| // function specified by Sym. If ReturnProbe is empty, no eBPF program will be attached to the return of the function. | |||
| ReturnProbe string | |||
| DependsOn []string | |||
|
|
|||
| closers atomic.Pointer[[]io.Closer] | |||
There was a problem hiding this comment.
Why not []io.Closer?
What are the concurrent scenarios we are trying to protect against?
There was a problem hiding this comment.
The Uprobe.Close is called by the Manager which can shutdown asynchronously. For example, this can occur when the context passed to the new Instrumentation is canceled asynchronously1. This is to prevent that data-race.
Footnotes
There was a problem hiding this comment.
I see. Do you think we can use sync.Once instead? I think it might be more readable
internal/pkg/instrumentation/bpf/go.opentelemetry.io/otel/traceglobal/probe.go
Outdated
Show resolved
Hide resolved
internal/pkg/instrumentation/bpf/go.opentelemetry.io/otel/traceglobal/probe.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Should we also add package constraints on the auto sdk probes?
| reader.Reset(record.RawSample) | ||
| err = binary.Read(reader, binary.LittleEndian, e) | ||
| case recordKindConrol: | ||
| c.uprobeNewStartMu.Lock() |
There was a problem hiding this comment.
The decodeEvent function of a probe instance can't be called from multiple goroutines , right? If so, maybe should remove the mutex
| @@ -365,50 +371,76 @@ type Uprobe struct { | |||
| // function specified by Sym. If ReturnProbe is empty, no eBPF program will be attached to the return of the function. | |||
| ReturnProbe string | |||
| DependsOn []string | |||
|
|
|||
| closers atomic.Pointer[[]io.Closer] | |||
There was a problem hiding this comment.
I see. Do you think we can use sync.Once instead? I think it might be more readable
Resolve #1399
This includes a
probe.Uproberefactor. Instead of passing backlinkswhen the uprobe loads, the uprobe itself now hold these values. When itsClosemethod is called it will, itself, call each of these close methods.This redesign is used so that the
newSpanuprobe can be closed prior to the others (which are closed by theManager).