Continue wiring up distributed tracing for the new Grid

shs96c · shs96c · commit 2778509e648e · 2018-11-11T01:32:01.000Z
The traces being generated don't quite match up
properly when viewed in Jaeger's UI, so I suspect
that there's more work to be done, but the
groundwork is now all in place.
diff --git a/java/client/src/org/openqa/selenium/remote/tracing/DistributedTracer.java b/java/client/src/org/openqa/selenium/remote/tracing/DistributedTracer.java
@@ -33,7 +33,7 @@
 public class DistributedTracer {
 
   private static volatile DistributedTracer INSTANCE = DistributedTracer.builder()
-      .registerDetectedTracers()
+//      .registerDetectedTracers()
       .build();
   private static final ThreadLocal<LinkedList<Span>> ACTIVE_SPANS =
       ThreadLocal.withInitial(LinkedList::new);
@@ -129,11 +129,14 @@ private Builder() {
     }
 
     public Builder registerDetectedTracers() {
-      io.opentracing.Tracer tracer = TracerResolver.resolveTracer();
-      if (tracer != null) {
-        register(tracer);
+      try {
+        io.opentracing.Tracer tracer = TracerResolver.resolveTracer();
+        if (tracer != null) {
+          register(tracer);
+        }
+      } catch (Exception e) {
+        // Carry on. This is fine.
       }
-
       return this;
     }
 
diff --git a/java/client/src/org/openqa/selenium/remote/tracing/OpenTracingSpan.java b/java/client/src/org/openqa/selenium/remote/tracing/OpenTracingSpan.java
@@ -104,6 +104,9 @@ void inject(HttpRequest request) {
   @Override
   void extract(HttpRequest request) {
     SpanContext context = tracer.extract(Format.Builtin.HTTP_HEADERS, new HttpRequestInjector(request));
+    if (context == null) {
+      return;
+    }
     for (Map.Entry<String, String> item : context.baggageItems()) {
       addTraceTag(item.getKey(), item.getValue());
     }
diff --git a/java/server/src/org/openqa/selenium/grid/commands/Hub.java b/java/server/src/org/openqa/selenium/grid/commands/Hub.java
@@ -88,12 +88,14 @@ public Executable configure(String... args) {
           new EnvConfig(),
           new ConcatenatingConfig("selenium", '.', System.getProperties()));
 
+      DistributedTracer tracer = DistributedTracer.getInstance();
+
       SessionMap sessions = new LocalSessionMap();
-      Distributor distributor = new LocalDistributor();
+      Distributor distributor = new LocalDistributor(tracer);
       Router router = new Router(sessions, distributor);
 
       Server<?> server = new BaseServer<>(
-          DistributedTracer.getInstance(),
+          tracer,
           new BaseServerOptions(config));
       server.addRoute(Routes.matching(router).using(router).decorateWith(W3CCommandHandler.class));
       server.start();
diff --git a/java/server/src/org/openqa/selenium/grid/commands/Standalone.java b/java/server/src/org/openqa/selenium/grid/commands/Standalone.java
@@ -94,8 +94,10 @@ public Executable configure(String... args) {
           new EnvConfig(),
           new ConcatenatingConfig("selenium", '.', System.getProperties()));
 
+      DistributedTracer tracer = DistributedTracer.getInstance();
+
       SessionMap sessions = new LocalSessionMap();
-      Distributor distributor = new LocalDistributor();
+      Distributor distributor = new LocalDistributor(tracer);
       Router router = new Router(sessions, distributor);
 
       String hostName;
@@ -114,8 +116,6 @@ public Executable configure(String... args) {
         throw new RuntimeException(e);
       }
 
-      DistributedTracer tracer = DistributedTracer.getInstance();
-
       LocalNode.Builder node = LocalNode.builder(tracer, localhost, sessions)
           .maximumConcurrentSessions(Runtime.getRuntime().availableProcessors() * 3);
       nodeFlags.configure(node);
diff --git a/java/server/src/org/openqa/selenium/grid/distributor/Distributor.java b/java/server/src/org/openqa/selenium/grid/distributor/Distributor.java
@@ -33,8 +33,10 @@
 import org.openqa.selenium.remote.http.HttpClient;
 import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.http.HttpResponse;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
 
 import java.io.IOException;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.UUID;
 import java.util.function.Predicate;
@@ -75,9 +77,11 @@ public abstract class Distributor implements Predicate<HttpRequest>, CommandHand
   private final Routes routes;
   private final Injector injector;
 
-  protected Distributor() {
+  protected Distributor(DistributedTracer tracer) {
+    Objects.requireNonNull(tracer);
     injector = Injector.builder()
         .register(this)
+        .register(tracer)
         .register(new Json())
         .register(HttpClient.Factory.createDefault())
         .build();
diff --git a/java/server/src/org/openqa/selenium/grid/distributor/httpd/DistributorServer.java b/java/server/src/org/openqa/selenium/grid/distributor/httpd/DistributorServer.java
@@ -39,6 +39,7 @@
 import org.openqa.selenium.grid.web.Routes;
 import org.openqa.selenium.remote.tracing.DistributedTracer;
 
+
 @AutoService(CliCommand.class)
 public class DistributorServer implements CliCommand {
 
@@ -83,11 +84,15 @@ public Executable configure(String... args) {
           new EnvConfig(),
           new ConcatenatingConfig("distributor", '.', System.getProperties()));
 
-      Distributor distributor = new LocalDistributor();
+      DistributedTracer tracer = DistributedTracer.builder()
+          .registerDetectedTracers()
+          .build();
+
+      Distributor distributor = new LocalDistributor(tracer);
 
       BaseServerOptions serverOptions = new BaseServerOptions(config);
 
-      Server<?> server = new BaseServer<>(DistributedTracer.getInstance(), serverOptions);
+      Server<?> server = new BaseServer<>(tracer, serverOptions);
       server.addRoute(
           Routes.matching(distributor)
               .using(distributor)
diff --git a/java/server/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java b/java/server/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java
@@ -29,6 +29,7 @@
 import org.openqa.selenium.grid.node.Node;
 import org.openqa.selenium.grid.node.NodeStatus;
 import org.openqa.selenium.remote.NewSessionPayload;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
 
 import java.util.HashSet;
 import java.util.Optional;
@@ -39,7 +40,8 @@ public class LocalDistributor extends Distributor {
 
   private final Set<Node> nodes = new HashSet<>();
 
-  public LocalDistributor() {
+  public LocalDistributor(DistributedTracer tracer) {
+    super(tracer);
   }
 
   @Override
diff --git a/java/server/src/org/openqa/selenium/grid/distributor/remote/RemoteDistributor.java b/java/server/src/org/openqa/selenium/grid/distributor/remote/RemoteDistributor.java
@@ -33,6 +33,7 @@
 import org.openqa.selenium.remote.http.HttpClient;
 import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.http.HttpResponse;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
 
 import java.io.IOException;
 import java.io.UncheckedIOException;
@@ -45,7 +46,9 @@ public class RemoteDistributor extends Distributor {
   public static final Json JSON = new Json();
   private final Function<HttpRequest, HttpResponse> client;
 
-  public RemoteDistributor(HttpClient client) {
+  public RemoteDistributor(DistributedTracer tracer, HttpClient client) {
+    super(tracer);
+
     Objects.requireNonNull(client);
     this.client = req -> {
       try {
diff --git a/java/server/src/org/openqa/selenium/grid/node/httpd/NodeServer.java b/java/server/src/org/openqa/selenium/grid/node/httpd/NodeServer.java
@@ -108,8 +108,12 @@ public Executable configure(String... args) {
 
       BaseServerOptions serverOptions = new BaseServerOptions(config);
 
+      DistributedTracer tracer = DistributedTracer.builder()
+          .registerDetectedTracers()
+          .build();
+
       LocalNode.Builder builder = LocalNode.builder(
-          DistributedTracer.getInstance(),
+          tracer,
           serverOptions.getExternalUri(),
           sessions);
       nodeFlags.configure(builder);
@@ -118,9 +122,10 @@ public Executable configure(String... args) {
       DistributorOptions distributorOptions = new DistributorOptions(config);
       URL distributorUrl = distributorOptions.getDistributorUri().toURL();
       Distributor distributor = new RemoteDistributor(
+          tracer,
           HttpClient.Factory.createDefault().createClient(distributorUrl));
 
-      Server<?> server = new BaseServer<>(DistributedTracer.getInstance(), serverOptions);
+      Server<?> server = new BaseServer<>(tracer, serverOptions);
       server.addRoute(Routes.matching(node).using(node).decorateWith(W3CCommandHandler.class));
       server.start();
 
diff --git a/java/server/src/org/openqa/selenium/grid/router/httpd/RouterServer.java b/java/server/src/org/openqa/selenium/grid/router/httpd/RouterServer.java
@@ -95,6 +95,10 @@ public Executable configure(String... args) {
           new EnvConfig(),
           new ConcatenatingConfig("router", '.', System.getProperties()));
 
+      DistributedTracer tracer = DistributedTracer.builder()
+          .registerDetectedTracers()
+          .build();
+
       SessionMapOptions sessionsOptions = new SessionMapOptions(config);
       URL sessionMapUrl = sessionsOptions.getSessionMapUri().toURL();
       SessionMap sessions = new RemoteSessionMap(
@@ -105,11 +109,12 @@ public Executable configure(String... args) {
       DistributorOptions distributorOptions = new DistributorOptions(config);
       URL distributorUrl = distributorOptions.getDistributorUri().toURL();
       Distributor distributor = new RemoteDistributor(
+          tracer,
           HttpClient.Factory.createDefault().createClient(distributorUrl));
 
       Router router = new Router(sessions, distributor);
 
-      Server<?> server = new BaseServer<>(DistributedTracer.getInstance(), serverOptions);
+      Server<?> server = new BaseServer<>(tracer, serverOptions);
       server.addRoute(Routes.matching(router).using(router).decorateWith(W3CCommandHandler.class));
       server.start();
     };
diff --git a/java/server/src/org/openqa/selenium/grid/server/CommandHandlerServlet.java b/java/server/src/org/openqa/selenium/grid/server/CommandHandlerServlet.java
@@ -62,6 +62,8 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws
     HttpRequest request = new ServletRequestWrappingHttpRequest(req);
     HttpResponse response = new ServletResponseWrappingHttpResponse(resp);
 
+    log(String.format("(%s) %s", request.getMethod(), request.getUri()));
+
     try (Span span = tracer.createSpan("handler-servlet", tracer.getActiveSpan())) {
       HttpTracing.extract(request, span);
 
diff --git a/java/server/test/org/openqa/selenium/grid/distributor/DistributorTest.java b/java/server/test/org/openqa/selenium/grid/distributor/DistributorTest.java
@@ -49,8 +49,8 @@ public class DistributorTest {
   @Before
   public void setUp() {
     tracer = DistributedTracer.builder().build();
-    local = new LocalDistributor();
-    distributor = new RemoteDistributor(new PassthroughHttpClient<>(local));
+    local = new LocalDistributor(tracer);
+    distributor = new RemoteDistributor(tracer, new PassthroughHttpClient<>(local));
 
     caps = new ImmutableCapabilities("browserName", "cheese");
   }
@@ -73,7 +73,7 @@ public void shouldBeAbleToAddANodeAndCreateASession() throws URISyntaxException
         .add(caps, c -> new Session(new SessionId(UUID.randomUUID()), nodeUri, c))
         .build();
 
-    Distributor distributor = new LocalDistributor();
+    Distributor distributor = new LocalDistributor(tracer);
     distributor.add(node);
 
     MutableCapabilities sessionCaps = new MutableCapabilities(caps);
@@ -96,8 +96,8 @@ public void shouldBeAbleToRemoveANode() throws URISyntaxException {
         .add(caps, c -> new Session(new SessionId(UUID.randomUUID()), nodeUri, c))
         .build();
 
-    Distributor local = new LocalDistributor();
-    distributor = new RemoteDistributor(new PassthroughHttpClient<>(local));
+    Distributor local = new LocalDistributor(tracer);
+    distributor = new RemoteDistributor(tracer, new PassthroughHttpClient<>(local));
     distributor.add(node);
     distributor.remove(node.getId());
 
diff --git a/java/server/test/org/openqa/selenium/grid/router/EndToEndTest.java b/java/server/test/org/openqa/selenium/grid/router/EndToEndTest.java
@@ -59,7 +59,7 @@ public class EndToEndTest {
   public void inMemory() throws URISyntaxException {
 
     SessionMap sessions = new LocalSessionMap();
-    Distributor distributor = new LocalDistributor();
+    Distributor distributor = new LocalDistributor(tracer);
     URI nodeUri = new URI("http://localhost:4444");
     LocalNode node = LocalNode.builder(tracer, nodeUri, sessions)
         .add(driverCaps, createFactory(nodeUri))
@@ -93,12 +93,12 @@ public void withServers() throws URISyntaxException {
 
     SessionMap sessions = new RemoteSessionMap(getClient(sessionServer));
 
-    LocalDistributor localDistributor = new LocalDistributor();
+    LocalDistributor localDistributor = new LocalDistributor(tracer);
     Server<?> distributorServer = createServer();
     distributorServer.addRoute(Routes.matching(localDistributor).using(localDistributor));
     distributorServer.start();
 
-    Distributor distributor = new RemoteDistributor(getClient(distributorServer));
+    Distributor distributor = new RemoteDistributor(tracer, getClient(distributorServer));
 
     int port = PortProber.findFreePort();
     URI nodeUri = new URI("http://localhost:" + port);
