open-telemetry / opentelemetry-go-instrumentation

OpenTelemetry Auto Instrumentation using eBPF
https://opentelemetry.io
Apache License 2.0
493 stars 77 forks source link

Don't track outgoing span contexts with the context map #873

Closed RonFed closed 2 months ago

RonFed commented 3 months ago

The start_tracking_span and stop_tracking_span functions are used for in-process context propagation, hence there is no need to call them on outgoing spans.

RonFed commented 3 months ago

This is still in draft because I'm not sure about the gRPC client probe which is outgoing but makes use of the start_tracking_span in a confusing way. It is needed there to correlate between the different probes using the context.

MrAlias commented 3 months ago

This is still in draft because I'm not sure about the gRPC client probe which is outgoing but makes use of the start_tracking_span in a confusing way. It is needed there to correlate between the different probes using the context.

Does this mean we are lacking testing for a particular case?

RonFed commented 3 months ago

Does this mean we are lacking testing for a particular case?

No, I meant that the use of start_tracking_span and stop_tracking_span looks redundant in the gRPC client probe since it deals with an outgoing span, and these functions are meant for in-process context propagation. However, it is required in the gRPC case because of the way the instrumentation is written: the http2Client_NewStream probe needs to be correlated with the ClientConn_Invoke probe, and this is done with contexts. I added a comment about this, and about the stop_tracking_span function being a no-op in case the passed span is not tracked.