Skip to content

Commit afa60e7

Browse files
committed
[java] Implementing TracedCommandExecutor as a decorator instead of polluting HttpCommandExecutor
1 parent e35bcab commit afa60e7

File tree

3 files changed

+115
-68
lines changed

3 files changed

+115
-68
lines changed

java/client/src/org/openqa/selenium/remote/HttpCommandExecutor.java

Lines changed: 58 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,6 @@
3131
import org.openqa.selenium.remote.http.HttpClient;
3232
import org.openqa.selenium.remote.http.HttpRequest;
3333
import org.openqa.selenium.remote.http.HttpResponse;
34-
import org.openqa.selenium.remote.tracing.Span;
35-
import org.openqa.selenium.remote.tracing.TracedHttpClient;
36-
import org.openqa.selenium.remote.tracing.Tracer;
37-
import org.openqa.selenium.remote.tracing.opentelemetry.OpenTelemetryTracer;
3834

3935
import java.io.IOException;
4036
import java.net.MalformedURLException;
@@ -57,7 +53,6 @@ public class HttpCommandExecutor implements CommandExecutor, NeedsLocalLogs {
5753
private final HttpClient client;
5854
private final HttpClient.Factory httpClientFactory;
5955
private final Map<String, CommandInfo> additionalCommands;
60-
private final Tracer tracer;
6156
private CommandCodec<HttpRequest> commandCodec;
6257
private ResponseCodec<HttpResponse> responseCodec;
6358

@@ -115,8 +110,7 @@ public HttpCommandExecutor(
115110
}
116111
remoteServer = config.baseUrl();
117112
this.additionalCommands = additionalCommands;
118-
tracer = OpenTelemetryTracer.getInstance();
119-
this.httpClientFactory = new TracedHttpClient.Factory(tracer, httpClientFactory);
113+
this.httpClientFactory = httpClientFactory;
120114
this.client = this.httpClientFactory.createClient(config);
121115
}
122116

