Add the distributed tracer to the grid

SeleniumHQ · Nov 10, 2018 · b66fe3e · b66fe3e
1 parent 465fc74
commit b66fe3e
Show file tree

Hide file tree

Showing 13 changed files with 58 additions and 26 deletions.
diff --git a/java/server/src/org/openqa/selenium/grid/commands/Hub.java b/java/server/src/org/openqa/selenium/grid/commands/Hub.java
@@ -41,6 +41,7 @@
 import org.openqa.selenium.grid.sessionmap.SessionMap;
 import org.openqa.selenium.grid.sessionmap.local.LocalSessionMap;
 import org.openqa.selenium.grid.web.Routes;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
 
 @AutoService(CliCommand.class)
 public class Hub implements CliCommand {
@@ -91,7 +92,9 @@ public Executable configure(String... args) {
       Distributor distributor = new LocalDistributor();
       Router router = new Router(sessions, distributor);
 
-      Server<?> server = new BaseServer<>(new BaseServerOptions(config));
+      Server<?> server = new BaseServer<>(
+          DistributedTracer.getInstance(),
+          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
@@ -44,6 +44,7 @@
 import org.openqa.selenium.grid.sessionmap.local.LocalSessionMap;
 import org.openqa.selenium.grid.web.Routes;
 import org.openqa.selenium.net.NetworkUtils;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
 
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -119,7 +120,9 @@ public Executable configure(String... args) {
 
       distributor.add(node.build());
 
-      Server<?> server = new BaseServer<>(new BaseServerOptions(config));
+      Server<?> server = new BaseServer<>(
+          DistributedTracer.getInstance(),
+          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/distributor/httpd/DistributorServer.java b/java/server/src/org/openqa/selenium/grid/distributor/httpd/DistributorServer.java
@@ -37,6 +37,7 @@
 import org.openqa.selenium.grid.server.Server;
 import org.openqa.selenium.grid.server.W3CCommandHandler;
 import org.openqa.selenium.grid.web.Routes;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
 
 @AutoService(CliCommand.class)
 public class DistributorServer implements CliCommand {
@@ -86,7 +87,7 @@ public Executable configure(String... args) {
 
       BaseServerOptions serverOptions = new BaseServerOptions(config);
 
-      Server<?> server = new BaseServer<>(serverOptions);
+      Server<?> server = new BaseServer<>(DistributedTracer.getInstance(), serverOptions);
       server.addRoute(
           Routes.matching(distributor)
               .using(distributor)

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
@@ -45,6 +45,7 @@
 import org.openqa.selenium.grid.sessionmap.remote.RemoteSessionMap;
 import org.openqa.selenium.grid.web.Routes;
 import org.openqa.selenium.remote.http.HttpClient;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
 
 import java.net.URL;
 import java.time.Duration;
@@ -116,7 +117,7 @@ public Executable configure(String... args) {
       Distributor distributor = new RemoteDistributor(
           HttpClient.Factory.createDefault().createClient(distributorUrl));
 
-      Server<?> server = new BaseServer<>(serverOptions);
+      Server<?> server = new BaseServer<>(DistributedTracer.getInstance(), 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
@@ -44,6 +44,7 @@
 import org.openqa.selenium.grid.sessionmap.remote.RemoteSessionMap;
 import org.openqa.selenium.grid.web.Routes;
 import org.openqa.selenium.remote.http.HttpClient;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
 
 import java.net.URL;
 
@@ -108,7 +109,7 @@ public Executable configure(String... args) {
 
       Router router = new Router(sessions, distributor);
 
-      Server<?> server = new BaseServer<>(serverOptions);
+      Server<?> server = new BaseServer<>(DistributedTracer.getInstance(), serverOptions);
       server.addRoute(Routes.matching(router).using(router).decorateWith(W3CCommandHandler.class));
       server.start();
     };

diff --git a/java/server/src/org/openqa/selenium/grid/server/BaseServer.java b/java/server/src/org/openqa/selenium/grid/server/BaseServer.java
@@ -32,6 +32,7 @@
 import org.openqa.selenium.net.NetworkUtils;
 import org.openqa.selenium.net.PortProber;
 import org.openqa.selenium.remote.http.HttpRequest;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
 import org.seleniumhq.jetty9.security.ConstraintMapping;
 import org.seleniumhq.jetty9.security.ConstraintSecurityHandler;
 import org.seleniumhq.jetty9.server.Connector;
@@ -70,8 +71,10 @@ public class BaseServer<T extends BaseServer> implements Server<T> {
   private final ServletContextHandler servletContextHandler;
   private final Injector injector;
   private final URL url;
+  private final DistributedTracer tracer;
 
-  public BaseServer(BaseServerOptions options) {
+  public BaseServer(DistributedTracer tracer, BaseServerOptions options) {
+    this.tracer = Objects.requireNonNull(tracer);
     int port = options.getPort() == 0 ? PortProber.findFreePort() : options.getPort();
 
     String host = options.getHostname().orElseGet(() -> {
@@ -197,7 +200,7 @@ public T start() {
           .decorateWith(W3CCommandHandler.class)
           .build();
 
-      addServlet(new CommandHandlerServlet(routes), "/*");
+      addServlet(new CommandHandlerServlet(tracer, routes), "/*");
 
       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
@@ -26,6 +26,9 @@
 import org.openqa.selenium.json.Json;
 import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.http.HttpResponse;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
+import org.openqa.selenium.remote.tracing.HttpTracing;
+import org.openqa.selenium.remote.tracing.Span;
 
 import java.io.IOException;
 import java.util.Objects;
@@ -39,8 +42,10 @@ class CommandHandlerServlet extends HttpServlet {
 
   private final Routes routes;
   private final Injector injector;
+  private final DistributedTracer tracer;
 
-  public CommandHandlerServlet(Routes routes) {
+  public CommandHandlerServlet(DistributedTracer tracer, Routes routes) {
+    this.tracer = Objects.requireNonNull(tracer);
     Objects.requireNonNull(routes);
     this.routes = combine(routes)
         .fallbackTo(
@@ -57,13 +62,15 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws
     HttpRequest request = new ServletRequestWrappingHttpRequest(req);
     HttpResponse response = new ServletResponseWrappingHttpResponse(resp);
 
-    System.out.println(String.format("(%s) %s", request.getMethod(), request.getUri()));
+    try (Span span = tracer.createSpan("handler-servlet", tracer.getActiveSpan())) {
+      HttpTracing.extract(request, span);
 
-    Optional<CommandHandler> possibleMatch = routes.match(injector, request);
-    if (possibleMatch.isPresent()) {
-      possibleMatch.get().execute(request, response);
-    } else {
-      throw new IllegalStateException("It should not be possible to get here");
+      Optional<CommandHandler> possibleMatch = routes.match(injector, request);
+      if (possibleMatch.isPresent()) {
+        possibleMatch.get().execute(request, response);
+      } else {
+        throw new IllegalStateException("It should not be possible to get here");
+      }
     }
   }
 }
diff --git a/java/server/src/org/openqa/selenium/grid/sessionmap/httpd/SessionMapServer.java b/java/server/src/org/openqa/selenium/grid/sessionmap/httpd/SessionMapServer.java
@@ -38,7 +38,7 @@
 import org.openqa.selenium.grid.server.W3CCommandHandler;
 import org.openqa.selenium.grid.sessionmap.SessionMap;
 import org.openqa.selenium.grid.sessionmap.local.LocalSessionMap;
-import org.openqa.selenium.grid.web.Routes;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
 
 @AutoService(CliCommand.class)
 public class SessionMapServer implements CliCommand {
@@ -88,7 +88,7 @@ public Executable configure(String... args) {
 
       BaseServerOptions serverOptions = new BaseServerOptions(config);
 
-      Server<?> server = new BaseServer<>(serverOptions);
+      Server<?> server = new BaseServer<>(DistributedTracer.getInstance(), serverOptions);
       server.addRoute(matching(sessions).using(sessions).decorateWith(W3CCommandHandler.class));
       server.start();
     };

diff --git a/java/server/src/org/openqa/selenium/remote/server/SeleniumServer.java b/java/server/src/org/openqa/selenium/remote/server/SeleniumServer.java
@@ -38,6 +38,7 @@
 import org.openqa.selenium.json.Json;
 import org.openqa.selenium.remote.server.jmx.JMXHelper;
 import org.openqa.selenium.remote.server.jmx.ManagedService;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
 
 import java.util.Map;
 import java.util.logging.Logger;
@@ -62,7 +63,9 @@ public class SeleniumServer extends BaseServer implements GridNodeServer {
   private ActiveSessions allSessions;
 
   public SeleniumServer(StandaloneConfiguration configuration) {
-    super(new BaseServerOptions(new AnnotatedConfig(configuration)));
+    super(
+        DistributedTracer.getInstance(),
+        new BaseServerOptions(new AnnotatedConfig(configuration)));
     this.configuration = configuration;
 
     objectName = new JMXHelper().register(this).getObjectName();

diff --git a/java/server/test/org/openqa/grid/BUCK b/java/server/test/org/openqa/grid/BUCK
@@ -30,6 +30,7 @@ java_library(name = 'test',
     '//java/client/src/org/openqa/selenium/safari:safari',
     '//java/client/src/org/openqa/selenium/support/ui:wait',
     '//java/server/src/org/openqa/grid:grid',
+    '//java/server/src/org/openqa/selenium/grid/web:web',
     '//java/server/src/org/openqa/selenium/remote/server:server',
     '//java/client/test/org/openqa/selenium/testing:test-base',
     '//java/client/test/org/openqa/selenium/support:clock',

diff --git a/java/server/test/org/openqa/selenium/grid/router/EndToEndTest.java b/java/server/test/org/openqa/selenium/grid/router/EndToEndTest.java
@@ -43,6 +43,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.net.URI;
 import java.net.URISyntaxException;
@@ -103,8 +104,10 @@ public void withServers() throws URISyntaxException {
     LocalNode localNode = LocalNode.builder(nodeUri, sessions)
         .add(driverCaps, createFactory(nodeUri))
         .build();
-    Server<?> nodeServer = new BaseServer<>(new BaseServerOptions(
-        new MapConfig(ImmutableMap.of("server", ImmutableMap.of("port", port)))));
+    Server<?> nodeServer = new BaseServer<>(
+        DistributedTracer.builder().build(),
+        new BaseServerOptions(
+            new MapConfig(ImmutableMap.of("server", ImmutableMap.of("port", port)))));
     nodeServer.addRoute(Routes.matching(localNode).using(localNode));
     nodeServer.start();
 
@@ -124,7 +127,7 @@ private HttpClient getClient(Server<?> server) {
 
   private Server<?> createServer() {
     int port = PortProber.findFreePort();
-    return new BaseServer<>(new BaseServerOptions(
+    return new BaseServer<>(DistributedTracer.builder().build(), new BaseServerOptions(
         new MapConfig(ImmutableMap.of("server", ImmutableMap.of("port", port)))));
   }
 
@@ -144,4 +147,4 @@ public void execute(HttpRequest req, HttpResponse resp) {
     return caps -> new SpoofSession(caps);
   }
 
-}
+}
diff --git a/java/server/test/org/openqa/selenium/grid/server/BaseServerTest.java b/java/server/test/org/openqa/selenium/grid/server/BaseServerTest.java
@@ -32,17 +32,19 @@
 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.net.URL;
 
 public class BaseServerTest {
 
   private BaseServerOptions emptyOptions = new BaseServerOptions(new MapConfig(ImmutableMap.of()));
+  private DistributedTracer tracer = DistributedTracer.builder().build();
 
   @Test
   public void baseServerStartsAndDoesNothing() throws IOException {
-    Server<?> server = new BaseServer<>(emptyOptions).start();
+    Server<?> server = new BaseServer<>(tracer, emptyOptions).start();
 
     URL url = server.getUrl();
     HttpClient client = HttpClient.Factory.createDefault().createClient(url);
@@ -57,7 +59,7 @@ public void baseServerStartsAndDoesNothing() throws IOException {
 
   @Test
   public void shouldAllowAHandlerToBeRegistered() throws IOException {
-    Server<?> server = new BaseServer<>(emptyOptions);
+    Server<?> server = new BaseServer<>(tracer, emptyOptions);
     server.addRoute(get("/cheese").using((req, res) -> res.setContent("cheddar".getBytes(UTF_8))));
 
     server.start();
@@ -70,7 +72,7 @@ public void shouldAllowAHandlerToBeRegistered() throws IOException {
 
   @Test
   public void ifTwoHandlersRespondToTheSameRequestTheLastOneAddedWillBeUsed() throws IOException {
-    Server<?> server = new BaseServer<>(emptyOptions);
+    Server<?> server = new BaseServer<>(tracer, emptyOptions);
     server.addRoute(get("/status").using((req, res) -> res.setContent("one".getBytes(UTF_8))));
     server.addRoute(get("/status").using((req, res) -> res.setContent("two".getBytes(UTF_8))));
 
@@ -85,7 +87,7 @@ public void ifTwoHandlersRespondToTheSameRequestTheLastOneAddedWillBeUsed() thro
 
   @Test
   public void addHandlersOnceServerIsStartedIsAnError() {
-    Server<BaseServer> server = new BaseServer<>(emptyOptions);
+    Server<BaseServer> server = new BaseServer<>(tracer, emptyOptions);
     server.start();
 
     Assertions.assertThatExceptionOfType(IllegalStateException.class).isThrownBy(

diff --git a/java/server/test/org/openqa/selenium/grid/server/CommandHandlerServletTest.java b/java/server/test/org/openqa/selenium/grid/server/CommandHandlerServletTest.java
@@ -33,6 +33,7 @@
 import org.openqa.selenium.remote.ErrorHandler;
 import org.openqa.selenium.remote.Response;
 import org.openqa.selenium.remote.http.HttpRequest;
+import org.openqa.selenium.remote.tracing.DistributedTracer;
 import org.openqa.testing.FakeHttpServletRequest;
 import org.openqa.testing.FakeHttpServletResponse;
 import org.openqa.testing.UrlInfo;
@@ -72,6 +73,7 @@ public void shouldReturnValueFromHandlerIfUrlMatches() throws IOException {
     String cheerfulGreeting = "Hello, world!";
 
     CommandHandlerServlet servlet = new CommandHandlerServlet(
+        DistributedTracer.builder().build(),
         Routes.matching(req -> true).using((req, res) -> res.setContent(cheerfulGreeting.getBytes(UTF_8))).build());
 
     HttpServletRequest request = requestConverter.apply(new HttpRequest(GET, "/hello-world"));
@@ -86,6 +88,7 @@ public void shouldReturnValueFromHandlerIfUrlMatches() throws IOException {
   @Test
   public void shouldCorrectlyReturnAnUnknownCommandExceptionForUnmappableUrls() throws IOException {
     CommandHandlerServlet servlet = new CommandHandlerServlet(
+        DistributedTracer.builder().build(),
         Routes.matching(req -> false).using((req, res) -> {}).decorateWith(W3CCommandHandler.class).build());
 
     HttpServletRequest request = requestConverter.apply(new HttpRequest(GET, "/missing"));
@@ -102,6 +105,7 @@ public void exceptionsThrownByHandlersAreConvertedToAProperPayload() throws IOEx
     Injector injector = Injector.builder().register(new Json()).build();
 
     CommandHandlerServlet servlet = new CommandHandlerServlet(
+        DistributedTracer.builder().build(),
         Routes.matching(req -> true).using((req, res) -> {
           throw new UnableToSetCookieException("Yowza");
         }).decorateWith(W3CCommandHandler.class).build());
@@ -117,4 +121,4 @@ public void exceptionsThrownByHandlersAreConvertedToAProperPayload() throws IOEx
     assertThat(thrown.getMessage()).startsWith("Yowza");
   }
 
-}
+}