Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request addresses a logger registration leak in the BigQuery JDBC driver by refactoring BigQueryJdbcResultSetLogger into a final facade class that delegates to shared, class-level loggers, and by converting instance loggers in various JDBC classes to static fields. Additionally, unit tests are added to verify that creating statements, metadata, and result-set loggers does not leak child loggers. The review feedback suggests declaring the lazily-initialized logger fields in BigQueryJdbcResultSetLogger as volatile to ensure thread safety across shared global facades, and modifying the warning method to avoid calling String.format when no arguments are provided, preventing potential UnknownFormatConversionException errors.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| */ | ||
| /** Cold path: builds a record whose caller is inferred lazily by the formatter. */ | ||
| private void logWithCaller(Level level, Throwable thrown, String msg) { | ||
| if (!delegate.isLoggable(level)) { |
There was a problem hiding this comment.
should we override isLoggable to do that check rather than always do delegate.isLoggable?
| logTrace(Level.FINE, "unknown", msgSupplier); | ||
| } | ||
|
|
||
| void warning(String format, Object... args) { |
b/576218509
One registered BigQueryJdbcCustomLogger per class, shared; the facade holds that delegate plus the connection ID and stamps the ID into LogRecord params[0] exactly as before, so per-connection file routing for un-proxied ResultSet calls is unchanged. Hot-path trace methods still set the source class/method explicitly and allocate nothing
when the level is disabled.