Skip to content

[java] deprecate unused logging implementation#16889

Merged
titusfortner merged 1 commit into
trunkfrom
java_logs
Jan 12, 2026
Merged

[java] deprecate unused logging implementation#16889
titusfortner merged 1 commit into
trunkfrom
java_logs

Conversation

@titusfortner

@titusfortner titusfortner commented Jan 11, 2026

Copy link
Copy Markdown
Member

User description

We removed a lot of this functionality in the Selenium 4 upgrade to Grid, without marking the methods as deprecated.
The tests for logging have been completely guarded so nothing has been running
Pretty much the only thing that works is what chromedriver supports: https://github.com/bayandin/chromedriver/blob/2ef5f580eeb49ae992f4ad4c9fada75614212d85/logging.h#L30-L33

💥 What does this PR do?

  • Deprecate everything that does not currently work

🔧 Implementation Notes

A lot of this doesn't do anything and could be completely removed, but I think a lot of Selenium users currently have vestigial references to logging that don't do anything. Rather than figuring out what would and wouldn't break, I'm just throwing a deprecation on everything.

Eventually all of this will go away once we have BiDi enabled.

💡 Additional Considerations

I think we keep the user facing methods in Selenium 5 just deprecated, but can probably remove all the implementation code.

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Enhancement


Description

  • Mark logging classes and methods as deprecated with forRemoval=true

  • Update RemoteLogs to handle deprecated log types gracefully

  • Modify tests to verify deprecated behavior returns empty results

  • Add @SuppressWarnings annotations to suppress deprecation warnings


File Walkthrough

Relevant files
Documentation
15 files
CompositeLocalLogs.java
Add deprecation annotation and javadoc                                     
+4/-0     
HandlerBasedLocalLogs.java
Add deprecation annotation and javadoc                                     
+7/-1     
LocalLogs.java
Add deprecation annotation and javadoc                                     
+7/-1     
LogCombiner.java
Add deprecation annotation and javadoc                                     
+7/-0     
LogType.java
Deprecate CLIENT, PROFILER, and SERVER log types                 
+18/-3   
LoggingHandler.java
Add deprecation annotation and javadoc                                     
+4/-0     
NeedsLocalLogs.java
Add deprecation annotation and javadoc                                     
+7/-0     
SessionLogHandler.java
Add deprecation annotation and javadoc                                     
+7/-0     
SessionLogs.java
Add deprecation annotation and javadoc                                     
+7/-1     
StoringLocalLogs.java
Add deprecation annotation and javadoc                                     
+4/-0     
EventType.java
Add deprecation annotation and javadoc                                     
+7/-0     
HttpProfilerLogEntry.java
Add deprecation annotation and javadoc                                     
+7/-0     
ProfilerLogEntry.java
Add deprecation annotation and javadoc                                     
+7/-0     
HttpCommandExecutor.java
Deprecate setLocalLogs method with suppressions                   
+8/-0     
TracedCommandExecutor.java
Deprecate setLocalLogs method with suppressions                   
+5/-0     
Bug fix
1 files
RemoteLogs.java
Handle deprecated log types and add null-safe LocalLogs   
+58/-12 
Enhancement
1 files
RemoteWebDriver.java
Extract LocalLogs initialization with deprecation handling
+13/-5   
Tests
5 files
JsonOutputTest.java
Add deprecation suppression to logging test                           
+1/-0     
AvailableLogsTest.java
Update tests to verify deprecated log types return empty 
+20/-21 
PerformanceLoggingMockTest.java
Add deprecation suppression and test comment                         
+2/-0     
PerformanceLoggingTest.java
Update test to verify deprecated profiler logs return empty
+5/-2     
RemoteLogsTest.java
Update tests for deprecated log type behavior                       
+6/-9     

@qodo-code-review

qodo-code-review Bot commented Jan 11, 2026

Copy link
Copy Markdown
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Stacktrace in warnings: The new warning log for omitted remote profiler logs logs the caught exception e, which
may expose internal details if application logs are user-visible.

Referred Code
  LOG.log(
      Level.WARNING, "Remote profiler logs are not available and have been omitted.", e);
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Exception logged: The new LOG.log(Level.WARNING, ..., e) may emit stack traces and potentially sensitive
runtime details depending on downstream log configuration and handlers.

Referred Code
  LOG.log(
      Level.WARNING, "Remote profiler logs are not available and have been omitted.", e);
}

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review

