Skip to content
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
ec94fd3
[java] Unify debug-logging switches into one real mechanism
MohabMohie Jul 29, 2026
ff32eab
[java][bidi][devtools] Raise the debug-logger switch from direct-cons…
MohabMohie Jul 29, 2026
a31a75a
[java][bidi][devtools] Document the debug-logging Connection construc…
MohabMohie Jul 29, 2026
4296a51
[java] Repair Debug's externally-removed handler; scope LoggingOption…
MohabMohie Jul 29, 2026
072512c
[java] Decide Debug's level raise off the effective level, not just i…
MohabMohie Jul 29, 2026
4d51cec
Merge branch 'trunk' into debug-logging-mechanism-stage1
MohabMohie Jul 29, 2026
c569d65
[java] Enforce, not just assert, the null-own-level precondition in t…
MohabMohie Jul 29, 2026
325168c
[java] Stop suppressing Selenium debug records from Grid's root handl…
MohabMohie Jul 29, 2026
d270074
[java] Assert the no-duplicate marker appears exactly once on stderr
MohabMohie Jul 29, 2026
59383f6
[java] Force the stubbed SE_DEBUG off before re-syncing Debug in cleanup
MohabMohie Jul 29, 2026
b35594b
Merge branch 'trunk' into debug-logging-mechanism-stage1
MohabMohie Jul 29, 2026
529bb21
Merge branch 'trunk' into debug-logging-mechanism-stage1
MohabMohie Jul 29, 2026
96ec9cf
Fix logging snapshot in RetryRequest: use live Debug.getDebugLogLevel…
MohabMohie Jul 29, 2026
ae54eb0
Move Debug.configureLogger() out of constructors' pre-validation path…
MohabMohie Jul 29, 2026
f9e03d9
Merge branch 'trunk' into debug-logging-mechanism-stage1
MohabMohie Jul 30, 2026
2f5c101
[java] Scope debug property to JUL diagnostics
MohabMohie Aug 1, 2026
1a69c0d
[java] Propagate requested debug level
MohabMohie Aug 1, 2026
65a4e4e
[java] Repair configured debug logger level
MohabMohie Aug 1, 2026
9b8a93e
[java] Restore repaired debug logger level
MohabMohie Aug 1, 2026
d536e3a
[java] Make debug handler inspection atomic
MohabMohie Aug 1, 2026
f87fd10
[java] Remove unused debug handler APIs
MohabMohie Aug 1, 2026
c7ba454
[java] Clarify debug logger configuration
MohabMohie Aug 3, 2026
7372c1e
[java] Clarify debug logger repair behavior
MohabMohie Aug 3, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions java/src/org/openqa/selenium/bidi/Connection.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.jspecify.annotations.Nullable;
import org.openqa.selenium.Beta;
import org.openqa.selenium.WebDriverException;
import org.openqa.selenium.internal.Debug;
import org.openqa.selenium.internal.Either;
import org.openqa.selenium.internal.Require;
import org.openqa.selenium.json.Json;
Expand Down Expand Up @@ -77,7 +78,21 @@ public class Connection implements Closeable {
private final WebSocket socket;
private final AtomicBoolean underlyingSocketClosed = new AtomicBoolean(false);

/**
* Creates a new BiDi connection to the given URL using the given HTTP client. Before the socket
* opens, the current Selenium debug switches are reflected onto the {@code org.openqa.selenium}
* logger via {@link Debug#configureLogger()}, so connections constructed directly (bypassing
* {@code RemoteWebDriver}/{@code DriverFinder}) still honor {@code -Dselenium.debug} and friends.
*
* @param client the HTTP client used to open the underlying web socket; must not be null
* @param url the URL to open the web socket connection to; must not be null
*/
public Connection(HttpClient client, String url) {
// Reflect the current debug switches before this connection starts logging its wire
// diagnostics at FINE -- callers that construct a Connection directly (never going through
// RemoteWebDriver or DriverFinder) would otherwise never trigger the raise. Idempotent and
// cheap, same pattern as DriverFinder.getBinaryPaths().
Debug.configureLogger();
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Outdated
Require.nonNull("HTTP client", client);
Require.nonNull("URL to connect to", url);

Expand Down
19 changes: 19 additions & 0 deletions java/src/org/openqa/selenium/devtools/Connection.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.jspecify.annotations.Nullable;
import org.openqa.selenium.WebDriverException;
import org.openqa.selenium.devtools.idealized.target.model.SessionID;
import org.openqa.selenium.internal.Debug;
import org.openqa.selenium.internal.Either;
import org.openqa.selenium.internal.Require;
import org.openqa.selenium.json.Json;
Expand Down Expand Up @@ -91,7 +92,25 @@ public Connection(HttpClient client, String url) {
this(client, url, ClientConfig.defaultConfig());
}

/**
* Creates a new CDP connection to the given URL using the given HTTP client and client
* configuration. Before the socket opens, the current Selenium debug switches are reflected onto
* the {@code org.openqa.selenium} logger via {@link Debug#configureLogger()}, so connections
* constructed directly (bypassing {@code RemoteWebDriver}/{@code DriverFinder}) still honor
* {@code -Dselenium.debug} and friends. The deprecated 2-arg constructor delegates here, so this
* single call point covers both.
*
* @param client the HTTP client used to open the underlying web socket; must not be null
* @param url the URL to open the web socket connection to
* @param clientConfig the client configuration to use when opening the connection
*/
public Connection(HttpClient client, String url, ClientConfig clientConfig) {
// Reflect the current debug switches before this connection starts logging its wire
// diagnostics at FINE -- callers that construct a Connection directly (never going through
// RemoteWebDriver or DriverFinder) would otherwise never trigger the raise. Idempotent and
// cheap, same pattern as DriverFinder.getBinaryPaths(). The deprecated 2-arg constructor
// delegates here, so this single call point covers both.
Debug.configureLogger();
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Outdated
this.client = Require.nonNull("HTTP client", client);
this.wsConfig = wsClientConfig(clientConfig, url);
this.socket = this.client.openSocket(new HttpRequest(GET, wsConfig.baseUri()), new Listener());
Expand Down
1 change: 1 addition & 0 deletions java/src/org/openqa/selenium/grid/log/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ java_library(
visibility = [
"//java/src/org/openqa/selenium/grid:__subpackages__",
"//java/src/org/openqa/selenium/remote/server:__subpackages__",
"//java/test/org/openqa/selenium/grid/log:__pkg__",
],
deps = [
"//java:auto-service",
Expand Down
82 changes: 78 additions & 4 deletions java/src/org/openqa/selenium/grid/log/LoggingOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.Enumeration;
import java.util.List;
import java.util.Locale;
import java.util.logging.Filter;
import java.util.logging.Handler;
import java.util.logging.Level;
import java.util.logging.LogManager;
Expand Down Expand Up @@ -85,11 +86,21 @@ public String getLogEncoding() {
return config.get(LOGGING_SECTION, "log-encoding").orElse(null);
}

/**
* Resolves the Grid log level from the {@code log-level} entry of the logging config section and
* stores it for {@link #configureLogging()}. Any active Selenium debug switch ({@code SE_DEBUG},
* {@code -Dselenium.debug=true}, {@code -Dselenium.webdriver.verbose=true}) overrides the
* configured value and forces {@link Level#FINE}. An unparseable configured value falls back to
* the default ({@code INFO}).
*
* @return this instance, for method chaining
*/
public LoggingOptions setLoggingLevel() {
String configLevel = config.get(LOGGING_SECTION, "log-level").orElse(DEFAULT_LOG_LEVEL);
if (Debug.isDebugAll()) {
if (Debug.isDebugAll() || Debug.isDebugging()) {
System.err.println(
"WARNING: Environment Variable `SE_DEBUG` is set; forcing Grid log level to FINE and"
"WARNING: Selenium debug logging is enabled (`SE_DEBUG`, `-Dselenium.debug=true`, or"
+ " `-Dselenium.webdriver.verbose=true`); forcing Grid log level to FINE and"
+ " overriding configured log level.");
configLevel = Level.FINE.getName();
}
Expand Down Expand Up @@ -123,11 +134,29 @@ public Tracer getTracer() {
return OpenTelemetryTracer.getInstance();
}

/**
* Configures logging for the Grid, wiring up Grid's own log handlers and, before doing so,
* reflecting the current Selenium debug switches onto the {@code org.openqa.selenium} logger via
* {@link Debug#configureLogger()}. Returns early without changing anything if logging is disabled
* ({@code enable} is {@code false}) or if an external {@code java.util.logging.config} class/file
* is configured, in which case that configuration takes priority. Otherwise, every other
* registered logger has its handlers stripped to start from a clean slate before Grid's root
* handlers are installed.
*/
public void configureLogging() {
if (!config.getBool(LOGGING_SECTION, "enable").orElse(DEFAULT_CONFIGURE_LOGGING)) {
return;
}

// Reflect the current debug switches onto the shared org.openqa.selenium logger before
// anything else below -- in particular, before the external-JUL-config early return just
// below hands the rest of logging setup off entirely. Without this, Selenium's own FINE-level
// wire diagnostics (RequestConverter, the BiDi/CDP Connection classes) stay invisible under
// -Dselenium.debug=true whenever an external `java.util.logging.config.*` property is set,
// since nothing else on Grid's startup path would ever call this. Idempotent and cheap, same
// chokepoint pattern as DriverFinder.getBinaryPaths().
Debug.configureLogger();

Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Outdated
String configClass = System.getProperty("java.util.logging.config.class");
String configFile = System.getProperty("java.util.logging.config.file");

Expand All @@ -137,15 +166,32 @@ public void configureLogging() {
return;
}

// Remove all handlers from existing loggers
// Remove all handlers from existing loggers, except org.openqa.selenium:
// Debug.configureLogger()
// above may have just installed a handler there for debug-mode output, and this loop would
// otherwise strip it moments later (Debug holds a strong static reference so that logger stays
// registered here too). Debug's own installed-handler bookkeeping has no way to learn a handler
// was removed out from under it, so once stripped its idempotency guard would prevent ever
// reinstalling one until the debug switch is toggled off and back on.
LogManager logManager = LogManager.getLogManager();
Enumeration<String> names = logManager.getLoggerNames();
while (names.hasMoreElements()) {
Logger logger = logManager.getLogger(names.nextElement());
String name = names.nextElement();
Logger logger = logManager.getLogger(name);
if (logger == null) {
continue;
}

if ("org.openqa.selenium".equals(name)) {
// Strip everything except the handler Debug.configureLogger() installed above -- an
// unrelated handler some other code attached to this logger must still be reset here,
// same as on any other logger; only Debug's own handler is exempt.
Arrays.stream(logger.getHandlers())
.filter(handler -> !Debug.isOwnHandler(handler))
.forEach(logger::removeHandler);
continue;
}

Arrays.stream(logger.getHandlers()).forEach(logger::removeHandler);
}

Expand All @@ -160,17 +206,45 @@ public void configureLogging() {
Handler handler = new FlushingHandler(out);
handler.setFormatter(new TerseFormatter(getLogTimestampFormat()));
handler.setLevel(level);
handler.setFilter(rootHandlerFilter());
configureLogEncoding(logger, encoding, handler);
}

if (isUsingStructuredLogging()) {
Handler handler = new FlushingHandler(out);
handler.setFormatter(new JsonFormatter());
handler.setLevel(level);
handler.setFilter(rootHandlerFilter());
configureLogEncoding(logger, encoding, handler);
}
}

/**
* Records that Debug.configureLogger()'s own handler on {@code org.openqa.selenium} already
* prints (FINE/CONFIG-range records from that logger or a descendant, while its handler is
* installed) must not also print through this root handler, PROVIDED this root handler's
* destination is the one Debug's handler also writes to -- that handler's own useParentHandlers
* is never disabled, so the same record reaches both. Debug's own handler is always a {@code
* ConsoleHandler}, which per its JDK contract always targets {@code System.err}; this root
* handler's destination only coincides with that when {@link Debug#isDebugAll()} ({@code
* SE_DEBUG}) is set AND no {@code log-file} is configured -- {@link #getOutputStream()} then
* defaults this handler to {@code System.err} too. When debugging is instead enabled via {@code
* -Dselenium.debug=true}/{@code -Dselenium.webdriver.verbose=true} without {@code SE_DEBUG},
* {@link #getOutputStream()} defaults to {@code System.out} -- a genuinely different destination
* from Debug's handler -- so suppressing there would make the record invisible to an operator
* watching Grid's own stdout/structured log output instead of de-duplicating it. A configured
* log-file is a genuinely separate destination Debug never writes to either, for the same reason.
* INFO-and-above {@code org.openqa.selenium} records, and everything from every other logger, are
* untouched either way: Debug's handler never covered those in the first place.
*/
private Filter rootHandlerFilter() {
boolean logFileConfigured = config.get(LOGGING_SECTION, "log-file").isPresent();
boolean sameDestinationAsDebugHandler = !logFileConfigured && Debug.isDebugAll();
return record ->
!sameDestinationAsDebugHandler
|| !Debug.isHandledBySeleniumDebugHandler(record.getLoggerName(), record.getLevel());
}

private void configureLogEncoding(Logger logger, @Nullable String encoding, Handler handler) {
String message;
try {
Expand Down
Loading