Address feedback

Signed-off-by: Prithvi Raj <p.r@uber.com>
jaegertracing · Oct 10, 2017 · 55d04df · 55d04df
1 parent 8277a5b
commit 55d04df
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 19 deletions.
diff --git a/jaeger-context/src/main/java/com/uber/jaeger/context/ActiveSpanSourceTraceContext.java b/jaeger-context/src/main/java/com/uber/jaeger/context/ActiveSpanSourceTraceContext.java
@@ -29,7 +29,6 @@
  */
 public class ActiveSpanSourceTraceContext implements TraceContext {
 
-  private final ActiveSpanSource activeSpanSource;
   /**
    * This is a hack to retrieve the span wrapped by the {@link ThreadLocalActiveSpan} implementation
    * to shoehorn into the {@link TraceContext} implementation. This is being done so that
@@ -47,20 +46,19 @@ public class ActiveSpanSourceTraceContext implements TraceContext {
     }
   }
 
-  public ActiveSpanSourceTraceContext(ActiveSpanSource activeSpanSource) {
-    this.activeSpanSource = activeSpanSource;
+  public ActiveSpanSourceTraceContext() {
   }
 
   /** Makes the span active. */
   @Override
   public void push(Span span) {
-    activeSpanSource.makeActive(span);
+    GlobalTracer.get().makeActive(span);
   }
 
   /** Deactivates the current active span. */
   @Override
   public Span pop() {
-    ActiveSpan activeSpan = activeSpanSource.activeSpan();
+    ActiveSpan activeSpan = GlobalTracer.get().activeSpan();
     Span span = getSpan(activeSpan);
     activeSpan.deactivate();
     return span;
@@ -69,13 +67,13 @@ public Span pop() {
   /** Retrieves the current active span. */
   @Override
   public Span getCurrentSpan() {
-    ActiveSpan activeSpan = activeSpanSource.activeSpan();
+    ActiveSpan activeSpan = GlobalTracer.get().activeSpan();
     return getSpan(activeSpan);
   }
 
   @Override
   public boolean isEmpty() {
-    return activeSpanSource.activeSpan() == null;
+    return GlobalTracer.get().activeSpan() == null;
   }
 
   private Span getSpan(ActiveSpan activeSpan) {

diff --git a/jaeger-context/src/main/java/com/uber/jaeger/context/TracingUtils.java b/jaeger-context/src/main/java/com/uber/jaeger/context/TracingUtils.java
@@ -19,24 +19,21 @@
 import java.util.concurrent.ExecutorService;
 
 public class TracingUtils {
-  private static TraceContext traceContext;
+  private static TraceContext traceContext = new ActiveSpanSourceTraceContext();
 
   public static TraceContext getTraceContext() {
-    initializeTraceContext();
+    assertGlobalTracerIsRegistered();
     return traceContext;
   }
 
   public static ExecutorService tracedExecutor(ExecutorService wrappedExecutorService) {
-    initializeTraceContext();
+    assertGlobalTracerIsRegistered();
     return new TracedExecutorService(wrappedExecutorService, traceContext);
   }
 
-  private static synchronized void initializeTraceContext() {
-    if (traceContext == null) {
-      if (!GlobalTracer.isRegistered()) {
-        throw new IllegalStateException("Please register a io.opentracing.util.GlobalTracer.");
-      }
-      traceContext = new ActiveSpanSourceTraceContext(GlobalTracer.get());
+  private static void assertGlobalTracerIsRegistered() {
+    if (!GlobalTracer.isRegistered()) {
+      throw new IllegalStateException("Please register a io.opentracing.util.GlobalTracer.");
     }
   }
 

diff --git a/jaeger-jaxrs2/src/test/java/com/uber/jaeger/filters/jaxrs2/ClientFilterTest.java b/jaeger-jaxrs2/src/test/java/com/uber/jaeger/filters/jaxrs2/ClientFilterTest.java
@@ -26,14 +26,17 @@
 import com.uber.jaeger.propagation.FilterIntegrationTest;
 import com.uber.jaeger.reporters.InMemoryReporter;
 import com.uber.jaeger.samplers.ConstSampler;
+import com.uber.jaeger.utils.TestUtils;
 import io.opentracing.Tracer;
 import io.opentracing.tag.Tags;
+import io.opentracing.util.GlobalTracer;
 import java.net.URI;
 import java.util.List;
 import java.util.Map;
 import javax.ws.rs.client.ClientRequestContext;
 import javax.ws.rs.client.ClientResponseContext;
 import javax.ws.rs.core.MultivaluedHashMap;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -64,10 +67,16 @@ public void setUp() throws Exception {
     tracer =
         new com.uber.jaeger.Tracer.Builder("Angry Machine", reporter, new ConstSampler(true))
             .build();
-    traceContext = new ActiveSpanSourceTraceContext(tracer);
+    GlobalTracer.register(tracer);
+    traceContext = new ActiveSpanSourceTraceContext();
     undertest = new ClientFilter(tracer, traceContext);
   }
 
+  @After
+  public void tearDown() throws Exception {
+    TestUtils.resetGlobalTracer();
+  }
+
   @Test
   public void testFilter() throws Exception {
     String method = "GET";

diff --git a/jaeger-jaxrs2/src/test/java/com/uber/jaeger/filters/jaxrs2/ServerFilterTest.java b/jaeger-jaxrs2/src/test/java/com/uber/jaeger/filters/jaxrs2/ServerFilterTest.java
@@ -26,14 +26,18 @@
 import com.uber.jaeger.propagation.FilterIntegrationTest;
 import com.uber.jaeger.reporters.InMemoryReporter;
 import com.uber.jaeger.samplers.ConstSampler;
+import com.uber.jaeger.utils.TestUtils;
 import io.opentracing.tag.Tags;
+import io.opentracing.util.GlobalTracer;
 import java.net.URI;
 import java.util.List;
 import java.util.Map;
 import javax.ws.rs.container.ContainerRequestContext;
 import javax.ws.rs.container.ContainerResponseContext;
 import javax.ws.rs.core.MultivaluedHashMap;
 import javax.ws.rs.core.UriInfo;
+
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -60,10 +64,16 @@ public void setUp() {
     tracer =
         new com.uber.jaeger.Tracer.Builder("Angry Machine", reporter, new ConstSampler(true))
             .build();
-    traceContext = new ActiveSpanSourceTraceContext(tracer);
+    GlobalTracer.register(tracer);
+    traceContext = new ActiveSpanSourceTraceContext();
     undertest = new ServerFilter(tracer, traceContext);
   }
 
+  @After
+  public void tearDown() throws Exception {
+    TestUtils.resetGlobalTracer();
+  }
+
   @Test
   public void filter() throws Exception {
     String method = "GET";

diff --git a/jaeger-jaxrs2/src/test/java/com/uber/jaeger/propagation/FilterIntegrationTest.java b/jaeger-jaxrs2/src/test/java/com/uber/jaeger/propagation/FilterIntegrationTest.java
@@ -29,9 +29,11 @@
 import com.uber.jaeger.metrics.InMemoryStatsReporter;
 import com.uber.jaeger.reporters.InMemoryReporter;
 import com.uber.jaeger.samplers.ConstSampler;
+import com.uber.jaeger.utils.TestUtils;
 import io.opentracing.SpanContext;
 import io.opentracing.Tracer;
 import io.opentracing.propagation.Format;
+import io.opentracing.util.GlobalTracer;
 import javax.ws.rs.client.Client;
 import javax.ws.rs.client.ClientBuilder;
 import javax.ws.rs.client.WebTarget;
@@ -66,7 +68,8 @@ public void setUp() throws Exception {
             .withStatsReporter(metricsReporter)
             .build();
 
-    traceContext = new ActiveSpanSourceTraceContext(tracer);
+    GlobalTracer.register(tracer);
+    traceContext = new ActiveSpanSourceTraceContext();
 
     // start the server
     server = new JerseyServer(tracer, traceContext);
@@ -81,6 +84,7 @@ public void setUp() throws Exception {
   @After
   public void tearDown() throws Exception {
     server.stop();
+    TestUtils.resetGlobalTracer();
   }
 
   @Test