Skip to content

Commit

Permalink
Continue wiring up distributed tracing for the new Grid
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
shs96c committed Nov 11, 2018
1 parent 00a7ba7 commit 2778509
Show file tree
Hide file tree
Showing 13 changed files with 60 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
6 changes: 4 additions & 2 deletions java/server/src/org/openqa/selenium/grid/commands/Hub.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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();
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand All @@ -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);
Expand All @@ -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());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 2778509

Please sign in to comment.