SeleniumHQ / SeleniumHQ/selenium

[🚀 Feature]: Make Java Logging Consistent

Open
#12,892 3 comments 1 reaction 0 assignees View on GitHub
A-needs decision C-java
Dominant language
Java
Stars
34.5k
Forks
8.7k
Avg merge
2d 1h
Merged PRs (30d)
92

Description

### Feature and motivation

Selenium library is currently taking an interesting approach to logging.

Many places in the code use [`getDebugLogLevel()`](https://github.com/SeleniumHQ/selenium/blob/selenium-4.13.0/java/src/org/openqa/selenium/internal/Debug.java#L46) to determine the level that something should be logged at.

That code is checking to see whether either of [two property values](https://github.com/SeleniumHQ/selenium/blob/selenium-4.13.0/java/src/org/openqa/selenium/internal/Debug.java#L32-L33) have been set or the code is in [Debug mode](https://github.com/SeleniumHQ/selenium/blob/selenium-4.13.0/java/src/org/openqa/selenium/internal/Debug.java#L29-L31) (e.g., IntelliJ debugger), and if it is, it logs things at the `INFO` level instead of the `FINE` level.

The nice thing about this approach is that we can tell people to do:
```
System.setProperty("selenium.debug", "true")
```
instead of having them learn how to use the root logger, etc as described here: https://www.selenium.dev/documentation/webdriver/troubleshooting/logging/

The other nice thing is that we don't have to worry about what other logging handlers they might be using, we don't have to override anything, we can just make our code log at a default visible level.

The less nice thing about this approach is that it isn't obvious why it's happening or how to change it.

Also, the toggle is based on a static block on the class, so changing the System property during the test will not change the behavior, which is limiting.

More importantly, right now some things are logged this way and some things aren't and I think we need to be consistent one way or another.

Options:
1. Deprecate `selenium.webdriver.verbose` and `selenium.debug` and remove the whole class
2. Move everything that is "general debug" information to use `getDebugLogLevel()` (keep the things I changed to log at FINER level at the FINER level — https://github.com/SeleniumHQ/selenium/pull/12866)
3. Remove the conditional to toggle this on automatically in Debug mode
4. Have the system property change the level the user is logging things rather than changing the level the line is getting logged at (this could break things for users if they have other implementations)
5. Let the the user change the debug mode on the fly via System property

The first feels too drastic, but I'm ok with requesting people use an alternate solution if we agree that's better.

Changing the user's logging level for them could be, depending:
```java
Logger rootLogger = Logger.getLogger("");
Arrays.stream(logger.getHandlers()).forEach(handler -> {
handler.setLevel(Level.FINE);
});
```

### Usage example

n/a

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.