[WIP] added ThreadLocalScopeManager

[Aaronontheweb]

Need to add some specs for this still, but since I was already working on this in another project I thought I'd contribute it back.

When I'm using OpenTracing in combination with Akka.NET or DotNetty, it's been really helpful to me to have a ThreadStatic means of looking up active scopes since the operations inside one actor message process or the processing of one parsed message on the socket occur within the same thread but the handlers for those operations might be implemented in different, disconnected stages potentially. Propagating active scope data down to all of the handlers via ThreadStatic storage has been very helpful, rather than trying to do it through something I implemented at the application layer.

[cwe1ss]

Thx for the PR. Could you please explain in more detail why the builtin AsyncLocalScopeManager (that uses AsyncLocal/ThreadLocal) doesn't work for your scenario? Using ThreadStatic seems problematic as it doesn't work properly with async/await (callback can be executed on different threads). Are you not using async/await and does using ThreadStatic give you better performance?

[Aaronontheweb]

@cwe1ss yeah, in this case it's that all of the work for a "logical" operation all occurs inside the same thread, but executed by disconnected parts in an event-processing pipeline. So there's no real async flow across multiple contexts and threads. But each thread does need to maintain its own isolated context since you might have multiple operations like this executing in parallel.

The IScopeManager in this case is really just a wrapper around that ThreadStatic variable that the disconnected parts can use to flow context together. Each stage in the processing pipeline should have it's own span, i.e. for a socket server receiving a message:

Decode --> Deserialize --> [application-layer stuff] --> response to client (async)

I wouldn't use this implementation for a traditional TPL application where a single logical operation can be executed across multiple threads (i.e. each time there's an await, depending upon a variety of factors) - it's (probably) not safe for that.

Nor would I use an AsyncLocal implementation for this because there's some unnecessary allocation overhead there, but I chose not to use it because all of these methods that rely on tracing in this scenario are effectively "synchronous" (because the async work happens below them in the call stack) I'm not sure what their ambient context looks like behind the scenes. I also figured that having ThreadStatic use null as a default active scope value was fine. But if we're making the NoOp stuff easier to access then going with a ThreadLocal<ISpan> that defaults to NoOpSpan.Instance might be a better choice.

This use case is probably less common than the AsyncLocal stuff, but in environments like the two I mentioned it's pretty handy.

[cwe1ss]

I still do not really understand the usability of this because:

• A single ITracer instance can only have a single IScopeManager and typically, there's only one ITracer per application. This means that using a ThreadStatic-based scope manager breaks as soon as there is a single instrumented await call anywhere in the application.
• ThreadStatic data exists for the whole lifetime of a thread so if you are using anything that uses ThreadPool under the hood, this might lead to unintended behavior.

