From b66fe3e8dcbf1f5a9f4a3e234773e40f8f46b396 Mon Sep 17 00:00:00 2001 From: Simon Stewart Date: Sat, 10 Nov 2018 21:44:24 +0000 Subject: [PATCH] Add the distributed tracer to the grid --- .../openqa/selenium/grid/commands/Hub.java | 5 ++++- .../selenium/grid/commands/Standalone.java | 5 ++++- .../distributor/httpd/DistributorServer.java | 3 ++- .../selenium/grid/node/httpd/NodeServer.java | 3 ++- .../grid/router/httpd/RouterServer.java | 3 ++- .../selenium/grid/server/BaseServer.java | 7 +++++-- .../grid/server/CommandHandlerServlet.java | 21 ++++++++++++------- .../sessionmap/httpd/SessionMapServer.java | 4 ++-- .../remote/server/SeleniumServer.java | 5 ++++- java/server/test/org/openqa/grid/BUCK | 1 + .../selenium/grid/router/EndToEndTest.java | 11 ++++++---- .../selenium/grid/server/BaseServerTest.java | 10 +++++---- .../server/CommandHandlerServletTest.java | 6 +++++- 13 files changed, 58 insertions(+), 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 index 15e0cb81a2a43..d9cd56346e9b3 100644 --- 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 index 26d3e4471a2c8..c71007272b0d3 100644 --- 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 index 08889e57507ad..691896cef4a2e 100644 --- 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 index 6820824090268..11c9186275411 100644 --- 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 index c857c5b153b7a..0eee3cc7a341b 100644 --- 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 index af5dcfe514b53..82bc2eafc7d39 100644 --- 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 implements Server { 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 index 0fce103bab874..81de58160c396 100644 --- 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 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 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 index c213b83fbc4ff..9ef059af2a77a 100644 --- 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 index 661d329f03763..cda185d8c28f7 100644 --- 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 index a064bac23b74f..6db82e128ccef 100644 --- 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 index da5271dea84e8..b8b801026fbcb 100644 --- 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); } -} \ No newline at end of file +} diff --git a/java/server/test/org/openqa/selenium/grid/server/BaseServerTest.java b/java/server/test/org/openqa/selenium/grid/server/BaseServerTest.java index fbb46755be6a0..5b1723a4a12e3 100644 --- a/java/server/test/org/openqa/selenium/grid/server/BaseServerTest.java +++ b/java/server/test/org/openqa/selenium/grid/server/BaseServerTest.java @@ -32,6 +32,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.net.URL; @@ -39,10 +40,11 @@ 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 server = new BaseServer<>(emptyOptions); + Server 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 index dd04c1635abe7..f56631561f54a 100644 --- 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"); } -} \ No newline at end of file +}