CLIENT spans should update their parent span's kind to INTERNAL

[owais]

Today all instrumentations that trace outgoing network calls generate spans with kind set to CLIENT. Most higher level libraries internally use lower level code to make such calls. If a user uses instrumentations for both of the libraries, they'll end up with two (or more) parent - child spans both with kind set to CLIENT. For example, requests may internally use urllib or a database client such as etcd may internally use requests, etc.

To remedy such cases, I propose all instrumentations that generate such client spans should always update their parent span's kind to INTERNAL. This way instrumentations do not need to be aware of each other and do not need to communicate with each other via context or any other mechanism.

Depends on: open-telemetry/opentelemetry-python#1774

[lzchen]

I propose all instrumentations that generate such client spans should always update their parent span's kind to INTERNAL.

Wouldn't this only make sense for the specific use case that was outlined above? (instrumented with multiple libraries that have the same code path).

[owais]

Yes, but as a rule it sounds much simpler to just always update parent to INTERNAL if the generated span is being set to CLIENT. This simple rule takes care of all cases. If we force the code to first check the kind of parent span, both the rule and implementations might get more complicated for no apparent benefit. Not to mention some span implementations may be write only and not even allow ready the any values like kind.

[lzchen]

@owais
But how is this possible? Do we have access to the parent span when we are generating the Client span? Don't we only have the parent's SpanContext?

[owais]

I didn't mean that SDK Span should do it automatically. I was suggesting that every instrumentation does it independently. Each instrumentation can fetch the current local span (it's eventual parent span) with api.get_current_span() before activating the client span it generates.

[lzchen]

@owais
Ahh I see.

This simple rule takes care of all cases.

Is this true? Aren't there valid use cases where the parent span of a (to be) generated CLIENT span shouldn't be INTERNAL>

For example, if a user has a flask application instrumented with flask instrumentation and requests instrumentation. They hit an endpoint whose logic makes an http request call to some other endpoint. A SERVER span is generated for hitting the flask endpoint, and it's child, a CLIENT span is generated in the logic that results from hitting the flask endpoint. In the requests instrumentation, it doesn't make sense to automatically change the parent span (current span) 's kind to INTERNAL right?

As well, span kinds cannot be changed after creation, so in instrumentations, we would actually have to copy the span and create a new one. After this, we would have to replace it in the current context. Not only that, if users copied/used the original copy of the span in their code somewhere via get_current_span, we have no way of updating those either.

Unless if I am misunderstanding something, this seems quite complex for this automatic "magic" logic.

[owais]

You're right. There is the use case where SERVER's spans immediate child is a CLIENT span and such a CLIENT span shouldn't change it's parent's kind.

Unless if I am misunderstanding something, this seems quite complex for this automatic "magic" logic.

yea, that would be complex but if we agree with the solution, we can try adding an update_kind() method to the API just like we have update_name(). I've opened another issue that is linked. One issue that has come up so far is that updating kind may not work well with streaming exporter implementations. If it was part of the API however, may be then we can expect streaming exporters to account for it.

[lzchen]

@owais
I think the missing piece is the logic that is included in: #445 . If this is covered sufficiently, then changing the kind makes sense ONLY if the "get current span" logic is implemented with it.