chore(bqjdbc): refactor log levels#13166
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the logging infrastructure of the BigQuery JDBC driver to improve performance and consistency. It removes numerous manual method entry/exit logs across the codebase, replacing them with a centralized logging mechanism within BigQueryJdbcContextProxy using dynamic proxies. The BigQueryJdbcResultSetLogger and BigQueryJdbcRootLogger were updated to better handle LogRecord objects and thread IDs. Feedback highlights potential reliability risks in the proxy's logging logic where calling toString() on arguments or results could trigger exceptions, and notes that setting thread IDs manually in the thread factory is misleading and redundant.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the logging infrastructure across the BigQuery JDBC driver, primarily shifting method entry/exit tracing from Level.FINEST to Level.FINER and introducing a more structured LogRecord approach to avoid expensive stack trace parsing. It also enhances the BigQueryJdbcContextProxy to automatically log method calls and results. Feedback focuses on a security concern regarding the potential for sensitive data exposure (CWE-532) when logging all method arguments and return values at the FINER level, as well as the performance implications of wrapping high-frequency JDBC operations.
b/505825091
- Causal Exception Unwrapping: Updated async stream readers (BigQueryStatement) to check e.getCause(), properly categorizing wrapped network interrupts during query shutdown to prevent false severe error logs.
- POJO Log Stripping: Removed repetitive, low-level diagnostic logs from intermediate state wrappers (BigQueryCallableStatement) to improve per-connection file readability.
- Standardized Level Routing: All Non-ResultSet trace logs now logs at Level.FINER, cleanly separating infrastructure execution boundaries from raw data iteration outputs (Level.FINEST).