Skip to content

Commit ee03118

Browse files
committed
[tracing] Better handling of tracing through distributor
With these changes in place, it's now possible to follow the 'New Session' command through all the services it touches. *phew*
1 parent 7c27774 commit ee03118

File tree

5 files changed

+26
-6
lines changed

5 files changed

+26
-6
lines changed

java/client/src/org/openqa/selenium/remote/tracing/HttpTracing.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,12 @@
2929
import java.util.LinkedHashMap;
3030
import java.util.Map;
3131
import java.util.Objects;
32+
import java.util.logging.Logger;
3233

3334
public class HttpTracing {
3435

36+
private static final Logger LOG = Logger.getLogger(HttpTracing.class.getName());
37+
3538
private HttpTracing() {
3639
// Utility classes
3740
}
@@ -52,6 +55,9 @@ public static void inject(Tracer tracer, Span span, HttpRequest request) {
5255
Objects.requireNonNull(tracer, "Tracer to use must be set.");
5356
Objects.requireNonNull(request, "Request must be set.");
5457

58+
StackTraceElement caller = Thread.currentThread().getStackTrace()[2];
59+
LOG.info(String.format("Injecting %s into %s at %s:%d", request, span, caller.getClassName(), caller.getLineNumber()));
60+
5561
span.setTag(Tags.HTTP_METHOD.getKey(), request.getMethod().toString());
5662
span.setTag(Tags.HTTP_URL.getKey(), request.getUri());
5763

java/server/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ public CreateSessionResponse newSession(HttpRequest request)
106106
Span previous = tracer.scopeManager().activeSpan();
107107
SpanContext parent = HttpTracing.extract(tracer, request);
108108
Span span = tracer.buildSpan("distributor.new_session").asChildOf(parent).start();
109+
tracer.scopeManager().activate(span);
109110

110111
try (Reader reader = reader(request);
111112
NewSessionPayload payload = NewSessionPayload.create(reader)) {

java/server/src/org/openqa/selenium/grid/distributor/remote/RemoteDistributor.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ public RemoteDistributor add(Node node) {
9090
public void remove(UUID nodeId) {
9191
Objects.requireNonNull(nodeId, "Node ID must be set");
9292
HttpRequest request = new HttpRequest(DELETE, "/se/grid/distributor/node/" + nodeId);
93-
Span span = tracer.scopeManager().activeSpan();
94-
HttpTracing.inject(tracer, span, request);
93+
HttpTracing.inject(tracer, tracer.scopeManager().activeSpan(), request);
9594

9695
HttpResponse response = client.execute(request);
9796

java/server/src/org/openqa/selenium/grid/node/Node.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,14 +111,14 @@ protected Node(Tracer tracer, UUID id, URI uri) {
111111
// "getSessionId" is aggressive about finding session ids, so this needs to be the last
112112
// route the is checked.
113113
matching(req -> getSessionId(req.getUri()).map(SessionId::new).map(this::isSessionOwner).orElse(false))
114-
.to(() -> new ForwardWebDriverCommand(this)),
114+
.to(() -> new ForwardWebDriverCommand(this)).with(new SpanDecorator(tracer, req -> "node.forward_command")),
115115
get("/se/grid/node/owner/{sessionId}")
116116
.to((params) -> new IsSessionOwner(this, json, new SessionId(params.get("sessionId")))).with(new SpanDecorator(tracer, req -> "node.is_session_owner")),
117117
delete("/se/grid/node/session/{sessionId}")
118-
.to((params) -> new StopNodeSession(this, new SessionId(params.get("sessionId")))),
118+
.to((params) -> new StopNodeSession(this, new SessionId(params.get("sessionId")))).with(new SpanDecorator(tracer, req -> "node.stop_session")),
119119
get("/se/grid/node/session/{sessionId}")
120-
.to((params) -> new GetNodeSession(this, json, new SessionId(params.get("sessionId")))),
121-
post("/se/grid/node/session").to(() -> new NewNodeSession(this, json)),
120+
.to((params) -> new GetNodeSession(this, json, new SessionId(params.get("sessionId")))).with(new SpanDecorator(tracer, req -> "node.get_session")),
121+
post("/se/grid/node/session").to(() -> new NewNodeSession(this, json)).with(new SpanDecorator(tracer, req -> "node.new_session")),
122122
get("/se/grid/node/status")
123123
.to(() -> req -> new HttpResponse().setContent(utf8String(json.toJson(getStatus())))).with(new SpanDecorator(tracer, req -> "node.node_status")),
124124
get("/status").to(() -> new StatusHandler(this, json)).with(new SpanDecorator(tracer, req -> "node.status")));

java/server/src/org/openqa/selenium/grid/node/local/LocalNode.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.google.common.collect.ImmutableList;
2727
import com.google.common.collect.ImmutableMap;
2828
import com.google.common.collect.ImmutableSet;
29+
import io.opentracing.Span;
2930
import io.opentracing.Tracer;
3031
import org.openqa.selenium.Capabilities;
3132
import org.openqa.selenium.NoSuchSessionException;
@@ -53,6 +54,7 @@
5354
import java.util.Objects;
5455
import java.util.Optional;
5556
import java.util.UUID;
57+
import java.util.logging.Logger;
5658
import java.util.stream.Collectors;
5759

5860
import static com.google.common.collect.ImmutableSet.toImmutableSet;
@@ -130,8 +132,12 @@ public boolean isSupporting(Capabilities capabilities) {
130132

131133
@Override
132134
public Optional<CreateSessionResponse> newSession(CreateSessionRequest sessionRequest) {
135+
Span span = tracer.scopeManager().activeSpan();
136+
Logger.getLogger(LocalNode.class.getName()).info("Creating new session using span: " + span);
133137
Objects.requireNonNull(sessionRequest, "Session request has not been set.");
134138

139+
if (span != null) {span.setTag("session_count", getCurrentSessionCount());}
140+
135141
if (getCurrentSessionCount() >= maxSessionCount) {
136142
return Optional.empty();
137143
}
@@ -157,6 +163,14 @@ public Optional<CreateSessionResponse> newSession(CreateSessionRequest sessionRe
157163
ActiveSession session = possibleSession.get();
158164
currentSessions.put(session.getId(), slot);
159165

166+
if (span != null) {
167+
span.setTag("session.id", session.getId().toString());
168+
span.setTag("session.capabilities", session.getCapabilities().toString());
169+
span.setTag("session.downstream.dialect", session.getDownstreamDialect().toString());
170+
span.setTag("session.upstream.dialect", session.getUpstreamDialect().toString());
171+
span.setTag("session.uri", session.getUri().toString());
172+
}
173+
160174
// The session we return has to look like it came from the node, since we might be dealing
161175
// with a webdriver implementation that only accepts connections from localhost
162176
Session externalSession = createExternalSession(session, externalUri);

0 commit comments

Comments
 (0)