scopeManager#activate creates new scope every time it's called

[Sam-Kruglov]

For me, it was intuitive that if I activate a span that is already active then nothing happens. Turns out, I leak Scope instances if I do that.

Span span = ...
Scope scope1 = tracer.scopeManager().activate(span);
Scope scope2 = tracer.scopeManager().activate(span);
Scope scope3 = tracer.scopeManager().activate(span);

Would be great if scope1, scope2, and scope3 would be the same Scope instance. It is bit more code for the library but it is also a bit more user-friendly.

Found out about it in https://github.com/AxonFramework/extension-tracing/pull/17

[sjoerdtalsma]

As intuitive as that may seem to you, how intuitive would the various close() calls become in this case? When would this single Scope instance actually be closed?

[Sam-Kruglov]

I assume that nothing should happen when you close a Scope that is already closed.

[whiskeysierra]

For me, it was intuitive that if I activate a span that is already active then nothing happens.

If that would be the case then the most inner close would close the most outer scope. That doesn't sound right.

I assume that nothing should happen when you close a Scope that is already closed.

I don't know how it's implemented but I really hope that you can stack/nest scopes and when you close them the next outer scope's span will become active.

Turns out, I leak Scope instances if I do that.

Not if you properly close every scope that you activate.

[Sam-Kruglov]

If that would be the case then the most inner close would close the most outer scope.

But the best practice is to use try-with-resources which is assigned to a specific Scope, so should be ok.

I really hope that you can stack/nest scopes and when you close them the next outer scope's span will become active.

If you stick to the above it will work.

Not if you properly close every scope that you activate.

Sure, I do not even intend to do that mistake anyway.

[Sam-Kruglov]

I just propose that Scope To Span should be One To One, instead of Many To One, that doesn't even make sense

[whiskeysierra]

No, it does make sense. Otherwise you'd need to know whether it's safe to close a scope or not. Now it's safe.

What problem are you trying to solve?

[Sam-Kruglov]

Can you elaborate on why it's not "safe"? Either way the method close() is located in the Scope class, no difference at all. The difference is in the activation of a Span. It's more robust if you activate the same Span twice and get the same Scope instance activated than having 2 Scopes for this Span.

[whiskeysierra]

It's more robust if you activate the same Span twice and get the same Scope instance activated than having 2 Scopes for this Span.

No, it's not. Here is why:

(L1 and L2 are two different layers in your application, e.g. two classes and L1 calls L2.)

1. L1: Activate span
2. L2: Activate span
3. L2: Close scope
4. L1: Close scope

The scope is closed after 3. Now in L1 before step 4, the layer has a reasonable expectation that the span that it activated is still active, but it no longer is. :boom:

What you're trying is to introduce an unnecessary entangling of span lifecycle (start + finish) and scope management (activate + close). Those two are orthogonal concerns and are better be kept apart. You can see this reflected in the most recent API versions because all methods that couple both concepts together are now removed:

• startActive()
• .activate(span, closeOnFinish)

Whether you create a new span or use the currently active span is independent of whether you activate this span afterwards. If you couple them you'd violate the Principle of Least Knowledge because suddenly your inner layers need to know whether it's safe to close the scopes that they obtain.

Now, when you keep those two things apart, as they are now you don't have any of those problems. Every layer can choose to activate a span or not without the need to know about the outer/upper layer and without the fear of breaking anyone.

[Sam-Kruglov]

That makes sense, thanks for the explanation. I guess I don't really get why do we even have Scope, why can't we simply start and stop the Span? I can't imagine a scenario where I would want multiple active scopes simultaneously for the same span, I would just split this span into multiple child spans and give them different names. Do you have an example for that?

[sjoerdtalsma]

@Sam-Kruglov Scope management exists to facilitate the concept of an active span. This allows tracing-aware code in different application levels to cooperate without having to explicitly pass along the span throughout the code.

E.g. an HTTP rest layer can create a new child span from a parent it detected in the HTTP headers of an incoming request and activate it in a new scope. Then, a database layer can create a new child span from the active span detected in that scope. Each span has its own start..finish lifecycle and as explained by @whiskeysierra above, each scope is considered active until it is closed.

Orthogonal concerns.

[Sam-Kruglov]

@sjoerdtalsma right, I saw there is this thread-local scope that has the current span in it, so we don't have to pass the span around manually. But why can't we store the span in a thread-local variable without wrapping it in something? Seems like we could "detect" the currently active span without the concept of Scope

[sjoerdtalsma]

Seems like we could "detect" the currently active span without the concept of Scope

Not in a standardised way; that's kind of the raison d'être of the ScopeManager

[Sam-Kruglov]

OK then, thanks for clarifying

[whiskeysierra]

Seems like we could "detect" the currently active span without the concept of Scope

The thread local approach doesn't work for reactive, non-blocking applications that work with an event loop.

[yurishkuro]

The thread local approach doesn't work for reactive, non-blocking applications that work with an event loop.

That's a problem to be solved by those frameworks. They need to provide a mechanism for keeping request-scoped context without requiring passing it around explicitly. For example, continuation-local storage in Nodejs or StackContext in Python's Tornado framework.