Fix Flaky Ratpack Fork Test - #622
Conversation
Ratpack tests occasionally fail with the handler span and manually started span out of order. This will ensure that if a span is manually started inside of the handler then it will be finished after the handler span is finished, which is when the request has been responded to.
| context.onClose { | ||
| if (startSpanInHandler) { | ||
| final Scope activeScope = GlobalTracer.get().scopeManager().active() | ||
| ((TraceScope) activeScope).setAsyncPropagation(false) |
There was a problem hiding this comment.
@realark does this even do anything given the scope is closed on the next line?
There was a problem hiding this comment.
Not it won't. Closing the scope has the same effect as turning off async.
There was a problem hiding this comment.
So ((TraceScope) activeScope).setAsyncPropagation(false) is not needed? Was this an effect of the new change in the executor instrumentation?
There was a problem hiding this comment.
Well it's only needed if the scope is not being closed. Since the active scope is being closed turning off async is not required.
I'm not following the instrumentation and testing approach here, but it's usually a bad idea to close a scope you didn't create. Should we be closing out the scope here, or should we let the creator of the scope handle that?
There was a problem hiding this comment.
The creator of the scope here is the test itself. I.e. the current active scope is the scope started by the test. I just pushed a change, perhaps that's a better approach to this test?
| ((TraceScope) scope).setAsyncPropagation(false) | ||
| scope.close() | ||
| context.onClose { | ||
| startedSpan.finish() |
There was a problem hiding this comment.
@realark is this a better way to handle the scope?
3 participants
Ratpack tests occasionally fail with the handler span and manually started span out of order. This will ensure that if a span is manually started inside of the handler then it will be finished after the handler span is finished, which is when the request has been responded to.