@@ -149,75 +143,72 @@ public URL getAddressOfRemoteServer() {
149143

150144
@Override
151145
public Response execute(Command command) throws IOException {
152-
try (Span commandSpan = tracer.getCurrentContext().createSpan("command")) {
153-
commandSpan.setAttribute("command", command.toString());
154-
if (command.getSessionId() == null) {
155-
if (QUIT.equals(command.getName())) {
156-
return new Response();
157-
}
158-
if (!GET_ALL_SESSIONS.equals(command.getName())
159-
&& !NEW_SESSION.equals(command.getName())) {
160-
throw new NoSuchSessionException(
161-
"Session ID is null. Using WebDriver after calling quit()?");
162-
}
146+
if (command.getSessionId() == null) {
147+
if (QUIT.equals(command.getName())) {
148+
return new Response();
163149
}
164-
165-
if (NEW_SESSION.equals(command.getName())) {
166-
if (commandCodec != null) {
167-
throw new SessionNotCreatedException("Session already exists");
168-
}
169-
ProtocolHandshake handshake = new ProtocolHandshake();
170-
log(LogType.PROFILER, new HttpProfilerLogEntry(command.getName(), true));
171-
ProtocolHandshake.Result result = handshake.createSession(client, command);
172-
Dialect dialect = result.getDialect();
173-
commandCodec = dialect.getCommandCodec();
174-
for (Map.Entry<String, CommandInfo> entry : additionalCommands.entrySet()) {
175-
defineCommand(entry.getKey(), entry.getValue());
176-
}
177-
responseCodec = dialect.getResponseCodec();
178-
log(LogType.PROFILER, new HttpProfilerLogEntry(command.getName(), false));
179-
return result.createResponse();
150+
if (!GET_ALL_SESSIONS.equals(command.getName())
151+
&& !NEW_SESSION.equals(command.getName())) {
152+
throw new NoSuchSessionException(
153+
"Session ID is null. Using WebDriver after calling quit()?");
180154
}
155+
}
181156

182-
if (commandCodec == null || responseCodec == null) {
183-
throw new WebDriverException(
184-
"No command or response codec has been defined. Unable to proceed");
157+
if (NEW_SESSION.equals(command.getName())) {
158+
if (commandCodec != null) {
159+
throw new SessionNotCreatedException("Session already exists");
160+
}
161+
ProtocolHandshake handshake = new ProtocolHandshake();
162+
log(LogType.PROFILER, new HttpProfilerLogEntry(command.getName(), true));
163+
ProtocolHandshake.Result result = handshake.createSession(client, command);
164+
Dialect dialect = result.getDialect();
165+
commandCodec = dialect.getCommandCodec();
166+
for (Map.Entry<String, CommandInfo> entry : additionalCommands.entrySet()) {
167+
defineCommand(entry.getKey(), entry.getValue());
185168
}
169+
responseCodec = dialect.getResponseCodec();
170+
log(LogType.PROFILER, new HttpProfilerLogEntry(command.getName(), false));
171+
return result.createResponse();
172+
}
186173

187-
HttpRequest httpRequest = commandCodec.encode(command);
174+
if (commandCodec == null || responseCodec == null) {
175+
throw new WebDriverException(
176+
"No command or response codec has been defined. Unable to proceed");
177+
}
188178

189-
// Ensure that we set the required headers
190-
if (httpRequest.getHeader("Content-Type") == null) {
191-
httpRequest.addHeader("Content-Type", JSON_UTF_8);
192-
}
179+
HttpRequest httpRequest = commandCodec.encode(command);
193180

194-
try {
195-
log(LogType.PROFILER, new HttpProfilerLogEntry(command.getName(), true));
196-
HttpResponse httpResponse = client.execute(httpRequest);
197-
log(LogType.PROFILER, new HttpProfilerLogEntry(command.getName(), false));
198-
199-
Response response = responseCodec.decode(httpResponse);
200-
if (response.getSessionId() == null) {
201-
if (httpResponse.getTargetHost() != null) {
202-
response.setSessionId(getSessionId(httpResponse.getTargetHost()).orElse(null));
203-
} else {
204-
// Spam in the session id from the request
205-
response.setSessionId(command.getSessionId().toString());
206-
}
207-
}
208-
if (QUIT.equals(command.getName())) {
209-
client.close();
210-
httpClientFactory.cleanupIdleClients();
211-
}
212-
return response;
213-
} catch (UnsupportedCommandException e) {
214-
if (e.getMessage() == null || "".equals(e.getMessage())) {
215-
throw new UnsupportedOperationException(
216-
"No information from server. Command name was: " + command.getName(),
217-
e.getCause());
181+
// Ensure that we set the required headers
182+
if (httpRequest.getHeader("Content-Type") == null) {
183+
httpRequest.addHeader("Content-Type", JSON_UTF_8);
184+
}
185+
186+
try {
187+
log(LogType.PROFILER, new HttpProfilerLogEntry(command.getName(), true));
188+
HttpResponse httpResponse = client.execute(httpRequest);
189+
log(LogType.PROFILER, new HttpProfilerLogEntry(command.getName(), false));
190+
191+
Response response = responseCodec.decode(httpResponse);
192+
if (response.getSessionId() == null) {
193+
if (httpResponse.getTargetHost() != null) {
194+
response.setSessionId(getSessionId(httpResponse.getTargetHost()).orElse(null));
195+
} else {
196+
// Spam in the session id from the request
197+
response.setSessionId(command.getSessionId().toString());
218198
}
219-
throw e;
220199
}
200+
if (QUIT.equals(command.getName())) {
201+
client.close();
202+
httpClientFactory.cleanupIdleClients();
203+
}
204+
return response;
205+
} catch (UnsupportedCommandException e) {
206+
if (e.getMessage() == null || "".equals(e.getMessage())) {
207+
throw new UnsupportedOperationException(
208+
"No information from server. Command name was: " + command.getName(),
209+
e.getCause());
210+
}
211+
throw e;
221212
}
222213
}
223214
}

java/client/src/org/openqa/selenium/remote/RemoteWebDriver.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,12 @@
5858
import org.openqa.selenium.logging.Logs;
5959
import org.openqa.selenium.logging.NeedsLocalLogs;
6060
import org.openqa.selenium.print.PrintOptions;
61+
import org.openqa.selenium.remote.http.ClientConfig;
62+
import org.openqa.selenium.remote.http.HttpClient;
6163
import org.openqa.selenium.remote.internal.WebElementToJsonConverter;
64+
import org.openqa.selenium.remote.tracing.TracedHttpClient;
65+
import org.openqa.selenium.remote.tracing.Tracer;
66+
import org.openqa.selenium.remote.tracing.opentelemetry.OpenTelemetryTracer;
6267
import org.openqa.selenium.virtualauthenticator.Credential;
6368
import org.openqa.selenium.virtualauthenticator.HasVirtualAuthenticator;
6469
import org.openqa.selenium.virtualauthenticator.VirtualAuthenticator;
@@ -134,7 +139,16 @@ public RemoteWebDriver(Capabilities capabilities) {
134139
}
135140

136141
public RemoteWebDriver(URL remoteAddress, Capabilities capabilities) {
137-
this(new HttpCommandExecutor(remoteAddress), capabilities);
142+
this(createTracedExecutorWithTracedHttpClient(remoteAddress), capabilities);
143+
}
144+
145+
private static CommandExecutor createTracedExecutorWithTracedHttpClient(URL remoteAddress) {
146+
Tracer tracer = OpenTelemetryTracer.getInstance();
147+
CommandExecutor executor = new HttpCommandExecutor(
148+
Collections.emptyMap(),
149+
ClientConfig.defaultConfig().baseUrl(remoteAddress),
150+
new TracedHttpClient.Factory(tracer, HttpClient.Factory.createDefault()));
151+
return new TracedCommandExecutor(executor, tracer);
138152
}
139153

140154
public RemoteWebDriver(CommandExecutor executor, Capabilities capabilities) {
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Licensed to the Software Freedom Conservancy (SFC) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The SFC licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.openqa.selenium.remote;
19+
20+
import org.openqa.selenium.remote.tracing.Span;
21+
import org.openqa.selenium.remote.tracing.Tracer;
22+
23+
import java.io.IOException;
24+
25+
public class TracedCommandExecutor implements CommandExecutor {
26+
27+
private final CommandExecutor delegate;
28+
private final Tracer tracer;
29+
30+
public TracedCommandExecutor(CommandExecutor delegate, Tracer tracer) {
31+
this.delegate = delegate;
32+
this.tracer = tracer;
33+
}
34+
35+
@Override
36+
public Response execute(Command command) throws IOException {
37+
try (Span commandSpan = tracer.getCurrentContext().createSpan("command")) {
38+
commandSpan.setAttribute("command", command.toString());
39+
return delegate.execute(command);
40+
}
41+
}
42+
}

0 commit comments

Comments
 (0)