Pass and close scope directly (#88)

Avoid using ScopeManager.active() as some tracer implementations return different scope with invalid scope.close() method. Signed-off-by: Pavol Loffay <ploffay@redhat.com>
opentracing-contrib · Jun 4, 2018 · f153e0b · f153e0b
1 parent eaa1a4e
commit f153e0b
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 15 deletions.
diff --git a/...racing-jaxrs2/src/main/java/io/opentracing/contrib/jaxrs2/client/ClientTracingFilter.java b/...racing-jaxrs2/src/main/java/io/opentracing/contrib/jaxrs2/client/ClientTracingFilter.java
@@ -7,6 +7,7 @@
 import io.opentracing.Tracer;
 import io.opentracing.contrib.jaxrs2.internal.CastUtils;
 import io.opentracing.contrib.jaxrs2.internal.SpanWrapper;
+import io.opentracing.noop.NoopScopeManager.NoopScope;
 import io.opentracing.propagation.Format;
 import io.opentracing.tag.Tags;
 import java.io.IOException;
@@ -61,7 +62,10 @@ public void filter(ClientRequestContext requestContext) throws IOException {
                 .asChildOf(parentSpanContext);
         }
 
-        Span span = spanBuilder.start();
+        /**
+         * Do not create Scope - there is no way to deactivate it on UnknownHostException
+         */
+        final Span span = spanBuilder.start();
 
         if (spanDecorators != null) {
             for (ClientSpanDecorator decorator: spanDecorators) {
@@ -74,7 +78,16 @@ public void filter(ClientRequestContext requestContext) throws IOException {
         }
 
         tracer.inject(span.context(), Format.Builtin.HTTP_HEADERS, new ClientHeadersInjectTextMap(requestContext.getHeaders()));
-        requestContext.setProperty(PROPERTY_NAME, new SpanWrapper(span));
+        requestContext.setProperty(PROPERTY_NAME, new SpanWrapper(new NoopScope() {
+            @Override
+            public void close() {
+            }
+
+            @Override
+            public Span span() {
+                return span;
+            }
+        }));
     }
 
     @Override

diff --git a/opentracing-jaxrs2/src/main/java/io/opentracing/contrib/jaxrs2/internal/SpanWrapper.java b/opentracing-jaxrs2/src/main/java/io/opentracing/contrib/jaxrs2/internal/SpanWrapper.java
@@ -1,6 +1,8 @@
 package io.opentracing.contrib.jaxrs2.internal;
 
+import io.opentracing.Scope;
 import io.opentracing.Span;
+import java.sql.Wrapper;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
@@ -12,21 +14,25 @@ public class SpanWrapper {
 
     public static final String PROPERTY_NAME = SpanWrapper.class.getName() + ".activeSpanWrapper";
 
-    private Span span;
+    private Scope scope;
     private AtomicBoolean finished = new AtomicBoolean();
 
-    public SpanWrapper(Span span) {
-        this.span = span;
+    public SpanWrapper(Scope scope) {
+        this.scope = scope;
     }
 
     public Span get() {
-        return span;
+        return scope.span();
+    }
+
+    public Scope getScope() {
+        return scope;
     }
 
     public synchronized void finish() {
         if (!finished.get()) {
             finished.set(true);
-            span.finish();
+            scope.span().finish();
         }
     }
 

diff --git a/...racing-jaxrs2/src/main/java/io/opentracing/contrib/jaxrs2/server/ServerTracingFilter.java b/...racing-jaxrs2/src/main/java/io/opentracing/contrib/jaxrs2/server/ServerTracingFilter.java
@@ -1,5 +1,6 @@
 package io.opentracing.contrib.jaxrs2.server;
 
+import io.opentracing.Scope;
 import io.opentracing.Span;
 import io.opentracing.SpanContext;
 import io.opentracing.Tracer;
@@ -70,27 +71,33 @@ public void filter(ContainerRequestContext requestContext) {
                 spanBuilder.asChildOf(parentSpanContext);
             }
 
-            Span span = spanBuilder.startActive(false).span();
+            Scope scope = spanBuilder.startActive(false);
 
             if (spanDecorators != null) {
                 for (ServerSpanDecorator decorator: spanDecorators) {
-                    decorator.decorateRequest(requestContext, span);
+                    decorator.decorateRequest(requestContext, scope.span());
                 }
             }
 
             // override operation name set by @Traced
             if (this.operationName != null) {
-                span.setOperationName(operationName);
+                scope.span().setOperationName(operationName);
             }
 
             if (log.isLoggable(Level.FINEST)) {
                 log.finest("Creating server span: " + operationName);
             }
 
-            requestContext.setProperty(PROPERTY_NAME, new SpanWrapper(span));
+            requestContext.setProperty(PROPERTY_NAME, new SpanWrapper(scope));
         }
     }
 
+    /**
+     * Returns a parent for a span created by this filter (jax-rs span).
+     * The context from the active span takes precedence over context in the request.
+     * The current active span should be child-of extracted context and for example
+     * created at a lower level e.g. jersey filter.
+     */
     private SpanContext parentSpanContext(ContainerRequestContext requestContext) {
         Span activeSpan = tracer.activeSpan();
         if (activeSpan != null) {

diff --git a/...racing-jaxrs2/src/main/java/io/opentracing/contrib/jaxrs2/server/SpanFinishingFilter.java b/...racing-jaxrs2/src/main/java/io/opentracing/contrib/jaxrs2/server/SpanFinishingFilter.java
@@ -61,12 +61,9 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
       }
       throw ex;
     } finally {
-      Scope scope = tracer.scopeManager().active();
-      if (scope != null) {
-        scope.close();
-      }
       SpanWrapper spanWrapper = getSpanWrapper(httpRequest);
       if (spanWrapper != null) {
+        spanWrapper.getScope().close();
         if (request.isAsyncStarted()) {
           request.getAsyncContext().addListener(new SpanFinisher(spanWrapper), request, response);
         } else {