Update to OpenTracing 0.33. #353
Conversation
|
@Ladicek we will need this for the MP 4.0 update. |
| activeScope.close(); | ||
| Span span = scopeManager.activeSpan(); | ||
| if (span != null) { | ||
| scopeManager.activate(span).close(); |
There was a problem hiding this comment.
So this line is effectively a noop. It creates a new scope and immediately destroys it, which leads to restoring the previously active scope (if there was one). Looks like the new OpenTracing API doesn't let us do what ConProp requires :-/
There was a problem hiding this comment.
... which is temporarily suspending the current span. That would be done by closing the currently active scope (and later activating the span again, which would create a new scope; we currently don't do this as highlighted by the TODO below). But we can't get to the currently active scope anymore.
There was a problem hiding this comment.
This actually seems to be a deliberate decision by OpenTracing to prevent API misuse. Which makes advanced use cases, like context propagation, impossible :-)
There was a problem hiding this comment.
I suppose the OpenTracing API is meant to be used in a way that there is no currently active scope (and hence no currently active span) by the time context propagation happens. Which essentially means that this method should be a real noop.
There was a problem hiding this comment.
Not sure on this one, might be good to see if @pavolloffay can offer advice
There was a problem hiding this comment.
From the java doc:
/**
* @deprecated use {@link #activeSpan()} instead.
* Return the currently active {@link Scope} which can be used to deactivate the currently active
* {@link Span}.
*
* <p>
* Observe that {@link Scope} is expected to be used only in the same thread where it was
* created, and thus should not be passed across threads.
*
* <p>
* Because both {@link #active()} and {@link #activeSpan()} reference the current
* active state, they both will be either null or non-null.
*
* @return the {@link Scope active scope}, or null if none could be found.
*/
There was a problem hiding this comment.
My interpretation is that it doesn't necessarily create a new span. The implementation seems equivalent, but I'm no expert here :)
There was a problem hiding this comment.
That javadoc is for ScopeManager.active() and comes from 0.32, right? Because that method is gone in 0.33.
ScopeManager.activate(Span) creates a new scope. The javadoc for Span and Scope could be clearer, but my understanding is that scope is a slice of time during which you do something with the span. During the lifetime of a span, many scopes can be created and subsequently closed, but the span itself can only be closed once.
There was a problem hiding this comment.
That javadoc is for
ScopeManager.active()and comes from 0.32, right? Because that method is gone in 0.33.
Yes, it is.
|
Hmm, I wanted to force-push the commit I amended locally, but somehow failed. Sorry about that, will open a new PR from my branch :-) |
|
Ha, I know what happened, instead of pushing the commit, I accidentally pushed the |
|
Submitted #360. |
No description provided.