[Tracing Instrumentation] Add instrumentation at Local Transport Layer

[Gaganjuneja]

Is your feature request related to a problem? Please describe.
Add tracing/instrumentation at LocalTransport layer.

Describe the solution you'd like
A clear and concise description of what you want to happen.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

[reta]

The Local Transport uses context preservation (from the thread pool) and the trace context propagation is working as expected, closing this for now.

[Gaganjuneja]

@reta

While analyzing the span context propagation through ThreadContext, I have found 2 issues that need to be discussed.

1. When ThreadA receives the span SpanA from the parent thread during hand off, what will be the lifecycle of this received span.

   1. Should It stay with the thread till it gets automatically recycled from the ContextPreservingRunnable/ContextPreservingAbstractRunnable?
   2. If a new Span Span11 is created from this thread (ThreadA) with the parent SpanA(ThreadA received it from the hand off) and when Span11 will be ended it should be able to override the SpanReference to null in the ThreadContext? (Span11 has parent SpanA but this parent was not created by this thread.) This is the current behaviour.

   Here we have one issue, in case ThreadA creates 3 spans Span11, Span12, Span13 one after another (Span11 created and ended before Span12 and so on.) then Span11 will be having the parent SpanA, and Span12 and Span13 will be the independent root spans and not related to SpanA. We can handle this by creating a Span10 when starts and ends after Span13 then Span10 would have SpanA as parent and all other spans Span11, Span12, Span13 will have Span10 as a parent.

2. We discussed earlier that, scheduled runnable span shouldn't have the parent span from the calling thread (in this particular scenario createShard schedules the RetentionLeaseBackgroundSyncAction). Here 2 scenarios

   1. Should the Index create request have the first occurrence as part of the trace and further scheduled occurrences should occur on its own. (Since this is happening in the sync path.)
   2. Even, it's fine to have the first occurrence also outside of this IndexCreate trace.

   One possible fix here is that when we are preserving the context for the scheduled task, we should create the newStoreContext which will clear the specific transient header(current_span) here.

[reta]

Thanks @Gaganjuneja

1. When ThreadA receives the span SpanA from the parent thread during hand off, what will be the lifecycle of this received span.

We need the hand off to propagate parent relationship only. Thinking out loud here, if any thread was handed off the parent span, it should (unless specifically told not to) use it for further spans creation, but it should not do anything with it - I think it corresponds to option i

i. Should It stay with the thread till it gets automatically recycled from the ContextPreservingRunnable/ContextPreservingAbstractRunnable?

Here we have one issue, in case ThreadA creates 3 spans Span11, Span12, Span13 one after another (Span11 created and ended before Span12 and so on.) then Span11 will be having the parent SpanA, and Span12 and Span13 will be the independent root spans and not related to SpanA. We can handle this by creating a Span10 when starts and ends after Span13 then Span10 would have SpanA as parent and all other spans Span11, Span12, Span13 will have Span10 as a parent.

Important note here: span creation does not do anything to context, only creating the explicit span scope does context manipulation. With the example you've described, I don't see this boundary, could you express it in terms of span & scopes please?

2. We discussed earlier that, scheduled runnable span shouldn't have the parent span from the calling thread (in this particular scenario createShard schedules the RetentionLeaseBackgroundSyncAction). Here 2 scenarios

Correct, there seems to be an issue, may be newStoreContext should not even propagate the span to thread context (but only stashContext does it)?

[Gaganjuneja]

Important note here: span creation does not do anything to context, only creating the explicit span scope does context manipulation. With the example you've described, I don't see this boundary, could you express it in terms of span & scopes please?

Let's say this method is executing on the ThreadA
void method(){
        final Span span11 = tracer.startSpan(SpanBuilder.from("span11")); //span11 will have spanA as parent
        try (final SpanScope spanScope = tracer.withSpanInScope(span)) {
           // do some work.
        }// detaching the span11 will set the previous parent as null in the threadcontext
        

        final Span span12 = tracer.startSpan(SpanBuilder.from("span12")); 
        // threadContext has null spanReference from the above detach call so here the span12 will not have any parent.
        try (final SpanScope spanScope = tracer.withSpanInScope(span)) {
           // do some work.
        }

       final Span span13 = tracer.startSpan(SpanBuilder.from("span13"));
        // threadContext has null spanReference from the above detach call so here the span12 will not have any parent.
        try (final SpanScope spanScope = tracer.withSpanInScope(span)) {
           // do some work.
        }
}

Correct, there seems to be an issue, may be newStoreContext should not even propagate the span to thread context (but only stashContext does it)?

Yes for the scheduled runnable.

[reta]

Let's say this method is executing on the ThreadA

To me the example is straightforward: span11 / span12 / span13 should have no parent (assuming that void method() was not executed in any scoped span).

[Gaganjuneja]

Agreed. But today, span11 will have spanA parent and span12 and span13 will be the root spans.

[reta]

Agreed. But today, span11 will have spanA parent and span12 and span13 will be the root spans.

Gotcha, so I converted that to test cases and see the problem in case when void method() was is executed in scoped span (not what I've assumed). I think I understand the problem and how to fix it in #11316

[Gaganjuneja]

We discussed earlier that, scheduled runnable span shouldn't have the parent span from the calling thread (in this particular scenario createShard schedules the RetentionLeaseBackgroundSyncAction). Here 2 scenarios

Correct, there seems to be an issue, may be newStoreContext should not even propagate the span to thread context (but only stashContext does it)?

We need newStoreContext to propagate the span in most of the cases except scheduled tasks. I will raise a PR for it.