Given these constraints, using ThreadStatic seems quite dangerous/wrong IMO. Could you therefore please elaborate more or point me to some code (if that's possible)?

[Aaronontheweb]

Given these constraints, using ThreadStatic seems quite dangerous/wrong IMO.

That's not really for you to judge, though. The goal of this project is to provide infrastructure for being able to do tracing, not to be perspective in how to do it in all cases. To give you some background, I'm one of the authors of Akka.NET, an implementation of the actor model in .NET. Actors are a concurrency primitive and eliminate the need for things like locks, critical regions, and even async / await.

In a project I'm developing we use OpenTracing-enabled tracers to correlate messages between actors, and each actor runs on its own thread and holds onto that thread while it processes a burst of N messages at a time. While that actor processes messages it might do other things synchronously in that processing scope, such as emit log events and so forth. Logging infrastructure and actors are disconnected by implementation, but connected temporally at the time of event publication - therefore using ThreadStatic is safe way of answering the question "what is the current ISpan these events should be published to?" How do I know? Because that's how we also do correlation of "which actor owns this thread right now?" using ThreadStatic parking, and we've been doing that successfully for 4-5 years at millions of messages per second per machine scale.

ThreadLocal / ThreadStatic parking is an extremely common technique inside concurrent systems. It's also how we do things like buffer pooling inside DotNetty. So I'm intending this PR for developers with use cases like mine, where they are familiar with the execution model of their application and aren't working inside the TPL.

[ndrwrbgs]

Hey Aaron,

Might I suggest putting this into its own Contrib library? You’re totally right that code is code and if it has a use, it’s useful! I lean personally towards caution when anywhere near async/await though since many professional programmers get confused in that area (again, a what is rather than a what should be).

Would this be an okay compromise? I think Christian could help get an official repo set up.

(Other random thoughts as I composed this: thread seems more often to be used by framework developers than application developers, but I’ve come to realize most of the tracing code, especially propagation, won’t be used/written by application developers directly. If that’s the hard philosophy of OT, then I’d be okay keeping it here, but the warnings that would need to accompany it still make me feel safer having it factored out)

[cwe1ss]

I know who you are. You are doing great OSS work and I really appreciate discussing this with you. Any new topic is an opportunity for me to learn something new! I hope I'm not annoying you but I want to make sure I properly understand this (and the risks associated with it).

Hopefully, someone else will jump in as well and provide additional feedback!

I understand how the synchronous execution of the actor model works and I understand that your use case "which actor owns this thread right now?" can be done using ThreadStatic/ThreadLocal because this is an isolated, internal feature of Akka.NET.

My concern still is that Akka.NET is just one part of an application and that using one global IScopeManager that's based on ThreadStatic is problematic.

Let's take an example: We have OpenTracing.Contrib.NetCore which auto-instruments .NET Core applications by subscribing to DiagnosticSource/Activity events. There's many cases where we also rely on the ScopeManager to persist the scope between a Start/Stop event (e.g. in Entity Framework calls or in the generic Activity listener).

These actions usually involve async/await under the hood. If someone using Akka.NET and this proposed global scopemanager, also uses the contrib project and if she/he does e.g. a single HttpClient call/SqlClient call/EF Core call etc somewhere in the application, that instrumentation might be broken.

Do you know how much slower using the existing AsyncLocalScopeManager (which uses AsyncLocal under the hood) is? I have not done any tests on this yet.

Another potentially interesting thing is that if the framework/library is under your control, you could also try to completely avoid using a scope manager by storing the parent/root span in some kind of context dictionary (e.g. HttpContext.Items in ASP.NET Core). This obviously comes at a cost because you manually have to set the parent whenever you create a new span, but it would probably give you the best performance.

[cwe1ss]

A note on the code itself: I think it would be better to call this "ThreadStaticScopeManager" as ThreadStatic is what it uses under the hood. ThreadLocal is a separate C# type and has slightly different behavior.

[Aaronontheweb]

@ndrwrbgs @cwe1ss this IScopeManager implementation is the default in other languages, such as Java - why should it exist in a contrib package for .NET when it's the default implementation for other runtimes?

My concern still is that Akka.NET is just one part of an application and that using one global IScopeManager that's based on ThreadStatic is problematic.

It works for Java developers and for C# developers who already use thread-local storage for other stuff, because it's the responsibility of the developer to wrap up whatever they're working on before their turn on the thread cycles onto a new piece of work, which will clear the active scope back to whatever it was before.

It's up to the end-user or the library maintainer to use the right tool for the job. I had to abandon one OpenTracing implementation that tried to use AsyncLocal because it stitched unrelated, concurrent workloads together inside my applications. I'm sure I won't be the only user who has that problem, hence why I added this.

Don't assume that there's going to be a single paradigm for any given platform - async / await isn't the only way to do concurrency or asynchronous programming in .NET. What I'm asking for here isn't anything that is application specific - I'm asking that we have parity with what the other OpenTracing libraries currently do and provide some out of the box tool sets for supporting common cases like not using await for concurrency.

[carlosalberto]

My two cents:

IScopeManager implementation is the default in other languages, such as Java - why should it exist in a contrib package for .NET when it's the default implementation for other runtimes?

I think this would be fine (as an example: opentracing-util in Java also has AutoFinishScopeManager, besides the ThreadLocalScopeManager one).

Don't assume that there's going to be a single paradigm for any given platform - async / await isn't the only way to do concurrency or asynchronous programming in .NET.

Agreed with that. Still, I'm curious about those aforementioned problems with AsyncLocal. That being said, I think AsyncLocalScopeManager should be the default recommended one (for the reasons that @cwe1ss has mentioned previously in this thread).

[cwe1ss]

@Aaronontheweb this IScopeManager implementation is the default in other languages, such as Java - why should it exist in a contrib package for .NET when it's the default implementation for other runtimes?

The default in opentracing-java uses Java's ThreadLocal. Are you saying that that maps to .NET's ThreadStatic (which you use in your PR) and not .NET's ThreadLocal? (I don't know much about the internals of Java).

@Aaronontheweb I had to abandon one OpenTracing implementation that tried to use AsyncLocal because it stitched unrelated, concurrent workloads together inside my applications.

Could you elaborate on that? Which implementation do you mean? Have you used the built-in AsyncLocalScopeManager or was it some different solution?

One thing I still couldn't read/understand from your comments is the following: Does using the built-in AsyncLocalScopeManager actually break your application/Akka.NET (which means you have to use one that's based on ThreadStatic) or does it work fine but performs badly?

[cwe1ss]

fyi, I'm currently not adding this to the next release (targeting next week) for the following reasons:

• I currently simply don't know enough about the internals of ThreadStatic, ThreadLocal, AsyncLocal, CallContext, ExecutionContext to decide this. As described, using this as the single IScopeManager in an entire application sounds dangerous to me and I therefore would like to wait for more people who want this.
• People who want this now can easily copy these two classes.
• The types should be renamed to ThreadStaticScope* IMO (see previous comment)

I'd also be interested in a benchmark that compares AsyncLocal with other mechanisms. Anyone interested in this? I do not really have much time right now.