qodo-code-review Bot commented Jan 11, 2026

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Deprecate entire logging API

Deprecate the entire user-facing logging API, including driver.manage().logs(),
Logs, and LogEntries, to clearly signal that the feature is obsolete and will be
removed.

Examples:

Solution Walkthrough:

Before:

// In org.openqa.selenium.WebDriver.Options
public interface Options {
  // The main entry point to the logging API is not deprecated.
  Logs logs();
  // ...
}

// In org.openqa.selenium.logging.Logs
// The main interface for log retrieval is not deprecated.
public interface Logs {
  LogEntries get(String logType);
  Set<String> getAvailableLogTypes();
}

// In org.openqa.selenium.logging.LogEntries
// The class holding log entries is not deprecated.
public class LogEntries implements Iterable<LogEntry> { //...
}

After:

// In org.openqa.selenium.WebDriver.Options
public interface Options {
  /**
   * @deprecated The W3C WebDriver specification does not support logging.
   *   This will be removed in a future release.
   */
  @Deprecated(forRemoval = true)
  Logs logs();
  // ...
}

// In org.openqa.selenium.logging.Logs
@Deprecated(forRemoval = true)
public interface Logs { //...
}

// In org.openqa.selenium.logging.LogEntries
@Deprecated(forRemoval = true)
public class LogEntries implements Iterable<LogEntry> { //...
}

Suggestion importance[1-10]: 8

__

Why: This is a valid and significant suggestion that addresses the overall deprecation strategy, proposing a more complete and clearer approach for users by deprecating the entire public logging API.

Medium
General
Avoid confusing deprecation warnings

Move the deprecation warning for CLIENT and PROFILER log types to only be logged
when empty logs are returned, avoiding confusion when the old functionality is
still active.

java/src/org/openqa/selenium/remote/RemoteLogs.java [68-105]

 @Override
 @SuppressWarnings("deprecation")
 public LogEntries get(String logType) {
   if (LogType.CLIENT.equals(logType)) {
+    if (localLogs != null) {
+      return getLocalEntries(logType);
+    }
     LOG.warning(
         "LogType.CLIENT is deprecated and not part of the W3C WebDriver specification. "
             + "Returning empty log entries.");
-    if (localLogs != null) {
-      return getLocalEntries(logType);
-    }
     return new LogEntries(Collections.emptyList());
   }
   if (LogType.PROFILER.equals(logType)) {
-    LOG.warning(
-        "LogType.PROFILER is deprecated and not part of the W3C WebDriver specification. "
-            + "Returning empty log entries.");
     if (localLogs != null) {
       LogEntries remoteEntries = new LogEntries(new ArrayList<>());
       try {
         remoteEntries = getRemoteEntries(logType);
       } catch (WebDriverException e) {
         // An exception may be thrown if the WebDriver server does not recognize profiler logs.
         // In this case, the user should be able to see the local profiler logs.
         LOG.log(
             Level.WARNING, "Remote profiler logs are not available and have been omitted.", e);
       }
       return LogCombiner.combine(remoteEntries, getLocalEntries(logType));
     }
+    LOG.warning(
+        "LogType.PROFILER is deprecated and not part of the W3C WebDriver specification. "
+            + "Returning empty log entries.");
     return new LogEntries(Collections.emptyList());
   }
   if (LogType.SERVER.equals(logType)) {
     LOG.warning(
         "LogType.SERVER is deprecated. Selenium Grid no longer supports server logs. "
             + "Returning empty log entries.");
     return new LogEntries(Collections.emptyList());
   }
   return getRemoteEntries(logType);
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the deprecation warning is logged even when the old functionality is executed, which is confusing. The proposed change improves the logic by showing the warning only when the new behavior (returning empty logs) is triggered.

Low
  • Update

@asolntsev asolntsev added this to the 4.40.0 milestone Jan 11, 2026
@asolntsev asolntsev added I-cleanup Something needs to be tidied J-logging Applied to issues where logging information would help troubleshoot labels Jan 11, 2026
@titusfortner titusfortner merged commit a0f9f81 into trunk Jan 12, 2026
21 checks passed
@titusfortner titusfortner deleted the java_logs branch January 12, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-java Java Bindings I-cleanup Something needs to be tidied J-logging Applied to issues where logging information would help troubleshoot Review effort 3/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants