Fix client scope not being closed #82
Conversation
pavolloffay
changed the title
|
👍 |
|
You are right, I thought that exception is thrown before the request filter, but it's not. So this is a much better option. |
| @@ -2,6 +2,7 @@ | |||
|
|
|||
| import static io.opentracing.contrib.jaxrs2.internal.SpanWrapper.PROPERTY_NAME; | |||
|
|
|||
| import io.opentracing.Scope; | |||
There was a problem hiding this comment.
Doesn't look like this is needed.
| @@ -61,7 +62,7 @@ public void filter(ClientRequestContext requestContext) throws IOException { | |||
| .asChildOf(parentSpanContext); | |||
| } | |||
|
|
|||
| Span span = spanBuilder.startActive(false).span(); | |||
| Span span = spanBuilder.start(); | |||
There was a problem hiding this comment.
Based on the previous comments on the PR - what happens if an exception is thrown? If the scope wouldn't be closed, this also means the span is not finished?
There was a problem hiding this comment.
yes, there is an issue for it and TODO in the test
| @@ -56,14 +65,15 @@ protected void initTracing(ServletContextHandler context) { | |||
| public void testDefaultConfiguration() { | |||
| MockTracer mockTracer = new MockTracer(new ThreadLocalScopeManager(), Propagator.TEXT_MAP); | |||
There was a problem hiding this comment.
Not related to this PR, but could simplify this to just use default constructor.
Fixes #78
There are two solutions:
cc @mabn